|
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) {
|