kde-plasma/kwallet-pam: Fix memleaks and dropping privileges

Package-Manager: Portage-2.3.8, Repoman-2.3.3

3 files changed, 309 insertions, 0 deletions

diff --git a/kde-plasma/kwallet-pam/files/kwallet-pam-5.10.5-check-graphical.patch b/kde-plasma/kwallet-pam/files/kwallet-pam-5.10.5-check-graphical.patch
new file mode 100644
index 000000000000..61ea4604586f
--- /dev/null
+++ b/kde-plasma/kwallet-pam/files/kwallet-pam-5.10.5-check-graphical.patch
@@ -0,0 +1,87 @@
+From f3b230f7f3bf39dc46b97a216aa7c28595d20a7a Mon Sep 17 00:00:00 2001
+From: Fabian Vogt <fabian@ritter-vogt.de>
+Date: Thu, 3 Aug 2017 09:50:30 +0200
+Subject: Check for a graphical session
+
+Summary:
+Avoid running if it detects a text session. This can be overridden by adding
+"force_run" as argument.
+
+Test Plan:
+Put pam_kwallet5.so as optional in a global common-session pam file
+that is included by all other services. It is not invoked when logging in from
+a tty with getty, sudo or su and still works when using SDDM. When adding
+force_run it runs in all cases.
+
+Reviewers: #plasma
+
+Subscribers: plasma-devel
+
+Tags: #plasma
+
+Differential Revision: https://phabricator.kde.org/D7125
+---
+ pam_kwallet.c | 26 ++++++++++++++++++++++++++
+ 1 file changed, 26 insertions(+)
+
+diff --git a/pam_kwallet.c b/pam_kwallet.c
+index cba57e7..46720a5 100644
+--- a/pam_kwallet.c
++++ b/pam_kwallet.c
+@@ -72,6 +72,7 @@ const static char *kwalletd = NULL;
+ const static char *socketPath = NULL;
+ const static char *kwalletPamDataKey = NULL;
+ const static char *logPrefix = NULL;
++static int force_run = 0;
+ 
+ #ifdef KWALLET5
+ const static char *envVar = "PAM_KWALLET5_LOGIN";
+@@ -98,6 +99,8 @@ static void parseArguments(int argc, const char **argv)
+             kwalletd = argv[x] + 9;
+         } else if (strstr(argv[x], "socketPath=") != NULL) {
+             socketPath= argv[x] + 11;
++        } else if (strcmp(argv[x], "force_run") == 0) {
++            force_run = 1;
+         }
+     }
+ #ifdef KWALLET5
+@@ -246,6 +249,24 @@ static void cleanup_free(pam_handle_t *pamh, void *ptr, int error_status)
+     free(ptr);
+ }
+ 
++static int is_graphical_session(pam_handle_t *pamh)
++{
++    //Detect a graphical session
++    const char *pam_tty = NULL, *pam_xdisplay = NULL,
++               *xdg_session_type = NULL, *display = NULL;
++
++    pam_get_item(pamh, PAM_TTY, (const void**) &pam_tty);
++#ifdef PAM_XDISPLAY
++    pam_get_item(pamh, PAM_XDISPLAY, (const void**) &pam_xdisplay);
++#endif
++    xdg_session_type = get_env(pamh, "XDG_SESSION_TYPE");
++
++    return (pam_xdisplay && strlen(pam_xdisplay) != 0)
++           || (pam_tty && pam_tty[0] == ':')
++           || (xdg_session_type && strcmp(xdg_session_type, "x11") == 0)
++           || (xdg_session_type && strcmp(xdg_session_type, "wayland") == 0);
++}
++
+ PAM_EXTERN int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, const char **argv)
+ {
+     pam_syslog(pamh, LOG_INFO, "%s: pam_sm_authenticate\n", logPrefix);
+@@ -537,6 +558,11 @@ PAM_EXTERN int pam_sm_open_session(pam_handle_t *pamh, int flags, int argc, cons
+ 
+     parseArguments(argc, argv);
+ 
++    if (!force_run && !is_graphical_session(pamh)) {
++        pam_syslog(pamh, LOG_INFO, "%s: not a graphical session, skipping. Use force_run parameter to ignore this.", logPrefix);
++        return PAM_IGNORE;
++    }
++
+     int result;
+     result = pam_set_data(pamh, "sm_open_session", "1", NULL);
+     if (result != PAM_SUCCESS) {
+-- 
+cgit v0.11.2
+
diff --git a/kde-plasma/kwallet-pam/files/kwallet-pam-5.10.5-cleanups.patch b/kde-plasma/kwallet-pam/files/kwallet-pam-5.10.5-cleanups.patch
new file mode 100644
index 000000000000..38a333131e93
--- /dev/null
+++ b/kde-plasma/kwallet-pam/files/kwallet-pam-5.10.5-cleanups.patch
@@ -0,0 +1,173 @@
+From a33ec22b96e837899528b05963eae8ea6b01171a Mon Sep 17 00:00:00 2001
+From: Fabian Vogt <fabian@ritter-vogt.de>
+Date: Thu, 3 Aug 2017 09:02:14 +0200
+Subject: Several cleanups
+
+Summary:
+- No cppcheck warnings anymore
+- Use snprintf everywhere
+- Avoid pointless multiplication with sizeof(char)
+- Avoid memory leaks
+
+Test Plan: Still builds, works the same as before.
+
+Reviewers: #plasma
+
+Subscribers: plasma-devel
+
+Tags: #plasma
+
+Differential Revision: https://phabricator.kde.org/D7123
+---
+ pam_kwallet.c | 44 ++++++++++++++++++++++++++++++++------------
+ 1 file changed, 32 insertions(+), 12 deletions(-)
+
+diff --git a/pam_kwallet.c b/pam_kwallet.c
+index d88c5e0..cba57e7 100644
+--- a/pam_kwallet.c
++++ b/pam_kwallet.c
+@@ -151,13 +151,14 @@ static int set_env(pam_handle_t *pamh, const char *name, const char *value)
+         //We do not return because pam_putenv might work
+     }
+ 
+-    char *pamEnv = malloc(strlen(name) + strlen(value) + 2); //2 is for = and \0
++    size_t pamEnvSize = strlen(name) + strlen(value) + 2; //2 is for = and \0
++    char *pamEnv = malloc(pamEnvSize);
+     if (!pamEnv) {
+         pam_syslog(pamh, LOG_WARNING, "%s: Impossible to allocate memory for pamEnv", logPrefix);
+         return -1;
+     }
+ 
+-    sprintf (pamEnv, "%s=%s", name, value);
++    snprintf (pamEnv, pamEnvSize, "%s=%s", name, value);
+     int ret = pam_putenv(pamh, pamEnv);
+     free(pamEnv);
+ 
+@@ -240,6 +241,11 @@ cleanup:
+     return result;
+ }
+ 
++static void cleanup_free(pam_handle_t *pamh, void *ptr, int error_status)
++{
++    free(ptr);
++}
++
+ PAM_EXTERN int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, const char **argv)
+ {
+     pam_syslog(pamh, LOG_INFO, "%s: pam_sm_authenticate\n", logPrefix);
+@@ -297,14 +303,17 @@ PAM_EXTERN int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, cons
+         return PAM_IGNORE;
+     }
+ 
+-    char *key = malloc(sizeof(char) * KWALLET_PAM_KEYSIZE);
+-    if (kwallet_hash(password, userInfo, key) != 0) {
++    char *key = malloc(KWALLET_PAM_KEYSIZE);
++    if (!key || kwallet_hash(password, userInfo, key) != 0) {
++        free(key);
+         pam_syslog(pamh, LOG_ERR, "%s: Fail into creating the hash", logPrefix);
+         return PAM_IGNORE;
+     }
+ 
+-    result = pam_set_data(pamh, kwalletPamDataKey, key, NULL);
++    result = pam_set_data(pamh, kwalletPamDataKey, key, cleanup_free);
++
+     if (result != PAM_SUCCESS) {
++        free(key);
+         pam_syslog(pamh, LOG_ERR, "%s: Impossible to store the hashed password: %s", logPrefix
+             , pam_strerror(pamh, result));
+         return PAM_IGNORE;
+@@ -385,9 +394,8 @@ cleanup:
+ static int better_write(int fd, const char *buffer, int len)
+ {
+     size_t writtenBytes = 0;
+-    int result;
+     while(writtenBytes < len) {
+-        result = write(fd, buffer + writtenBytes, len - writtenBytes);
++        int result = write(fd, buffer + writtenBytes, len - writtenBytes);
+         if (result < 0) {
+             if (errno != EAGAIN && errno != EINTR) {
+                 return -1;
+@@ -450,6 +458,7 @@ static void start_kwallet(pam_handle_t *pamh, struct passwd *userInfo, const cha
+     if (result != PAM_SUCCESS) {
+         pam_syslog(pamh, LOG_ERR, "%s: Impossible to set %s env, %s",
+                    logPrefix, envVar, pam_strerror(pamh, result));
++        free(fullSocket);
+         return;
+     }
+ 
+@@ -459,12 +468,15 @@ static void start_kwallet(pam_handle_t *pamh, struct passwd *userInfo, const cha
+     if (strlen(fullSocket) > sizeof(local.sun_path)) {
+         pam_syslog(pamh, LOG_ERR, "%s: socket path %s too long to open",
+                    logPrefix, fullSocket);
++        free(fullSocket);
+         return;
+     }
+     strcpy(local.sun_path, fullSocket);
++    free(fullSocket);
++    fullSocket = NULL;
+     unlink(local.sun_path);//Just in case it exists from a previous login
+ 
+-    pam_syslog(pamh, LOG_INFO, "%s: final socket path: %s", logPrefix, fullSocket);
++    pam_syslog(pamh, LOG_INFO, "%s: final socket path: %s", logPrefix, local.sun_path);
+ 
+     size_t len = strlen(local.sun_path) + sizeof(local.sun_family);
+     if (bind(envSocket, (struct sockaddr *)&local, len) == -1) {
+@@ -477,7 +489,7 @@ static void start_kwallet(pam_handle_t *pamh, struct passwd *userInfo, const cha
+         return;
+     }
+ 
+-    if (chown(fullSocket, userInfo->pw_uid, userInfo->pw_gid) == -1) {
++    if (chown(local.sun_path, userInfo->pw_uid, userInfo->pw_gid) == -1) {
+         pam_syslog(pamh, LOG_INFO, "%s: Couldn't change ownership of the socket", logPrefix);
+         return;
+     }
+@@ -655,7 +667,8 @@ int kwallet_hash(const char *passphrase, struct passwd *userInfo, char *key)
+ #else
+     char *fixpath = "share/apps/kwallet/kdewallet.salt";
+ #endif
+-    char *path = (char*) malloc(strlen(userInfo->pw_dir) + strlen(kdehome) + strlen(fixpath) + 3);//3 == / and \0
++    size_t pathSize = strlen(userInfo->pw_dir) + strlen(kdehome) + strlen(fixpath) + 3;//3 == /, / and \0
++    char *path = (char*) malloc(pathSize);
+     sprintf(path, "%s/%s/%s", userInfo->pw_dir, kdehome, fixpath);
+ 
+     struct stat info;
+@@ -666,21 +679,26 @@ int kwallet_hash(const char *passphrase, struct passwd *userInfo, char *key)
+         FILE *fd = fopen(path, "r");
+         if (fd == NULL) {
+             syslog(LOG_ERR, "%s: Couldn't open file: %s because: %d-%s", logPrefix, path, errno, strerror(errno));
++            free(path);
+             return 1;
+         }
+-        salt = (char*) malloc(sizeof(char) * KWALLET_PAM_SALTSIZE);
++        salt = (char*) malloc(KWALLET_PAM_SALTSIZE);
+         memset(salt, '\0', KWALLET_PAM_SALTSIZE);
+         fread(salt, KWALLET_PAM_SALTSIZE, 1, fd);
+         fclose(fd);
+     }
++    free(path);
++
+     if (salt == NULL) {
+         syslog(LOG_ERR, "%s-kwalletd: Couldn't create or read the salt file", logPrefix);
+         return 1;
+     }
+ 
+     gcry_error_t error;
++
+     error = gcry_control(GCRYCTL_INIT_SECMEM, 32768, 0);
+     if (error != 0) {
++        free(salt);
+         syslog(LOG_ERR, "%s-kwalletd: Can't get secure memory: %d", logPrefix, error);
+         return 1;
+     }
+@@ -691,5 +709,7 @@ int kwallet_hash(const char *passphrase, struct passwd *userInfo, char *key)
+                             GCRY_KDF_PBKDF2, GCRY_MD_SHA512,
+                             salt, KWALLET_PAM_SALTSIZE,
+                             KWALLET_PAM_ITERATIONS,KWALLET_PAM_KEYSIZE, key);
+-    return 0;
++
++    free(salt);
++    return (int) error; // gcry_kdf_derive returns 0 on success
+ }
+-- 
+cgit v0.11.2
+
diff --git a/kde-plasma/kwallet-pam/files/kwallet-pam-5.10.5-privileges.patch b/kde-plasma/kwallet-pam/files/kwallet-pam-5.10.5-privileges.patch
new file mode 100644
index 000000000000..8b45b293bbf9
--- /dev/null
+++ b/kde-plasma/kwallet-pam/files/kwallet-pam-5.10.5-privileges.patch
@@ -0,0 +1,49 @@
+From 1a01e1eb870e1ab1d96a8641f1f3500af646c974 Mon Sep 17 00:00:00 2001
+From: Fabian Vogt <fabian@ritter-vogt.de>
+Date: Thu, 3 Aug 2017 09:27:10 +0200
+Subject: Avoid dropping privileges by initializing gcrypt secmem
+
+Summary:
+It's a documented side effect that initialization of secure memory in gcrypt
+drops privileges if getuid() != geteuid(). This results in breaking setuid
+callers, like sudo or su.
+
+Test Plan: Can use sudo again when pam_kwallet is involved.
+
+Reviewers: #plasma
+
+Subscribers: plasma-devel
+
+Tags: #plasma
+
+Differential Revision: https://phabricator.kde.org/D7124
+---
+ pam_kwallet.c | 6 ++++++
+ 1 file changed, 6 insertions(+)
+
+diff --git a/pam_kwallet.c b/pam_kwallet.c
+index 46720a5..20d9603 100644
+--- a/pam_kwallet.c
++++ b/pam_kwallet.c
+@@ -722,12 +722,18 @@ int kwallet_hash(const char *passphrase, struct passwd *userInfo, char *key)
+ 
+     gcry_error_t error;
+ 
++    /* We cannot call GCRYCTL_INIT_SECMEM as it drops privileges if getuid() != geteuid().
++     * PAM modules are in many cases executed through setuid binaries, which this call
++     * would break.
++     * It was never effective anyway as neither key nor passphrase are in secure memory,
++     * which is a prerequisite for secure operation...
+     error = gcry_control(GCRYCTL_INIT_SECMEM, 32768, 0);
+     if (error != 0) {
+         free(salt);
+         syslog(LOG_ERR, "%s-kwalletd: Can't get secure memory: %d", logPrefix, error);
+         return 1;
+     }
++    */
+ 
+     gcry_control (GCRYCTL_INITIALIZATION_FINISHED, 0);
+ 
+-- 
+cgit v0.11.2
+