From: Todd C. Miller Date: Wed, 22 Feb 2012 18:04:03 +0000 (-0500) Subject: Relax the user/group/mode checks on sudoers files. As long as the X-Git-Tag: SUDO_1_8_5~1^2~193 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=415454ff59c1dab3de86365bf5aa022dd4845562;p=sudo Relax the user/group/mode checks on sudoers files. As long as the file is owned by the right user, not world-writable and not writable by a group other than the one specified at configure time (gid 0 by default), the file is considered OK. Note that visudo will still set the mode to the value specified at configure time. --- diff --git a/MANIFEST b/MANIFEST index 03106b272..df88d8c3a 100644 --- a/MANIFEST +++ b/MANIFEST @@ -47,6 +47,7 @@ compat/regress/fnmatch/fnm_test.in compat/regress/glob/files compat/regress/glob/globtest.c compat/regress/glob/globtest.in +compat/secure_path.c compat/setenv.c compat/siglist.in compat/snprintf.c @@ -107,6 +108,7 @@ include/gettext.h include/lbuf.h include/list.h include/missing.h +include/secure_path.h include/sudo_conf.h include/sudo_debug.h include/sudo_plugin.h diff --git a/common/Makefile.in b/common/Makefile.in index 245616c66..695febec7 100644 --- a/common/Makefile.in +++ b/common/Makefile.in @@ -42,8 +42,8 @@ DEFS = @OSDEFS@ -D_PATH_SUDO_CONF=\"$(sysconfdir)/sudo.conf\" SHELL = @SHELL@ -LTOBJS = alloc.lo atobool.lo fileops.lo fmt_string.lo lbuf.lo \ - list.lo setgroups.lo sudo_conf.lo sudo_debug.lo term.lo \ +LTOBJS = alloc.lo atobool.lo fileops.lo fmt_string.lo lbuf.lo list.lo \ + secure_path.lo setgroups.lo sudo_conf.lo sudo_debug.lo term.lo \ zero_bytes.lo @COMMON_OBJS@ all: libcommon.la @@ -117,6 +117,10 @@ lbuf.lo: $(srcdir)/lbuf.c $(top_builddir)/config.h $(incdir)/missing.h \ list.lo: $(srcdir)/list.c $(top_builddir)/config.h $(incdir)/missing.h \ $(incdir)/list.h $(incdir)/error.h $(LIBTOOL) --mode=compile $(CC) -c -o $@ $(CPPFLAGS) $(CFLAGS) $(DEFS) $(srcdir)/list.c +secure_path.lo: $(srcdir)/secure_path.c $(top_builddir)/config.h \ + $(incdir)/missing.h $(incdir)/sudo_debug.h \ + $(incdir)/secure_path.h + $(LIBTOOL) --mode=compile $(CC) -c -o $@ $(CPPFLAGS) $(CFLAGS) $(DEFS) $(srcdir)/secure_path.c setgroups.lo: $(srcdir)/setgroups.c $(top_builddir)/config.h \ $(incdir)/missing.h $(incdir)/sudo_debug.h $(LIBTOOL) --mode=compile $(CC) -c -o $@ $(CPPFLAGS) $(CFLAGS) $(DEFS) $(srcdir)/setgroups.c @@ -124,7 +128,8 @@ sudo_conf.lo: $(srcdir)/sudo_conf.c $(top_builddir)/config.h \ $(top_srcdir)/compat/stdbool.h $(incdir)/missing.h \ $(incdir)/alloc.h $(incdir)/error.h $(incdir)/fileops.h \ $(top_builddir)/pathnames.h $(incdir)/sudo_plugin.h \ - $(incdir)/sudo_conf.h $(incdir)/list.h $(incdir)/sudo_debug.h + $(incdir)/sudo_conf.h $(incdir)/list.h $(incdir)/sudo_debug.h \ + $(incdir)/secure_path.h $(incdir)/gettext.h $(LIBTOOL) --mode=compile $(CC) -c -o $@ $(CPPFLAGS) $(CFLAGS) $(DEFS) $(srcdir)/sudo_conf.c sudo_debug.lo: $(srcdir)/sudo_debug.c $(top_builddir)/config.h \ $(top_srcdir)/compat/stdbool.h $(incdir)/missing.h \ diff --git a/common/secure_path.c b/common/secure_path.c new file mode 100644 index 000000000..db2b87f90 --- /dev/null +++ b/common/secure_path.c @@ -0,0 +1,66 @@ +/* + * Copyright (c) 2012 Todd C. Miller + * + * Permission to use, copy, modify, and distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ + +#include + +#include +#include +#include +#include +#ifdef HAVE_STRING_H +# include +#endif /* HAVE_STRING_H */ +#ifdef HAVE_STRINGS_H +# include +#endif /* HAVE_STRINGS_H */ +#ifdef HAVE_UNISTD_H +# include +#endif /* HAVE_UNISTD_H */ +#include + +#include "missing.h" +#include "sudo_debug.h" +#include "secure_path.h" + +/* + * Verify that path is a regular file and not writable by other users. + */ +int +sudo_secure_path(const char *path, uid_t uid, gid_t gid, struct stat *sbp) +{ + struct stat sb; + int rval = SUDO_PATH_MISSING; + debug_decl(sudo_secure_path, SUDO_DEBUG_UTIL) + + if (path != NULL && stat_sudoers(path, &sb) == 0) { + if (!S_ISREG(sb.st_mode)) { + rval = SUDO_PATH_BAD_TYPE; + } else if (uid != (uid_t)-1 && sb.st_uid != uid) { + rval = SUDO_PATH_WRONG_OWNER; + } else if (sb.st_mode & S_IWOTH) { + rval = SUDO_PATH_WORLD_WRITABLE; + } else if (ISSET(sb.st_mode, S_IWGRP) && + (gid == (gid_t)-1 || sb.st_gid != gid)) { + rval = SUDO_PATH_GROUP_WRITABLE; + } else { + rval = SUDO_PATH_SECURE; + } + if (sbp) + (void) memcpy(sbp, &sb, sizeof(struct stat)); + } + + debug_return_int(rval); +} diff --git a/common/sudo_conf.c b/common/sudo_conf.c index 3ddd8619e..c4bfd211d 100644 --- a/common/sudo_conf.c +++ b/common/sudo_conf.c @@ -43,6 +43,7 @@ # include #endif /* HAVE_UNISTD_H */ #include +#include #define SUDO_ERROR_WRAP 0 @@ -54,6 +55,14 @@ #include "sudo_plugin.h" #include "sudo_conf.h" #include "sudo_debug.h" +#include "secure_path.h" +#include "gettext.h" + +#ifdef __TANDEM +# define ROOT_UID 65535 +#else +# define ROOT_UID 0 +#endif #ifndef _PATH_SUDO_ASKPASS # define _PATH_SUDO_ASKPASS NULL @@ -250,19 +259,48 @@ sudo_conf_disable_coredump(void) } /* - * Reads in /etc/sudo.conf - * Returns a list of plugins. + * Reads in /etc/sudo.conf and populates sudo_conf_data. */ void sudo_conf_read(void) { struct sudo_conf_table *cur; struct plugin_info *info; + struct stat sb; FILE *fp; char *cp; - if ((fp = fopen(_PATH_SUDO_CONF, "r")) == NULL) + switch (sudo_secure_path(_PATH_SUDO_CONF, ROOT_UID, -1, &sb)) { + case SUDO_PATH_SECURE: + break; + case SUDO_PATH_MISSING: + /* Root should always be able to read sudo.conf. */ + if (errno != ENOENT && geteuid() == ROOT_UID) + warning(_("unable to stat %s"), _PATH_SUDO_CONF); + goto done; + case SUDO_PATH_BAD_TYPE: + warningx(_("%s is not a regular file"), _PATH_SUDO_CONF); + goto done; + case SUDO_PATH_WRONG_OWNER: + warningx(_("%s is owned by uid %u, should be %u"), + _PATH_SUDO_CONF, (unsigned int) sb.st_uid, ROOT_UID); + goto done; + case SUDO_PATH_WORLD_WRITABLE: + warningx(_("%s is world writable"), _PATH_SUDO_CONF); + goto done; + case SUDO_PATH_GROUP_WRITABLE: + warningx(_("%s is group writable"), _PATH_SUDO_CONF); + goto done; + default: + /* NOTREACHED */ + goto done; + } + + if ((fp = fopen(_PATH_SUDO_CONF, "r")) == NULL) { + if (errno != ENOENT && geteuid() == ROOT_UID) + warning(_("unable to open %s"), _PATH_SUDO_CONF); goto done; + } while ((cp = sudo_parseln(fp)) != NULL) { /* Skip blank or comment lines */ diff --git a/include/secure_path.h b/include/secure_path.h new file mode 100644 index 000000000..37218864d --- /dev/null +++ b/include/secure_path.h @@ -0,0 +1,29 @@ +/* + * Copyright (c) 2012 Todd C. Miller + * + * Permission to use, copy, modify, and distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ + +#ifndef _SUDO_SECURE_PATH_H +#define _SUDO_SECURE_PATH_H + +#define SUDO_PATH_SECURE 0 +#define SUDO_PATH_MISSING -1 +#define SUDO_PATH_BAD_TYPE -2 +#define SUDO_PATH_WRONG_OWNER -3 +#define SUDO_PATH_WORLD_WRITABLE -4 +#define SUDO_PATH_GROUP_WRITABLE -5 + +int sudo_secure_path(const char *path, uid_t uid, gid_t gid, struct stat *sbp); + +#endif /* _SUDO_SECURE_PATH_H */ diff --git a/plugins/sudoers/Makefile.in b/plugins/sudoers/Makefile.in index c338e5592..c60cbf9bb 100644 --- a/plugins/sudoers/Makefile.in +++ b/plugins/sudoers/Makefile.in @@ -715,7 +715,8 @@ sudoers.lo: $(srcdir)/sudoers.c $(top_builddir)/config.h \ $(srcdir)/defaults.h $(devdir)/def_data.h $(srcdir)/logging.h \ $(srcdir)/sudo_nss.h $(incdir)/sudo_plugin.h \ $(incdir)/sudo_debug.h $(incdir)/gettext.h $(srcdir)/interfaces.h \ - $(srcdir)/sudoers_version.h $(srcdir)/auth/sudo_auth.h + $(srcdir)/sudoers_version.h $(srcdir)/auth/sudo_auth.h \ + $(incdir)/secure_path.h $(LIBTOOL) --mode=compile $(CC) -c $(CPPFLAGS) $(CFLAGS) $(DEFS) $(srcdir)/sudoers.c sudoreplay.o: $(srcdir)/sudoreplay.c $(top_builddir)/config.h \ $(top_srcdir)/compat/timespec.h $(top_srcdir)/compat/stdbool.h \ diff --git a/plugins/sudoers/sudoers.c b/plugins/sudoers/sudoers.c index 168b23307..ec9f81694 100644 --- a/plugins/sudoers/sudoers.c +++ b/plugins/sudoers/sudoers.c @@ -84,6 +84,7 @@ #include "interfaces.h" #include "sudoers_version.h" #include "auth/sudo_auth.h" +#include "secure_path.h" /* * Prototypes @@ -940,74 +941,57 @@ set_cmnd(void) FILE * open_sudoers(const char *sudoers, bool doedit, bool *keepopen) { - struct stat statbuf; + struct stat sb; FILE *fp = NULL; - int rootstat; debug_decl(open_sudoers, SUDO_DEBUG_PLUGIN) - /* - * Fix the mode and group on sudoers file from old default. - * Only works if file system is readable/writable by root. - */ - if ((rootstat = stat_sudoers(sudoers, &statbuf)) == 0 && - sudoers_uid == statbuf.st_uid && sudoers_mode != 0400 && - (statbuf.st_mode & 0007777) == 0400) { - - if (chmod(sudoers, sudoers_mode) == 0) { - warningx(_("fixed mode on %s"), sudoers); - SET(statbuf.st_mode, sudoers_mode); - if (statbuf.st_gid != sudoers_gid) { - if (chown(sudoers, (uid_t) -1, sudoers_gid) == 0) { - warningx(_("set group on %s"), sudoers); - statbuf.st_gid = sudoers_gid; - } else - warning(_("unable to set group on %s"), sudoers); - } - } else - warning(_("unable to fix mode on %s"), sudoers); - } - - /* - * Sanity checks on sudoers file. Must be done as sudoers - * file owner. We already did a stat as root, so use that - * data if we can't stat as sudoers file owner. - */ set_perms(PERM_SUDOERS); - if (rootstat != 0 && stat_sudoers(sudoers, &statbuf) != 0) - log_error(USE_ERRNO|NO_EXIT, _("unable to stat %s"), sudoers); - else if (!S_ISREG(statbuf.st_mode)) - log_error(NO_EXIT, _("%s is not a regular file"), sudoers); - else if ((statbuf.st_mode & 07577) != (sudoers_mode & 07577)) - log_error(NO_EXIT, _("%s is mode 0%o, should be 0%o"), sudoers, - (unsigned int) (statbuf.st_mode & 07777), - (unsigned int) sudoers_mode); - else if (statbuf.st_uid != sudoers_uid) - log_error(NO_EXIT, _("%s is owned by uid %u, should be %u"), sudoers, - (unsigned int) statbuf.st_uid, (unsigned int) sudoers_uid); - else if (statbuf.st_gid != sudoers_gid && ISSET(statbuf.st_mode, S_IRGRP|S_IWGRP)) - log_error(NO_EXIT, _("%s is owned by gid %u, should be %u"), sudoers, - (unsigned int) statbuf.st_gid, (unsigned int) sudoers_gid); - else if ((fp = fopen(sudoers, "r")) == NULL) - log_error(USE_ERRNO|NO_EXIT, _("unable to open %s"), sudoers); - else { - /* - * Make sure we can actually read sudoers so we can present the - * user with a reasonable error message (unlike the lexer). - */ - if (statbuf.st_size != 0 && fgetc(fp) == EOF) { - log_error(USE_ERRNO|NO_EXIT, _("unable to read %s"), sudoers); - fclose(fp); - fp = NULL; - } - } - - if (fp != NULL) { - rewind(fp); - (void) fcntl(fileno(fp), F_SETFD, 1); + switch (sudo_secure_path(sudoers, sudoers_uid, sudoers_gid, &sb)) { + case SUDO_PATH_SECURE: + if ((fp = fopen(sudoers, "r")) == NULL) { + log_error(USE_ERRNO|NO_EXIT, _("unable to open %s"), sudoers); + } else { + /* + * Make sure we can actually read sudoers so we can present the + * user with a reasonable error message (unlike the lexer). + */ + if (sb.st_size != 0 && fgetc(fp) == EOF) { + log_error(USE_ERRNO|NO_EXIT, _("unable to read %s"), + sudoers); + fclose(fp); + fp = NULL; + } else { + /* Rewind fp and set close on exec flag. */ + rewind(fp); + (void) fcntl(fileno(fp), F_SETFD, 1); + } + } + break; + case SUDO_PATH_MISSING: + log_error(USE_ERRNO|NO_EXIT, _("unable to stat %s"), sudoers); + break; + case SUDO_PATH_BAD_TYPE: + log_error(NO_EXIT, _("%s is not a regular file"), sudoers); + break; + case SUDO_PATH_WRONG_OWNER: + log_error(NO_EXIT, _("%s is owned by uid %u, should be %u"), + sudoers, (unsigned int) sb.st_uid, (unsigned int) sudoers_uid); + break; + case SUDO_PATH_WORLD_WRITABLE: + log_error(NO_EXIT, _("%s is world writable"), sudoers); + break; + case SUDO_PATH_GROUP_WRITABLE: + log_error(NO_EXIT, _("%s is owned by gid %u, should be %u"), + sudoers, (unsigned int) sb.st_gid, (unsigned int) sudoers_gid); + break; + default: + /* NOTREACHED */ + break; } restore_perms(); /* change back to root */ + debug_return_ptr(fp); }