Commit 20f66485 authored by Ulrich Müller's avatar Ulrich Müller
Browse files

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.
parent 59e7fe6d
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> 2015-01-16 Paul Eggert <eggert@cs.ucla.edu>
Give up on -Wsuggest-attribute=const Give up on -Wsuggest-attribute=const
......
...@@ -392,10 +392,25 @@ OPTION_DEFAULT_ON([compress-install], ...@@ -392,10 +392,25 @@ OPTION_DEFAULT_ON([compress-install],
make GZIP_PROG= install]) make GZIP_PROG= install])
AC_ARG_WITH(gameuser,dnl AC_ARG_WITH(gameuser,dnl
[AS_HELP_STRING([--with-gameuser=USER],[user for shared game score files])]) [AS_HELP_STRING([--with-gameuser=USER_OR_GROUP],
test "X${with_gameuser}" != X && test "${with_gameuser}" != yes \ [user for shared game score files.
&& gameuser="${with_gameuser}" An argument prefixed by ':' specifies a group instead.])])
test "X$gameuser" = X && gameuser=games 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 AC_ARG_WITH([gnustep-conf],dnl
[AS_HELP_STRING([--with-gnustep-conf=FILENAME], [AS_HELP_STRING([--with-gnustep-conf=FILENAME],
...@@ -4684,6 +4699,7 @@ AC_SUBST(etcdocdir) ...@@ -4684,6 +4699,7 @@ AC_SUBST(etcdocdir)
AC_SUBST(bitmapdir) AC_SUBST(bitmapdir)
AC_SUBST(gamedir) AC_SUBST(gamedir)
AC_SUBST(gameuser) AC_SUBST(gameuser)
AC_SUBST(gamegroup)
## FIXME? Nothing uses @LD_SWITCH_X_SITE@. ## FIXME? Nothing uses @LD_SWITCH_X_SITE@.
## src/Makefile.in did add LD_SWITCH_X_SITE (as a cpp define) to the ## src/Makefile.in did add LD_SWITCH_X_SITE (as a cpp define) to the
## end of LIBX_BASE, but nothing ever set it. ## end of LIBX_BASE, but nothing ever set it.
......
...@@ -45,6 +45,13 @@ and silent rules are now quieter. To get the old behavior where ...@@ -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 'make' chatters a lot, configure with '--disable-silent-rules' or
build with 'make V=1'. 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. ** 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, It has no particular connection to Emacs and has not changed in years,
......
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> 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 * Makefile.in (AM_V_RC, am__v_RC_, am__v_RC_0, am__v_RC_1): New
......
...@@ -122,6 +122,7 @@ archlibdir=@archlibdir@ ...@@ -122,6 +122,7 @@ archlibdir=@archlibdir@
gamedir=@gamedir@ gamedir=@gamedir@
gameuser=@gameuser@ gameuser=@gameuser@
gamegroup=@gamegroup@
# ==================== Utility Programs for the Build ================= # ==================== Utility Programs for the Build =================
...@@ -263,10 +264,17 @@ $(DESTDIR)${archlibdir}: all ...@@ -263,10 +264,17 @@ $(DESTDIR)${archlibdir}: all
umask 022; ${MKDIR_P} "$(DESTDIR)${gamedir}"; \ umask 022; ${MKDIR_P} "$(DESTDIR)${gamedir}"; \
touch "$(DESTDIR)${gamedir}/snake-scores"; \ touch "$(DESTDIR)${gamedir}/snake-scores"; \
touch "$(DESTDIR)${gamedir}/tetris-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 \ ifneq ($(gameuser),)
chown ${gameuser} "$(DESTDIR)${gamedir}"; \ chown ${gameuser} "$(DESTDIR)${archlibdir}/update-game-score${EXEEXT}"
chmod u=rwx,g=rwx,o=rx "$(DESTDIR)${gamedir}"; \ chmod u+s,go-r "$(DESTDIR)${archlibdir}/update-game-score${EXEEXT}"
fi 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`; \ exp_archlibdir=`cd "$(DESTDIR)${archlibdir}" && /bin/pwd`; \
if [ "$$exp_archlibdir" != "`cd ${srcdir} && /bin/pwd`" ]; then \ if [ "$$exp_archlibdir" != "`cd ${srcdir} && /bin/pwd`" ]; then \
for file in ${SCRIPTS}; do \ for file in ${SCRIPTS}; do \
......
...@@ -21,8 +21,8 @@ along with GNU Emacs. If not, see <http://www.gnu.org/licenses/>. */ ...@@ -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 /* This program allows a game to securely and atomically update a
score file. It should be installed setuid, owned by an appropriate score file. It should be installed either setuid or setgid, owned
user like `games'. by an appropriate user or group like `games'.
Alternatively, it can be compiled without HAVE_SHARED_GAME_DIR Alternatively, it can be compiled without HAVE_SHARED_GAME_DIR
defined, and in that case it will store scores in the user's home 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, ...@@ -88,7 +88,7 @@ static int push_score (struct score_entry **scores, ptrdiff_t *count,
ptrdiff_t *size, struct score_entry const *newscore); ptrdiff_t *size, struct score_entry const *newscore);
static void sort_scores (struct score_entry *scores, ptrdiff_t count, static void sort_scores (struct score_entry *scores, ptrdiff_t count,
bool reverse); 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); const struct score_entry *scores, ptrdiff_t count);
static _Noreturn void static _Noreturn void
...@@ -122,18 +122,19 @@ get_user_id (void) ...@@ -122,18 +122,19 @@ get_user_id (void)
} }
static const char * 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) if (privileged)
lose ("Not using a shared game directory, and no prefix given.");
if (running_suid)
{ {
#ifdef HAVE_SHARED_GAME_DIR #ifdef HAVE_SHARED_GAME_DIR
return HAVE_SHARED_GAME_DIR; return HAVE_SHARED_GAME_DIR;
#else #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 #endif
} }
if (user_prefix == NULL)
lose ("Not using a shared game directory, and no prefix given.");
return user_prefix; return user_prefix;
} }
...@@ -173,7 +174,7 @@ int ...@@ -173,7 +174,7 @@ int
main (int argc, char **argv) main (int argc, char **argv)
{ {
int c; int c;
bool running_suid; bool running_suid, running_sgid;
void *lockstate; void *lockstate;
char *scorefile; char *scorefile;
char *end, *nl, *user, *data; char *end, *nl, *user, *data;
...@@ -214,8 +215,11 @@ main (int argc, char **argv) ...@@ -214,8 +215,11 @@ main (int argc, char **argv)
usage (EXIT_FAILURE); usage (EXIT_FAILURE);
running_suid = (getuid () != geteuid ()); 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); scorefile = malloc (strlen (prefix) + strlen (argv[optind]) + 2);
if (!scorefile) if (!scorefile)
...@@ -270,7 +274,8 @@ main (int argc, char **argv) ...@@ -270,7 +274,8 @@ main (int argc, char **argv)
scores += scorecount - max_scores; scores += scorecount - max_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); unlock_file (scorefile, lockstate);
lose_syserr ("Failed to write scores file"); lose_syserr ("Failed to write scores file");
...@@ -421,8 +426,8 @@ sort_scores (struct score_entry *scores, ptrdiff_t count, bool reverse) ...@@ -421,8 +426,8 @@ sort_scores (struct score_entry *scores, ptrdiff_t count, bool reverse)
} }
static int static int
write_scores (const char *filename, const struct score_entry *scores, write_scores (const char *filename, mode_t mode,
ptrdiff_t count) const struct score_entry *scores, ptrdiff_t count)
{ {
int fd; int fd;
FILE *f; FILE *f;
...@@ -435,7 +440,7 @@ write_scores (const char *filename, const struct score_entry *scores, ...@@ -435,7 +440,7 @@ write_scores (const char *filename, const struct score_entry *scores,
if (fd < 0) if (fd < 0)
return -1; return -1;
#ifndef DOS_NT #ifndef DOS_NT
if (fchmod (fd, 0644) != 0) if (fchmod (fd, mode) != 0)
return -1; return -1;
#endif #endif
f = fdopen (fd, "w"); f = fdopen (fd, "w");
......
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> 2015-01-21 Stefan Monnier <monnier@iro.umontreal.ca>
* emacs-lisp/eieio.el: Use cl-defmethod. * emacs-lisp/eieio.el: Use cl-defmethod.
......
...@@ -486,13 +486,13 @@ FILE is created there." ...@@ -486,13 +486,13 @@ FILE is created there."
(not (zerop (logand (file-modes (not (zerop (logand (file-modes
(expand-file-name "update-game-score" (expand-file-name "update-game-score"
exec-directory)) exec-directory))
#o4000))))) #o6000)))))
(cond ((file-name-absolute-p file) (cond ((file-name-absolute-p file)
(gamegrid-add-score-insecure file score)) (gamegrid-add-score-insecure file score))
((and gamegrid-shared-game-dir ((and gamegrid-shared-game-dir
(file-exists-p (expand-file-name file shared-game-score-directory))) (file-exists-p (expand-file-name file shared-game-score-directory)))
;; Use the setuid "update-game-score" program to update a ;; Use the setuid (or setgid) "update-game-score" program
;; system-wide score file. ;; to update a system-wide score file.
(gamegrid-add-score-with-update-game-score-1 file (gamegrid-add-score-with-update-game-score-1 file
(expand-file-name file shared-game-score-directory) score)) (expand-file-name file shared-game-score-directory) score))
;; Else: Add the score to a score file in the user's home ;; Else: Add the score to a score file in the user's home
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment