commit:     f1744b94bb12908e35666a1322d06615b7f7657b
Author:     Andreas Sturmlechner <asturm <AT> gentoo <DOT> org>
AuthorDate: Sat May  5 11:16:14 2018 +0000
Commit:     Andreas Sturmlechner <asturm <AT> gentoo <DOT> org>
CommitDate: Sat May  5 11:16:14 2018 +0000
URL:        https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=f1744b94

kde-plasma/kwallet-pam: Fix CVE-2018-10380

Bug: https://bugs.gentoo.org/654746
Package-Manager: Portage-2.3.32, Repoman-2.3.9

 .../kwallet-pam-5.11.5-CVE-2018-10380-1.patch      | 206 +++++++++++++++++++++
 .../kwallet-pam-5.11.5-CVE-2018-10380-2.patch      | 135 ++++++++++++++
 .../kwallet-pam-5.11.5-CVE-2018-10380-3.patch      |  54 ++++++
 .../kwallet-pam/kwallet-pam-5.11.5-r1.ebuild       |  35 ++++
 .../kwallet-pam/kwallet-pam-5.12.5-r1.ebuild       |  35 ++++
 5 files changed, 465 insertions(+)

diff --git 
a/kde-plasma/kwallet-pam/files/kwallet-pam-5.11.5-CVE-2018-10380-1.patch 
b/kde-plasma/kwallet-pam/files/kwallet-pam-5.11.5-CVE-2018-10380-1.patch
new file mode 100644
index 00000000000..70ade02a825
--- /dev/null
+++ b/kde-plasma/kwallet-pam/files/kwallet-pam-5.11.5-CVE-2018-10380-1.patch
@@ -0,0 +1,206 @@
+From 2134dec85ce19d6378d03cddfae9e5e464cb24c0 Mon Sep 17 00:00:00 2001
+From: Albert Astals Cid <aa...@kde.org>
+Date: Tue, 1 May 2018 12:29:02 +0200
+Subject: Move salt creation to an unprivileged process
+
+Opening files for writing as root is very tricky since through the power
+of symlinks we can get tricked to write in places we don't want to and
+we don't really need to be root to create the salt file
+---
+ pam_kwallet.c | 121 ++++++++++++++++++++++++++++++++++------------------------
+ 1 file changed, 71 insertions(+), 50 deletions(-)
+
+diff --git a/pam_kwallet.c b/pam_kwallet.c
+index 20d9603..083c9aa 100644
+--- a/pam_kwallet.c
++++ b/pam_kwallet.c
+@@ -82,7 +82,7 @@ const static char *envVar = "PAM_KWALLET_LOGIN";
+ 
+ static int argumentsParsed = -1;
+ 
+-int kwallet_hash(const char *passphrase, struct passwd *userInfo, char *key);
++int kwallet_hash(pam_handle_t *pamh, const char *passphrase, struct passwd 
*userInfo, char *key);
+ 
+ static void parseArguments(int argc, const char **argv)
+ {
+@@ -325,7 +325,7 @@ PAM_EXTERN int pam_sm_authenticate(pam_handle_t *pamh, int 
flags, int argc, cons
+     }
+ 
+     char *key = malloc(KWALLET_PAM_KEYSIZE);
+-    if (!key || kwallet_hash(password, userInfo, key) != 0) {
++    if (!key || kwallet_hash(pamh, password, userInfo, key) != 0) {
+         free(key);
+         pam_syslog(pamh, LOG_ERR, "%s: Fail into creating the hash", 
logPrefix);
+         return PAM_IGNORE;
+@@ -352,6 +352,26 @@ PAM_EXTERN int pam_sm_authenticate(pam_handle_t *pamh, 
int flags, int argc, cons
+     return PAM_SUCCESS;
+ }
+ 
++static int drop_privileges(struct passwd *userInfo)
++{
++    /* When dropping privileges from root, the `setgroups` call will
++    * remove any extraneous groups. If we don't call this, then
++    * even though our uid has dropped, we may still have groups
++    * that enable us to do super-user things. This will fail if we
++    * aren't root, so don't bother checking the return value, this
++    * is just done as an optimistic privilege dropping function.
++    */
++    setgroups(0, NULL);
++
++    //Change to the user in case we are not it yet
++    if (setgid (userInfo->pw_gid) < 0 || setuid (userInfo->pw_uid) < 0 ||
++        setegid (userInfo->pw_gid) < 0 || seteuid (userInfo->pw_uid) < 0) {
++        return -1;
++    }
++
++    return 0;
++}
++
+ static void execute_kwallet(pam_handle_t *pamh, struct passwd *userInfo, int 
toWalletPipe[2], int envSocket)
+ {
+     //In the child pam_syslog does not work, using syslog directly
+@@ -366,18 +386,8 @@ static void execute_kwallet(pam_handle_t *pamh, struct 
passwd *userInfo, int toW
+     //This is the side of the pipe PAM will send the hash to
+     close (toWalletPipe[1]);
+ 
+-    /* When dropping privileges from root, the `setgroups` call will
+-    * remove any extraneous groups. If we don't call this, then
+-    * even though our uid has dropped, we may still have groups
+-    * that enable us to do super-user things. This will fail if we
+-    * aren't root, so don't bother checking the return value, this
+-    * is just done as an optimistic privilege dropping function.
+-    */
+-    setgroups(0, NULL);
+-
+     //Change to the user in case we are not it yet
+-    if (setgid (userInfo->pw_gid) < 0 || setuid (userInfo->pw_uid) < 0 ||
+-        setegid (userInfo->pw_gid) < 0 || seteuid (userInfo->pw_uid) < 0) {
++    if (drop_privileges(userInfo) < 0) {
+         syslog(LOG_ERR, "%s: could not set gid/uid/euid/egit for kwalletd", 
logPrefix);
+         goto cleanup;
+     }
+@@ -619,7 +629,7 @@ PAM_EXTERN int pam_sm_chauthtok(pam_handle_t *pamh, int 
flags, int argc, const c
+     return PAM_SUCCESS;
+ }
+ 
+-int mkpath(char *path, struct passwd *userInfo)
++static int mkpath(char *path)
+ {
+     struct stat sb;
+     char *slash;
+@@ -639,10 +649,6 @@ int mkpath(char *path, struct passwd *userInfo)
+                 errno != EEXIST)) {
+                 syslog(LOG_ERR, "%s: Couldn't create directory: %s because: 
%d-%s", logPrefix, path, errno, strerror(errno));
+                 return (-1);
+-            } else {
+-                if (chown(path, userInfo->pw_uid, userInfo->pw_gid) == -1) {
+-                    syslog(LOG_INFO, "%s: Couldn't change ownership of: %s", 
logPrefix, path);
+-                }
+             }
+         } else if (!S_ISDIR(sb.st_mode)) {
+             return (-1);
+@@ -654,34 +660,49 @@ int mkpath(char *path, struct passwd *userInfo)
+     return (0);
+ }
+ 
+-static char* createNewSalt(const char *path, struct passwd *userInfo)
++static void createNewSalt(pam_handle_t *pamh, const char *path, struct passwd 
*userInfo)
+ {
+-    unlink(path);//in case the file already exists
++    const int pid = fork();
++    if (pid == -1) {
++        pam_syslog(pamh, LOG_ERR, "%s: Couldn't fork to create salt file", 
logPrefix);
++    } else if (pid == 0) {
++        // Child process
++        if (drop_privileges(userInfo) < 0) {
++            syslog(LOG_ERR, "%s: could not set gid/uid/euid/egit for salt 
file creation", logPrefix);
++            exit(-1);
++        }
+ 
+-    char *dir = strdup(path);
+-    dir[strlen(dir) - 14] = '\0';//remove kdewallet.salt
+-    mkpath(dir, userInfo);//create the path in case it does not exists
+-    free(dir);
++        unlink(path);//in case the file already exists
+ 
+-    char *salt = gcry_random_bytes(KWALLET_PAM_SALTSIZE, GCRY_STRONG_RANDOM);
+-    FILE *fd = fopen(path, "w");
++        char *dir = strdup(path);
++        dir[strlen(dir) - 14] = '\0';//remove kdewallet.salt
++        mkpath(dir); //create the path in case it does not exists
++        free(dir);
+ 
+-    //If the file can't be created
+-    if (fd == NULL) {
+-        syslog(LOG_ERR, "%s: Couldn't open file: %s because: %d-%s", 
logPrefix, path, errno, strerror(errno));
+-        return NULL;
+-    }
++        char *salt = gcry_random_bytes(KWALLET_PAM_SALTSIZE, 
GCRY_STRONG_RANDOM);
++        FILE *fd = fopen(path, "w");
+ 
+-    fwrite(salt, KWALLET_PAM_SALTSIZE, 1, fd);
+-    fclose(fd);
++        //If the file can't be created
++        if (fd == NULL) {
++            syslog(LOG_ERR, "%s: Couldn't open file: %s because: %d-%s", 
logPrefix, path, errno, strerror(errno));
++            exit(-2);
++        }
+ 
+-    if (chown(path, userInfo->pw_uid, userInfo->pw_gid) == -1) {
+-        syslog(LOG_ERR, "%s: Couldn't change ownership of the created salt 
file", logPrefix);
+-    }
++        fwrite(salt, KWALLET_PAM_SALTSIZE, 1, fd);
++        fclose(fd);
+ 
+-    return salt;
++        exit(0); // success
++    } else {
++        // pam process, just wait for child to finish
++        int status;
++        waitpid(pid, &status, 0);
++        if (status != 0) {
++            pam_syslog(pamh, LOG_ERR, "%s: Couldn't create salt file", 
logPrefix);
++        }
++    }
+ }
+-int kwallet_hash(const char *passphrase, struct passwd *userInfo, char *key)
++
++int kwallet_hash(pam_handle_t *pamh, const char *passphrase, struct passwd 
*userInfo, char *key)
+ {
+     if (!gcry_check_version("1.5.0")) {
+         syslog(LOG_ERR, "%s-kwalletd: libcrypt version is too old", 
logPrefix);
+@@ -700,19 +721,19 @@ int kwallet_hash(const char *passphrase, struct passwd 
*userInfo, char *key)
+     struct stat info;
+     char *salt = NULL;
+     if (stat(path, &info) != 0 || info.st_size == 0) {
+-        salt = createNewSalt(path, userInfo);
+-    } else {
+-        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(KWALLET_PAM_SALTSIZE);
+-        memset(salt, '\0', KWALLET_PAM_SALTSIZE);
+-        fread(salt, KWALLET_PAM_SALTSIZE, 1, fd);
+-        fclose(fd);
++        createNewSalt(pamh, path, userInfo);
+     }
++
++    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(KWALLET_PAM_SALTSIZE);
++    memset(salt, '\0', KWALLET_PAM_SALTSIZE);
++    fread(salt, KWALLET_PAM_SALTSIZE, 1, fd);
++    fclose(fd);
+     free(path);
+ 
+     if (salt == NULL) {
+-- 
+cgit v0.11.2
+

diff --git 
a/kde-plasma/kwallet-pam/files/kwallet-pam-5.11.5-CVE-2018-10380-2.patch 
b/kde-plasma/kwallet-pam/files/kwallet-pam-5.11.5-CVE-2018-10380-2.patch
new file mode 100644
index 00000000000..2f88e0c3cea
--- /dev/null
+++ b/kde-plasma/kwallet-pam/files/kwallet-pam-5.11.5-CVE-2018-10380-2.patch
@@ -0,0 +1,135 @@
+From 01d4143fda5bddb6dca37b23304dc239a5fb38b5 Mon Sep 17 00:00:00 2001
+From: Albert Astals Cid <aa...@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
+

diff --git 
a/kde-plasma/kwallet-pam/files/kwallet-pam-5.11.5-CVE-2018-10380-3.patch 
b/kde-plasma/kwallet-pam/files/kwallet-pam-5.11.5-CVE-2018-10380-3.patch
new file mode 100644
index 00000000000..de882e45453
--- /dev/null
+++ b/kde-plasma/kwallet-pam/files/kwallet-pam-5.11.5-CVE-2018-10380-3.patch
@@ -0,0 +1,54 @@
+From 8da1a47035fc92bc1496059583772bc4bd6e8ba6 Mon Sep 17 00:00:00 2001
+From: Maximiliano Curia <m...@gnuservers.com.ar>
+Date: Fri, 4 May 2018 22:06:06 +0200
+Subject: Avoid giving an stderr to kwallet
+
+Summary:
+The fixes for CVE-2018-10380 introduced a regression for most users not
+using kde, and some for kde sessions. In particular the reorder of the
+close calls and creating a new socket caused that the socket is always
+assigned the file descriptor 2, aka stderr.
+
+BUG: 393856
+
+Test Plan: It works
+
+Reviewers: #plasma, aacid
+
+Reviewed By: aacid
+
+Subscribers: asturmlechner, rdieter, davidedmundson, plasma-devel
+
+Tags: #plasma
+
+Differential Revision: https://phabricator.kde.org/D12702
+---
+ pam_kwallet.c | 5 ++++-
+ 1 file changed, 4 insertions(+), 1 deletion(-)
+
+diff --git a/pam_kwallet.c b/pam_kwallet.c
+index b9c984a..661ed8d 100644
+--- a/pam_kwallet.c
++++ b/pam_kwallet.c
+@@ -375,7 +375,8 @@ static int drop_privileges(struct passwd *userInfo)
+ 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;
++    //keep stderr open so socket doesn't returns us that fd
++    int x = 3;
+     //Close fd that are not of interest of kwallet
+     for (; x < 64; ++x) {
+         if (x != toWalletPipe[0]) {
+@@ -424,6 +425,8 @@ static void execute_kwallet(pam_handle_t *pamh, struct 
passwd *userInfo, int toW
+         pam_syslog(pamh, LOG_INFO, "%s-kwalletd: Couldn't listen in 
socket\n", logPrefix);
+         return;
+     }
++    //finally close stderr
++    close(2);
+ 
+     // Fork twice to daemonize kwallet
+     setsid();
+-- 
+cgit v0.11.2
+

diff --git a/kde-plasma/kwallet-pam/kwallet-pam-5.11.5-r1.ebuild 
b/kde-plasma/kwallet-pam/kwallet-pam-5.11.5-r1.ebuild
new file mode 100644
index 00000000000..fdfa93bb219
--- /dev/null
+++ b/kde-plasma/kwallet-pam/kwallet-pam-5.11.5-r1.ebuild
@@ -0,0 +1,35 @@
+# Copyright 1999-2018 Gentoo Foundation
+# Distributed under the terms of the GNU General Public License v2
+
+EAPI=6
+
+inherit kde5
+
+DESCRIPTION="KWallet PAM module to not enter password again"
+LICENSE="LGPL-2.1"
+KEYWORDS="~amd64 ~arm ~x86"
+IUSE=""
+
+DEPEND="
+       dev-libs/libgcrypt:0=
+       virtual/pam
+"
+RDEPEND="${DEPEND}
+       net-misc/socat
+"
+
+PATCHES=( "${FILESDIR}"/${P}-CVE-2018-10380-{1,2,3}.patch )
+
+src_configure() {
+       local mycmakeargs=(
+               -DCMAKE_INSTALL_LIBDIR="/$(get_libdir)"
+               -DKWALLET4=0
+       )
+       kde5_src_configure
+}
+
+pkg_postinst() {
+       kde5_pkg_postinst
+       elog "This package enables auto-unlocking of kde-frameworks/kwallet:5."
+       elog "See also: https://wiki.gentoo.org/wiki/KDE#KWallet_auto-unlocking";
+}

diff --git a/kde-plasma/kwallet-pam/kwallet-pam-5.12.5-r1.ebuild 
b/kde-plasma/kwallet-pam/kwallet-pam-5.12.5-r1.ebuild
new file mode 100644
index 00000000000..46782411966
--- /dev/null
+++ b/kde-plasma/kwallet-pam/kwallet-pam-5.12.5-r1.ebuild
@@ -0,0 +1,35 @@
+# Copyright 1999-2018 Gentoo Foundation
+# Distributed under the terms of the GNU General Public License v2
+
+EAPI=6
+
+inherit kde5
+
+DESCRIPTION="KWallet PAM module to not enter password again"
+LICENSE="LGPL-2.1"
+KEYWORDS="~amd64 ~arm ~x86"
+IUSE=""
+
+DEPEND="
+       dev-libs/libgcrypt:0=
+       virtual/pam
+"
+RDEPEND="${DEPEND}
+       net-misc/socat
+"
+
+PATCHES=( "${FILESDIR}"/${PN}-5.11.5-CVE-2018-10380-{1,2,3}.patch )
+
+src_configure() {
+       local mycmakeargs=(
+               -DCMAKE_INSTALL_LIBDIR="/$(get_libdir)"
+               -DKWALLET4=0
+       )
+       kde5_src_configure
+}
+
+pkg_postinst() {
+       kde5_pkg_postinst
+       elog "This package enables auto-unlocking of kde-frameworks/kwallet:5."
+       elog "See also: https://wiki.gentoo.org/wiki/KDE#KWallet_auto-unlocking";
+}

Reply via email to