1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
|
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: 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
|
