cifs.upcall: fix regression in kerberos mount

The fix for CVE-2021-20208 in commit e461afd ("cifs.upcall: try to use
container ipc/uts/net/pid/mnt/user namespaces") introduced a
regression for kerberos mounts when cifs-utils is built with
libcap-ng. It makes mount fail with ENOKEY "Required key not
available".

Current state:

mount.cifs
 '---> mount() ---> kernel
       	           negprot, session setup (need security blob for krb)
                   request_key("cifs.spnego",  payload="pid=%d;username=...")
                               upcall
  /sbin/request-key <--------------'
   reads /etc/request-keys.conf
   dispatch cifs.spnego request
   calls /usr/sbin/cifs.upcall <key id>
   - drop privileges (capabilities)
   - fetch keyid
   - parse payload
   - switch to mount.cifs namespaces
   - call krb5_xxx() funcs
   - generate security blob
   - set key value to security blob
      '-----------------------------------> kernel
                                         put blob in session setup packet
  				       continue auth
  				       open tcon
  				       get share root
  				       setup superblock
mount.cifs mount() returns    <-----------'

By the time cifs.upcall tries to switch to namespaces, enough
capabilities have dropped in trim_capabilities() that it makes setns()
fail with EPERM.

setns() requires CAP_SYS_ADMIN.

With libcap trim_capabilities() is a no-op.

This fix:

- moves the namespace switch earlier so that operations like
  setgroups(), setgid(), scanning of pid environment, ... happens in the
  contained namespaces.
- moves trim_capabilities() after the namespace switch
- moves the string processing to decode the key request payload in a
  child process with minimum capabilities. the decoded data is shared
  with the parent process via shared memory obtained with mmap().

Fixes: e461afd ("cifs.upcall: try to use container ipc/uts/net/pid/mnt/user namespaces")
Signed-off-by: Aurelien Aptel <aaptel@suse.com>

diff --git a/cifs.upcall.c b/cifs.upcall.c
index e4139349fea7539dbef8b781c19f07b0ec5f34e6..ad0430157958a1067f28af6df0cbcda8ed2e0f63 100644 (file)
--- a/cifs.upcall.c
+++ b/cifs.upcall.c
@@ -52,6 +52,9 @@
 #include <stdbool.h>
 #include <errno.h>
 #include <sched.h>
+#include <sys/mman.h>
+#include <sys/types.h>
+#include <sys/wait.h>
 
 #include "data_blob.h"
 #include "spnego.h"
@@ -787,6 +790,25 @@ handle_krb5_mech(const char *oid, const char *host, DATA_BLOB * secblob,
        return retval;
 }
 
+
+
+struct decoded_args {
+       int ver;
+       char hostname[NI_MAXHOST + 1];
+       char ip[NI_MAXHOST + 1];
+
+/* Max user name length. */
+#define MAX_USERNAME_SIZE 256
+       char username[MAX_USERNAME_SIZE + 1];
+
+       uid_t uid;
+       uid_t creduid;
+       pid_t pid;
+       sectype_t sec;
+
+/*
+ * Flags to keep track of what was provided
+ */
 #define DKD_HAVE_HOSTNAME      0x1
 #define DKD_HAVE_VERSION       0x2
 #define DKD_HAVE_SEC           0x4
@@ -796,23 +818,13 @@ handle_krb5_mech(const char *oid, const char *host, DATA_BLOB * secblob,
 #define DKD_HAVE_CREDUID       0x40
 #define DKD_HAVE_USERNAME      0x80
 #define DKD_MUSTHAVE_SET (DKD_HAVE_HOSTNAME|DKD_HAVE_VERSION|DKD_HAVE_SEC)
-
-struct decoded_args {
-       int ver;
-       char *hostname;
-       char *ip;
-       char *username;
-       uid_t uid;
-       uid_t creduid;
-       pid_t pid;
-       sectype_t sec;
+       int have;
 };
 
 static unsigned int
-decode_key_description(const char *desc, struct decoded_args *arg)
+__decode_key_description(const char *desc, struct decoded_args *arg)
 {
-       int len;
-       int retval = 0;
+       size_t len;
        char *pos;
        const char *tkn = desc;
 
@@ -826,13 +838,13 @@ decode_key_description(const char *desc, struct decoded_args *arg)
                                len = pos - tkn;
 
                        len -= 5;
-                       free(arg->hostname);
-                       arg->hostname = strndup(tkn + 5, len);
-                       if (arg->hostname == NULL) {
-                               syslog(LOG_ERR, "Unable to allocate memory");
+                       if (len > sizeof(arg->hostname)-1) {
+                               syslog(LOG_ERR, "host= value too long for buffer");
                                return 1;
                        }
-                       retval |= DKD_HAVE_HOSTNAME;
+                       memset(arg->hostname, 0, sizeof(arg->hostname));
+                       strncpy(arg->hostname, tkn + 5, len);
+                       arg->have |= DKD_HAVE_HOSTNAME;
                        syslog(LOG_DEBUG, "host=%s", arg->hostname);
                } else if (!strncmp(tkn, "ip4=", 4) || !strncmp(tkn, "ip6=", 4)) {
                        if (pos == NULL)
@@ -841,13 +853,13 @@ decode_key_description(const char *desc, struct decoded_args *arg)
                                len = pos - tkn;
 
                        len -= 4;
-                       free(arg->ip);
-                       arg->ip = strndup(tkn + 4, len);
-                       if (arg->ip == NULL) {
-                               syslog(LOG_ERR, "Unable to allocate memory");
+                       if (len > sizeof(arg->ip)-1) {
+                               syslog(LOG_ERR, "ip[46]= value too long for buffer");
                                return 1;
                        }
-                       retval |= DKD_HAVE_IP;
+                       memset(arg->ip, 0, sizeof(arg->ip));
+                       strncpy(arg->ip, tkn + 4, len);
+                       arg->have |= DKD_HAVE_IP;
                        syslog(LOG_DEBUG, "ip=%s", arg->ip);
                } else if (strncmp(tkn, "user=", 5) == 0) {
                        if (pos == NULL)
@@ -856,13 +868,13 @@ decode_key_description(const char *desc, struct decoded_args *arg)
                                len = pos - tkn;
 
                        len -= 5;
-                       free(arg->username);
-                       arg->username = strndup(tkn + 5, len);
-                       if (arg->username == NULL) {
-                               syslog(LOG_ERR, "Unable to allocate memory");
+                       if (len > sizeof(arg->username)-1) {
+                               syslog(LOG_ERR, "user= value too long for buffer");
                                return 1;
                        }
-                       retval |= DKD_HAVE_USERNAME;
+                       memset(arg->username, 0, sizeof(arg->username));
+                       strncpy(arg->username, tkn + 5, len);
+                       arg->have |= DKD_HAVE_USERNAME;
                        syslog(LOG_DEBUG, "user=%s", arg->username);
                } else if (strncmp(tkn, "pid=", 4) == 0) {
                        errno = 0;
@@ -873,13 +885,13 @@ decode_key_description(const char *desc, struct decoded_args *arg)
                                return 1;
                        }
                        syslog(LOG_DEBUG, "pid=%u", arg->pid);
-                       retval |= DKD_HAVE_PID;
+                       arg->have |= DKD_HAVE_PID;
                } else if (strncmp(tkn, "sec=", 4) == 0) {
                        if (strncmp(tkn + 4, "krb5", 4) == 0) {
-                               retval |= DKD_HAVE_SEC;
+                               arg->have |= DKD_HAVE_SEC;
                                arg->sec = KRB5;
                        } else if (strncmp(tkn + 4, "mskrb5", 6) == 0) {
-                               retval |= DKD_HAVE_SEC;
+                               arg->have |= DKD_HAVE_SEC;
                                arg->sec = MS_KRB5;
                        }
                        syslog(LOG_DEBUG, "sec=%d", arg->sec);
@@ -891,7 +903,7 @@ decode_key_description(const char *desc, struct decoded_args *arg)
                                       strerror(errno));
                                return 1;
                        }
-                       retval |= DKD_HAVE_UID;
+                       arg->have |= DKD_HAVE_UID;
                        syslog(LOG_DEBUG, "uid=%u", arg->uid);
                } else if (strncmp(tkn, "creduid=", 8) == 0) {
                        errno = 0;
@@ -901,7 +913,7 @@ decode_key_description(const char *desc, struct decoded_args *arg)
                                       strerror(errno));
                                return 1;
                        }
-                       retval |= DKD_HAVE_CREDUID;
+                       arg->have |= DKD_HAVE_CREDUID;
                        syslog(LOG_DEBUG, "creduid=%u", arg->creduid);
                } else if (strncmp(tkn, "ver=", 4) == 0) {      /* if version */
                        errno = 0;
@@ -911,14 +923,56 @@ decode_key_description(const char *desc, struct decoded_args *arg)
                                       strerror(errno));
                                return 1;
                        }
