From c3c23f9515e5894719b1c08e2062f0280c147537165438024d9e804dd33f68f4 Mon Sep 17 00:00:00 2001 From: Christophe Marin Date: Mon, 13 Mar 2023 16:17:31 +0000 Subject: [PATCH] Accepting request 1071111 from home:Vogtinator:plasma5.27 - Add patches for handling edge cases and hardening: * 0001-Verify-that-XDG_RUNTIME_DIR-is-usable.patch * 0002-Don-t-do-anything-if-the-password-is-empty.patch * 0003-Exit-early-if-the-target-user-is-root.patch * 0004-Don-t-call-pam_sm_open_session-within-pam_sm_authent.patch OBS-URL: https://build.opensuse.org/request/show/1071111 OBS-URL: https://build.opensuse.org/package/show/KDE:Frameworks5/pam_kwallet?expand=0&rev=264 --- ...erify-that-XDG_RUNTIME_DIR-is-usable.patch | 37 +++++++++++ ...do-anything-if-the-password-is-empty.patch | 30 +++++++++ ...xit-early-if-the-target-user-is-root.patch | 42 +++++++++++++ ...m_open_session-within-pam_sm_authent.patch | 62 +++++++++++++++++++ pam_kwallet.changes | 9 +++ pam_kwallet.spec | 7 ++- 6 files changed, 186 insertions(+), 1 deletion(-) create mode 100644 0001-Verify-that-XDG_RUNTIME_DIR-is-usable.patch create mode 100644 0002-Don-t-do-anything-if-the-password-is-empty.patch create mode 100644 0003-Exit-early-if-the-target-user-is-root.patch create mode 100644 0004-Don-t-call-pam_sm_open_session-within-pam_sm_authent.patch diff --git a/0001-Verify-that-XDG_RUNTIME_DIR-is-usable.patch b/0001-Verify-that-XDG_RUNTIME_DIR-is-usable.patch new file mode 100644 index 0000000..cf81743 --- /dev/null +++ b/0001-Verify-that-XDG_RUNTIME_DIR-is-usable.patch @@ -0,0 +1,37 @@ +From 42f4dbd10b0f1a24d38513399f07936360920fa2 Mon Sep 17 00:00:00 2001 +From: Fabian Vogt +Date: Mon, 13 Mar 2023 10:07:22 +0100 +Subject: [PATCH 1/4] Verify that XDG_RUNTIME_DIR is usable + +It needs to be an existing directory with mode 0700 and owned by the user. +--- + pam_kwallet.c | 13 +++++++++++++ + 1 file changed, 13 insertions(+) + +diff --git a/pam_kwallet.c b/pam_kwallet.c +index e8fbc27..31e93aa 100644 +--- a/pam_kwallet.c ++++ b/pam_kwallet.c +@@ -455,6 +455,19 @@ static void start_kwallet(pam_handle_t *pamh, struct passwd *userInfo, const cha + snprintf(fullSocket, needed, "%s/%s_%s%s", socketPath, socketPrefix, userInfo->pw_name, ".socket"); + } else { + socketPath = get_env(pamh, "XDG_RUNTIME_DIR"); ++ // Check whether XDG_RUNTIME_DIR is usable ++ if (socketPath) { ++ struct stat rundir_stat; ++ if (stat(socketPath, &rundir_stat) != 0) { ++ pam_syslog(pamh, LOG_ERR, "%s: Failed to stat %s", logPrefix, socketPath); ++ socketPath = NULL; ++ } else if(!S_ISDIR(rundir_stat.st_mode) || (rundir_stat.st_mode & ~S_IFMT) != 0700 ++ || rundir_stat.st_uid != userInfo->pw_uid) { ++ pam_syslog(pamh, LOG_ERR, "%s: %s has wrong type, perms or ownership", logPrefix, socketPath); ++ socketPath = NULL; ++ } ++ } ++ + if (socketPath) { + size_t needed = snprintf(NULL, 0, "%s/%s%s", socketPath, socketPrefix, ".socket"); + needed += 1; +-- +2.39.2 + diff --git a/0002-Don-t-do-anything-if-the-password-is-empty.patch b/0002-Don-t-do-anything-if-the-password-is-empty.patch new file mode 100644 index 0000000..463f0e8 --- /dev/null +++ b/0002-Don-t-do-anything-if-the-password-is-empty.patch @@ -0,0 +1,30 @@ +From 09659874cc6cc3ab21314dc3b24a2db1bc77c46c Mon Sep 17 00:00:00 2001 +From: Fabian Vogt +Date: Mon, 13 Mar 2023 10:09:10 +0100 +Subject: [PATCH 2/4] Don't do anything if the password is empty + +If for some reason the password is empty (bug or intentionally configured), +avoid creating a possibly insecure hash. +--- + pam_kwallet.c | 5 +++++ + 1 file changed, 5 insertions(+) + +diff --git a/pam_kwallet.c b/pam_kwallet.c +index 31e93aa..2cd3758 100644 +--- a/pam_kwallet.c ++++ b/pam_kwallet.c +@@ -294,6 +294,11 @@ PAM_EXTERN int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, cons + return PAM_IGNORE; + } + ++ if (password[0] == '\0') { ++ pam_syslog(pamh, LOG_NOTICE, "%s: Empty or missing password, doing nothing", logPrefix); ++ return PAM_IGNORE; ++ } ++ + char *key = strdup(password); + result = pam_set_data(pamh, kwalletPamDataKey, key, cleanup_free); + +-- +2.39.2 + diff --git a/0003-Exit-early-if-the-target-user-is-root.patch b/0003-Exit-early-if-the-target-user-is-root.patch new file mode 100644 index 0000000..42e10a5 --- /dev/null +++ b/0003-Exit-early-if-the-target-user-is-root.patch @@ -0,0 +1,42 @@ +From 2126d9f148506d71ebc5576a91259c80e095f5ec Mon Sep 17 00:00:00 2001 +From: Fabian Vogt +Date: Mon, 13 Mar 2023 10:12:18 +0100 +Subject: [PATCH 3/4] Exit early if the target user is root + +kwallet should not be used as root user, so just refuse doing anything if +root is the target user. +--- + pam_kwallet.c | 10 ++++++++++ + 1 file changed, 10 insertions(+) + +diff --git a/pam_kwallet.c b/pam_kwallet.c +index 2cd3758..49be6c0 100644 +--- a/pam_kwallet.c ++++ b/pam_kwallet.c +@@ -265,6 +265,11 @@ PAM_EXTERN int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, cons + return PAM_IGNORE; + } + ++ if (userInfo->pw_uid == 0) { ++ pam_syslog(pamh, LOG_DEBUG, "%s: Refusing to do anything for the root user", logPrefix); ++ return PAM_IGNORE; ++ } ++ + const char *password; + result = pam_get_item(pamh, PAM_AUTHTOK, (const void**)&password); + +@@ -569,6 +574,11 @@ PAM_EXTERN int pam_sm_open_session(pam_handle_t *pamh, int flags, int argc, cons + return PAM_IGNORE; + } + ++ if (userInfo->pw_uid == 0) { ++ pam_syslog(pamh, LOG_DEBUG, "%s: Refusing to do anything for the root user", logPrefix); ++ return PAM_IGNORE; ++ } ++ + char *password; + result = pam_get_data(pamh, kwalletPamDataKey, (const void **)&password); + +-- +2.39.2 + diff --git a/0004-Don-t-call-pam_sm_open_session-within-pam_sm_authent.patch b/0004-Don-t-call-pam_sm_open_session-within-pam_sm_authent.patch new file mode 100644 index 0000000..365c271 --- /dev/null +++ b/0004-Don-t-call-pam_sm_open_session-within-pam_sm_authent.patch @@ -0,0 +1,62 @@ +From 574d9a78e0b4dc3fe898a109a27bf650e9a80cc3 Mon Sep 17 00:00:00 2001 +From: Fabian Vogt +Date: Mon, 13 Mar 2023 10:38:36 +0100 +Subject: [PATCH 4/4] Don't call pam_sm_open_session within pam_sm_authenticate + +It doesn't make sense to open a session before performing authentication. +Don't work around application (or configuration) bugs here. +--- + pam_kwallet.c | 22 +++------------------- + 1 file changed, 3 insertions(+), 19 deletions(-) + +diff --git a/pam_kwallet.c b/pam_kwallet.c +index 49be6c0..841e766 100644 +--- a/pam_kwallet.c ++++ b/pam_kwallet.c +@@ -314,14 +314,6 @@ PAM_EXTERN int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, cons + return PAM_IGNORE; + } + +- //if sm_open_session has already been called (but we did not have password), call it now +- const char *session_bit; +- result = pam_get_data(pamh, "sm_open_session", (const void **)&session_bit); +- if (result == PAM_SUCCESS) { +- pam_syslog(pamh, LOG_ERR, "%s: open_session was called before us, calling it now", logPrefix); +- return pam_sm_open_session(pamh, flags, argc, argv); +- } +- + //TODO unlock kwallet that is already executed + return PAM_IGNORE; + } +@@ -550,17 +542,9 @@ PAM_EXTERN int pam_sm_open_session(pam_handle_t *pamh, int flags, int argc, cons + return PAM_IGNORE; + } + +- int result; +- result = pam_set_data(pamh, "sm_open_session", "1", NULL); +- if (result != PAM_SUCCESS) { +- pam_syslog(pamh, LOG_ERR, "%s: Impossible to store sm_open_session: %s", +- logPrefix, pam_strerror(pamh, result)); +- return PAM_IGNORE; +- } +- +- //Fetch the user, needed to get user information ++ //Fetch the user, needed to get user information + const char *username; +- result = pam_get_user(pamh, &username, NULL); ++ int result = pam_get_user(pamh, &username, NULL); + if (result != PAM_SUCCESS) { + pam_syslog(pamh, LOG_ERR, "%s: Couldn't get username %s", + logPrefix, pam_strerror(pamh, result)); +@@ -584,7 +568,7 @@ PAM_EXTERN int pam_sm_open_session(pam_handle_t *pamh, int flags, int argc, cons + + if (result != PAM_SUCCESS) { + pam_syslog(pamh, LOG_INFO, "%s: open_session called without %s", logPrefix, kwalletPamDataKey); +- return PAM_SUCCESS;//We will wait for pam_sm_authenticate ++ return PAM_IGNORE; + } + + char *key = malloc(KWALLET_PAM_KEYSIZE); +-- +2.39.2 + diff --git a/pam_kwallet.changes b/pam_kwallet.changes index f6ba6f5..dd949a6 100644 --- a/pam_kwallet.changes +++ b/pam_kwallet.changes @@ -1,3 +1,12 @@ +------------------------------------------------------------------- +Mon Mar 13 10:29:41 UTC 2023 - Fabian Vogt + +- Add patches for handling edge cases and hardening: + * 0001-Verify-that-XDG_RUNTIME_DIR-is-usable.patch + * 0002-Don-t-do-anything-if-the-password-is-empty.patch + * 0003-Exit-early-if-the-target-user-is-root.patch + * 0004-Don-t-call-pam_sm_open_session-within-pam_sm_authent.patch + ------------------------------------------------------------------- Tue Feb 28 17:34:17 UTC 2023 - Fabian Vogt diff --git a/pam_kwallet.spec b/pam_kwallet.spec index 505de01..5af936d 100644 --- a/pam_kwallet.spec +++ b/pam_kwallet.spec @@ -30,6 +30,11 @@ Source1: https://download.kde.org/stable/plasma/%{version}/kwallet-pam-%{ Source2: plasma.keyring %endif Source3: baselibs.conf +# PATCH-FIX-UPSTREAM https://invent.kde.org/plasma/kwallet-pam/-/merge_requests/12 +Patch1: 0001-Verify-that-XDG_RUNTIME_DIR-is-usable.patch +Patch2: 0002-Don-t-do-anything-if-the-password-is-empty.patch +Patch3: 0003-Exit-early-if-the-target-user-is-root.patch +Patch4: 0004-Don-t-call-pam_sm_open_session-within-pam_sm_authent.patch BuildRequires: extra-cmake-modules >= 1.2.0 BuildRequires: kf5-filesystem BuildRequires: libgcrypt-devel >= 1.5.0 @@ -64,7 +69,7 @@ This package contains support files used by the KWallet PAM module. %prep -%setup -q -n kwallet-pam-%{version} +%autosetup -p1 -n kwallet-pam-%{version} %build # Before usrmerge, the PAM module goes into /lib*/security/