Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
openSUSE:Step:FrontRunner
sudo.32786
sudo-CVE-2023-42465-2of2.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File sudo-CVE-2023-42465-2of2.patch of Package sudo.32786
From 7873f8334c8d31031f8cfa83bd97ac6029309e4f Mon Sep 17 00:00:00 2001 From: "Todd C. Miller" <Todd.Miller@sudo.ws> Date: Sat, 9 Sep 2023 14:07:04 -0600 Subject: [PATCH] Try to make sudo less vulnerable to ROWHAMMER attacks. We now use ROWHAMMER-resistent values for ALLOW, DENY, AUTH_SUCCESS, AUTH_FAILURE, AUTH_FATAL and AUTH_NONINTERACTIVE. In addition, we explicitly test for expected values instead of using a negated test against an error value. In the parser match functions this means explicitly checking for ALLOW or DENY instead of accepting anything that is not set to UNSPEC. Thanks to Andrew J. Adiletta, M. Caner Tol, Yarkin Doroz, and Berk Sunar, all affiliated with the Vernam Applied Cryptography and Cybersecurity Lab at Worcester Polytechnic Institute, for the report. Paper preprint: https://arxiv.org/abs/2309.02545 --- plugins/sudoers/auth/passwd.c | 27 ++++++++++------- plugins/sudoers/auth/sudo_auth.c | 51 ++++++++++++++++++++++---------- plugins/sudoers/auth/sudo_auth.h | 12 ++++---- plugins/sudoers/parse.c | 12 ++++---- plugins/sudoers/match.c | 25 ++++++++-------- plugins/sudoers/parse.h | 23 ++++++++++---- 6 files changed, 96 insertions(+), 54 deletions(-) Index: sudo-1.9.9/plugins/sudoers/auth/passwd.c =================================================================== --- sudo-1.9.9.orig/plugins/sudoers/auth/passwd.c +++ sudo-1.9.9/plugins/sudoers/auth/passwd.c @@ -66,7 +66,7 @@ sudo_passwd_verify(struct passwd *pw, ch char des_pass[9], *epass; char *pw_epasswd = auth->data; size_t pw_len; - int matched = 0; + int ret; debug_decl(sudo_passwd_verify, SUDOERS_DEBUG_AUTH); /* An empty plain-text password must match an empty encrypted password. */ @@ -78,7 +78,7 @@ sudo_passwd_verify(struct passwd *pw, ch */ pw_len = strlen(pw_epasswd); if (pw_len == DESLEN || HAS_AGEINFO(pw_epasswd, pw_len)) { - strlcpy(des_pass, pass, sizeof(des_pass)); + (void)strlcpy(des_pass, pass, sizeof(des_pass)); pass = des_pass; } @@ -88,27 +88,34 @@ sudo_passwd_verify(struct passwd *pw, ch * only compare the first DESLEN characters in that case. */ epass = (char *) crypt(pass, pw_epasswd); + ret = AUTH_FAILURE; if (epass != NULL) { - if (HAS_AGEINFO(pw_epasswd, pw_len) && strlen(epass) == DESLEN) - matched = !strncmp(pw_epasswd, epass, DESLEN); - else - matched = !strcmp(pw_epasswd, epass); + if (HAS_AGEINFO(pw_epasswd, pw_len) && strlen(epass) == DESLEN) { + if (strncmp(pw_epasswd, epass, DESLEN) == 0) + ret = AUTH_SUCCESS; + } else { + if (strcmp(pw_epasswd, epass) == 0) + ret = AUTH_SUCCESS; + } } - debug_return_int(matched ? AUTH_SUCCESS : AUTH_FAILURE); + debug_return_int(ret); } #else int sudo_passwd_verify(struct passwd *pw, char *pass, sudo_auth *auth, struct sudo_conv_callback *callback) { char *pw_passwd = auth->data; - int matched; + int ret; debug_decl(sudo_passwd_verify, SUDOERS_DEBUG_AUTH); /* Simple string compare for systems without crypt(). */ - matched = !strcmp(pass, pw_passwd); + if (strcmp(pass, pw_passwd) == 0) + ret = AUTH_SUCCESS; + else + ret = AUTH_FAILURE; - debug_return_int(matched ? AUTH_SUCCESS : AUTH_FAILURE); + debug_return_int(ret); } #endif Index: sudo-1.9.9/plugins/sudoers/parse.c =================================================================== --- sudo-1.9.9.orig/plugins/sudoers/parse.c +++ sudo-1.9.9/plugins/sudoers/parse.c @@ -188,7 +188,7 @@ sudoers_lookup_check(struct sudo_nss *ns if (runas_match == ALLOW) { cmnd_match = cmnd_matches(nss->parse_tree, cs->cmnd, cs->runchroot, info); - if (cmnd_match != UNSPEC) { + if (SPECIFIED(cmnd_match)) { /* * If user is running command as himself, * set runas_pw = sudo_user.pw. @@ -431,7 +431,7 @@ sudoers_lookup(struct sudo_nss_list *snl } m = sudoers_lookup_check(nss, pw, &validated, &info, &cs, &defs, now); - if (m != UNSPEC) { + if (SPECIFIED(m)) { match = m; parse_tree = nss->parse_tree; } @@ -439,7 +439,7 @@ sudoers_lookup(struct sudo_nss_list *snl if (!sudo_nss_can_continue(nss, m)) break; } - if (match != UNSPEC) { + if (SPECIFIED(match)) { if (info.cmnd_path != NULL) { /* Update user_cmnd, user_stat, cmnd_status from matching entry. */ free(user_cmnd); Index: sudo-1.9.9/plugins/sudoers/match.c =================================================================== --- sudo-1.9.9.orig/plugins/sudoers/match.c +++ sudo-1.9.9/plugins/sudoers/match.c @@ -92,7 +92,7 @@ user_matches(struct sudoers_parse_tree * if ((a = alias_get(parse_tree, m->name, USERALIAS)) != NULL) { /* XXX */ const int rc = userlist_matches(parse_tree, pw, &a->members); - if (rc != UNSPEC) { + if (SPECIFIED(rc)) { if (m->negated) { matched = rc == ALLOW ? DENY : ALLOW; } else { @@ -124,7 +124,8 @@ userlist_matches(struct sudoers_parse_tr debug_decl(userlist_matches, SUDOERS_DEBUG_MATCH); TAILQ_FOREACH_REVERSE(m, list, member_list, entries) { - if ((matched = user_matches(parse_tree, pw, m)) != UNSPEC) + matched = user_matches(parse_tree, pw, m); + if (SPECIFIED(matched)) break; } debug_return_int(matched); @@ -194,7 +195,7 @@ runaslist_matches(struct sudoers_parse_t if (a != NULL) { rc = runaslist_matches(parse_tree, &a->members, &empty, matching_user, NULL); - if (rc != UNSPEC) { + if (SPECIFIED(rc)) { if (m->negated) { user_matched = rc == ALLOW ? DENY : ALLOW; } else { @@ -215,7 +216,7 @@ runaslist_matches(struct sudoers_parse_t user_matched = m->negated ? DENY : ALLOW; break; } - if (user_matched != UNSPEC) { + if (SPECIFIED(user_matched)) { if (matching_user != NULL && m->type != ALIAS) *matching_user = m; break; @@ -228,7 +229,7 @@ runaslist_matches(struct sudoers_parse_t * Skip checking runas group if none was specified. */ if (ISSET(sudo_user.flags, RUNAS_GROUP_SPECIFIED)) { - if (user_matched == UNSPEC) { + if (!SPECIFIED(user_matched)) { if (strcmp(user_name, runas_pw->pw_name) == 0) user_matched = ALLOW; /* only changing group */ } @@ -243,7 +244,7 @@ runaslist_matches(struct sudoers_parse_t if (a != NULL) { rc = runaslist_matches(parse_tree, &empty, &a->members, NULL, matching_group); - if (rc != UNSPEC) { + if (SPECIFIED(rc)) { if (m->negated) { group_matched = rc == ALLOW ? DENY : ALLOW; } else { @@ -259,14 +260,14 @@ runaslist_matches(struct sudoers_parse_t group_matched = m->negated ? DENY : ALLOW; break; } - if (group_matched != UNSPEC) { + if (SPECIFIED(group_matched)) { if (matching_group != NULL && m->type != ALIAS) *matching_group = m; break; } } } - if (group_matched == UNSPEC) { + if (!SPECIFIED(group_matched)) { struct gid_list *runas_groups; /* * The runas group was not explicitly allowed by sudoers. @@ -310,7 +311,7 @@ hostlist_matches_int(struct sudoers_pars TAILQ_FOREACH_REVERSE(m, list, member_list, entries) { matched = host_matches(parse_tree, pw, lhost, shost, m); - if (matched != UNSPEC) + if (SPECIFIED(matched)) break; } debug_return_int(matched); @@ -361,7 +362,7 @@ host_matches(struct sudoers_parse_tree * /* XXX */ const int rc = hostlist_matches_int(parse_tree, pw, lhost, shost, &a->members); - if (rc != UNSPEC) { + if (SPECIFIED(rc)) { if (m->negated) { matched = rc == ALLOW ? DENY : ALLOW; } else { @@ -395,7 +396,7 @@ cmndlist_matches(struct sudoers_parse_tr TAILQ_FOREACH_REVERSE(m, list, member_list, entries) { matched = cmnd_matches(parse_tree, m, runchroot, info); - if (matched != UNSPEC) + if (SPECIFIED(matched)) break; } debug_return_int(matched); @@ -425,7 +426,7 @@ cmnd_matches(struct sudoers_parse_tree * a = alias_get(parse_tree, m->name, CMNDALIAS); if (a != NULL) { rc = cmndlist_matches(parse_tree, &a->members, runchroot, info); - if (rc != UNSPEC) { + if (SPECIFIED(rc)) { if (m->negated) { matched = rc == ALLOW ? DENY : ALLOW; } else { Index: sudo-1.9.9/plugins/sudoers/parse.h =================================================================== --- sudo-1.9.9.orig/plugins/sudoers/parse.h +++ sudo-1.9.9/plugins/sudoers/parse.h @@ -34,16 +34,29 @@ # define SUDOERS_NAME_MATCH #endif +/* Allowed by policy (rowhammer resistent). */ +#undef ALLOW +#define ALLOW 0x52a2925 /* 0101001010100010100100100101 */ + +/* Denied by policy (rowhammer resistent). */ +#undef DENY +#define DENY 0xad5d6da /* 1010110101011101011011011010 */ + +/* Neither allowed, nor denied. */ #undef UNSPEC #define UNSPEC -1 -#undef DENY -#define DENY 0 -#undef ALLOW -#define ALLOW 1 + +/* Tag implied by root access (SETENV only). */ #undef IMPLIED #define IMPLIED 2 /* + * We must explicitly check against ALLOW and DENY instead testing + * that the value is not UNSPEC to avoid potential ROWHAMMER issues. + */ +#define SPECIFIED(_v) ((_v) == ALLOW || (_v) == DENY) + +/* * Initialize all tags to UNSPEC. */ #define TAGS_INIT(t) do { \ @@ -92,7 +105,7 @@ * Returns true if the specified tag is not UNSPEC or IMPLIED, else false. */ #define TAG_SET(tt) \ - ((tt) != UNSPEC && (tt) != IMPLIED) + ((tt) == true || (tt) == false) /* * Returns true if any tags set in nt differ between ot and nt, else false. Index: sudo-1.9.9/plugins/sudoers/auth/sudo_auth.h =================================================================== --- sudo-1.9.9.orig/plugins/sudoers/auth/sudo_auth.h +++ sudo-1.9.9/plugins/sudoers/auth/sudo_auth.h @@ -19,12 +19,12 @@ #ifndef SUDO_AUTH_H #define SUDO_AUTH_H -/* Auth function return values. */ -#define AUTH_SUCCESS 0 -#define AUTH_FAILURE 1 -#define AUTH_INTR 2 -#define AUTH_FATAL 3 -#define AUTH_NONINTERACTIVE 4 +/* Auth function return values (rowhammer resistent). */ +#define AUTH_SUCCESS 0x52a2925 /* 0101001010100010100100100101 */ +#define AUTH_FAILURE 0xad5d6da /* 1010110101011101011011011010 */ +#define AUTH_INTR 0x69d61fc8 /* 1101001110101100001111111001000 */ +#define AUTH_FATAL 0x1629e037 /* 0010110001010011110000000110111 */ +#define AUTH_NONINTERACTIVE 0x1fc8d3ac /* 11111110010001101001110101100 */ typedef struct sudo_auth { int flags; /* various flags, see below */ Index: sudo-1.9.9/plugins/sudoers/auth/sudo_auth.c =================================================================== --- sudo-1.9.9.orig/plugins/sudoers/auth/sudo_auth.c +++ sudo-1.9.9/plugins/sudoers/auth/sudo_auth.c @@ -114,10 +114,16 @@ sudo_auth_init(struct passwd *pw, int mo if (auth->init && !IS_DISABLED(auth)) { /* Disable if it failed to init unless there was a fatal error. */ status = (auth->init)(pw, auth); - if (status == AUTH_FAILURE) + switch (status) { + case AUTH_SUCCESS: + break; + case AUTH_FAILURE: SET(auth->flags, FLAG_DISABLED); - else if (status == AUTH_FATAL) - break; /* assume error msg already printed */ + break; + default: + /* Assume error msg already printed. */ + debug_return_int(-1); + } } } @@ -163,7 +169,7 @@ sudo_auth_init(struct passwd *pw, int mo } } - debug_return_int(status == AUTH_FATAL ? -1 : 0); + debug_return_int(0); } /* @@ -204,7 +210,7 @@ sudo_auth_cleanup(struct passwd *pw, boo for (auth = auth_switch; auth->name; auth++) { if (auth->cleanup && !IS_DISABLED(auth)) { int status = (auth->cleanup)(pw, auth, force); - if (status == AUTH_FATAL) { + if (status != AUTH_SUCCESS) { /* Assume error msg already printed. */ debug_return_int(-1); } @@ -301,7 +307,7 @@ verify_user(struct passwd *pw, char *pro SET(auth->flags, FLAG_DISABLED); else if (status == AUTH_NONINTERACTIVE) goto done; - else if (status == AUTH_FATAL || user_interrupted()) + else if (status != AUTH_SUCCESS || user_interrupted()) goto done; /* assume error msg already printed */ } } @@ -359,7 +365,6 @@ done: case AUTH_NONINTERACTIVE: SET(validated, FLAG_NO_USER_INPUT); FALLTHROUGH; - case AUTH_FATAL: default: log_auth_failure(validated, 0); ret = -1; @@ -371,24 +376,32 @@ done: /* * Call authentication method begin session hooks. - * Returns 1 on success and -1 on error. + * Returns true on success, false on failure and -1 on error. */ int sudo_auth_begin_session(struct passwd *pw, char **user_env[]) { sudo_auth *auth; + int ret = true; debug_decl(sudo_auth_begin_session, SUDOERS_DEBUG_AUTH); for (auth = auth_switch; auth->name; auth++) { if (auth->begin_session && !IS_DISABLED(auth)) { int status = (auth->begin_session)(pw, user_env, auth); - if (status != AUTH_SUCCESS) { + switch (status) { + case AUTH_SUCCESS: + break; + case AUTH_FAILURE: + ret = false; + break; + default: /* Assume error msg already printed. */ - debug_return_int(-1); + ret = -1; + break; } } } - debug_return_int(1); + debug_return_int(ret); } bool @@ -409,25 +422,33 @@ sudo_auth_needs_end_session(void) /* * Call authentication method end session hooks. - * Returns 1 on success and -1 on error. + * Returns true on success, false on failure and -1 on error. */ int sudo_auth_end_session(struct passwd *pw) { sudo_auth *auth; + int ret = true; int status; debug_decl(sudo_auth_end_session, SUDOERS_DEBUG_AUTH); for (auth = auth_switch; auth->name; auth++) { if (auth->end_session && !IS_DISABLED(auth)) { status = (auth->end_session)(pw, auth); - if (status == AUTH_FATAL) { + switch (status) { + case AUTH_SUCCESS: + break; + case AUTH_FAILURE: + ret = false; + break; + default: /* Assume error msg already printed. */ - debug_return_int(-1); + ret = -1; + break; } } } - debug_return_int(1); + debug_return_int(ret); } /*
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
