From ab88bb2f381b5e4dfa9de674ef51d93458c7242c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 15 Sep 2026 13:18:33 +0200 Subject: [PATCH] pam_systemd: even more debug messages This level of debugging might be over the top, so I kept this commit separate. If reviewers think that it's too much, let's drop it. It turns out that SELinux defeats getenv_secure(), so the expected variables are not read, c.f. https://bugzilla.redhat.com/show_bug.cgi?id=2533000#c8. --- src/login/pam_systemd.c | 80 ++++++++++++++++++++++------------------- 1 file changed, 44 insertions(+), 36 deletions(-) diff --git a/src/login/pam_systemd.c b/src/login/pam_systemd.c index 13330c9e3c..ee8b942ea4 100644 --- a/src/login/pam_systemd.c +++ b/src/login/pam_systemd.c @@ -474,7 +474,7 @@ static int append_session_io_weight(pam_handle_t *pamh, sd_bus_message *m, const return sd_bus_message_append(m, "(sv)", "IOWeight", "t", val); } -static const char* getenv_harder(pam_handle_t *pamh, const char *key, const char *fallback) { +static const char* getenv_harder(pam_handle_t *pamh, bool debug, const char *key, const char *fallback) { const char *v; assert(pamh); @@ -487,62 +487,70 @@ static const char* getenv_harder(pam_handle_t *pamh, const char *key, const char * can still be set from the unit file Environment= block. */ v = sym_pam_getenv(pamh, key); - if (!isempty(v)) + if (!isempty(v)) { + pam_debug_syslog(pamh, debug, "Found PAM environment variable %s='%s'", key, v); return v; + } /* We use secure_getenv() here, since we might get loaded into su/sudo, which are SUID. Ideally * they'd clean up the environment before invoking foreign code (such as PAM modules), but alas they * currently don't (to be precise, they clean up the environment they pass to their children, but * not their own environ[]). */ v = secure_getenv(key); - if (!isempty(v)) + if (!isempty(v)) { + pam_debug_syslog(pamh, debug, "Found normal environment variable %s='%s'", key, v); return v; + } + if (fallback) + pam_debug_syslog(pamh, debug, "Using fallback %s='%s'", key, fallback); return fallback; } -static bool getenv_harder_bool(pam_handle_t *pamh, const char *key, bool fallback) { +static bool getenv_harder_bool(pam_handle_t *pamh, bool debug, const char *key, bool fallback) { const char *v; int r; assert(pamh); assert(key); - v = getenv_harder(pamh, key, NULL); - if (isempty(v)) - return fallback; - - r = parse_boolean(v); - if (r < 0) { - sym_pam_syslog(pamh, LOG_WARNING, - "Failed to parse environment variable value '%s' of '%s', falling back to using '%s'.", - v, key, true_false(fallback)); + v = getenv_harder(pamh, debug, key, NULL); + if (!v) { + pam_debug_syslog(pamh, debug, "Using fallback %s=%s", key, true_false(fallback)); return fallback; } - return r; + r = parse_boolean(v); + if (r >= 0) + return r; + + sym_pam_syslog(pamh, LOG_WARNING, + "Failed to parse environment variable value '%s', using fallback %s=%s", + v, key, true_false(fallback)); + return fallback; } -static uint32_t getenv_harder_uint32(pam_handle_t *pamh, const char *key, uint32_t fallback) { +static uint32_t getenv_harder_uint32(pam_handle_t *pamh, bool debug, const char *key, uint32_t fallback) { int r; assert(pamh); assert(key); - const char *v = getenv_harder(pamh, key, NULL); - if (isempty(v)) - return fallback; - - uint32_t u; - r = safe_atou32(v, &u); - if (r < 0) { - sym_pam_syslog(pamh, LOG_WARNING, - "Failed to parse environment variable value '%s' of '%s' as unsigned integer, falling back to using %" PRIu32 ".", - v, key, fallback); + const char *v = getenv_harder(pamh, debug, key, NULL); + if (!v) { + pam_debug_syslog(pamh, debug, "Using fallback %s=%"PRIu32, key, fallback); return fallback; } - return u; + uint32_t u; + r = safe_atou32(v, &u); + if (r >= 0) + return u; + + pam_syslog_errno(pamh, LOG_WARNING, r, + "Failed to parse environment variable value '%s' as unsigned integer, using fallback %s=%"PRIu32 ": %m", + v, key, fallback); + return fallback; } static int update_environment(pam_handle_t *pamh, const char *key, const char *value) { @@ -1854,15 +1862,15 @@ _public_ PAM_EXTERN int pam_sm_open_session( if (r != PAM_SUCCESS) return pam_syslog_pam_error(pamh, LOG_ERR, r, "Failed to get PAM items: @PAMERR@"); - c.seat = getenv_harder(pamh, "XDG_SEAT", NULL); - c.vtnr = getenv_harder_uint32(pamh, "XDG_VTNR", 0); - c.type = getenv_harder(pamh, "XDG_SESSION_TYPE", type_pam); - c.class = getenv_harder(pamh, "XDG_SESSION_CLASS", class_pam); - c.desktop = getenv_harder(pamh, "XDG_SESSION_DESKTOP", desktop_pam); - c.area = getenv_harder(pamh, "XDG_AREA", area_pam); - c.incomplete = getenv_harder_bool(pamh, "XDG_SESSION_INCOMPLETE", false); - inhibit_what = getenv_harder(pamh, "XDG_SESSION_INHIBIT", inhibit_what); - inhibit_why = getenv_harder(pamh, "XDG_SESSION_INHIBIT_WHY", inhibit_why); + c.seat = getenv_harder(pamh, debug, "XDG_SEAT", NULL); + c.vtnr = getenv_harder_uint32(pamh, debug, "XDG_VTNR", 0); + c.type = getenv_harder(pamh, debug, "XDG_SESSION_TYPE", type_pam); + c.class = getenv_harder(pamh, debug, "XDG_SESSION_CLASS", class_pam); + c.desktop = getenv_harder(pamh, debug, "XDG_SESSION_DESKTOP", desktop_pam); + c.area = getenv_harder(pamh, debug, "XDG_AREA", area_pam); + c.incomplete = getenv_harder_bool(pamh, debug, "XDG_SESSION_INCOMPLETE", false); + inhibit_what = getenv_harder(pamh, debug, "XDG_SESSION_INHIBIT", inhibit_what); + inhibit_why = getenv_harder(pamh, debug, "XDG_SESSION_INHIBIT_WHY", inhibit_why); pam_debug_syslog(pamh, debug, "Initial state: " @@ -1879,7 +1887,7 @@ _public_ PAM_EXTERN int pam_sm_open_session( yes_no(c.incomplete), strempty(inhibit_what)); - const char *extra_device_access = getenv_harder(pamh, "XDG_SESSION_EXTRA_DEVICE_ACCESS", NULL); + const char *extra_device_access = getenv_harder(pamh, debug, "XDG_SESSION_EXTRA_DEVICE_ACCESS", NULL); if (extra_device_access) { c.extra_device_access = strv_split(extra_device_access, ":"); if (!c.extra_device_access)