From 01d4143fda5bddb6dca37b23304dc239a5fb38b5 Mon Sep 17 00:00:00 2001
From: Albert Astals Cid <aacid@kde.org>
Date: Tue, 1 May 2018 12:32:24 +0200
Subject: Move socket creation to unprivileged codepath

We don't need to be creating the socket as root, and doing so,
specially having a chown is problematic security wise.
---
 pam_kwallet.c | 77 ++++++++++++++++++++++++++++-------------------------------
 1 file changed, 36 insertions(+), 41 deletions(-)

diff --git a/pam_kwallet.c b/pam_kwallet.c
index 083c9aa..b9c984a 100644
--- a/pam_kwallet.c
+++ b/pam_kwallet.c
@@ -372,13 +372,13 @@ static int drop_privileges(struct passwd *userInfo)
     return 0;
 }
 
-static void execute_kwallet(pam_handle_t *pamh, struct passwd *userInfo, int toWalletPipe[2], int envSocket)
+static void execute_kwallet(pam_handle_t *pamh, struct passwd *userInfo, int toWalletPipe[2], char *fullSocket)
 {
     //In the child pam_syslog does not work, using syslog directly
     int x = 2;
     //Close fd that are not of interest of kwallet
     for (; x < 64; ++x) {
-        if (x != toWalletPipe[0] && x != envSocket) {
+        if (x != toWalletPipe[0]) {
             close (x);
         }
     }
@@ -392,6 +392,39 @@ static void execute_kwallet(pam_handle_t *pamh, struct passwd *userInfo, int toW
         goto cleanup;
     }
 
+    int envSocket;
+    if ((envSocket = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) {
+        pam_syslog(pamh, LOG_ERR, "%s: couldn't create socket", logPrefix);
+        return;
+    }
+
+    struct sockaddr_un local;
+    local.sun_family = AF_UNIX;
+
+    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, local.sun_path);
+
+    size_t len = strlen(local.sun_path) + sizeof(local.sun_family);
+    if (bind(envSocket, (struct sockaddr *)&local, len) == -1) {
+        pam_syslog(pamh, LOG_INFO, "%s-kwalletd: Couldn't bind to local file\n", logPrefix);
+        return;
+    }
+
+    if (listen(envSocket, 5) == -1) {
+        pam_syslog(pamh, LOG_INFO, "%s-kwalletd: Couldn't listen in socket\n", logPrefix);
+        return;
+    }
+
     // Fork twice to daemonize kwallet
     setsid();
     pid_t pid = fork();
@@ -452,12 +485,6 @@ static void start_kwallet(pam_handle_t *pamh, struct passwd *userInfo, const cha
         pam_syslog(pamh, LOG_ERR, "%s: Couldn't create pipes", logPrefix);
     }
 
-    int envSocket;
-    if ((envSocket = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) {
-        pam_syslog(pamh, LOG_ERR, "%s: couldn't create socket", logPrefix);
-        return;
-    }
-
 #ifdef KWALLET5
     const char *socketPrefix = "kwallet5";
 #else
@@ -493,38 +520,6 @@ static void start_kwallet(pam_handle_t *pamh, struct passwd *userInfo, const cha
         return;
     }
 
-    struct sockaddr_un local;
-    local.sun_family = AF_UNIX;
-
-    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, local.sun_path);
-
-    size_t len = strlen(local.sun_path) + sizeof(local.sun_family);
-    if (bind(envSocket, (struct sockaddr *)&local, len) == -1) {
-        pam_syslog(pamh, LOG_INFO, "%s-kwalletd: Couldn't bind to local file\n", logPrefix);
-        return;
-    }
-
-    if (listen(envSocket, 5) == -1) {
-        pam_syslog(pamh, LOG_INFO, "%s-kwalletd: Couldn't listen in socket\n", logPrefix);
-        return;
-    }
-
-    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;
-    }
-
     pid_t pid;
     int status;
     switch (pid = fork ()) {
@@ -534,7 +529,7 @@ static void start_kwallet(pam_handle_t *pamh, struct passwd *userInfo, const cha
 
     //Child fork, will contain kwalletd
     case 0:
-        execute_kwallet(pamh, userInfo, toWalletPipe, envSocket);
+        execute_kwallet(pamh, userInfo, toWalletPipe, fullSocket);
         /* Should never be reached */
         break;
 
-- 
cgit v0.11.2