From: Todd C. Miller Date: Fri, 20 Aug 1999 20:37:16 +0000 (+0000) Subject: Run most of the code as root, not the invoking user. It doesn't really X-Git-Tag: SUDO_1_6_0~122 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=275c2fc980d8ddd851867d2ad126bb5e2b73879f;p=sudo Run most of the code as root, not the invoking user. It doesn't really gain us anything to run as the user since an attacker can just have an setuid(0) in their egg. Running as root solves potential problems wrt signalling. --- diff --git a/auth/API b/auth/API index 88991b66c..fcb2f1b6d 100644 --- a/auth/API +++ b/auth/API @@ -31,8 +31,8 @@ The variables in the struct are as follows: initialized in the "init" or "setup" routines. Possible values of sudo_auth.flags: - FLAG_ROOT Whether or not the auth functions should run with - an euid of 0 or the uid of the invoking user. + FLAG_USER Whether or not the auth functions should run with + the euid of the invoking user instead of 0. FLAG_CONFIGURED If set then the auth method is assumed to have been configured successfully. All auth methods start out @@ -102,23 +102,23 @@ in sudo_auth.h. For instance, for a method, ``fooauth'', add: #elif defined(HAVE_FOOAUTH) # define AUTH_STANDALONE \ - AUTH_ENTRY(FLAG_ROOT, "foo", \ + AUTH_ENTRY(0, "foo", \ foo_init, foo_setup, foo_verify, foo_cleanup) -If the method doesn't need to run as root, replace FLAG_ROOT with 0. -If you don't have a init/setup/cleanup routine, just use a NULL for that -field. +If the method needs to run as the user, not root, replace the first +parameter to AUTH_ENTRY (0) with FLAG_USER. If you don't have a +init/setup/cleanup routine, just use a NULL for that field. For a normal authentication method, add it to the ``auth_switch'' in sudo_auth.c. If ``fooauth'' is a normal auth method, its entry would look like: # ifdef HAVE_FOOAUTH - AUTH_ENTRY(FLAG_ROOT, "foo", foo_init, foo_setup, foo_verify, foo_cleanup) + AUTH_ENTRY(0, "foo", foo_init, foo_setup, foo_verify, foo_cleanup) # endif -Again, if the method doesn't need to run as root, replace FLAG_ROOT -with 0. Likewise, if you don't have a init/setup/cleanup routine, +Again, if the method doesn't need to run as root, replace the 0 with +FLAG_USER. Likewise, if you don't have a init/setup/cleanup routine, just use a NULL for that field. NOTE: You should not make a method both ``standalone'' and diff --git a/auth/sudo_auth.c b/auth/sudo_auth.c index b93ef19a3..ff14ec7c8 100644 --- a/auth/sudo_auth.c +++ b/auth/sudo_auth.c @@ -65,25 +65,25 @@ sudo_auth auth_switch[] = { AUTH_STANDALONE #else # ifndef WITHOUT_PASSWD - AUTH_ENTRY(FLAG_ROOT, "passwd", NULL, NULL, passwd_verify, NULL) + AUTH_ENTRY(0, "passwd", NULL, NULL, passwd_verify, NULL) # endif # if defined(HAVE_SECUREWARE) && !defined(WITHOUT_PASSWD) - AUTH_ENTRY(FLAG_ROOT, "secureware", secureware_init, NULL, secureware_verify, NULL) + AUTH_ENTRY(0, "secureware", secureware_init, NULL, secureware_verify, NULL) # endif # ifdef HAVE_AFS - AUTH_ENTRY(FLAG_ROOT, "afs", NULL, NULL, afs_verify, NULL) + AUTH_ENTRY(0, "afs", NULL, NULL, afs_verify, NULL) # endif # ifdef HAVE_KERB4 - AUTH_ENTRY(FLAG_ROOT, "kerb4", kerb4_init, NULL, kerb4_verify, NULL) + AUTH_ENTRY(0, "kerb4", kerb4_init, NULL, kerb4_verify, NULL) # endif # ifdef HAVE_KERB5 - AUTH_ENTRY(FLAG_ROOT, "kerb5", kerb5_init, NULL, kerb5_verify, NULL) + AUTH_ENTRY(0, "kerb5", kerb5_init, NULL, kerb5_verify, NULL) # endif # ifdef HAVE_SKEY - AUTH_ENTRY(FLAG_ROOT, "S/Key", NULL, rfc1938_setup, rfc1938_verify, NULL) + AUTH_ENTRY(0, "S/Key", NULL, rfc1938_setup, rfc1938_verify, NULL) # endif # ifdef HAVE_OPIE - AUTH_ENTRY(FLAG_ROOT, "OPIE", NULL, rfc1938_setup, rfc1938_verify, NULL) + AUTH_ENTRY(0, "OPIE", NULL, rfc1938_setup, rfc1938_verify, NULL) # endif #endif /* AUTH_STANDALONE */ AUTH_ENTRY(0, NULL, NULL, NULL, NULL, NULL) @@ -107,8 +107,8 @@ verify_user() /* Initialize auth methods and unconfigure the method if necessary. */ for (auth = auth_switch; auth->name; auth++) { if (auth->init && IS_CONFIGURED(auth)) { - if (NEEDS_ROOT(auth)) - set_perms(PERM_ROOT, 0); + if (NEEDS_USER(auth)) + set_perms(PERM_USER, 0); status = (auth->init)(sudo_user.pw, &user_prompt, auth); if (status == AUTH_FAILURE) @@ -116,8 +116,8 @@ verify_user() else if (status == AUTH_FATAL) /* XXX log */ exit(1); /* assume error msg already printed */ - if (NEEDS_ROOT(auth)) - set_perms(PERM_USER, 0); + if (NEEDS_USER(auth)) + set_perms(PERM_ROOT, 0); } } @@ -125,8 +125,8 @@ verify_user() /* Do any per-method setup and unconfigure the method if needed */ for (auth = auth_switch; auth->name; auth++) { if (auth->setup && IS_CONFIGURED(auth)) { - if (NEEDS_ROOT(auth)) - set_perms(PERM_ROOT, 0); + if (NEEDS_USER(auth)) + set_perms(PERM_USER, 0); status = (auth->setup)(sudo_user.pw, &user_prompt, auth); if (status == AUTH_FAILURE) @@ -134,8 +134,8 @@ verify_user() else if (status == AUTH_FATAL) /* XXX log */ exit(1); /* assume error msg already printed */ - if (NEEDS_ROOT(auth)) - set_perms(PERM_USER, 0); + if (NEEDS_USER(auth)) + set_perms(PERM_ROOT, 0); } } @@ -154,14 +154,14 @@ verify_user() if (!IS_CONFIGURED(auth)) continue; - if (NEEDS_ROOT(auth)) - set_perms(PERM_ROOT, 0); + if (NEEDS_USER(auth)) + set_perms(PERM_USER, 0); success = auth->status = (auth->verify)(sudo_user.pw, p, auth); (void) memset(p, 0, strlen(p)); - if (NEEDS_ROOT(auth)) - set_perms(PERM_USER, 0); + if (NEEDS_USER(auth)) + set_perms(PERM_ROOT, 0); if (auth->status != AUTH_FAILURE) goto cleanup; @@ -182,15 +182,15 @@ cleanup: /* Call cleanup routines. */ for (auth = auth_switch; auth->name; auth++) { if (auth->cleanup && IS_CONFIGURED(auth)) { - if (NEEDS_ROOT(auth)) - set_perms(PERM_ROOT, 0); + if (NEEDS_USER(auth)) + set_perms(PERM_USER, 0); status = (auth->cleanup)(sudo_user.pw, auth); if (status == AUTH_FATAL) /* XXX log */ exit(1); /* assume error msg already printed */ - if (NEEDS_ROOT(auth)) - set_perms(PERM_USER, 0); + if (NEEDS_USER(auth)) + set_perms(PERM_ROOT, 0); } } diff --git a/auth/sudo_auth.h b/auth/sudo_auth.h index 821822cf4..e6aa3ef4e 100644 --- a/auth/sudo_auth.h +++ b/auth/sudo_auth.h @@ -55,12 +55,12 @@ typedef struct sudo_auth { /* Values for sudo_auth.flags. */ /* XXX - these names are too long for my liking */ -#define FLAG_ROOT 0x01 /* functions must run as root */ +#define FLAG_USER 0x01 /* functions must run as root */ #define FLAG_CONFIGURED 0x02 /* method configured ok */ #define FLAG_ONEANDONLY 0x04 /* one and only auth method */ /* Shortcuts for using the flags above. */ -#define NEEDS_ROOT(x) ((x)->flags & FLAG_ROOT) +#define NEEDS_USER(x) ((x)->flags & FLAG_USER) #define IS_CONFIGURED(x) ((x)->flags & FLAG_CONFIGURED) #define IS_ONEANDONLY(x) ((x)->flags & FLAG_ONEANDONLY) @@ -99,27 +99,27 @@ int securid_verify __P((struct passwd *pw, char *pass, sudo_auth *auth)); /* Some methods cannots (or should not) interoperate with any others */ #if defined(HAVE_PAM) # define AUTH_STANDALONE \ - AUTH_ENTRY(FLAG_ROOT, "pam", \ + AUTH_ENTRY(0, "pam", \ pam_init, NULL, pam_verify, pam_cleanup) #elif defined(HAVE_SECURID) # define AUTH_STANDALONE \ - AUTH_ENTRY(FLAG_ROOT, "SecurId", \ + AUTH_ENTRY(0, "SecurId", securid_init, securid_setup, securid_verify, NULL) #elif defined(HAVE_SIA) # define AUTH_STANDALONE \ - AUTH_ENTRY(FLAG_ROOT, "sia", \ + AUTH_ENTRY(0, "sia", \ NULL, sia_setup, sia_verify, sia_cleanup) #elif defined(HAVE_DCE) # define AUTH_STANDALONE \ - AUTH_ENTRY(FLAG_ROOT, "dce", \ + AUTH_ENTRY(0, "dce", \ NULL, NULL, dce_verify, NULL) #elif defined(HAVE_AUTHENTICATE) # define AUTH_STANDALONE \ - AUTH_ENTRY(FLAG_ROOT, "aixauth", \ + AUTH_ENTRY(0, "aixauth", \ NULL, NULL, aixauth_verify, NULL) #elif defined(HAVE_FWTK) # define AUTH_STANDALONE \ - AUTH_ENTRY(FLAG_ROOT, "fwtk", fwtk_init, \ + AUTH_ENTRY(0, "fwtk", fwtk_init, \ NULL, fwtk_verify, fwtk_cleanup) #endif diff --git a/check.c b/check.c index 8fb32e1a2..170f02991 100644 --- a/check.c +++ b/check.c @@ -137,8 +137,6 @@ update_timestamp(timestampdir, timestampfile) char *timestampfile; { - set_perms(PERM_ROOT, 0); /* become root */ - if (touch(timestampfile ? timestampfile : timestampdir, time(NULL)) == -1) { if (timestampfile) { int fd = open(timestampfile, O_WRONLY|O_CREAT|O_TRUNC, 0600); @@ -152,8 +150,6 @@ update_timestamp(timestampdir, timestampfile) log_error(NO_EXIT|USE_ERRNO, "Can't mkdir %s", timestampdir); } } - - set_perms(PERM_USER, 0); /* relinquish root */ } /* @@ -288,8 +284,6 @@ timestamp_status(timestampdir, timestampfile, user, make_dirs) time_t now; int status = TS_ERROR; /* assume the worst */ - set_perms(PERM_ROOT, 0); /* become root */ - /* * Sanity check _PATH_SUDO_TIMEDIR and make it if it doesn't already exist. * We start out assuming the worst (that the dir is not sane) and @@ -325,10 +319,8 @@ timestamp_status(timestampdir, timestampfile, user, make_dirs) status = TS_MISSING; } } - if (status == TS_ERROR) { - set_perms(PERM_USER, 0); /* relinquish root */ + if (status == TS_ERROR) return(status); - } /* * Sanity check the user's ticket dir. We start by downgrading @@ -435,7 +427,6 @@ timestamp_status(timestampdir, timestampfile, user, make_dirs) } } - set_perms(PERM_USER, 0); /* relinquish root */ return(status); } @@ -455,7 +446,6 @@ remove_timestamp(remove) status = timestamp_status(timestampdir, timestampfile, user_name, FALSE); if (status == TS_OLD || status == TS_CURRENT) { ts = timestampfile ? timestampfile : timestampdir; - set_perms(PERM_ROOT, 0); /* become root */ if (remove) { if (timestampfile) status = unlink(timestampfile); @@ -471,7 +461,6 @@ remove_timestamp(remove) (void) fprintf(stderr, "%s: can't reset %s to epoch: %s\n", Argv[0], ts, strerror(errno)); } - set_perms(PERM_USER, 0); /* relinquish root */ } (void) free(timestampdir); diff --git a/find_path.c b/find_path.c index cf6342907..be92d3bba 100644 --- a/find_path.c +++ b/find_path.c @@ -112,7 +112,6 @@ find_path(infile, outfile) path = estrdup(path); origpath = path; - /* XXX use strtok() */ do { if ((n = strchr(path, ':'))) *n = '\0'; diff --git a/goodpath.c b/goodpath.c index 4808da9df..f950d9442 100644 --- a/goodpath.c +++ b/goodpath.c @@ -67,19 +67,12 @@ sudo_goodpath(path) const char *path; { struct stat sb; - int err; /* Check for brain damage */ if (path == NULL || path[0] == '\0') return(NULL); - /* Do the stat() as root. */ - set_perms(PERM_ROOT, 0); - err = stat(path, &sb); - set_perms(PERM_USER, 0); - - /* stat() failed */ - if (err) + if (stat(path, &sb)) return(NULL); /* Make sure path describes an executable regular file. */ diff --git a/logging.c b/logging.c index 74174e11d..bf8743c28 100644 --- a/logging.c +++ b/logging.c @@ -161,15 +161,10 @@ do_logfile(msg) FILE *fp; mode_t oldmask; time_t now; - int oldeuid = geteuid(); int maxlen = MAXLOGFILELEN; now = time((time_t) 0); - /* Become root if we are not already. */ - if (oldeuid) - set_perms(PERM_ROOT, 0); - oldmask = umask(077); fp = fopen(_PATH_SUDO_LOGFILE, "a"); (void) umask(oldmask); @@ -254,9 +249,6 @@ do_logfile(msg) (void) lock_file(fileno(fp), SUDO_UNLOCK); (void) fclose(fp); } - - if (oldeuid) - set_perms(PERM_USER, 0); /* relinquish root */ } #endif /* LOGGING & SLOG_FILE */ @@ -299,20 +291,18 @@ log_auth(status, inform_user) mail_auth(status, logline); /* send mail based on status */ /* Inform the user if they failed to authenticate. */ - if (inform_user) { + if (inform_user && (status & VALIDATE_NOT_OK)) { if (status & FLAG_NO_USER) (void) fprintf(stderr, "%s is not in the sudoers file. %s", user_name, "This incident will be reported.\n"); else if (status & FLAG_NO_HOST) (void) fprintf(stderr, "%s is not allowed to run sudo on %s. %s", user_name, user_shost, "This incident will be reported.\n"); - else if (status & VALIDATE_NOT_OK) + else (void) fprintf(stderr, "Sorry, user %s is not allowed to execute '%s%s%s' as %s on %s.\n", user_name, user_cmnd, user_args ? " " : "", user_args ? user_args : "", user_runas, user_host); - else - (void) fprintf(stderr, "An unknown error has occurred.\n"); } /* @@ -351,6 +341,10 @@ log_error(va_alist) fmt = va_arg(ap, const char *); #endif + /* Become root if we are not already to avoid user control */ + if (geteuid() != 0) + set_perms(PERM_ROOT, 0); + /* Expand printf-style format + args. */ evasprintf(&message, fmt, ap); va_end(ap); @@ -409,6 +403,9 @@ log_error(va_alist) free(message); } +/* + * Send a message to ALERTMAIL + */ #ifdef _PATH_SENDMAIL static void send_mail(line) @@ -481,7 +478,7 @@ send_mail(line) user_name, line); fclose(mail); reapchild(0); - exit(0); + _exit(0); } else { /* Parent, just return unless there is an error. */ if (pid == -1) { @@ -510,6 +507,7 @@ mail_auth(status, line) { int mail_mask; + /* If any of these bits are set in status, we send mail. */ mail_mask = VALIDATE_ERROR; #ifdef SEND_MAIL_WHEN_OK mail_mask |= VALIDATE_OK; diff --git a/parse.c b/parse.c index 51daa8f51..62bc842f8 100644 --- a/parse.c +++ b/parse.c @@ -124,26 +124,17 @@ sudoers_lookup(check_cmnd) yyin = sudoers_fp; yyout = stdout; - /* - * Allocate space for data structures in the parser. - */ + /* Allocate space for data structures in the parser. */ init_parser(); - /* - * Need to be root while stat'ing things in the parser. - */ + /* Need to be root while stat'ing things in the parser. */ set_perms(PERM_ROOT, 0); error = yyparse(); - /* - * Don't need to keep this open... - */ + /* Close the sudoers file now that we are done with it. */ (void) fclose(sudoers_fp); sudoers_fp = NULL; - /* relinquish extra privs */ - set_perms(PERM_USER, 0); - if (error || parse_error) return(VALIDATE_ERROR); @@ -159,10 +150,9 @@ sudoers_lookup(check_cmnd) } /* - * Only check the actual command if the check_cmnd - * flag is set. It is not set for the "validate" - * and "list" pseudo-commands. Always check the - * host and user. + * Only check the actual command if the check_cmnd flag is set. + * It is not set for the "validate" and "list" pseudo-commands. + * Always check the host and user. */ if (check_cmnd == FALSE) while (top) { diff --git a/sudo.c b/sudo.c index 06b9c34ea..c8b68b152 100644 --- a/sudo.c +++ b/sudo.c @@ -258,7 +258,7 @@ main(argc, argv) cmnd_status = init_vars(sudo_mode); - set_perms(PERM_USER, sudo_mode); + /* At this point, ruid == euid == 0 */ check_sudoers(); /* check mode/owner on _PATH_SUDOERS */ @@ -763,7 +763,6 @@ check_sudoers() * Fix the mode and group on sudoers file from old default. * Only works if filesystem is readable/writable by root. */ - set_perms(PERM_ROOT, 0); if ((rootstat = lstat(_PATH_SUDOERS, &statbuf)) == 0 && SUDOERS_UID == statbuf.st_uid && SUDOERS_MODE != 0400 && (statbuf.st_mode & 0007777) == 0400) { @@ -824,8 +823,7 @@ check_sudoers() log_error(USE_ERRNO, "can't open %s", _PATH_SUDOERS); } - set_perms(PERM_ROOT, 0); - set_perms(PERM_USER, 0); + set_perms(PERM_ROOT, 0); /* change back to root */ } /*