-                       retval |= DKD_HAVE_VERSION;
+                       arg->have |= DKD_HAVE_VERSION;
                        syslog(LOG_DEBUG, "ver=%d", arg->ver);
                }
                if (pos == NULL)
                        break;
                tkn = pos + 1;
        } while (tkn);
-       return retval;
+       return 0;
+}
+
+static unsigned int
+decode_key_description(const char *desc, struct decoded_args **arg)
+{
+       pid_t pid;
+       pid_t rc;
+       int status;
+
+       /*
+        * Do all the decoding/string processing in a child process
+        * with low privileges.
+        */
+
+       *arg = mmap(NULL, sizeof(struct decoded_args), PROT_READ | PROT_WRITE,
+                   MAP_ANONYMOUS | MAP_SHARED, -1, 0);
+       if (*arg == MAP_FAILED) {
+               syslog(LOG_ERR, "%s: mmap failed: %s", __func__, strerror(errno));
+               return -1;
+       }
+
+       pid = fork();
+       if (pid < 0) {
+               syslog(LOG_ERR, "%s: fork failed: %s", __func__, strerror(errno));
+               munmap(*arg, sizeof(struct decoded_args));
+               *arg = NULL;
+               return -1;
+       }
+       if (pid == 0) {
+               /* do the parsing in child */
+               drop_all_capabilities();
+               exit(__decode_key_description(desc, *arg));
+       }
+
+       rc = waitpid(pid, &status, 0);
+       if (rc < 0 || !WIFEXITED(status) || WEXITSTATUS(status) != 0) {
+               munmap(*arg, sizeof(struct decoded_args));
+               *arg = NULL;
+               return 1;
+       }
+
+       return 0;
 }
 
 static int setup_key(const key_serial_t key, const void *data, size_t datalen)
