| 0 | |
From: Albert Astals Cid <aacid@kde.org>
|
| 1 | |
Date: Tue, 1 May 2018 12:29:02 +0200
|
| 2 | |
Subject: Move salt creation to an unprivileged process
|
| 3 | |
|
| 4 | |
Opening files for writing as root is very tricky since through the power
|
| 5 | |
of symlinks we can get tricked to write in places we don't want to and
|
| 6 | |
we don't really need to be root to create the salt file
|
| 7 | |
---
|
| 8 | |
pam_kwallet.c | 121 ++++++++++++++++++++++++++++++++++------------------------
|
| 9 | |
1 file changed, 71 insertions(+), 50 deletions(-)
|
| 10 | |
|
| 11 | |
diff --git a/pam_kwallet.c b/pam_kwallet.c
|
| 12 | |
index 20d9603..083c9aa 100644
|
| 13 | |
--- a/pam_kwallet.c
|
| 14 | |
+++ b/pam_kwallet.c
|
| 15 | |
@@ -82,7 +82,7 @@ const static char *envVar = "PAM_KWALLET_LOGIN";
|
| 16 | |
|
| 17 | |
static int argumentsParsed = -1;
|
| 18 | |
|
| 19 | |
-int kwallet_hash(const char *passphrase, struct passwd *userInfo, char *key);
|
| 20 | |
+int kwallet_hash(pam_handle_t *pamh, const char *passphrase, struct passwd *userInfo, char *key);
|
| 21 | |
|
| 22 | |
static void parseArguments(int argc, const char **argv)
|
| 23 | |
{
|
| 24 | |
@@ -325,7 +325,7 @@ PAM_EXTERN int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, cons
|
| 25 | |
}
|
| 26 | |
|
| 27 | |
char *key = malloc(KWALLET_PAM_KEYSIZE);
|
| 28 | |
- if (!key || kwallet_hash(password, userInfo, key) != 0) {
|
| 29 | |
+ if (!key || kwallet_hash(pamh, password, userInfo, key) != 0) {
|
| 30 | |
free(key);
|
| 31 | |
pam_syslog(pamh, LOG_ERR, "%s: Fail into creating the hash", logPrefix);
|
| 32 | |
return PAM_IGNORE;
|
| 33 | |
@@ -352,6 +352,26 @@ PAM_EXTERN int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, cons
|
| 34 | |
return PAM_SUCCESS;
|
| 35 | |
}
|
| 36 | |
|
| 37 | |
+static int drop_privileges(struct passwd *userInfo)
|
| 38 | |
+{
|
| 39 | |
+ /* When dropping privileges from root, the `setgroups` call will
|
| 40 | |
+ * remove any extraneous groups. If we don't call this, then
|
| 41 | |
+ * even though our uid has dropped, we may still have groups
|
| 42 | |
+ * that enable us to do super-user things. This will fail if we
|
| 43 | |
+ * aren't root, so don't bother checking the return value, this
|
| 44 | |
+ * is just done as an optimistic privilege dropping function.
|
| 45 | |
+ */
|
| 46 | |
+ setgroups(0, NULL);
|
| 47 | |
+
|
| 48 | |
+ //Change to the user in case we are not it yet
|
| 49 | |
+ if (setgid (userInfo->pw_gid) < 0 || setuid (userInfo->pw_uid) < 0 ||
|
| 50 | |
+ setegid (userInfo->pw_gid) < 0 || seteuid (userInfo->pw_uid) < 0) {
|
| 51 | |
+ return -1;
|
| 52 | |
+ }
|
| 53 | |
+
|
| 54 | |
+ return 0;
|
| 55 | |
+}
|
| 56 | |
+
|
| 57 | |
static void execute_kwallet(pam_handle_t *pamh, struct passwd *userInfo, int toWalletPipe[2], int envSocket)
|
| 58 | |
{
|
| 59 | |
//In the child pam_syslog does not work, using syslog directly
|
| 60 | |
@@ -366,18 +386,8 @@ static void execute_kwallet(pam_handle_t *pamh, struct passwd *userInfo, int toW
|
| 61 | |
//This is the side of the pipe PAM will send the hash to
|
| 62 | |
close (toWalletPipe[1]);
|
| 63 | |
|
| 64 | |
- /* When dropping privileges from root, the `setgroups` call will
|
| 65 | |
- * remove any extraneous groups. If we don't call this, then
|
| 66 | |
- * even though our uid has dropped, we may still have groups
|
| 67 | |
- * that enable us to do super-user things. This will fail if we
|
| 68 | |
- * aren't root, so don't bother checking the return value, this
|
| 69 | |
- * is just done as an optimistic privilege dropping function.
|
| 70 | |
- */
|
| 71 | |
- setgroups(0, NULL);
|
| 72 | |
-
|
| 73 | |
//Change to the user in case we are not it yet
|
| 74 | |
- if (setgid (userInfo->pw_gid) < 0 || setuid (userInfo->pw_uid) < 0 ||
|
| 75 | |
- setegid (userInfo->pw_gid) < 0 || seteuid (userInfo->pw_uid) < 0) {
|
| 76 | |
+ if (drop_privileges(userInfo) < 0) {
|
| 77 | |
syslog(LOG_ERR, "%s: could not set gid/uid/euid/egit for kwalletd", logPrefix);
|
| 78 | |
goto cleanup;
|
| 79 | |
}
|
| 80 | |
@@ -619,7 +629,7 @@ PAM_EXTERN int pam_sm_chauthtok(pam_handle_t *pamh, int flags, int argc, const c
|
| 81 | |
return PAM_SUCCESS;
|
| 82 | |
}
|
| 83 | |
|
| 84 | |
-int mkpath(char *path, struct passwd *userInfo)
|
| 85 | |
+static int mkpath(char *path)
|
| 86 | |
{
|
| 87 | |
struct stat sb;
|
| 88 | |
char *slash;
|
| 89 | |
@@ -639,10 +649,6 @@ int mkpath(char *path, struct passwd *userInfo)
|
| 90 | |
errno != EEXIST)) {
|
| 91 | |
syslog(LOG_ERR, "%s: Couldn't create directory: %s because: %d-%s", logPrefix, path, errno, strerror(errno));
|
| 92 | |
return (-1);
|
| 93 | |
- } else {
|
| 94 | |
- if (chown(path, userInfo->pw_uid, userInfo->pw_gid) == -1) {
|
| 95 | |
- syslog(LOG_INFO, "%s: Couldn't change ownership of: %s", logPrefix, path);
|
| 96 | |
- }
|
| 97 | |
}
|
| 98 | |
} else if (!S_ISDIR(sb.st_mode)) {
|
| 99 | |
return (-1);
|
| 100 | |
@@ -654,34 +660,49 @@ int mkpath(char *path, struct passwd *userInfo)
|
| 101 | |
return (0);
|
| 102 | |
}
|
| 103 | |
|
| 104 | |
-static char* createNewSalt(const char *path, struct passwd *userInfo)
|
| 105 | |
+static void createNewSalt(pam_handle_t *pamh, const char *path, struct passwd *userInfo)
|
| 106 | |
{
|
| 107 | |
- unlink(path);//in case the file already exists
|
| 108 | |
+ const int pid = fork();
|
| 109 | |
+ if (pid == -1) {
|
| 110 | |
+ pam_syslog(pamh, LOG_ERR, "%s: Couldn't fork to create salt file", logPrefix);
|
| 111 | |
+ } else if (pid == 0) {
|
| 112 | |
+ // Child process
|
| 113 | |
+ if (drop_privileges(userInfo) < 0) {
|
| 114 | |
+ syslog(LOG_ERR, "%s: could not set gid/uid/euid/egit for salt file creation", logPrefix);
|
| 115 | |
+ exit(-1);
|
| 116 | |
+ }
|
| 117 | |
|
| 118 | |
- char *dir = strdup(path);
|
| 119 | |
- dir[strlen(dir) - 14] = '\0';//remove kdewallet.salt
|
| 120 | |
- mkpath(dir, userInfo);//create the path in case it does not exists
|
| 121 | |
- free(dir);
|
| 122 | |
+ unlink(path);//in case the file already exists
|
| 123 | |
|
| 124 | |
- char *salt = gcry_random_bytes(KWALLET_PAM_SALTSIZE, GCRY_STRONG_RANDOM);
|
| 125 | |
- FILE *fd = fopen(path, "w");
|
| 126 | |
+ char *dir = strdup(path);
|
| 127 | |
+ dir[strlen(dir) - 14] = '\0';//remove kdewallet.salt
|
| 128 | |
+ mkpath(dir); //create the path in case it does not exists
|
| 129 | |
+ free(dir);
|
| 130 | |
|
| 131 | |
- //If the file can't be created
|
| 132 | |
- if (fd == NULL) {
|
| 133 | |
- syslog(LOG_ERR, "%s: Couldn't open file: %s because: %d-%s", logPrefix, path, errno, strerror(errno));
|
| 134 | |
- return NULL;
|
| 135 | |
- }
|
| 136 | |
+ char *salt = gcry_random_bytes(KWALLET_PAM_SALTSIZE, GCRY_STRONG_RANDOM);
|
| 137 | |
+ FILE *fd = fopen(path, "w");
|
| 138 | |
|
| 139 | |
- fwrite(salt, KWALLET_PAM_SALTSIZE, 1, fd);
|
| 140 | |
- fclose(fd);
|
| 141 | |
+ //If the file can't be created
|
| 142 | |
+ if (fd == NULL) {
|
| 143 | |
+ syslog(LOG_ERR, "%s: Couldn't open file: %s because: %d-%s", logPrefix, path, errno, strerror(errno));
|
| 144 | |
+ exit(-2);
|
| 145 | |
+ }
|
| 146 | |
|
| 147 | |
- if (chown(path, userInfo->pw_uid, userInfo->pw_gid) == -1) {
|
| 148 | |
- syslog(LOG_ERR, "%s: Couldn't change ownership of the created salt file", logPrefix);
|
| 149 | |
- }
|
| 150 | |
+ fwrite(salt, KWALLET_PAM_SALTSIZE, 1, fd);
|
| 151 | |
+ fclose(fd);
|
| 152 | |
|
| 153 | |
- return salt;
|
| 154 | |
+ exit(0); // success
|
| 155 | |
+ } else {
|
| 156 | |
+ // pam process, just wait for child to finish
|
| 157 | |
+ int status;
|
| 158 | |
+ waitpid(pid, &status, 0);
|
| 159 | |
+ if (status != 0) {
|
| 160 | |
+ pam_syslog(pamh, LOG_ERR, "%s: Couldn't create salt file", logPrefix);
|
| 161 | |
+ }
|
| 162 | |
+ }
|
| 163 | |
}
|
| 164 | |
-int kwallet_hash(const char *passphrase, struct passwd *userInfo, char *key)
|
| 165 | |
+
|
| 166 | |
+int kwallet_hash(pam_handle_t *pamh, const char *passphrase, struct passwd *userInfo, char *key)
|
| 167 | |
{
|
| 168 | |
if (!gcry_check_version("1.5.0")) {
|
| 169 | |
syslog(LOG_ERR, "%s-kwalletd: libcrypt version is too old", logPrefix);
|
| 170 | |
@@ -700,19 +721,19 @@ int kwallet_hash(const char *passphrase, struct passwd *userInfo, char *key)
|
| 171 | |
struct stat info;
|
| 172 | |
char *salt = NULL;
|
| 173 | |
if (stat(path, &info) != 0 || info.st_size == 0) {
|
| 174 | |
- salt = createNewSalt(path, userInfo);
|
| 175 | |
- } else {
|
| 176 | |
- FILE *fd = fopen(path, "r");
|
| 177 | |
- if (fd == NULL) {
|
| 178 | |
- syslog(LOG_ERR, "%s: Couldn't open file: %s because: %d-%s", logPrefix, path, errno, strerror(errno));
|
| 179 | |
- free(path);
|
| 180 | |
- return 1;
|
| 181 | |
- }
|
| 182 | |
- salt = (char*) malloc(KWALLET_PAM_SALTSIZE);
|
| 183 | |
- memset(salt, '\0', KWALLET_PAM_SALTSIZE);
|
| 184 | |
- fread(salt, KWALLET_PAM_SALTSIZE, 1, fd);
|
| 185 | |
- fclose(fd);
|
| 186 | |
+ createNewSalt(pamh, path, userInfo);
|
| 187 | |
}
|
| 188 | |
+
|
| 189 | |
+ FILE *fd = fopen(path, "r");
|
| 190 | |
+ if (fd == NULL) {
|
| 191 | |
+ syslog(LOG_ERR, "%s: Couldn't open file: %s because: %d-%s", logPrefix, path, errno, strerror(errno));
|
| 192 | |
+ free(path);
|
| 193 | |
+ return 1;
|
| 194 | |
+ }
|
| 195 | |
+ salt = (char*) malloc(KWALLET_PAM_SALTSIZE);
|
| 196 | |
+ memset(salt, '\0', KWALLET_PAM_SALTSIZE);
|
| 197 | |
+ fread(salt, KWALLET_PAM_SALTSIZE, 1, fd);
|
| 198 | |
+ fclose(fd);
|
| 199 | |
free(path);
|
| 200 | |
|
| 201 | |
if (salt == NULL) {
|