From 413e1100b843e2f7035baa8a36cf8d79b54e0bef Mon Sep 17 00:00:00 2001 From: "Todd C. Miller" Date: Sun, 22 Jan 2017 18:56:16 -0800 Subject: [PATCH] Add new fdexec sudoers setting to allow choose whether execve() or fexecve() is used. --- doc/sudoers.cat | 32 ++++- doc/sudoers.man.in | 52 +++++++++ doc/sudoers.mdoc.in | 45 +++++++ plugins/sudoers/def_data.c | 11 ++ plugins/sudoers/def_data.h | 5 +- plugins/sudoers/def_data.in | 4 + plugins/sudoers/defaults.c | 1 + plugins/sudoers/match.c | 226 +++++++++++++++++++++++++----------- 8 files changed, 308 insertions(+), 68 deletions(-) diff --git a/doc/sudoers.cat b/doc/sudoers.cat index ffad399c1..10ebc1422 100644 --- a/doc/sudoers.cat +++ b/doc/sudoers.cat @@ -416,7 +416,9 @@ SSUUDDOOEERRSS FFIILLEE FFOORRMMAATT command after the digest check has been performed but before the command is executed. A similar race condition exists on systems that lack the fexecve(2) system call when the directory in which the command is located - is writable by the user. + is writable by the user. See the description of the _f_d_e_x_e_c setting for + more information on how ssuuddoo executes commands that have an associated + digest. Command digests are only supported by version 1.8.7 or higher. @@ -1728,6 +1730,34 @@ SSUUDDOOEERRSS OOPPTTIIOONNSS requirements. The group name specified should not include a % prefix. This is not set by default. + fdexec Determines whether ssuuddoo will execute a command by its path + or by an open file descriptor. It has the following + possible values: + + always Always execute by file descriptor. + + never Never execute by file descriptor. + + digest_only + Only execute by file descriptor if the command has + an associated digest in the _s_u_d_o_e_r_s file. + + The default value is _d_i_g_e_s_t___o_n_l_y. This avoids a time of + check versus time of use race condition when the command is + located in a directory writable by the invoking user. + + Note that _f_d_e_x_e_c will change the first element of the + argument vector for scripts ($0 in the shell) due to the + way the kernel runs script interpreters. Instead of being + a normal path, it will refer to a file descriptor. For + example, _/_d_e_v_/_f_d_/_4 on Solaris and _/_p_r_o_c_/_s_e_l_f_/_f_d_/_4 on Linux. + A workaround is to use the SUDO_COMMAND environment + variable instead. + + This setting is only supported by version 1.8.20 or higher. + If the operating system does not support the fexecve(2) + system call, this setting has no effect. + group_plugin A string containing a ssuuddooeerrss group plugin with optional arguments. The string should consist of the plugin path, either fully-qualified or relative to the diff --git a/doc/sudoers.man.in b/doc/sudoers.man.in index bae06012c..9ab794eef 100644 --- a/doc/sudoers.man.in +++ b/doc/sudoers.man.in @@ -890,6 +890,11 @@ A similar race condition exists on systems that lack the fexecve(2) system call when the directory in which the command is located is writable by the user. +See the description of the +\fIfdexec\fR +setting for more information on how +\fBsudo\fR +executes commands that have an associated digest. .PP Command digests are only supported by version 1.8.7 or higher. .SS "Defaults" @@ -3485,6 +3490,53 @@ The group name specified should not include a prefix. This is not set by default. .TP 14n +fdexec +Determines whether +\fBsudo\fR +will execute a command by its path or by an open file descriptor. +It has the following possible values: +.PP +.RS 14n +.PD 0 +.TP 8n +always +Always execute by file descriptor. +.PD +.TP 8n +never +Never execute by file descriptor. +.TP 8n +digest_only +Only execute by file descriptor if the command has an associated digest +in the +\fIsudoers\fR +file. +.PP +The default value is +\fIdigest_only\fR. +This avoids a time of check versus time of use race condition when +the command is located in a directory writable by the invoking user. +.sp +Note that +\fIfdexec\fR +will change the first element of the argument vector for scripts +($0 in the shell) due to the way the kernel runs script interpreters. +Instead of being a normal path, it will refer to a file descriptor. +For example, +\fI/dev/fd/4\fR +on Solaris and +\fI/proc/self/fd/4\fR +on Linux. +A workaround is to use the +\fRSUDO_COMMAND\fR +environment variable instead. +.sp +This setting is only supported by version 1.8.20 or higher. +If the operating system does not support the +fexecve(2) +system call, this setting has no effect. +.RE +.TP 14n group_plugin A string containing a \fBsudoers\fR diff --git a/doc/sudoers.mdoc.in b/doc/sudoers.mdoc.in index 4a9508d8f..f343fa15f 100644 --- a/doc/sudoers.mdoc.in +++ b/doc/sudoers.mdoc.in @@ -847,6 +847,11 @@ A similar race condition exists on systems that lack the .Xr fexecve 2 system call when the directory in which the command is located is writable by the user. +See the description of the +.Em fdexec +setting for more information on how +.Nm sudo +executes commands that have an associated digest. .Pp Command digests are only supported by version 1.8.7 or higher. .Ss Defaults @@ -3254,6 +3259,46 @@ The group name specified should not include a .Li % prefix. This is not set by default. +.It fdexec +Determines whether +.Nm sudo +will execute a command by its path or by an open file descriptor. +It has the following possible values: +.Bl -tag -width 6n +.It always +Always execute by file descriptor. +.It never +Never execute by file descriptor. +.It digest_only +Only execute by file descriptor if the command has an associated digest +in the +.Em sudoers +file. +.El +.Pp +The default value is +.Em digest_only . +This avoids a time of check versus time of use race condition when +the command is located in a directory writable by the invoking user. +.Pp +Note that +.Em fdexec +will change the first element of the argument vector for scripts +($0 in the shell) due to the way the kernel runs script interpreters. +Instead of being a normal path, it will refer to a file descriptor. +For example, +.Pa /dev/fd/4 +on Solaris and +.Pa /proc/self/fd/4 +on Linux. +A workaround is to use the +.Dv SUDO_COMMAND +environment variable instead. +.Pp +This setting is only supported by version 1.8.20 or higher. +If the operating system does not support the +.Xr fexecve 2 +system call, this setting has no effect. .It group_plugin A string containing a .Nm sudoers diff --git a/plugins/sudoers/def_data.c b/plugins/sudoers/def_data.c index 00caa8b45..489cb23a9 100644 --- a/plugins/sudoers/def_data.c +++ b/plugins/sudoers/def_data.c @@ -21,6 +21,13 @@ static struct def_values def_data_verifypw[] = { { NULL, 0 }, }; +static struct def_values def_data_fdexec[] = { + { "never", never }, + { "digest_only", digest_only }, + { "always", always }, + { NULL, 0 }, +}; + struct sudo_defs_types sudo_defs_table[] = { { "syslog", T_LOGFAC|T_BOOL, @@ -434,6 +441,10 @@ struct sudo_defs_types sudo_defs_table[] = { "iolog_mode", T_MODE, N_("File mode to use for the I/O log files: 0%o"), NULL, + }, { + "fdexec", T_TUPLE|T_BOOL, + N_("Execute commands by file descriptor instead of by path: %s"), + def_data_fdexec, }, { NULL, 0, NULL } diff --git a/plugins/sudoers/def_data.h b/plugins/sudoers/def_data.h index d83d2c3b6..8b798a541 100644 --- a/plugins/sudoers/def_data.h +++ b/plugins/sudoers/def_data.h @@ -204,11 +204,14 @@ #define def_iolog_group (sudo_defs_table[I_IOLOG_GROUP].sd_un.str) #define I_IOLOG_MODE 102 #define def_iolog_mode (sudo_defs_table[I_IOLOG_MODE].sd_un.mode) +#define I_FDEXEC 103 +#define def_fdexec (sudo_defs_table[I_FDEXEC].sd_un.tuple) enum def_tuple { never, once, always, any, - all + all, + digest_only }; diff --git a/plugins/sudoers/def_data.in b/plugins/sudoers/def_data.in index 9f069f1ee..000b3a926 100644 --- a/plugins/sudoers/def_data.in +++ b/plugins/sudoers/def_data.in @@ -322,3 +322,7 @@ iolog_group iolog_mode T_MODE "File mode to use for the I/O log files: 0%o" +fdexec + T_TUPLE|T_BOOL + "Execute commands by file descriptor instead of by path: %s" + never digest_only always diff --git a/plugins/sudoers/defaults.c b/plugins/sudoers/defaults.c index 5eaf8ea93..70f81f669 100644 --- a/plugins/sudoers/defaults.c +++ b/plugins/sudoers/defaults.c @@ -533,6 +533,7 @@ init_defaults(void) def_netgroup_tuple = false; def_sudoedit_checkdir = true; def_iolog_mode = S_IRUSR|S_IWUSR; + def_fdexec = digest_only; /* Syslog options need special care since they both strings and ints */ #if (LOGGING & SLOG_SYSLOG) diff --git a/plugins/sudoers/match.c b/plugins/sudoers/match.c index a994beea7..008d7a7c9 100644 --- a/plugins/sudoers/match.c +++ b/plugins/sudoers/match.c @@ -75,6 +75,10 @@ # include "compat/sha2.h" #endif +#if !defined(O_SEARCH) && defined(O_PATH) +# define O_SEARCH O_PATH +#endif + static struct member_list empty = TAILQ_HEAD_INITIALIZER(empty); static bool command_matches_dir(const char *sudoers_dir, size_t dlen, const struct sudo_digest *digest); @@ -83,7 +87,7 @@ static bool command_matches_glob(const char *sudoers_cmnd, const char *sudoers_a #endif static bool command_matches_fnmatch(const char *sudoers_cmnd, const char *sudoers_args, const struct sudo_digest *digest); static bool command_matches_normal(const char *sudoers_cmnd, const char *sudoers_args, const struct sudo_digest *digest); -static bool digest_matches(const char *file, const struct sudo_digest *sd, int *fd); +static bool digest_matches(int fd, const char *file, const struct sudo_digest *sd); /* * Returns true if string 's' contains meta characters. @@ -433,10 +437,73 @@ done: debug_return_bool(rc); } +/* + * Stat file by fd is possible, else by path. + * Returns true on success, else false. + */ +static bool +do_stat(int fd, const char *path, struct stat *sb) +{ + debug_decl(do_stat, SUDOERS_DEBUG_MATCH) + + if (fd != -1) + debug_return_bool(fstat(fd, sb) == 0); + debug_return_bool(stat(path, sb) == 0); +} + +/* + * Open path if fdexec is enabled or if a digest is present. + * Returns false on error, else true. + */ +static bool +open_cmnd(const char *path, const struct sudo_digest *digest, int *fdp) +{ + int fd = -1; + bool is_script = false; + debug_decl(open_cmnd, SUDOERS_DEBUG_MATCH) + + /* Only open the file for fdexec or for digest matching. */ + if (def_fdexec != always && digest == NULL) + debug_return_bool(true); + + fd = open(path, O_RDONLY|O_NONBLOCK); +# ifdef O_SEARCH + if (fd == -1 && errno == EACCES && digest == NULL) { + /* Try again with O_SEARCH if no digest is specified. */ + const int saved_errno = errno; + if ((fd = open(path, O_SEARCH)) == -1) + errno = saved_errno; + } +# endif + if (fd == -1) + debug_return_bool(false); + +#ifdef HAVE_FEXECVE + do { + /* Check for #! cookie and set is_script. */ + char magic[2]; + if (read(fd, magic, 2) == 2) { + if (magic[0] == '#' && magic[1] == '!') + is_script = true; + } + (void) lseek(fd, (off_t)0, SEEK_SET); + } while (0); + /* + * Shell scripts go through namei twice and so we can't set the close + * on exec flag on the fd for fexecve(2). + */ + if (!is_script) + (void)fcntl(fd, F_SETFD, FD_CLOEXEC); +#endif /* HAVE_FEXECVE */ + *fdp = fd; + debug_return_bool(true); +} + static bool command_matches_fnmatch(const char *sudoers_cmnd, const char *sudoers_args, const struct sudo_digest *digest) { + struct stat sb; /* XXX - unused */ debug_decl(command_matches_fnmatch, SUDOERS_DEBUG_MATCH) /* @@ -453,11 +520,22 @@ command_matches_fnmatch(const char *sudoers_cmnd, const char *sudoers_args, close(cmnd_fd); cmnd_fd = -1; } + /* Open the file for fdexec or for digest matching. */ + if (!open_cmnd(user_cmnd, digest, &cmnd_fd)) + goto bad; + if (!do_stat(cmnd_fd, user_cmnd, &sb)) + goto bad; /* Check digest of user_cmnd since sudoers_cmnd is a pattern. */ - if (digest != NULL && !digest_matches(user_cmnd, digest, &cmnd_fd)) - debug_return_bool(false); + if (digest != NULL && !digest_matches(cmnd_fd, user_cmnd, digest)) + goto bad; /* No need to set safe_cmnd since user_cmnd matches sudoers_cmnd */ debug_return_bool(true); +bad: + if (cmnd_fd != -1) { + close(cmnd_fd); + cmnd_fd = -1; + } + debug_return_bool(false); } debug_return_bool(false); } @@ -502,17 +580,22 @@ command_matches_glob(const char *sudoers_cmnd, const char *sudoers_args, /* If user_cmnd is fully-qualified, check for an exact match. */ if (user_cmnd[0] == '/') { for (ap = gl.gl_pathv; (cp = *ap) != NULL; ap++) { - if (strcmp(cp, user_cmnd) != 0 || stat(cp, &sudoers_stat) == -1) + if (fd != -1) { + close(fd); + fd = -1; + } + if (strcmp(cp, user_cmnd) != 0) + continue; + /* Open the file for fdexec or for digest matching. */ + if (!open_cmnd(cp, digest, &fd)) + continue; + if (!do_stat(fd, cp, &sudoers_stat)) continue; if (user_stat == NULL || (user_stat->st_dev == sudoers_stat.st_dev && user_stat->st_ino == sudoers_stat.st_ino)) { - if (fd != -1) { - close(fd); - fd = -1; - } /* There could be multiple matches, check digest early. */ - if (digest != NULL && !digest_matches(cp, digest, &fd)) { + if (digest != NULL && !digest_matches(fd, cp, digest)) { bad_digest = true; continue; } @@ -532,6 +615,11 @@ command_matches_glob(const char *sudoers_cmnd, const char *sudoers_args, /* No exact match, compare basename, st_dev and st_ino. */ if (!bad_digest) { for (ap = gl.gl_pathv; (cp = *ap) != NULL; ap++) { + if (fd != -1) { + close(fd); + fd = -1; + } + /* If it ends in '/' it is a directory spec. */ dlen = strlen(cp); if (cp[dlen - 1] == '/') { @@ -545,17 +633,18 @@ command_matches_glob(const char *sudoers_cmnd, const char *sudoers_args, base++; else base = cp; - if (strcmp(user_base, base) != 0 || - stat(cp, &sudoers_stat) == -1) + if (strcmp(user_base, base) != 0) + continue; + + /* Open the file for fdexec or for digest matching. */ + if (!open_cmnd(cp, digest, &fd)) + continue; + if (!do_stat(fd, cp, &sudoers_stat)) continue; if (user_stat == NULL || (user_stat->st_dev == sudoers_stat.st_dev && user_stat->st_ino == sudoers_stat.st_ino)) { - if (fd != -1) { - close(fd); - fd = -1; - } - if (digest != NULL && !digest_matches(cp, digest, &fd)) + if (digest != NULL && !digest_matches(fd, cp, digest)) continue; free(safe_cmnd); if ((safe_cmnd = strdup(cp)) == NULL) { @@ -655,19 +744,16 @@ static struct digest_function { }; static bool -digest_matches(const char *file, const struct sudo_digest *sd, int *fd) +digest_matches(int fd, const char *file, const struct sudo_digest *sd) { unsigned char file_digest[SHA512_DIGEST_LENGTH]; unsigned char sudoers_digest[SHA512_DIGEST_LENGTH]; unsigned char buf[32 * 1024]; struct digest_function *func = NULL; -#ifdef HAVE_FEXECVE - bool first = true; - bool is_script = false; -#endif /* HAVE_FEXECVE */ size_t nread; SHA2_CTX ctx; FILE *fp; + int fd2; unsigned int i; debug_decl(digest_matches, SUDOERS_DEBUG_MATCH) @@ -700,22 +786,20 @@ digest_matches(const char *file, const struct sudo_digest *sd, int *fd) } } - if ((fp = fopen(file, "r")) == NULL) { + if ((fd2 = dup(fd)) == -1) { + sudo_debug_printf(SUDO_DEBUG_INFO, "unable to dup %s: %s", + file, strerror(errno)); + debug_return_bool(false); + } + if ((fp = fdopen(fd2, "r")) == NULL) { sudo_debug_printf(SUDO_DEBUG_INFO, "unable to open %s: %s", file, strerror(errno)); + close(fd2); debug_return_bool(false); } func->init(&ctx); while ((nread = fread(buf, 1, sizeof(buf), fp)) != 0) { -#ifdef HAVE_FEXECVE - /* Check for #! cookie and set is_script. */ - if (first) { - first = false; - if (nread >= 2 && buf[0] == '#' && buf[1] == '!') - is_script = true; - } -#endif /* HAVE_FEXECVE */ func->update(&ctx, buf, nread); } if (ferror(fp)) { @@ -733,24 +817,6 @@ digest_matches(const char *file, const struct sudo_digest *sd, int *fd) debug_return_bool(false); } -#ifdef HAVE_FEXECVE - /* - * On systems with fexecve(2) we can use that to execute the - * matching command even when the directory is writable. - */ - if ((*fd = dup(fileno(fp))) == -1) { - sudo_debug_printf(SUDO_DEBUG_INFO, "unable to dup %s: %s", - file, strerror(errno)); - fclose(fp); - debug_return_bool(false); - } - /* - * Shell scripts go through namei twice and so we can't set the close - * on exec flag on the fd for fexecve(2). - */ - if (!is_script) - (void)fcntl(*fd, F_SETFD, FD_CLOEXEC); -#endif /* HAVE_FEXECVE */ fclose(fp); debug_return_bool(true); bad_format: @@ -765,6 +831,7 @@ command_matches_normal(const char *sudoers_cmnd, const char *sudoers_args, const struct stat sudoers_stat; const char *base; size_t dlen; + int fd = -1; debug_decl(command_matches_normal, SUDOERS_DEBUG_MATCH) /* If it ends in '/' it is a directory spec. */ @@ -777,10 +844,15 @@ command_matches_normal(const char *sudoers_cmnd, const char *sudoers_args, const base = sudoers_cmnd; else base++; - if (strcmp(user_base, base) != 0 || - stat(sudoers_cmnd, &sudoers_stat) == -1) + if (strcmp(user_base, base) != 0) debug_return_bool(false); + /* Open the file for fdexec or for digest matching. */ + if (!open_cmnd(sudoers_cmnd, digest, &fd)) + goto bad; + if (!do_stat(fd, sudoers_cmnd, &sudoers_stat)) + goto bad; + /* * Return true if inode/device matches AND * a) there are no args in sudoers OR @@ -791,23 +863,38 @@ command_matches_normal(const char *sudoers_cmnd, const char *sudoers_args, const if (user_stat != NULL && (user_stat->st_dev != sudoers_stat.st_dev || user_stat->st_ino != sudoers_stat.st_ino)) - debug_return_bool(false); + goto bad; if (!command_args_match(sudoers_cmnd, sudoers_args)) - debug_return_bool(false); - if (cmnd_fd != -1) { - close(cmnd_fd); - cmnd_fd = -1; - } - if (digest != NULL && !digest_matches(sudoers_cmnd, digest, &cmnd_fd)) { + goto bad; + if (digest != NULL && !digest_matches(fd, sudoers_cmnd, digest)) { /* XXX - log functions not available but we should log very loudly */ - debug_return_bool(false); + goto bad; } free(safe_cmnd); if ((safe_cmnd = strdup(sudoers_cmnd)) == NULL) { sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory")); - debug_return_bool(false); + goto bad; + } + if (cmnd_fd != -1) { + close(cmnd_fd); + cmnd_fd = -1; + } +#ifdef HAVE_FEXECVE + /* Stash away fd if we are going to use fexecve(2) */ + if (def_fdexec == always || (digest != NULL && def_fdexec == digest_only)) { + cmnd_fd = fd; + } else +#endif /* HAVE_FEXECVE */ + { + /* Either fdexec is not in use or fexecve(2) is not present. */ + if (fd != -1) + close(fd); } debug_return_bool(true); +bad: + if (fd != -1) + close(fd); + debug_return_bool(false); } #endif /* SUDOERS_NAME_MATCH */ @@ -851,23 +938,30 @@ command_matches_dir(const char *sudoers_dir, size_t dlen, debug_return_bool(false); } while ((dent = readdir(dirp)) != NULL) { + if (fd != -1) { + close(fd); + fd = -1; + } + /* ignore paths > PATH_MAX (XXX - log) */ buf[dlen] = '\0'; if (strlcat(buf, dent->d_name, sizeof(buf)) >= sizeof(buf)) continue; /* only stat if basenames are the same */ - if (strcmp(user_base, dent->d_name) != 0 || - stat(buf, &sudoers_stat) == -1) + if (strcmp(user_base, dent->d_name) != 0) + continue; + + /* Open the file for fdexec or for digest matching. */ + if (!open_cmnd(buf, digest, &fd)) + continue; + if (!do_stat(fd, buf, &sudoers_stat)) continue; + if (user_stat == NULL || (user_stat->st_dev == sudoers_stat.st_dev && user_stat->st_ino == sudoers_stat.st_ino)) { - if (fd != -1) { - close(fd); - fd = -1; - } - if (digest != NULL && !digest_matches(buf, digest, &fd)) + if (digest != NULL && !digest_matches(fd, buf, digest)) continue; free(safe_cmnd); if ((safe_cmnd = strdup(buf)) == NULL) { -- 2.40.0