@@ -1098,7 +1152,7 @@ int main(const int argc, char *const argv[])
        bool try_dns = false, legacy_uid = false , env_probe = true;
        char *buf;
        char hostbuf[NI_MAXHOST], *host;
-       struct decoded_args arg;
+       struct decoded_args *arg = NULL;
        const char *oid;
        uid_t uid;
        char *keytab_name = NULL;
@@ -1109,7 +1163,6 @@ int main(const int argc, char *const argv[])
        const char *key_descr = NULL;
 
        hostbuf[0] = '\0';
-       memset(&arg, 0, sizeof(arg));
 
        openlog(prog, 0, LOG_DAEMON);
 
@@ -1150,9 +1203,6 @@ int main(const int argc, char *const argv[])
                }
        }
 
-       if (trim_capabilities(env_probe))
-               goto out;
-
        /* is there a key? */
        if (argc <= optind) {
                usage();
@@ -1178,6 +1228,10 @@ int main(const int argc, char *const argv[])
 
        syslog(LOG_DEBUG, "key description: %s", buf);
 
+       /*
+        * If we are requested a simple DNS query, do it and exit
+        */
+
        if (strncmp(buf, "cifs.resolver", sizeof("cifs.resolver") - 1) == 0)
                key_descr = ".cifs.resolver";
        else if (strncmp(buf, "dns_resolver", sizeof("dns_resolver") - 1) == 0)
@@ -1187,33 +1241,42 @@ int main(const int argc, char *const argv[])
                goto out;
        }
 
-       have = decode_key_description(buf, &arg);
+       /*
+        * Otherwise, it's a spnego key request
+        */
+
+       rc = decode_key_description(buf, &arg);
        free(buf);
