| blob | 7e370649bdd7ae80adf7038acade382eef3f1581 |
1 From 7f9711dd902a239c499682015d708f73ec884af2 Mon Sep 17 00:00:00 2001
2 From: Aurelien Aptel <aaptel@suse.com>
3 Date: Wed, 21 Apr 2021 16:22:15 +0200
4 Subject: [PATCH] cifs.upcall: fix regression in kerberos mount
6 The fix for CVE-2021-20208 in commit e461afd ("cifs.upcall: try to use
7 container ipc/uts/net/pid/mnt/user namespaces") introduced a
8 regression for kerberos mounts when cifs-utils is built with
9 libcap-ng. It makes mount fail with ENOKEY "Required key not
10 available".
12 Current state:
14 mount.cifs
15 '---> mount() ---> kernel
16 negprot, session setup (need security blob for krb)
17 request_key("cifs.spnego", payload="pid=%d;username=...")
18 upcall
19 /sbin/request-key <--------------'
20 reads /etc/request-keys.conf
21 dispatch cifs.spnego request
22 calls /usr/sbin/cifs.upcall <key id>
23 - drop privileges (capabilities)
24 - fetch keyid
25 - parse payload
26 - switch to mount.cifs namespaces
27 - call krb5_xxx() funcs
28 - generate security blob
29 - set key value to security blob
30 '-----------------------------------> kernel
31 put blob in session setup packet
32 continue auth
33 open tcon
34 get share root
35 setup superblock
36 mount.cifs mount() returns <-----------'
38 By the time cifs.upcall tries to switch to namespaces, enough
39 capabilities have dropped in trim_capabilities() that it makes setns()
40 fail with EPERM.
42 setns() requires CAP_SYS_ADMIN.
44 With libcap trim_capabilities() is a no-op.
46 This fix:
48 - moves the namespace switch earlier so that operations like
49 setgroups(), setgid(), scanning of pid environment, ... happens in the
50 contained namespaces.
51 - moves trim_capabilities() after the namespace switch
52 - moves the string processing to decode the key request payload in a
53 child process with minimum capabilities. the decoded data is shared
54 with the parent process via shared memory obtained with mmap().
56 Fixes: e461afd ("cifs.upcall: try to use container ipc/uts/net/pid/mnt/user namespaces")
57 Signed-off-by: Aurelien Aptel <aaptel@suse.com>
58 ---
59 cifs.upcall.c | 214 ++++++++++++++++++++++++++++++++------------------
60 1 file changed, 139 insertions(+), 75 deletions(-)
62 diff --git a/cifs.upcall.c b/cifs.upcall.c
63 index e413934..ad04301 100644
64 --- a/cifs.upcall.c
65 +++ b/cifs.upcall.c
66 @@ -52,6 +52,9 @@
67 #include <stdbool.h>
68 #include <errno.h>
69 #include <sched.h>
70 +#include <sys/mman.h>
71 +#include <sys/types.h>
72 +#include <sys/wait.h>
74 #include "data_blob.h"
75 #include "spnego.h"
76 @@ -787,6 +790,25 @@ handle_krb5_mech(const char *oid, const char *host, DATA_BLOB * secblob,
77 return retval;
78 }
80 +
81 +
82 +struct decoded_args {
83 + int ver;
84 + char hostname[NI_MAXHOST + 1];
85 + char ip[NI_MAXHOST + 1];
86 +
87 +/* Max user name length. */
88 +#define MAX_USERNAME_SIZE 256
89 + char username[MAX_USERNAME_SIZE + 1];
90 +
91 + uid_t uid;
92 + uid_t creduid;
93 + pid_t pid;
94 + sectype_t sec;
95 +
96 +/*
97 + * Flags to keep track of what was provided
98 + */
99 #define DKD_HAVE_HOSTNAME 0x1
100 #define DKD_HAVE_VERSION 0x2
101 #define DKD_HAVE_SEC 0x4
102 @@ -796,23 +818,13 @@ handle_krb5_mech(const char *oid, const char *host, DATA_BLOB * secblob,
103 #define DKD_HAVE_CREDUID 0x40
104 #define DKD_HAVE_USERNAME 0x80
105 #define DKD_MUSTHAVE_SET (DKD_HAVE_HOSTNAME|DKD_HAVE_VERSION|DKD_HAVE_SEC)
106 -
107 -struct decoded_args {
108 - int ver;
109 - char *hostname;
110 - char *ip;
111 - char *username;
112 - uid_t uid;
113 - uid_t creduid;
114 - pid_t pid;
115 - sectype_t sec;
116 + int have;
117 };
119 static unsigned int
120 -decode_key_description(const char *desc, struct decoded_args *arg)
121 +__decode_key_description(const char *desc, struct decoded_args *arg)
122 {
123 - int len;
124 - int retval = 0;
125 + size_t len;
126 char *pos;
127 const char *tkn = desc;
129 @@ -826,13 +838,13 @@ decode_key_description(const char *desc, struct decoded_args *arg)
130 len = pos - tkn;
132 len -= 5;
133 - free(arg->hostname);
134 - arg->hostname = strndup(tkn + 5, len);
135 - if (arg->hostname == NULL) {
136 - syslog(LOG_ERR, "Unable to allocate memory");
137 + if (len > sizeof(arg->hostname)-1) {
138 + syslog(LOG_ERR, "host= value too long for buffer");
139 return 1;
140 }
141 - retval |= DKD_HAVE_HOSTNAME;
142 + memset(arg->hostname, 0, sizeof(arg->hostname));
143 + strncpy(arg->hostname, tkn + 5, len);
144 + arg->have |= DKD_HAVE_HOSTNAME;
145 syslog(LOG_DEBUG, "host=%s", arg->hostname);
146 } else if (!strncmp(tkn, "ip4=", 4) || !strncmp(tkn, "ip6=", 4)) {
147 if (pos == NULL)
148 @@ -841,13 +853,13 @@ decode_key_description(const char *desc, struct decoded_args *arg)
149 len = pos - tkn;
151 len -= 4;
152 - free(arg->ip);
153 - arg->ip = strndup(tkn + 4, len);
154 - if (arg->ip == NULL) {
155 - syslog(LOG_ERR, "Unable to allocate memory");
156 + if (len > sizeof(arg->ip)-1) {
157 + syslog(LOG_ERR, "ip[46]= value too long for buffer");
158 return 1;
159 }
160 - retval |= DKD_HAVE_IP;
161 + memset(arg->ip, 0, sizeof(arg->ip));
162 + strncpy(arg->ip, tkn + 4, len);
163 + arg->have |= DKD_HAVE_IP;
164 syslog(LOG_DEBUG, "ip=%s", arg->ip);
165 } else if (strncmp(tkn, "user=", 5) == 0) {
166 if (pos == NULL)
167 @@ -856,13 +868,13 @@ decode_key_description(const char *desc, struct decoded_args *arg)
168 len = pos - tkn;
170 len -= 5;
171 - free(arg->username);
172 - arg->username = strndup(tkn + 5, len);
173 - if (arg->username == NULL) {
174 - syslog(LOG_ERR, "Unable to allocate memory");
175 + if (len > sizeof(arg->username)-1) {
176 + syslog(LOG_ERR, "user= value too long for buffer");
177 return 1;
178 }
179 - retval |= DKD_HAVE_USERNAME;
180 + memset(arg->username, 0, sizeof(arg->username));
181 + strncpy(arg->username, tkn + 5, len);
182 + arg->have |= DKD_HAVE_USERNAME;
183 syslog(LOG_DEBUG, "user=%s", arg->username);
184 } else if (strncmp(tkn, "pid=", 4) == 0) {
185 errno = 0;
186 @@ -873,13 +885,13 @@ decode_key_description(const char *desc, struct decoded_args *arg)
187 return 1;
188 }
189 syslog(LOG_DEBUG, "pid=%u", arg->pid);
190 - retval |= DKD_HAVE_PID;
191 + arg->have |= DKD_HAVE_PID;
192 } else if (strncmp(tkn, "sec=", 4) == 0) {
193 if (strncmp(tkn + 4, "krb5", 4) == 0) {
194 - retval |= DKD_HAVE_SEC;
195 + arg->have |= DKD_HAVE_SEC;
196 arg->sec = KRB5;
197 } else if (strncmp(tkn + 4, "mskrb5", 6) == 0) {
198 - retval |= DKD_HAVE_SEC;
199 + arg->have |= DKD_HAVE_SEC;
200 arg->sec = MS_KRB5;
201 }
202 syslog(LOG_DEBUG, "sec=%d", arg->sec);
203 @@ -891,7 +903,7 @@ decode_key_description(const char *desc, struct decoded_args *arg)
204 strerror(errno));
205 return 1;
206 }
207 - retval |= DKD_HAVE_UID;
208 + arg->have |= DKD_HAVE_UID;
209 syslog(LOG_DEBUG, "uid=%u", arg->uid);
210 } else if (strncmp(tkn, "creduid=", 8) == 0) {
211 errno = 0;
212 @@ -901,7 +913,7 @@ decode_key_description(const char *desc, struct decoded_args *arg)
213 strerror(errno));
214 return 1;
215 }
216 - retval |= DKD_HAVE_CREDUID;
217 + arg->have |= DKD_HAVE_CREDUID;
218 syslog(LOG_DEBUG, "creduid=%u", arg->creduid);
219 } else if (strncmp(tkn, "ver=", 4) == 0) { /* if version */
220 errno = 0;
221 @@ -911,14 +923,56 @@ decode_key_description(const char *desc, struct decoded_args *arg)
222 strerror(errno));
223 return 1;
224 }
225 - retval |= DKD_HAVE_VERSION;
226 + arg->have |= DKD_HAVE_VERSION;
227 syslog(LOG_DEBUG, "ver=%d", arg->ver);
228 }
229 if (pos == NULL)
230 break;
231 tkn = pos + 1;
232 } while (tkn);
233 - return retval;
234 + return 0;
235 +}
236 +
237 +static unsigned int
238 +decode_key_description(const char *desc, struct decoded_args **arg)
239 +{
240 + pid_t pid;
241 + pid_t rc;
242 + int status;
243 +
244 + /*
245 + * Do all the decoding/string processing in a child process
246 + * with low privileges.
247 + */
248 +
249 + *arg = mmap(NULL, sizeof(struct decoded_args), PROT_READ | PROT_WRITE,
250 + MAP_ANONYMOUS | MAP_SHARED, -1, 0);
251 + if (*arg == MAP_FAILED) {
252 + syslog(LOG_ERR, "%s: mmap failed: %s", __func__, strerror(errno));
253 + return -1;
254 + }
255 +
256 + pid = fork();
257 + if (pid < 0) {
258 + syslog(LOG_ERR, "%s: fork failed: %s", __func__, strerror(errno));
259 + munmap(*arg, sizeof(struct decoded_args));
260 + *arg = NULL;
261 + return -1;
262 + }
263 + if (pid == 0) {
264 + /* do the parsing in child */
265 + drop_all_capabilities();
266 + exit(__decode_key_description(desc, *arg));
267 + }
268 +
269 + rc = waitpid(pid, &status, 0);
270 + if (rc < 0 || !WIFEXITED(status) || WEXITSTATUS(status) != 0) {
271 + munmap(*arg, sizeof(struct decoded_args));
272 + *arg = NULL;
273 + return 1;
274 + }
275 +
276 + return 0;
277 }
279 static int setup_key(const key_serial_t key, const void *data, size_t datalen)
280 @@ -1098,7 +1152,7 @@ int main(const int argc, char *const argv[])
281 bool try_dns = false, legacy_uid = false , env_probe = true;
282 char *buf;
283 char hostbuf[NI_MAXHOST], *host;
284 - struct decoded_args arg;
285 + struct decoded_args *arg = NULL;
286 const char *oid;
287 uid_t uid;
288 char *keytab_name = NULL;
289 @@ -1109,7 +1163,6 @@ int main(const int argc, char *const argv[])
290 const char *key_descr = NULL;
292 hostbuf[0] = '\0';
293 - memset(&arg, 0, sizeof(arg));
295 openlog(prog, 0, LOG_DAEMON);
297 @@ -1150,9 +1203,6 @@ int main(const int argc, char *const argv[])
298 }
299 }
301 - if (trim_capabilities(env_probe))
302 - goto out;
303 -
304 /* is there a key? */
305 if (argc <= optind) {
306 usage();
307 @@ -1178,6 +1228,10 @@ int main(const int argc, char *const argv[])
309 syslog(LOG_DEBUG, "key description: %s", buf);
311 + /*
312 + * If we are requested a simple DNS query, do it and exit
313 + */
314 +
315 if (strncmp(buf, "cifs.resolver", sizeof("cifs.resolver") - 1) == 0)
316 key_descr = ".cifs.resolver";
317 else if (strncmp(buf, "dns_resolver", sizeof("dns_resolver") - 1) == 0)
318 @@ -1187,33 +1241,42 @@ int main(const int argc, char *const argv[])
319 goto out;
320 }
322 - have = decode_key_description(buf, &arg);
323 + /*
324 + * Otherwise, it's a spnego key request
325 + */
326 +
327 + rc = decode_key_description(buf, &arg);
328 free(buf);
329 - if ((have & DKD_MUSTHAVE_SET) != DKD_MUSTHAVE_SET) {
330 + if (rc) {
331 + syslog(LOG_ERR, "failed to decode key description");
332 + goto out;
333 + }
334 +
335 + if ((arg->have & DKD_MUSTHAVE_SET) != DKD_MUSTHAVE_SET) {
336 syslog(LOG_ERR, "unable to get necessary params from key "
337 "description (0x%x)", have);
338 rc = 1;
339 goto out;
340 }
342 - if (arg.ver > CIFS_SPNEGO_UPCALL_VERSION) {
343 + if (arg->ver > CIFS_SPNEGO_UPCALL_VERSION) {
344 syslog(LOG_ERR, "incompatible kernel upcall version: 0x%x",
345 - arg.ver);
346 + arg->ver);
347 rc = 1;
348 goto out;
349 }
351 - if (strlen(arg.hostname) >= NI_MAXHOST) {
352 + if (strlen(arg->hostname) >= NI_MAXHOST) {
353 syslog(LOG_ERR, "hostname provided by kernel is too long");
354 rc = 1;
355 goto out;
357 }
359 - if (!legacy_uid && (have & DKD_HAVE_CREDUID))
360 - uid = arg.creduid;
361 - else if (have & DKD_HAVE_UID)
362 - uid = arg.uid;
363 + if (!legacy_uid && (arg->have & DKD_HAVE_CREDUID))
364 + uid = arg->creduid;
365 + else if (arg->have & DKD_HAVE_UID)
366 + uid = arg->uid;
367 else {
368 /* no uid= or creduid= parm -- something is wrong */
369 syslog(LOG_ERR, "No uid= or creduid= parm specified");
370 @@ -1221,6 +1284,21 @@ int main(const int argc, char *const argv[])
371 goto out;
372 }
374 + /*
375 + * Change to the process's namespace. This means that things will work
376 + * acceptably in containers, because we'll be looking at the correct
377 + * filesystem and have the correct network configuration.
378 + */
379 + rc = switch_to_process_ns(arg->pid);
380 + if (rc == -1) {
381 + syslog(LOG_ERR, "unable to switch to process namespace: %s", strerror(errno));
382 + rc = 1;
383 + goto out;
384 + }
385 +
386 + if (trim_capabilities(env_probe))
387 + goto out;
388 +
389 /*
390 * The kernel doesn't pass down the gid, so we resort here to scraping
391 * one out of the passwd nss db. Note that this might not reflect the
392 @@ -1266,20 +1344,7 @@ int main(const int argc, char *const argv[])
393 * look at the environ file.
394 */
395 env_cachename =
396 - get_cachename_from_process_env(env_probe ? arg.pid : 0);
397 -
398 - /*
399 - * Change to the process's namespace. This means that things will work
400 - * acceptably in containers, because we'll be looking at the correct
401 - * filesystem and have the correct network configuration.
402 - */
403 - rc = switch_to_process_ns(arg.pid);
404 - if (rc == -1) {
405 - syslog(LOG_ERR, "unable to switch to process namespace: %s",
406 - strerror(errno));
407 - rc = 1;
408 - goto out;
409 - }
410 + get_cachename_from_process_env(env_probe ? arg->pid : 0);
412 rc = setuid(uid);
413 if (rc == -1) {
414 @@ -1301,18 +1366,18 @@ int main(const int argc, char *const argv[])
416 ccache = get_existing_cc(env_cachename);
417 /* Couldn't find credcache? Try to use keytab */
418 - if (ccache == NULL && arg.username != NULL)
419 - ccache = init_cc_from_keytab(keytab_name, arg.username);
420 + if (ccache == NULL && arg->username[0] != '\0')
421 + ccache = init_cc_from_keytab(keytab_name, arg->username);
423 if (ccache == NULL) {
424 rc = 1;
425 goto out;
426 }
428 - host = arg.hostname;
429 + host = arg->hostname;
431 // do mech specific authorization
432 - switch (arg.sec) {
433 + switch (arg->sec) {
434 case MS_KRB5:
435 case KRB5:
436 /*
437 @@ -1328,7 +1393,7 @@ int main(const int argc, char *const argv[])
438 * TRY only:
439 * cifs/bar.example.com@REALM
440 */
441 - if (arg.sec == MS_KRB5)
442 + if (arg->sec == MS_KRB5)
443 oid = OID_KERBEROS5_OLD;
444 else
445 oid = OID_KERBEROS5;
446 @@ -1385,10 +1450,10 @@ retry_new_hostname:
447 break;
448 }
450 - if (!try_dns || !(have & DKD_HAVE_IP))
451 + if (!try_dns || !(arg->have & DKD_HAVE_IP))
452 break;
454 - rc = ip_to_fqdn(arg.ip, hostbuf, sizeof(hostbuf));
455 + rc = ip_to_fqdn(arg->ip, hostbuf, sizeof(hostbuf));
456 if (rc)
457 break;
459 @@ -1396,7 +1461,7 @@ retry_new_hostname:
460 host = hostbuf;
461 goto retry_new_hostname;
462 default:
463 - syslog(LOG_ERR, "sectype: %d is not implemented", arg.sec);
464 + syslog(LOG_ERR, "sectype: %d is not implemented", arg->sec);
465 rc = 1;
466 break;
467 }
468 @@ -1414,7 +1479,7 @@ retry_new_hostname:
469 rc = 1;
470 goto out;
471 }
472 - keydata->version = arg.ver;
473 + keydata->version = arg->ver;
474 keydata->flags = 0;
475 keydata->sesskey_len = sess_key.length;
476 keydata->secblob_len = secblob.length;
477 @@ -1440,11 +1505,10 @@ out:
478 krb5_cc_close(context, ccache);
479 if (context)
480 krb5_free_context(context);
481 - free(arg.hostname);
482 - free(arg.ip);
483 - free(arg.username);
484 free(keydata);
485 free(env_cachename);
486 + if (arg)
487 + munmap(arg, sizeof(*arg));
488 syslog(LOG_DEBUG, "Exit status %ld", rc);
489 return rc;
490 }
491 --
492 2.33.0