https://github.com/sudo-project/sudo/commit/7873f8334c8d31031f8cfa83bd97ac6029309e4f Skip to content Toggle navigation Sign in * Product + Actions Automate any workflow + Packages Host and manage packages + Security Find and fix vulnerabilities + Codespaces Instant dev environments + Copilot Write better code with AI + Code review Manage code changes + Issues Plan and track work + Discussions Collaborate outside of code Explore + All features + Documentation + GitHub Skills + Blog * Solutions For + Enterprise + Teams + Startups + Education By Solution + CI/CD & Automation + DevOps + DevSecOps Resources + Learning Pathways + White papers, Ebooks, Webinars + Customer Stories + Partners * Open Source + GitHub Sponsors Fund open source developers + The ReadME Project GitHub community articles Repositories + Topics + Trending + Collections * Pricing Search or jump to... Search code, repositories, users, issues, pull requests... Search [ ] Clear Search syntax tips Provide feedback We read every piece of feedback, and take your input very seriously. [ ] [ ] Include my email address so I can be contacted Cancel Submit feedback Saved searches Use saved searches to filter your results more quickly Name [ ] Query [ ] To see all available qualifiers, see our documentation. Cancel Create saved search Sign in Sign up You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session. You switched accounts on another tab or window. Reload to refresh your session. Dismiss alert {{ message }} sudo-project / sudo Public * * Notifications * Fork 191 * Star 1k * Code * Issues 18 * Pull requests 1 * Actions * Projects 0 * Security * Insights Additional navigation options * Code * Issues * Pull requests * Actions * Projects * Security * Insights Commit Permalink This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository. Browse files Browse the repository at this point in the history Try to make sudo less vulnerable to ROWHAMMER attacks. We now use ROWHAMMER-resistent values for ALLOW, DENY, AUTH_SUCCESS, AUTH_FAILURE, AUTH_ERROR 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 * Loading branch information @millert millert committed Sep 9, 2023 1 parent 525803d commit 7873f83 Show file tree Hide file tree Showing 6 changed files with 96 additions and 54 deletions. * Whitespace * Ignore whitespace * Split * Unified [ ] * plugins/sudoers + auth o plugins/sudoers/auth/passwd.c passwd.c o plugins/sudoers/auth/sudo_auth.c sudo_auth.c o plugins/sudoers/auth/sudo_auth.h sudo_auth.h + plugins/sudoers/lookup.c lookup.c + plugins/sudoers/match.c match.c + plugins/sudoers/parse.h parse.h There are no files selected for viewing 27 changes: 17 additions & 10 deletions 27 plugins/sudoers/auth/ passwd.c [*] Show comments View file Edit file Delete file This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters Show hidden characters Original file Diff line number line Diff line change number @@ -68,7 +68,7 @@ sudo_passwd_verify(const Expand Up struct sudoers_context *ctx, struct passwd *pw, 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. */ @@ -80,7 +80,7 @@ sudo_passwd_verify(const Expand All struct sudoers_context *ctx, struct passwd *pw, */ 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; } @@ -90,30 +90,37 @@ sudo_passwd_verify(const Expand All struct sudoers_context *ctx, struct passwd *pw, * 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; } } explicit_bzero(des_pass, sizeof(des_pass)); debug_return_int(matched ? AUTH_SUCCESS : AUTH_FAILURE); debug_return_int(ret); } #else int sudo_passwd_verify(const struct sudoers_context *ctx, struct passwd *pw, const 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 Expand Down 51 changes: 36 additions & 15 deletions 51 plugins/sudoers/auth/ sudo_auth.c [*] Show comments View file Edit file Delete file This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters Show hidden characters Original Diff file line line Diff line change number number Expand Up @@ -116,10 +116,16 @@ sudo_auth_init(const struct sudoers_context *ctx, struct passwd *pw, if (auth->init && !IS_DISABLED(auth)) { /* Disable if it failed to init unless there was a fatal error. */ status = (auth->init)(ctx, pw, auth); if (status == AUTH_FAILURE) switch (status) { case AUTH_SUCCESS: break; case AUTH_FAILURE: SET(auth->flags, FLAG_DISABLED); else if (status == AUTH_ERROR) break; /* assume error msg already printed */ break; default: /* Assume error msg already printed. */ debug_return_int(-1); } } } Expand Down Expand @@ -166,7 +172,7 @@ sudo_auth_init(const struct Up sudoers_context *ctx, struct passwd *pw, } } debug_return_int(status == AUTH_ERROR ? -1 : 0 ); debug_return_int(0); } /* Expand Down Expand @@ -209,7 +215,7 @@ sudo_auth_cleanup(const Up struct sudoers_context *ctx, struct passwd *pw, for (auth = auth_switch; auth->name; auth++) { if (auth->cleanup && !IS_DISABLED(auth)) { int status = (auth->cleanup)(ctx, pw, auth, force); if (status == AUTH_ERROR) { if (status != AUTH_SUCCESS) { /* Assume error msg already printed. */ debug_return_int(-1); } Expand Down Expand @@ -306,7 +312,7 @@ verify_user(const struct Up sudoers_context *ctx, struct passwd *pw, char *prompt, SET(auth->flags, FLAG_DISABLED); else if (status == AUTH_NONINTERACTIVE) goto done; else if (status == AUTH_ERROR || user_interrupted()) else if (status != AUTH_SUCCESS || user_interrupted()) goto done; /* assume error msg already printed */ } } Expand Down Expand @@ -365,7 +371,6 @@ verify_user(const struct Up sudoers_context *ctx, struct passwd *pw, char *prompt, case AUTH_NONINTERACTIVE: SET(validated, FLAG_NO_USER_INPUT); FALLTHROUGH; case AUTH_ERROR: default: log_auth_failure(ctx, validated, 0); ret = -1; @@ -377,25 +382,33 @@ verify_user(const struct Expand All sudoers_context *ctx, struct passwd *pw, char *prompt, /* * 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(const struct sudoers_context *ctx, 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)(ctx, 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 Expand All @@ -416,25 +429,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(void) { 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)(auth); if (status == AUTH_ERROR) { 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); } /* Expand Down 12 changes: 6 additions & 6 deletions 12 plugins/sudoers/auth/ sudo_auth.h [*] Show comments View file Edit file Delete file This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters Show hidden characters Original file Diff line Diff line change line number number Expand Up @@ -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_ERROR 3 #define AUTH_NONINTERACTIVE 4 /* Auth function return values (rowhammer resistent). */ This comment has been minimized. Sign in to view Sorry, something went wrong. Copy link Quote reply @pavlinux pavlinux Dec 25, 2023 * edited Makefile: gcc -DRND1=0x$(openssl rand -hex 4) \ -DRND2=0x$(openssl rand -hex 4) \ -DRND3=0x$(openssl rand -hex 4) \ -DRND4=0x$(openssl rand -hex 4) \ -DRND5=0x$(openssl rand -hex 4) ... sudo_auth.h #define AUTH_SUCCESS RND1 /* kh.z. */ #define AUTH_FAILURE RND2 /* kh.z. */ #define AUTH_INTR RND3 /* kh.z. */ #define AUTH_ERROR RND4 /* kh.z. */ #define AUTH_NONINTERACTIVE RND5 /* kh.z. */ 3 pavlinux, finwarman, and hedger reacted with laugh emoji All reactions * 3 reactions This comment has been minimized. Sign in to view Sorry, something went wrong. Copy link Quote reply @millert millert Dec 26, 2023 Author Collaborator The values used were chosen such that it takes a large number of bit flips to change from allowed to denied. Using random values doesn't really protect against this attack. 2 henry701 and colejohnson66 reacted with thumbs up emoji 1 jakiestfu reacted with hooray emoji 2 danodonovan and jakiestfu reacted with rocket emoji All reactions * 2 reactions * 1 reaction * 2 reactions #define AUTH_SUCCESS 0x52a2925 /* 0101001010100010100100100101 */ #define AUTH_FAILURE 0xad5d6da /* 1010110101011101011011011010 */ #define AUTH_INTR 0x69d61fc8 /* 1101001110101100001111111001000 */ #define AUTH_ERROR 0x1629e037 /* 0010110001010011110000000110111 */ #define AUTH_NONINTERACTIVE 0x1fc8d3ac /* 11111110010001101001110101100 */ typedef struct sudo_auth { unsigned int flags; /* various flags, see below */ Expand Down 12 changes: 6 additions & 6 deletions 12 plugins/sudoers/lookup.c [*] Show comments View file Edit file Delete file This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters Show hidden characters Original file Diff line number line Diff line change number @@ -100,7 +100,7 @@ sudoers_lookup_pseudo Expand Up (struct sudo_nss_list *snl, struct sudoers_context *ctx, int user_match = userlist_matches(nss-> parse_tree, ctx->user.pw, &us->users); if (user_match != ALLOW) { if (callback != NULL && user_match != UNSPEC) { if (callback != NULL && user_match == DENY) { callback(nss->parse_tree, us, user_match, NULL , UNSPEC, NULL, UNSPEC, UNSPEC, UNSPEC, cb_data); } Expand Down Expand @@ -189,7 +189,7 @@ sudoers_lookup_pseudo Up (struct sudo_nss_list *snl, struct sudoers_context *ctx, host_match, cs, date_match, runas_match, cmnd_match, cb_data); } if (cmnd_match != UNSPEC) { if (SPECIFIED(cmnd_match)) { /* * We take the last match but must process * the entire policy for pwcheck == all. Expand Down Expand @@ -245,7 +245,7 @@ sudoers_lookup_check Up (struct sudo_nss *nss, struct sudoers_context *ctx, TAILQ_FOREACH_REVERSE(us, &nss->parse_tree-> userspecs, userspec_list, entries) { int user_match = userlist_matches(nss-> parse_tree, ctx->user.pw, &us->users); if (user_match != ALLOW) { if (callback != NULL && user_match != UNSPEC) { if (callback != NULL && user_match == DENY) { callback(nss->parse_tree, us, user_match, NULL , UNSPEC, NULL, UNSPEC, UNSPEC, UNSPEC, cb_data); } Expand Down Expand @@ -290,7 +290,7 @@ sudoers_lookup_check Up (struct sudo_nss *nss, struct sudoers_context *ctx, cs, date_match, runas_match, cmnd_match, cb_data); } if (cmnd_match != UNSPEC) { if (SPECIFIED(cmnd_match)) { /* * If user is running command as themselves, * set ctx->runas.pw = ctx->user.pw. Expand Down Expand @@ -542,15 +542,15 @@ sudoers_lookup(struct Up sudo_nss_list *snl, struct sudoers_context *ctx, m = sudoers_lookup_check(nss, ctx, &validated, &info, now, callback, cb_data, &cs, &defs); if (m != UNSPEC) { if (SPECIFIED(m)) { match = m; parse_tree = nss->parse_tree; } if (!sudo_nss_can_continue(nss, m)) break; } if (match != UNSPEC) { if (SPECIFIED(match)) { if (info.cmnd_path != NULL) { /* Update cmnd, cmnd_stat, cmnd_status from matching entry. */ free(ctx->user.cmnd); Expand Down 25 changes: 13 additions & 12 deletions 25 plugins/sudoers/match.c [*] Show comments View file Edit file Delete file This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters Show hidden characters Original file Diff line number line Diff line change number Expand Up @@ -91,7 +91,7 @@ user_matches(const struct sudoers_parse_tree *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 { Expand Down Expand @@ -123,7 +123,8 @@ userlist_matches(const Up struct sudoers_parse_tree *parse_tree, 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); Expand Down Expand @@ -184,7 +185,7 @@ runas_userlist_matches Up (const struct sudoers_parse_tree *parse_tree, if (a != NULL) { const int rc = runas_userlist_matches( parse_tree, &a->members, matching_user); if (rc != UNSPEC) { if (SPECIFIED(rc)) { if (m->negated) { user_matched = rc == ALLOW ? DENY : ALLOW; } else { Expand All @@ -211,7 +212,7 @@ runas_userlist_matches (const struct sudoers_parse_tree *parse_tree, 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; Expand Down Expand @@ -246,7 +247,7 @@ runas_grouplist_matches Up (const struct sudoers_parse_tree *parse_tree, if (a != NULL) { const int rc = runas_grouplist_matches( parse_tree, &a->members, matching_group); if (rc != UNSPEC) { if (SPECIFIED(rc)) { if (m->negated) { group_matched = rc == ALLOW ? DENY : ALLOW; } else { Expand All @@ -262,14 +263,14 @@ runas_grouplist_matches (const struct sudoers_parse_tree *parse_tree, 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. Expand Down Expand @@ -349,7 +350,7 @@ hostlist_matches_int(const Up struct sudoers_parse_tree *parse_tree, 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); Expand Down Expand @@ -402,7 +403,7 @@ host_matches(const struct Up sudoers_parse_tree *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 { Expand Down Expand @@ -440,7 +441,7 @@ cmndlist_matches(const Up struct sudoers_parse_tree *parse_tree, 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); Expand Down Expand @@ -471,7 +472,7 @@ cmnd_matches(const struct Up sudoers_parse_tree *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 { Expand Down Expand @@ -511,7 +512,7 @@ cmnd_matches_all(const Up struct sudoers_parse_tree *parse_tree, if (a != NULL) { TAILQ_FOREACH_REVERSE(m, &a->members, member_list, entries) { matched = cmnd_matches_all(parse_tree, m, runchroot, info); if (matched != UNSPEC) { if (SPECIFIED(matched)) { if (negated) matched = matched == ALLOW ? DENY : ALLOW; break; Expand Down Oops, something went wrong. Retry Toggle all file notes Toggle all file annotations 0 comments on commit 7873f83 Please sign in to comment. Footer (c) 2024 GitHub, Inc. Footer navigation * Terms * Privacy * Security * Status * Docs * Contact * Manage cookies * Do not share my personal information You can't perform that action at this time.