-       if ((have & DKD_MUSTHAVE_SET) != DKD_MUSTHAVE_SET) {
+       if (rc) {
+               syslog(LOG_ERR, "failed to decode key description");
+               goto out;
+       }
+
+       if ((arg->have & DKD_MUSTHAVE_SET) != DKD_MUSTHAVE_SET) {
                syslog(LOG_ERR, "unable to get necessary params from key "
                       "description (0x%x)", have);
                rc = 1;
                goto out;
        }
 
-       if (arg.ver > CIFS_SPNEGO_UPCALL_VERSION) {
+       if (arg->ver > CIFS_SPNEGO_UPCALL_VERSION) {
                syslog(LOG_ERR, "incompatible kernel upcall version: 0x%x",
-                      arg.ver);
+                      arg->ver);
                rc = 1;
                goto out;
        }
 
-       if (strlen(arg.hostname) >= NI_MAXHOST) {
+       if (strlen(arg->hostname) >= NI_MAXHOST) {
                syslog(LOG_ERR, "hostname provided by kernel is too long");
                rc = 1;
                goto out;
 
        }
 
-       if (!legacy_uid && (have & DKD_HAVE_CREDUID))
-               uid = arg.creduid;
-       else if (have & DKD_HAVE_UID)
-               uid = arg.uid;
+       if (!legacy_uid && (arg->have & DKD_HAVE_CREDUID))
+               uid = arg->creduid;
+       else if (arg->have & DKD_HAVE_UID)
+               uid = arg->uid;
        else {
                /* no uid= or creduid= parm -- something is wrong */
                syslog(LOG_ERR, "No uid= or creduid= parm specified");
@@ -1221,6 +1284,21 @@ int main(const int argc, char *const argv[])
                goto out;
        }
 
+       /*
+        * Change to the process's namespace. This means that things will work
+        * acceptably in containers, because we'll be looking at the correct
+        * filesystem and have the correct network configuration.
+        */
+       rc = switch_to_process_ns(arg->pid);
+       if (rc == -1) {
+               syslog(LOG_ERR, "unable to switch to process namespace: %s", strerror(errno));
+               rc = 1;
+               goto out;
+       }
+
+       if (trim_capabilities(env_probe))
+               goto out;
+
        /*
         * The kernel doesn't pass down the gid, so we resort here to scraping
         * one out of the passwd nss db. Note that this might not reflect the
@@ -1266,20 +1344,7 @@ int main(const int argc, char *const argv[])
         * look at the environ file.
         */
        env_cachename =
-               get_cachename_from_process_env(env_probe ? arg.pid : 0);
-
-       /*
-        * Change to the process's namespace. This means that things will work
-        * acceptably in containers, because we'll be looking at the correct
-        * filesystem and have the correct network configuration.
-        */
-       rc = switch_to_process_ns(arg.pid);
-       if (rc == -1) {
-               syslog(LOG_ERR, "unable to switch to process namespace: %s",
-                      strerror(errno));
-               rc = 1;
-               goto out;
-       }
+               get_cachename_from_process_env(env_probe ? arg->pid : 0);
 
        rc = setuid(uid);
        if (rc == -1) {
@@ -1301,18 +1366,18 @@ int main(const int argc, char *const argv[])
 
        ccache = get_existing_cc(env_cachename);
        /* Couldn't find credcache? Try to use keytab */
-       if (ccache == NULL && arg.username != NULL)
-               ccache = init_cc_from_keytab(keytab_name, arg.username);
+       if (ccache == NULL && arg->username[0] != '\0')
+               ccache = init_cc_from_keytab(keytab_name, arg->username);
 
        if (ccache == NULL) {
                rc = 1;
                goto out;
        }
 
-       host = arg.hostname;
+       host = arg->hostname;
 
        // do mech specific authorization
-       switch (arg.sec) {
+       switch (arg->sec) {
        case MS_KRB5:
        case KRB5:
                /*
@@ -1328,7 +1393,7 @@ int main(const int argc, char *const argv[])
                 * TRY only:
                 * cifs/bar.example.com@REALM
                 */
-               if (arg.sec == MS_KRB5)
+               if (arg->sec == MS_KRB5)
                        oid = OID_KERBEROS5_OLD;
                else
                        oid = OID_KERBEROS5;
@@ -1385,10 +1450,10 @@ retry_new_hostname:
                                break;
                }
 
-               if (!try_dns || !(have & DKD_HAVE_IP))
+               if (!try_dns || !(arg->have & DKD_HAVE_IP))
                        break;
 
-               rc = ip_to_fqdn(arg.ip, hostbuf, sizeof(hostbuf));
+               rc = ip_to_fqdn(arg->ip, hostbuf, sizeof(hostbuf));
                if (rc)
                        break;
 
@@ -1396,7 +1461,7 @@ retry_new_hostname:
                host = hostbuf;
                goto retry_new_hostname;
        default:
-               syslog(LOG_ERR, "sectype: %d is not implemented", arg.sec);
+               syslog(LOG_ERR, "sectype: %d is not implemented", arg->sec);
                rc = 1;
                break;
        }
@@ -1414,7 +1479,7 @@ retry_new_hostname:
                rc = 1;
                goto out;
        }
-       keydata->version = arg.ver;
+       keydata->version = arg->ver;
        keydata->flags = 0;
        keydata->sesskey_len = sess_key.length;
        keydata->secblob_len = secblob.length;
@@ -1440,11 +1505,10 @@ out:
                krb5_cc_close(context, ccache);
        if (context)
                krb5_free_context(context);
-       free(arg.hostname);
-       free(arg.ip);
-       free(arg.username);
        free(keydata);
        free(env_cachename);
+       if (arg)
+               munmap(arg, sizeof(*arg));
        syslog(LOG_DEBUG, "Exit status %ld", rc);
        return rc;
 }