SHA256
1
0
forked from pool/pam_kwallet

Accepting request 610925 from KDE:Frameworks5

Plasma 5.13 beta

OBS-URL: https://build.opensuse.org/request/show/610925
OBS-URL: https://build.opensuse.org/package/show/openSUSE:Factory/pam_kwallet?expand=0&rev=30
This commit is contained in:
Dominique Leuenberger 2018-05-29 08:35:38 +00:00 committed by Git OBS Bridge
commit 887c512fd1
7 changed files with 25 additions and 411 deletions

View File

@ -1,54 +0,0 @@
From 8da1a47035fc92bc1496059583772bc4bd6e8ba6 Mon Sep 17 00:00:00 2001
From: Maximiliano Curia <maxy@gnuservers.com.ar>
Date: Fri, 4 May 2018 22:06:06 +0200
Subject: [PATCH] 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();
--
2.16.2

View File

@ -1,206 +0,0 @@
From 2134dec85ce19d6378d03cddfae9e5e464cb24c0 Mon Sep 17 00:00:00 2001
From: Albert Astals Cid <aacid@kde.org>
Date: Tue, 1 May 2018 12:29:02 +0200
Subject: [PATCH 1/2] 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) {
--
2.16.2

View File

@ -1,135 +0,0 @@
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: [PATCH 2/2] 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;
--
2.16.2

View File

@ -1,3 +0,0 @@
version https://git-lfs.github.com/spec/v1
oid sha256:6374132fe2d142c9eb999e90c636b973fe9715335de3cfa86f0b5f4258d5eba2
size 18584

View File

@ -0,0 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:edc3a62e3e641303fa17716a2cf4f2d8aa9258df215e939cbd9a1cb04ef974b1
size 18600

View File

@ -1,3 +1,21 @@
-------------------------------------------------------------------
Sat May 19 14:16:37 CEST 2018 - fabian@ritter-vogt.de
- Update to 5.12.90
* New feature release
* For more details please see:
* https://www.kde.org/announcements/plasma-5.12.90.php
- Changes since 5.12.5:
* Don't create salt file if user home directory does not exist
* Return PAM_IGNORE from pam_sm_authenticate
* Avoid giving an stderr to kwallet (kde#393856)
* Move socket creation to unprivileged codepath
* Move salt creation to an unprivileged process
- Remove patches, now upstream:
* 0001-Move-salt-creation-to-an-unprivileged-process.patch
* 0002-Move-socket-creation-to-unprivileged-codepath.patch
* 0001-Avoid-giving-an-stderr-to-kwallet.patch
-------------------------------------------------------------------
Sat May 5 11:17:50 UTC 2018 - fabian@ritter-vogt.de

View File

@ -17,20 +17,14 @@
Name: pam_kwallet
Version: 5.12.5
Version: 5.12.90
Release: 0
Summary: A PAM Module for KWallet signing
License: LGPL-2.1 and GPL-2.0+ and GPL-3.0
Group: System/GUI/KDE
Url: http://www.kde.org/
Source: http://download.kde.org/stable/plasma/%{version}/kwallet-pam-%{version}.tar.xz
Source: http://download.kde.org/unstable/plasma/%{version}/kwallet-pam-%{version}.tar.xz
Source1: baselibs.conf
# PATCH-FIX-UPSTREAM
Patch1: 0001-Move-salt-creation-to-an-unprivileged-process.patch
# PATCH-FIX-UPSTREAM
Patch2: 0002-Move-socket-creation-to-unprivileged-codepath.patch
# PATCH-FIX-UPSTREAM
Patch3: 0001-Avoid-giving-an-stderr-to-kwallet.patch
BuildRequires: extra-cmake-modules >= 1.2.0
BuildRequires: kf5-filesystem
BuildRequires: libgcrypt-devel >= 1.5.0
@ -59,7 +53,6 @@ module.
%prep
%setup -q -n kwallet-pam-%{version}
%autopatch -p1
%build
%cmake_kf5 -d build -- -DLIBEXEC_INSTALL_DIR=%{_kf5_libexecdir} -DCMAKE_INSTALL_PREFIX=/
@ -81,13 +74,11 @@ module.
%endif
%files
%defattr(-,root,root)
%doc COPYING*
%license COPYING*
/%{_lib}/security/pam_kwallet5.so
%files common
%defattr(-,root,root)
%doc COPYING*
%license COPYING*
%config %{_kf5_configdir}/autostart/pam_kwallet_init.desktop
%{_kf5_libexecdir}/pam_kwallet_init