From: Ulrich Müller Date: Fri, 16 Jan 2015 08:25:25 +0000 (+0100) Subject: Allow update-game-score to run sgid instead of suid. X-Git-Tag: emacs-25.0.90~2586^2 X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=20f66485526b69eb26f2e70bd835a5e1333559d5;p=emacs.git Allow update-game-score to run sgid instead of suid. * configure.ac (gamegroup): New AC_SUBST. (--with-gameuser): Allow to specify a group instead of a user. In the default case, check at configure time if a 'games' user exists. * lib-src/update-game-score.c: Allow the program to run sgid instead of suid, in order to match common practice for most games. (main): Check if we are running sgid. Pass appropriate file permission bits to 'write_scores'. (write_scores): New 'mode' argument, instead of hardcoding 0644. (get_prefix): Update error message. * lib-src/Makefile.in (gamegroup): New variable, set by configure. ($(DESTDIR)${archlibdir}): Handle both suid or sgid when installing the 'update-game-score' program. * lisp/play/gamegrid.el (gamegrid-add-score-with-update-game-score): Allow the 'update-game-score' helper program to run suid or sgid. --- diff --git a/ChangeLog b/ChangeLog index 309b04f26ab..b02203dbe75 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +2015-01-21 Ulrich Müller + + * configure.ac (gamegroup): New AC_SUBST. + (--with-gameuser): Allow to specify a group instead of a user. + In the default case, check at configure time if a 'games' user + exists. + 2015-01-16 Paul Eggert Give up on -Wsuggest-attribute=const diff --git a/configure.ac b/configure.ac index 9db4bdecfcc..47b36fe1839 100644 --- a/configure.ac +++ b/configure.ac @@ -392,10 +392,25 @@ OPTION_DEFAULT_ON([compress-install], make GZIP_PROG= install]) AC_ARG_WITH(gameuser,dnl -[AS_HELP_STRING([--with-gameuser=USER],[user for shared game score files])]) -test "X${with_gameuser}" != X && test "${with_gameuser}" != yes \ - && gameuser="${with_gameuser}" -test "X$gameuser" = X && gameuser=games +[AS_HELP_STRING([--with-gameuser=USER_OR_GROUP], + [user for shared game score files. + An argument prefixed by ':' specifies a group instead.])]) +gameuser= +gamegroup= +case ${with_gameuser} in + no) ;; + "" | yes) + AC_MSG_CHECKING([whether a 'games' user exists]) + if id -u games >/dev/null 2>&1; then + AC_MSG_RESULT([yes]) + gameuser=games + else + AC_MSG_RESULT([no]) + fi + ;; + :*) gamegroup=`echo "${with_gameuser}" | sed -e "s/://"` ;; + *) gameuser=${with_gameuser} ;; +esac AC_ARG_WITH([gnustep-conf],dnl [AS_HELP_STRING([--with-gnustep-conf=FILENAME], @@ -4684,6 +4699,7 @@ AC_SUBST(etcdocdir) AC_SUBST(bitmapdir) AC_SUBST(gamedir) AC_SUBST(gameuser) +AC_SUBST(gamegroup) ## FIXME? Nothing uses @LD_SWITCH_X_SITE@. ## src/Makefile.in did add LD_SWITCH_X_SITE (as a cpp define) to the ## end of LIBX_BASE, but nothing ever set it. diff --git a/etc/NEWS b/etc/NEWS index 548b54df0da..120d8b920c6 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -45,6 +45,13 @@ and silent rules are now quieter. To get the old behavior where 'make' chatters a lot, configure with '--disable-silent-rules' or build with 'make V=1'. +--- +** The configure option '--with-gameuser' now allows to specify a +group instead of a user if its argument is prefixed by ':' (a colon). +This will cause the game score files in ${localstatedir}/games/emacs +to be owned by that group, and the helper program for updating them to +be installed setgid. + --- ** The `grep-changelog' script (and its manual page) are no longer included. It has no particular connection to Emacs and has not changed in years, diff --git a/lib-src/ChangeLog b/lib-src/ChangeLog index 37f037ef324..b67038ff81a 100644 --- a/lib-src/ChangeLog +++ b/lib-src/ChangeLog @@ -1,3 +1,15 @@ +2015-01-21 Ulrich Müller + + * update-game-score.c: Allow the program to run sgid instead + of suid, in order to match common practice for most games. + (main): Check if we are running sgid. Pass appropriate file + permission bits to 'write_scores'. + (write_scores): New 'mode' argument, instead of hardcoding 0644. + (get_prefix): Update error message. + * Makefile.in (gamegroup): New variable, set by configure. + ($(DESTDIR)${archlibdir}): Handle both suid or sgid when + installing the 'update-game-score' program. + 2015-01-16 Eli Zaretskii * Makefile.in (AM_V_RC, am__v_RC_, am__v_RC_0, am__v_RC_1): New diff --git a/lib-src/Makefile.in b/lib-src/Makefile.in index 01592bd21a5..2997f1b35a8 100644 --- a/lib-src/Makefile.in +++ b/lib-src/Makefile.in @@ -122,6 +122,7 @@ archlibdir=@archlibdir@ gamedir=@gamedir@ gameuser=@gameuser@ +gamegroup=@gamegroup@ # ==================== Utility Programs for the Build ================= @@ -263,10 +264,17 @@ $(DESTDIR)${archlibdir}: all umask 022; ${MKDIR_P} "$(DESTDIR)${gamedir}"; \ touch "$(DESTDIR)${gamedir}/snake-scores"; \ touch "$(DESTDIR)${gamedir}/tetris-scores" - -if chown ${gameuser} "$(DESTDIR)${archlibdir}/update-game-score${EXEEXT}" && chmod u+s "$(DESTDIR)${archlibdir}/update-game-score${EXEEXT}"; then \ - chown ${gameuser} "$(DESTDIR)${gamedir}"; \ - chmod u=rwx,g=rwx,o=rx "$(DESTDIR)${gamedir}"; \ - fi +ifneq ($(gameuser),) + chown ${gameuser} "$(DESTDIR)${archlibdir}/update-game-score${EXEEXT}" + chmod u+s,go-r "$(DESTDIR)${archlibdir}/update-game-score${EXEEXT}" + chown ${gameuser} "$(DESTDIR)${gamedir}" + chmod u=rwx,g=rx,o=rx "$(DESTDIR)${gamedir}" +else ifneq ($(gamegroup),) + chgrp ${gamegroup} "$(DESTDIR)${archlibdir}/update-game-score${EXEEXT}" + chmod g+s,o-r "$(DESTDIR)${archlibdir}/update-game-score${EXEEXT}" + chgrp ${gamegroup} "$(DESTDIR)${gamedir}" + chmod u=rwx,g=rwx,o=rx "$(DESTDIR)${gamedir}" +endif exp_archlibdir=`cd "$(DESTDIR)${archlibdir}" && /bin/pwd`; \ if [ "$$exp_archlibdir" != "`cd ${srcdir} && /bin/pwd`" ]; then \ for file in ${SCRIPTS}; do \ diff --git a/lib-src/update-game-score.c b/lib-src/update-game-score.c index d3354af2783..4f154832c81 100644 --- a/lib-src/update-game-score.c +++ b/lib-src/update-game-score.c @@ -21,8 +21,8 @@ along with GNU Emacs. If not, see . */ /* This program allows a game to securely and atomically update a - score file. It should be installed setuid, owned by an appropriate - user like `games'. + score file. It should be installed either setuid or setgid, owned + by an appropriate user or group like `games'. Alternatively, it can be compiled without HAVE_SHARED_GAME_DIR defined, and in that case it will store scores in the user's home @@ -88,7 +88,7 @@ static int push_score (struct score_entry **scores, ptrdiff_t *count, ptrdiff_t *size, struct score_entry const *newscore); static void sort_scores (struct score_entry *scores, ptrdiff_t count, bool reverse); -static int write_scores (const char *filename, +static int write_scores (const char *filename, mode_t mode, const struct score_entry *scores, ptrdiff_t count); static _Noreturn void @@ -122,18 +122,19 @@ get_user_id (void) } static const char * -get_prefix (bool running_suid, const char *user_prefix) +get_prefix (bool privileged, const char *user_prefix) { - if (!running_suid && user_prefix == NULL) - lose ("Not using a shared game directory, and no prefix given."); - if (running_suid) + if (privileged) { #ifdef HAVE_SHARED_GAME_DIR return HAVE_SHARED_GAME_DIR; #else - lose ("This program was compiled without HAVE_SHARED_GAME_DIR,\n and should not be suid."); + lose ("This program was compiled without HAVE_SHARED_GAME_DIR,\n" + "and should not run with elevated privileges."); #endif } + if (user_prefix == NULL) + lose ("Not using a shared game directory, and no prefix given."); return user_prefix; } @@ -173,7 +174,7 @@ int main (int argc, char **argv) { int c; - bool running_suid; + bool running_suid, running_sgid; void *lockstate; char *scorefile; char *end, *nl, *user, *data; @@ -214,8 +215,11 @@ main (int argc, char **argv) usage (EXIT_FAILURE); running_suid = (getuid () != geteuid ()); + running_sgid = (getgid () != getegid ()); + if (running_suid && running_sgid) + lose ("This program can run either suid or sgid, but not both."); - prefix = get_prefix (running_suid, user_prefix); + prefix = get_prefix (running_suid || running_sgid, user_prefix); scorefile = malloc (strlen (prefix) + strlen (argv[optind]) + 2); if (!scorefile) @@ -270,7 +274,8 @@ main (int argc, char **argv) scores += scorecount - max_scores; scorecount = max_scores; } - if (write_scores (scorefile, scores, scorecount) < 0) + if (write_scores (scorefile, running_sgid ? 0664 : 0644, + scores, scorecount) < 0) { unlock_file (scorefile, lockstate); lose_syserr ("Failed to write scores file"); @@ -421,8 +426,8 @@ sort_scores (struct score_entry *scores, ptrdiff_t count, bool reverse) } static int -write_scores (const char *filename, const struct score_entry *scores, - ptrdiff_t count) +write_scores (const char *filename, mode_t mode, + const struct score_entry *scores, ptrdiff_t count) { int fd; FILE *f; @@ -435,7 +440,7 @@ write_scores (const char *filename, const struct score_entry *scores, if (fd < 0) return -1; #ifndef DOS_NT - if (fchmod (fd, 0644) != 0) + if (fchmod (fd, mode) != 0) return -1; #endif f = fdopen (fd, "w"); diff --git a/lisp/ChangeLog b/lisp/ChangeLog index d13bacfd965..7aa66bf9ad5 100644 --- a/lisp/ChangeLog +++ b/lisp/ChangeLog @@ -1,3 +1,8 @@ +2015-01-21 Ulrich Müller + + * play/gamegrid.el (gamegrid-add-score-with-update-game-score): + Allow the 'update-game-score' helper program to run suid or sgid. + 2015-01-21 Stefan Monnier * emacs-lisp/eieio.el: Use cl-defmethod. diff --git a/lisp/play/gamegrid.el b/lisp/play/gamegrid.el index 1e265a635a0..b4c3c594731 100644 --- a/lisp/play/gamegrid.el +++ b/lisp/play/gamegrid.el @@ -486,13 +486,13 @@ FILE is created there." (not (zerop (logand (file-modes (expand-file-name "update-game-score" exec-directory)) - #o4000))))) + #o6000))))) (cond ((file-name-absolute-p file) (gamegrid-add-score-insecure file score)) ((and gamegrid-shared-game-dir (file-exists-p (expand-file-name file shared-game-score-directory))) - ;; Use the setuid "update-game-score" program to update a - ;; system-wide score file. + ;; Use the setuid (or setgid) "update-game-score" program + ;; to update a system-wide score file. (gamegrid-add-score-with-update-game-score-1 file (expand-file-name file shared-game-score-directory) score)) ;; Else: Add the score to a score file in the user's home