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.

8 files changed, 85 insertions, 25 deletions

diff --git a/ChangeLog b/ChangeLog
index 309b04f26ab..b02203dbe75 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2015-01-21  Ulrich Müller  <ulm@gentoo.org>
+
+        * 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  <eggert@cs.ucla.edu>
 
         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
@@ -46,6 +46,13 @@ and silent rules are now quieter.  To get the old behavior where
 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,
 so if you want to use it, you can always take a copy from an older Emacs.
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  <ulm@gentoo.org>
+
+        * 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  <eliz@gnu.org>
 
         * 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 <http://www.gnu.org/licenses/>.  */
 
 
 /* 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  <ulm@gentoo.org>
+
+        * 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  <monnier@iro.umontreal.ca>
 
         * 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