Hi,Following up the upstream announcement of a security flaw in kwallet-pam [1] I would like to upload the upstream fixes to stretch. All the versions prior the (not yet released) 5.12.6 are affected by this. The fix was backported by upstream to plasma 5.8, which is what we shipped in stretch.
The latest 5.8 upstream version (5.8.9), only has a version bump, and a minor translation update, which are not relevant. [2]
I have already uploaded the fixes to unstable. I'm attaching the corresponding debdiff. Happy hacking, [1]: https://marc.info/?l=kde-announce&m=152534806103730&w=2 [2]: https://cgit.kde.org/kwallet-pam.git/log/?h=Plasma/5.8 -- "If it ain't broke, don't fix it" -- Bert Lance "If we can't fix it, it ain't broke" -- Lieutenant Colonel Walt Weir Saludos /\/\ /\ >< `/
diff -Nru kwallet-pam-5.8.4/debian/changelog kwallet-pam-5.8.4/debian/changelog
--- kwallet-pam-5.8.4/debian/changelog 2016-11-23 18:36:40.000000000 +0100
+++ kwallet-pam-5.8.4/debian/changelog 2018-05-03 19:01:35.000000000 +0200
@@ -1,3 +1,11 @@
+kwallet-pam (5.8.4-1+deb9u1) stretch-security; urgency=high
+
+ * CVE-2018-10380 fix
+ Add upstream patches Move-salt-creation-to-an-unprivileged-process.patch
+ and Move-socket-creation-to-unprivileged-codepath.patch.
+
+ -- Maximiliano Curia <m...@debian.org> Thu, 03 May 2018 19:01:35 +0200
+
kwallet-pam (5.8.4-1) unstable; urgency=medium
* New upstream release (5.8.4)
diff -Nru
kwallet-pam-5.8.4/debian/patches/Move-salt-creation-to-an-unprivileged-process.patch
kwallet-pam-5.8.4/debian/patches/Move-salt-creation-to-an-unprivileged-process.patch
---
kwallet-pam-5.8.4/debian/patches/Move-salt-creation-to-an-unprivileged-process.patch
1970-01-01 01:00:00.000000000 +0100
+++
kwallet-pam-5.8.4/debian/patches/Move-salt-creation-to-an-unprivileged-process.patch
2018-05-03 19:01:35.000000000 +0200
@@ -0,0 +1,201 @@
+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 | 120 ++++++++++++++++++++++++++++++++++------------------------
+ 1 file changed, 71 insertions(+), 49 deletions(-)
+
+diff --git a/pam_kwallet.c b/pam_kwallet.c
+index 809ab9a..2617311 100644
+--- a/pam_kwallet.c
++++ b/pam_kwallet.c
+@@ -61,7 +61,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)
+ {
+@@ -282,7 +282,7 @@ PAM_EXTERN int pam_sm_authenticate(pam_handle_t *pamh, int
flags, int argc, cons
+ }
+
+ char *key = malloc(sizeof(char) * KWALLET_PAM_KEYSIZE);
+- if (kwallet_hash(password, userInfo, key) != 0) {
++ if (kwallet_hash(pamh, password, userInfo, key) != 0) {
+ pam_syslog(pamh, LOG_ERR, "%s: Fail into creating the hash",
logPrefix);
+ return PAM_IGNORE;
+ }
+@@ -306,6 +306,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
+@@ -320,18 +340,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;
+ }
+@@ -548,7 +558,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;
+@@ -568,10 +578,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);
+@@ -583,34 +589,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);
+@@ -628,18 +649,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));
+- return 1;
+- }
+- salt = (char*) malloc(sizeof(char) * 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));
++ return 1;
++ }
++ salt = (char*) malloc(sizeof(char) * KWALLET_PAM_SALTSIZE);
++ memset(salt, '\0', KWALLET_PAM_SALTSIZE);
++ fread(salt, KWALLET_PAM_SALTSIZE, 1, fd);
++ fclose(fd);
++
+ if (salt == NULL) {
+ syslog(LOG_ERR, "%s-kwalletd: Couldn't create or read the salt file",
logPrefix);
+ return 1;
diff -Nru
kwallet-pam-5.8.4/debian/patches/Move-socket-creation-to-unprivileged-codepath.patch
kwallet-pam-5.8.4/debian/patches/Move-socket-creation-to-unprivileged-codepath.patch
---
kwallet-pam-5.8.4/debian/patches/Move-socket-creation-to-unprivileged-codepath.patch
1970-01-01 01:00:00.000000000 +0100
+++
kwallet-pam-5.8.4/debian/patches/Move-socket-creation-to-unprivileged-codepath.patch
2018-05-03 19:01:35.000000000 +0200
@@ -0,0 +1,125 @@
+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 | 71 +++++++++++++++++++++++++++--------------------------------
+ 1 file changed, 33 insertions(+), 38 deletions(-)
+
+diff --git a/pam_kwallet.c b/pam_kwallet.c
+index 2617311..37bad79 100644
+--- a/pam_kwallet.c
++++ b/pam_kwallet.c
+@@ -326,13 +326,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);
+ }
+ }
+@@ -346,6 +346,36 @@ 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);
++ return;
++ }
++ strcpy(local.sun_path, fullSocket);
++ 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);
++
++ 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();
+@@ -407,12 +437,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
+@@ -430,35 +454,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 ((size_t)len > sizeof(local.sun_path)) {
+- pam_syslog(pamh, LOG_ERR, "%s: socket path %s too long to open",
+- logPrefix, fullSocket);
+- return;
+- }
+- strcpy(local.sun_path, fullSocket);
+- 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);
+-
+- 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(fullSocket, 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 ()) {
+@@ -468,7 +463,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;
+
diff -Nru kwallet-pam-5.8.4/debian/patches/series
kwallet-pam-5.8.4/debian/patches/series
--- kwallet-pam-5.8.4/debian/patches/series 1970-01-01 01:00:00.000000000
+0100
+++ kwallet-pam-5.8.4/debian/patches/series 2018-05-03 19:01:35.000000000
+0200
@@ -0,0 +1,2 @@
+Move-salt-creation-to-an-unprivileged-process.patch
+Move-socket-creation-to-unprivileged-codepath.patch
signature.asc
Description: PGP signature
