Skip to content

Commit e4f06d0

Browse files
crueggecruegge
committed
Fix file permission security issue
The config file was read with elevated privileges, allowing users to place a symlink to a sensitive file at ~/.pam-gnupg and have (partial) file contents sent to their gpg socket. This patch introduces an additional fork() followed by dropping privileges before reading the file. It also contains various minor cleanups. Thanks to @Duncaen for pointing this out. Closes #14.
1 parent fbd75b7 commit e4f06d0

File tree

1 file changed

+81
-181
lines changed

1 file changed

+81
-181
lines changed

‎src/pam_gnupg.c

Lines changed: 81 additions & 181 deletions
Original file line numberDiff line numberDiff line change
@@ -23,68 +23,12 @@
2323
#define READ_END 0
2424
#define WRITE_END 1
2525

26-
#define tohex(n) ((n) < 10 ? ((n) + '0') : (((n) - 10) + 'A'))
27-
28-
struct userinfo {
29-
int uid, gid;
30-
char *home;
31-
};
32-
33-
void free_userinfo(struct userinfo *userinfo) {
34-
35-
if (!userinfo)
36-
return;
37-
38-
free(userinfo->home);
39-
free(userinfo);
40-
}
41-
42-
bool get_userinfo(pam_handle_t *pamh, struct userinfo **userinfo) {
43-
const char *user = NULL;
44-
struct passwd pwd, *result = NULL;
45-
char *buf = NULL;
46-
size_t bufsize;
47-
48-
*userinfo = NULL;
49-
50-
if (pam_get_user(pamh, &user, NULL) != PAM_SUCCESS || user == NULL) {
51-
return false;
52-
}
53-
54-
bufsize = sysconf(_SC_GETPW_R_SIZE_MAX);
55-
if (bufsize == -1) {
56-
bufsize = 16384;
57-
}
58-
59-
buf = malloc(bufsize);
60-
if (buf == NULL) {
61-
return false;
62-
}
63-
64-
if (getpwnam_r(user, &pwd, buf, bufsize, &result) != 0 || result == NULL ||
65-
pwd.pw_dir == NULL || pwd.pw_dir[0] != '/') {
66-
free(buf);
67-
return false;
68-
}
69-
70-
*userinfo = malloc(sizeof(*userinfo));
71-
if (*userinfo == NULL) {
72-
free(buf);
73-
return false;
74-
}
75-
76-
(*userinfo)->uid = pwd.pw_uid;
77-
(*userinfo)->gid = pwd.pw_gid;
78-
(*userinfo)->home = strdup(pwd.pw_dir);
79-
free(buf);
80-
81-
if ((*userinfo)->home == NULL) {
82-
free_userinfo(*userinfo);
83-
*userinfo = NULL;
84-
return false;
26+
char tohex(char n) {
27+
if (n < 10) {
28+
return n + '0';
29+
} else {
30+
return n - 10 + 'A';
8531
}
86-
87-
return true;
8832
}
8933

9034
/* Copied from gnupg */
@@ -126,144 +70,111 @@ void cleanup_token(pam_handle_t *pamh, void *data, int error_status) {
12670
wipestr(data);
12771
}
12872

129-
void close_safe(int fd)
130-
{
131-
if (fd != -1) {
132-
close(fd);
73+
bool preset_passphrase(pam_handle_t *pamh, const char *tok, bool autostart) {
74+
const char *user = NULL;
75+
if (pam_get_user(pamh, &user, NULL) != PAM_SUCCESS || user == NULL) {
76+
return false;
13377
}
134-
}
13578

136-
void setup_sigs(struct sigaction **old) {
137-
struct sigaction sigchld, sigpipe;
138-
if ((*old = malloc(2*sizeof(struct sigaction))) == NULL) {
139-
return;
79+
struct passwd *pwd = getpwnam(user);
80+
if (pwd == NULL) {
81+
return false;
14082
}
141-
memset(*old, 0, 2*sizeof(struct sigaction));
83+
84+
struct sigaction sigchld, old_sigchld;
14285
memset(&sigchld, 0, sizeof(sigchld));
143-
memset(&sigpipe, 0, sizeof(sigpipe));
86+
memset(&old_sigchld, 0, sizeof(old_sigchld));
14487
sigchld.sa_handler = SIG_DFL;
145-
sigpipe.sa_handler = SIG_IGN;
146-
sigaction(SIGCHLD, &sigchld, *old+0);
147-
sigaction(SIGPIPE, &sigpipe, *old+1);
148-
}
88+
sigaction(SIGCHLD, &sigchld, &old_sigchld);
14989

150-
void restore_sigs(const struct sigaction *old) {
151-
if (old == NULL) {
152-
return;
153-
}
154-
sigaction(SIGCHLD, old+0, NULL);
155-
sigaction(SIGPIPE, old+1, NULL);
156-
free((void *) old);
157-
}
158-
159-
int run_as_user(const struct userinfo *user, const char * const cmd[], int *input, char **env) {
160-
int inp[2] = {-1, -1};
161-
int pid;
162-
int dev_null;
163-
164-
if (pipe(inp) < 0) {
165-
*input = -1;
166-
return 0;
90+
pid_t pid = fork();
91+
if (pid == -1) {
92+
sigaction(SIGCHLD, &old_sigchld, NULL);
93+
return false;
94+
} else if (pid > 0) {
95+
int status;
96+
waitpid(pid, &status, 0);
97+
sigaction(SIGCHLD, &old_sigchld, NULL);
98+
return (WIFEXITED(status) && WEXITSTATUS(status) == EXIT_SUCCESS);
16799
}
168-
*input = inp[WRITE_END];
169-
170-
switch (pid = fork()) {
171-
case -1:
172-
close_safe(inp[READ_END]);
173-
close_safe(inp[WRITE_END]);
174-
*input = -1;
175-
return 0;
176100

177-
case 0:
178-
break;
101+
// We're in the child process now. From here on, the function will not return.
179102

180-
default:
181-
close_safe(inp[READ_END]);
182-
return pid;
103+
if (setregid(pwd->pw_gid, pwd->pw_gid) < 0 || setreuid(pwd->pw_uid, pwd->pw_uid) < 0) {
104+
exit(EXIT_FAILURE);
183105
}
184106

185-
/* We're in the child process now */
186-
187-
if (dup2(inp[READ_END], STDIN_FILENO) < 0) {
107+
int inp[2] = {-1, -1};
108+
if (pipe(inp) < 0) {
188109
exit(EXIT_FAILURE);
189110
}
190-
close_safe(inp[READ_END]);
191-
close_safe(inp[WRITE_END]);
111+
signal(SIGPIPE, SIG_IGN);
192112

193-
if ((dev_null = open("/dev/null", O_WRONLY)) != -1) {
113+
int dev_null = open("/dev/null", O_RDWR);
114+
if (dev_null != -1) {
115+
dup2(dev_null, STDIN_FILENO);
194116
dup2(dev_null, STDOUT_FILENO);
195117
dup2(dev_null, STDERR_FILENO);
196118
close(dev_null);
197119
}
198120

199-
if (seteuid(getuid()) < 0 || setegid(getgid()) < 0 ||
200-
setgid(user->gid) < 0 || setuid(user->uid) < 0 ||
201-
setegid(user->gid) < 0 || seteuid(user->uid) < 0) {
121+
int dirfd = open(pwd->pw_dir, O_RDONLY | O_CLOEXEC);
122+
if (dirfd < 0) {
202123
exit(EXIT_FAILURE);
203124
}
204-
205-
if (env != NULL) {
206-
execve(cmd[0], (char * const *) cmd, env);
207-
} else {
208-
execv(cmd[0], (char * const *) cmd);
209-
}
210-
exit(EXIT_FAILURE);
211-
}
212-
213-
bool preset_passphrase(pam_handle_t *pamh, const char *tok, bool autostart) {
214-
bool ret = false;
215-
216-
struct userinfo *user;
217-
if (!get_userinfo(pamh, &user)) {
218-
return false;
219-
}
220-
221-
char *keygrip_file;
222-
if (asprintf(&keygrip_file, "%s/.pam-gnupg", user->home) < 0) {
223-
goto end;
125+
int fd = openat(dirfd, ".pam-gnupg", O_RDONLY | O_CLOEXEC);
126+
if (fd < 0) {
127+
exit(EXIT_FAILURE);
224128
}
225-
FILE *file = fopen(keygrip_file, "r");
226-
free(keygrip_file);
129+
FILE *file = fdopen(fd, "r");
227130
if (file == NULL) {
228-
return false;
131+
exit(EXIT_FAILURE);
229132
}
230133

231-
struct sigaction *handlers = NULL;
232-
setup_sigs(&handlers);
233-
if (handlers == NULL) {
234-
goto end;
235-
}
134+
pid = fork();
135+
if (pid == -1) {
136+
exit(EXIT_FAILURE);
137+
} else if (pid == 0) {
138+
// Grandchild
139+
if (dup2(inp[READ_END], STDIN_FILENO) < 0) {
140+
exit(EXIT_FAILURE);
141+
}
142+
close(inp[READ_END]);
143+
close(inp[WRITE_END]);
144+
145+
// gpg-connect-agent has an option --no-autostart, which *should* return
146+
// non-zero when the agent is not running. Unfortunately, the exit code is
147+
// always 0 in version 2.1. Passing an invalid agent program here is a
148+
// workaround. See https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=797334
149+
const char *cmd[] = {GPG_CONNECT_AGENT, "--agent-program", "/dev/null", NULL};
150+
if (autostart) {
151+
cmd[1] = NULL;
152+
}
236153

237-
/* gpg-connect-agent has an option --no-autostart, which *should* return
238-
* non-zero when the agent is not running. Unfortunately, the exit code is
239-
* always 0 in version 2.1. Passing an invalid agent program here is a
240-
* workaround. See https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=797334 */
241-
const char *cmd[] = {GPG_CONNECT_AGENT, "--agent-program", "/dev/null", NULL};
242-
if (autostart) {
243-
cmd[1] = NULL;
154+
char **env = pam_getenvlist(pamh);
155+
if (env != NULL) {
156+
execve(cmd[0], (char * const *) cmd, env);
157+
} else {
158+
execv(cmd[0], (char * const *) cmd);
159+
}
244160
}
245161

246-
int input;
247-
char **env = pam_getenvlist(pamh);
248-
const int pid = run_as_user(user, cmd, &input, env);
249-
if (env != NULL) {
250-
free(env);
251-
}
252-
if (pid == 0 || input < 0) {
253-
goto end;
254-
}
162+
close(inp[READ_END]);
255163

256164
char *presetcmd;
257165
const int presetlen = asprintf(&presetcmd, "PRESET_PASSPHRASE xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx -1 %s\n", tok);
258166
if (presetlen < 0) {
259-
presetcmd = NULL;
260-
goto end;
167+
exit(EXIT_FAILURE);
261168
}
262169
char * const keygrip = presetcmd + 18;
263170

264171
char *line = NULL;
265172
size_t len = 0;
266-
while (getline(&line, &len, file) != -1) {
173+
ssize_t rd;
174+
while ((rd = getline(&line, &len, file)) != -1) {
175+
if (line[rd-1] == '\n') {
176+
line[rd-1] = '\0';
177+
}
267178
const char *cur = line;
268179
while (*cur && strchr(" \t\n\r\f\v", *cur)) {
269180
cur++;
@@ -273,33 +184,22 @@ bool preset_passphrase(pam_handle_t *pamh, const char *tok, bool autostart) {
273184
}
274185
strncpy(keygrip, cur, KEYGRIP_LENGTH);
275186
if (strlen(keygrip) < KEYGRIP_LENGTH) {
276-
/* We hit eol sooner than expected. */
187+
// We hit an unexpected eol or null byte.
277188
continue;
278189
}
279-
if (write(input, presetcmd, presetlen) < 0) {
280-
/* If anything goes wrong, we just stop here. No attempt is made to
281-
* clean passphrases that were set in a previous iteration. */
282-
close(input);
283-
goto end;
190+
if (write(inp[WRITE_END], presetcmd, presetlen) < 0) {
191+
exit(EXIT_FAILURE);
284192
}
285193
}
286194

287195
int status;
288-
close(input);
196+
close(inp[WRITE_END]);
289197
waitpid(pid, &status, 0);
290-
ret = (WIFEXITED(status) && WEXITSTATUS(status) == 0);
291-
292-
end:
293-
wipestr(presetcmd);
294-
restore_sigs(handlers);
295-
free_userinfo(user);
296-
if (file != NULL) {
297-
fclose(file);
298-
}
299-
if (line != NULL) {
300-
free(line);
198+
if (WIFEXITED(status) && WEXITSTATUS(status) == 0) {
199+
exit(EXIT_SUCCESS);
200+
} else {
201+
exit(EXIT_FAILURE);
301202
}
302-
return ret;
303203
}
304204

305205
int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, const char **argv) {

0 commit comments

Comments
 (0)