From: Zbigniew Jędrzejewski-Szmek Date: Tue, 19 May 2020 08:09:14 +0000 (+0200) Subject: Merge pull request #15794 from poettering/pam-sudo-fixes-part2 X-Git-Tag: v246-rc1~340 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=5e375a1ef2d97b1d7561bd224003217d223c2f05;hp=201fa8f256ed4884839d9ec3dfef3c0f2c0d2bf0;p=thirdparty%2Fsystemd.git Merge pull request #15794 from poettering/pam-sudo-fixes-part2 pam_systemd/pam_systemd_home: fix caching --- diff --git a/src/home/pam_systemd_home.c b/src/home/pam_systemd_home.c index 7662fa69ebf..72ce062f54e 100644 --- a/src/home/pam_systemd_home.c +++ b/src/home/pam_systemd_home.c @@ -18,11 +18,6 @@ #include "user-record.h" #include "user-util.h" -/* Used for the "systemd-user-record-is-homed" PAM data field, to indicate whether we know whether this user - * record is managed by homed or by something else. */ -#define USER_RECORD_IS_HOMED INT_TO_PTR(1) -#define USER_RECORD_IS_OTHER INT_TO_PTR(2) - static int parse_argv( pam_handle_t *handle, int argc, const char **argv, @@ -67,27 +62,30 @@ static int parse_argv( static int acquire_user_record( pam_handle_t *handle, + const char *username, UserRecord **ret_record) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; _cleanup_(json_variant_unrefp) JsonVariant *v = NULL; _cleanup_(user_record_unrefp) UserRecord *ur = NULL; _cleanup_(sd_bus_unrefp) sd_bus *bus = NULL; - const char *username = NULL, *json = NULL; - const void *b = NULL; + _cleanup_free_ char *homed_field = NULL; + const char *json = NULL; int r; assert(handle); - r = pam_get_user(handle, &username, NULL); - if (r != PAM_SUCCESS) { - pam_syslog(handle, LOG_ERR, "Failed to get user name: %s", pam_strerror(handle, r)); - return r; - } + if (!username) { + r = pam_get_user(handle, &username, NULL); + if (r != PAM_SUCCESS) { + pam_syslog(handle, LOG_ERR, "Failed to get user name: %s", pam_strerror(handle, r)); + return r; + } - if (isempty(username)) { - pam_syslog(handle, LOG_ERR, "User name not set."); - return PAM_SERVICE_ERR; + if (isempty(username)) { + pam_syslog(handle, LOG_ERR, "User name not set."); + return PAM_SERVICE_ERR; + } } /* Let's bypass all IPC complexity for the two user names we know for sure we don't manage, and for @@ -95,31 +93,30 @@ static int acquire_user_record( if (STR_IN_SET(username, "root", NOBODY_USER_NAME) || !valid_user_group_name(username, 0)) return PAM_USER_UNKNOWN; - /* Let's check if a previous run determined that this user is not managed by homed. If so, let's exit early */ - r = pam_get_data(handle, "systemd-user-record-is-homed", &b); + /* We cache the user record in the PAM context. We use a field name that includes the username, since + * clients might change the user name associated with a PAM context underneath us. Notably, 'sudo' + * creates a single PAM context and first authenticates it with the user set to the originating user, + * then updates the user for the destination user and issues the session stack with the same PAM + * context. We thus must be prepared that the user record changes between calls and we keep any + * caching separate. */ + homed_field = strjoin("systemd-home-user-record-", username); + if (!homed_field) + return pam_log_oom(handle); + + /* Let's use the cache, so that we can share it between the session and the authentication hooks */ + r = pam_get_data(handle, homed_field, (const void**) &json); if (!IN_SET(r, PAM_SUCCESS, PAM_NO_MODULE_DATA)) { - /* Failure */ - pam_syslog(handle, LOG_ERR, "Failed to get PAM user-record-is-homed flag: %s", pam_strerror(handle, r)); + pam_syslog(handle, LOG_ERR, "Failed to get PAM user record data: %s", pam_strerror(handle, r)); return r; - } else if (b == NULL) - /* Nothing cached yet, need to acquire fresh */ - json = NULL; - else if (b != USER_RECORD_IS_HOMED) - /* Definitely not a homed record */ - return PAM_USER_UNKNOWN; - else { - /* It's a homed record, let's use the cache, so that we can share it between the session and - * the authentication hooks */ - r = pam_get_data(handle, "systemd-user-record", (const void**) &json); - if (!IN_SET(r, PAM_SUCCESS, PAM_NO_MODULE_DATA)) { - pam_syslog(handle, LOG_ERR, "Failed to get PAM user record data: %s", pam_strerror(handle, r)); - return r; - } } - - if (!json) { + if (r == PAM_SUCCESS && json) { + /* We determined earlier that this is not a homed user? Then exit early. (We use -1 as + * negative cache indicator) */ + if (json == (void*) -1) + return PAM_USER_UNKNOWN; + } else { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; - _cleanup_free_ char *json_copy = NULL; + _cleanup_free_ char *generic_field = NULL, *json_copy = NULL; r = pam_acquire_bus_connection(handle, &bus); if (r != PAM_SUCCESS) @@ -146,23 +143,38 @@ static int acquire_user_record( if (r < 0) return pam_bus_log_parse_error(handle, r); + /* First copy: for the homed-specific data field, i.e. where we know the user record is from + * homed */ json_copy = strdup(json); if (!json_copy) return pam_log_oom(handle); - r = pam_set_data(handle, "systemd-user-record", json_copy, pam_cleanup_free); + r = pam_set_data(handle, homed_field, json_copy, pam_cleanup_free); if (r != PAM_SUCCESS) { - pam_syslog(handle, LOG_ERR, "Failed to set PAM user record data: %s", pam_strerror(handle, r)); + pam_syslog(handle, LOG_ERR, "Failed to set PAM user record data '%s': %s", + homed_field, pam_strerror(handle, r)); return r; } - TAKE_PTR(json_copy); + /* Take a second copy: for the generic data field, the one which we share with + * pam_systemd. While we insist on only reusing homed records, pam_systemd is fine with homed + * and non-homed user records. */ + json_copy = strdup(json); + if (!json_copy) + return pam_log_oom(handle); + + generic_field = strjoin("systemd-user-record-", username); + if (!generic_field) + return pam_log_oom(handle); - r = pam_set_data(handle, "systemd-user-record-is-homed", USER_RECORD_IS_HOMED, NULL); + r = pam_set_data(handle, generic_field, json_copy, pam_cleanup_free); if (r != PAM_SUCCESS) { - pam_syslog(handle, LOG_ERR, "Failed to set PAM user record is homed flag: %s", pam_strerror(handle, r)); + pam_syslog(handle, LOG_ERR, "Failed to set PAM user record data '%s': %s", + homed_field, pam_strerror(handle, r)); return r; } + + TAKE_PTR(json_copy); } r = json_parse(json, JSON_PARSE_SENSITIVE, &v, NULL, NULL); @@ -181,6 +193,7 @@ static int acquire_user_record( return PAM_SERVICE_ERR; } + /* Safety check if cached record actually matches what we are looking for */ if (!streq_ptr(username, ur->user_name)) { pam_syslog(handle, LOG_ERR, "Acquired user record does not match user name."); return PAM_SERVICE_ERR; @@ -193,23 +206,36 @@ static int acquire_user_record( user_unknown: /* Cache this, so that we don't check again */ - r = pam_set_data(handle, "systemd-user-record-is-homed", USER_RECORD_IS_OTHER, NULL); + r = pam_set_data(handle, homed_field, (void*) -1, NULL); if (r != PAM_SUCCESS) - pam_syslog(handle, LOG_ERR, "Failed to set PAM user-record-is-homed flag, ignoring: %s", pam_strerror(handle, r)); + pam_syslog(handle, LOG_ERR, "Failed to set PAM user record data '%s' to invalid, ignoring: %s", + homed_field, pam_strerror(handle, r)); return PAM_USER_UNKNOWN; } -static int release_user_record(pam_handle_t *handle) { +static int release_user_record(pam_handle_t *handle, const char *username) { + _cleanup_free_ char *homed_field = NULL, *generic_field = NULL; int r, k; - r = pam_set_data(handle, "systemd-user-record", NULL, NULL); + assert(handle); + assert(username); + + homed_field = strjoin("systemd-home-user-record-", username); + if (!homed_field) + return pam_log_oom(handle); + + r = pam_set_data(handle, homed_field, NULL, NULL); if (r != PAM_SUCCESS) - pam_syslog(handle, LOG_ERR, "Failed to release PAM user record data: %s", pam_strerror(handle, r)); + pam_syslog(handle, LOG_ERR, "Failed to release PAM user record data '%s': %s", homed_field, pam_strerror(handle, r)); - k = pam_set_data(handle, "systemd-user-record-is-homed", NULL, NULL); + generic_field = strjoin("systemd-user-record-", username); + if (!generic_field) + return pam_log_oom(handle); + + k = pam_set_data(handle, generic_field, NULL, NULL); if (k != PAM_SUCCESS) - pam_syslog(handle, LOG_ERR, "Failed to release PAM user-record-is-homed flag: %s", pam_strerror(handle, k)); + pam_syslog(handle, LOG_ERR, "Failed to release PAM user record data '%s': %s", generic_field, pam_strerror(handle, k)); return IN_SET(r, PAM_SUCCESS, PAM_NO_MODULE_DATA) ? k : r; } @@ -395,7 +421,9 @@ static int acquire_home( bool do_auth = please_authenticate, home_not_active = false, home_locked = false; _cleanup_(sd_bus_unrefp) sd_bus *bus = NULL; _cleanup_close_ int acquired_fd = -1; + _cleanup_free_ char *fd_field = NULL; const void *home_fd_ptr = NULL; + const char *username = NULL; unsigned n_attempts = 0; int r; @@ -409,8 +437,27 @@ static int acquire_home( * authenticates, while the other PAM hooks unset it so that they can a ref of their own without * authentication if possible, but with authentication if necessary. */ + r = pam_get_user(handle, &username, NULL); + if (r != PAM_SUCCESS) { + pam_syslog(handle, LOG_ERR, "Failed to get user name: %s", pam_strerror(handle, r)); + return r; + } + + if (isempty(username)) { + pam_syslog(handle, LOG_ERR, "User name not set."); + return PAM_SERVICE_ERR; + } + /* If we already have acquired the fd, let's shortcut this */ - r = pam_get_data(handle, "systemd-home-fd", &home_fd_ptr); + fd_field = strjoin("systemd-home-fd-", username); + if (!fd_field) + return pam_log_oom(handle); + + r = pam_get_data(handle, fd_field, &home_fd_ptr); + if (!IN_SET(r, PAM_SUCCESS, PAM_NO_MODULE_DATA)) { + pam_syslog(handle, LOG_ERR, "Failed to retrieve PAM home reference fd: %s", pam_strerror(handle, r)); + return r; + } if (r == PAM_SUCCESS && PTR_TO_FD(home_fd_ptr) >= 0) return PAM_SUCCESS; @@ -418,7 +465,7 @@ static int acquire_home( if (r != PAM_SUCCESS) return r; - r = acquire_user_record(handle, &ur); + r = acquire_user_record(handle, username, &ur); if (r != PAM_SUCCESS) return r; @@ -534,7 +581,7 @@ static int acquire_home( do_auth = true; } - r = pam_set_data(handle, "systemd-home-fd", FD_TO_PTR(acquired_fd), cleanup_home_fd); + r = pam_set_data(handle, fd_field, FD_TO_PTR(acquired_fd), cleanup_home_fd); if (r < 0) { pam_syslog(handle, LOG_ERR, "Failed to set PAM bus data: %s", pam_strerror(handle, r)); return r; @@ -545,7 +592,7 @@ static int acquire_home( /* We likely just activated the home directory, let's flush out the user record, since a * newer embedded user record might have been acquired from the activation. */ - r = release_user_record(handle); + r = release_user_record(handle, ur->user_name); if (!IN_SET(r, PAM_SUCCESS, PAM_NO_MODULE_DATA)) return r; } @@ -555,15 +602,27 @@ static int acquire_home( return PAM_SUCCESS; } -static int release_home_fd(pam_handle_t *handle) { +static int release_home_fd(pam_handle_t *handle, const char *username) { + _cleanup_free_ char *fd_field = NULL; const void *home_fd_ptr = NULL; int r; - r = pam_get_data(handle, "systemd-home-fd", &home_fd_ptr); - if (r == PAM_NO_MODULE_DATA || PTR_TO_FD(home_fd_ptr) < 0) + assert(handle); + assert(username); + + fd_field = strjoin("systemd-home-fd-", username); + if (!fd_field) + return pam_log_oom(handle); + + r = pam_get_data(handle, fd_field, &home_fd_ptr); + if (r == PAM_NO_MODULE_DATA || (r == PAM_SUCCESS && PTR_TO_FD(home_fd_ptr) < 0)) return PAM_NO_MODULE_DATA; + if (r != PAM_SUCCESS) { + pam_syslog(handle, LOG_ERR, "Failed to retrieve PAM home reference fd: %s", pam_strerror(handle, r)); + return r; + } - r = pam_set_data(handle, "systemd-home-fd", NULL, NULL); + r = pam_set_data(handle, fd_field, NULL, NULL); if (r != PAM_SUCCESS) pam_syslog(handle, LOG_ERR, "Failed to release PAM home reference fd: %s", pam_strerror(handle, r)); @@ -650,20 +709,25 @@ _public_ PAM_EXTERN int pam_sm_close_session( if (debug) pam_syslog(handle, LOG_DEBUG, "pam-systemd-homed session end"); + r = pam_get_user(handle, &username, NULL); + if (r != PAM_SUCCESS) { + pam_syslog(handle, LOG_ERR, "Failed to get user name: %s", pam_strerror(handle, r)); + return r; + } + + if (isempty(username)) { + pam_syslog(handle, LOG_ERR, "User name not set."); + return PAM_SERVICE_ERR; + } + /* Let's explicitly drop the reference to the homed session, so that the subsequent ReleaseHome() * call will be able to do its thing. */ - r = release_home_fd(handle); + r = release_home_fd(handle, username); if (r == PAM_NO_MODULE_DATA) /* Nothing to do, we never acquired an fd */ return PAM_SUCCESS; if (r != PAM_SUCCESS) return r; - r = pam_get_user(handle, &username, NULL); - if (r != PAM_SUCCESS) { - pam_syslog(handle, LOG_ERR, "Failed to get user name: %s", pam_strerror(handle, r)); - return r; - } - r = pam_acquire_bus_connection(handle, &bus); if (r != PAM_SUCCESS) return r; @@ -715,7 +779,7 @@ _public_ PAM_EXTERN int pam_sm_acct_mgmt( if (r != PAM_SUCCESS) return r; - r = acquire_user_record(handle, &ur); + r = acquire_user_record(handle, NULL, &ur); if (r != PAM_SUCCESS) return r; @@ -823,7 +887,7 @@ _public_ PAM_EXTERN int pam_sm_chauthtok( if (r != PAM_SUCCESS) return r; - r = acquire_user_record(handle, &ur); + r = acquire_user_record(handle, NULL, &ur); if (r != PAM_SUCCESS) return r; diff --git a/src/login/pam_systemd.c b/src/login/pam_systemd.c index b6f5562707c..96ce4742f01 100644 --- a/src/login/pam_systemd.c +++ b/src/login/pam_systemd.c @@ -99,6 +99,7 @@ static int acquire_user_record( _cleanup_(user_record_unrefp) UserRecord *ur = NULL; const char *username = NULL, *json = NULL; + _cleanup_free_ char *field = NULL; int r; assert(handle); @@ -114,39 +115,18 @@ static int acquire_user_record( return PAM_SERVICE_ERR; } - /* If pam_systemd_homed (or some other module) already acqired the user record we can reuse it + /* If pam_systemd_homed (or some other module) already acquired the user record we can reuse it * here. */ - r = pam_get_data(handle, "systemd-user-record", (const void**) &json); - if (r != PAM_SUCCESS || !json) { - _cleanup_free_ char *formatted = NULL; - - if (!IN_SET(r, PAM_SUCCESS, PAM_NO_MODULE_DATA)) { - pam_syslog(handle, LOG_ERR, "Failed to get PAM user record data: %s", pam_strerror(handle, r)); - return r; - } - - /* Request the record ourselves */ - r = userdb_by_name(username, 0, &ur); - if (r < 0) { - pam_syslog(handle, LOG_ERR, "Failed to get user record: %s", strerror_safe(r)); - return PAM_USER_UNKNOWN; - } - - r = json_variant_format(ur->json, 0, &formatted); - if (r < 0) { - pam_syslog(handle, LOG_ERR, "Failed to format user JSON: %s", strerror_safe(r)); - return PAM_SERVICE_ERR; - } - - /* And cache it for everyone else */ - r = pam_set_data(handle, "systemd-user-record", formatted, pam_cleanup_free); - if (r < 0) { - pam_syslog(handle, LOG_ERR, "Failed to set PAM user record data: %s", pam_strerror(handle, r)); - return r; - } + field = strjoin("systemd-user-record-", username); + if (!field) + return pam_log_oom(handle); - TAKE_PTR(formatted); - } else { + r = pam_get_data(handle, field, (const void**) &json); + if (!IN_SET(r, PAM_SUCCESS, PAM_NO_MODULE_DATA)) { + pam_syslog(handle, LOG_ERR, "Failed to get PAM user record data: %s", pam_strerror(handle, r)); + return r; + } + if (r == PAM_SUCCESS && json) { _cleanup_(json_variant_unrefp) JsonVariant *v = NULL; /* Parse cached record */ @@ -171,6 +151,31 @@ static int acquire_user_record( pam_syslog(handle, LOG_ERR, "Acquired user record does not match user name."); return PAM_SERVICE_ERR; } + } else { + _cleanup_free_ char *formatted = NULL; + + /* Request the record ourselves */ + r = userdb_by_name(username, 0, &ur); + if (r < 0) { + pam_syslog(handle, LOG_ERR, "Failed to get user record: %s", strerror_safe(r)); + return PAM_USER_UNKNOWN; + } + + r = json_variant_format(ur->json, 0, &formatted); + if (r < 0) { + pam_syslog(handle, LOG_ERR, "Failed to format user JSON: %s", strerror_safe(r)); + return PAM_SERVICE_ERR; + } + + /* And cache it for everyone else */ + r = pam_set_data(handle, field, formatted, pam_cleanup_free); + if (r < 0) { + pam_syslog(handle, LOG_ERR, "Failed to set PAM user record data '%s': %s", + field, pam_strerror(handle, r)); + return r; + } + + TAKE_PTR(formatted); } if (!uid_is_valid(ur->uid)) {