Fix: runas: supplementary groups are ignored on lttng save
authorFrancis Deslauriers <francis.deslauriers@efficios.com>
Fri, 23 Jul 2021 20:27:00 +0000 (16:27 -0400)
committerJérémie Galarneau <jeremie.galarneau@efficios.com>
Thu, 5 Aug 2021 23:53:38 +0000 (19:53 -0400)
Observed issue
==============

On `lttng save` the following is reported to the user:

 $ sudo -u my_user lttng save -o /tmp/my_dir my_session_name
 Error: Permission denied

Note that:
  * the running lttng-sessiond is root,
  * "my_user" is part of the tracing group,
  * "my_user" primary group is "my_user" and is part of group "my_dummy_group"
  * The "/tmp/my_dir" has the following permissions:

    drwxrwx--- 2 root my_dummy_group 4096 Jul 26 16:39 /tmp/my_dir/

Cause
=====

The supplementary groups are not initialized when the run-as process
demote itself to the user "my_user" to perform the recursive mkdir
required by the `lttng save` command.

From the point of the view the kernel, at the moment of performing the
mkdir call the permissions looks like this:

 euid: uid of "my_user"
 egid: primary gid of "my_user"
 supplementary group list: "root"

Note that the kernel does not treat the presence of the root group in
the supplementary group list in any special way. Since "root gid" !=
"my_dummy_group gid" the directory creation is refused.

Solution
========

Use initgroups(3) to initialize the supplementary group list.

Known drawbacks
=========

None.

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I58656a3107e4f7b59a2391a4759988401cad7a2b

src/common/runas.c

index 5d63f640e3ca20d12b29ed870e4b96688b9e9043..cfc898f166d45e8f313b2e060c1518568d54fcce 100644 (file)
@@ -8,20 +8,20 @@
  */
 
 #define _LGPL_SOURCE
-#include <errno.h>
+#include <assert.h>
+#include <fcntl.h>
+#include <grp.h>
 #include <limits.h>
+#include <pwd.h>
+#include <sched.h>
+#include <signal.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
-#include <sys/wait.h>
-#include <sys/types.h>
 #include <sys/stat.h>
+#include <sys/types.h>
+#include <sys/wait.h>
 #include <unistd.h>
-#include <fcntl.h>
-#include <sched.h>
-#include <signal.h>
-#include <assert.h>
-#include <signal.h>
 
 #include <common/lttng-kernel.h>
 #include <common/common.h>
@@ -36,6 +36,8 @@
 
 #include "runas.h"
 
+#define GETPW_BUFFER_FALLBACK_SIZE 4096
+
 struct run_as_data;
 struct run_as_ret;
 typedef int (*run_as_fct)(struct run_as_data *data, struct run_as_ret *ret_value);
@@ -823,18 +825,207 @@ end:
        return ret;
 }
 
+static int get_user_infos_from_uid(
+               uid_t uid, char **username, gid_t *primary_gid)
+{
+       int ret;
+       char *buf = NULL;
+       size_t buf_size;
+       struct passwd pwd;
+       struct passwd *result = NULL;
+
+       /* Fetch the max size for the temporary buffer. */
+       errno = 0;
+       buf_size = sysconf(_SC_GETPW_R_SIZE_MAX);
+       if (buf_size < 0) {
+               if (errno != 0) {
+                       PERROR("Failed to query _SC_GETPW_R_SIZE_MAX");
+                       goto error;
+               }
+
+               /* Limit is indeterminate. */
+               WARN("Failed to query _SC_GETPW_R_SIZE_MAX as it is "
+                       "indeterminate; falling back to default buffer size");
+               buf_size = GETPW_BUFFER_FALLBACK_SIZE;
+       }
+
+       buf = zmalloc(buf_size);
+       if (buf == NULL) {
+               PERROR("Failed to allocate buffer to get password file entries");
+               goto error;
+       }
+
+       ret = getpwuid_r(uid, &pwd, buf, buf_size, &result);
+       if (ret < 0) {
+               PERROR("Failed to get user information for user:  uid = %d",
+                               (int) uid);
+               goto error;
+       }
+
+       if (result == NULL) {
+               ERR("Failed to find user information in password entries: uid = %d",
+                               (int) uid);
+               ret = -1;
+               goto error;
+       }
+
+       *username = strdup(result->pw_name);
+       if (*username == NULL) {
+               PERROR("Failed to copy user name");
+               goto error;
+       }
+
+       *primary_gid = result->pw_gid;
+
+end:
+       free(buf);
+       return ret;
+error:
+       *username = NULL;
+       *primary_gid = -1;
+       ret = -1;
+       goto end;
+}
+
+static int demote_creds(
+               uid_t prev_uid, gid_t prev_gid, uid_t new_uid, gid_t new_gid)
+{
+       int ret = 0;
+       gid_t primary_gid;
+       char *username = NULL;
+
+       /* Change the group id. */
+       if (prev_gid != new_gid) {
+               ret = setegid(new_gid);
+               if (ret < 0) {
+                       PERROR("Failed to set effective group id: new_gid = %d",
+                                       (int) new_gid);
+                       goto end;
+               }
+       }
+
+       /* Change the user id. */
+       if (prev_uid != new_uid) {
+               ret = get_user_infos_from_uid(new_uid, &username, &primary_gid);
+               if (ret < 0) {
+                       goto end;
+               }
+
+               /*
+                * Initialize the supplementary group access list.
+                *
+                * This is needed to handle cases where the supplementary groups
+                * of the user the process is demoting-to would give it access
+                * to a given file/folder, but not it's primary group.
+                *
+                * e.g
+                *   username: User1
+                *   Primary Group: User1
+                *   Secondary group: Disk, Network
+                *
+                *   mkdir inside the following directory must work since User1
+                *   is part of the Network group.
+                *
+                *   drwxrwx--- 2 root Network 4096 Jul 23 17:17 /tmp/my_folder/
+                *
+                *
+                * The order of the following initgroups and seteuid calls is
+                * important here;
+                * Only a root process or one with CAP_SETGID capability can
+                * call the the initgroups() function. We must initialize the
+                * supplementary groups before we change the effective
+                * UID to a less-privileged user.
+                */
+               ret = initgroups(username, primary_gid);
+               if (ret < 0) {
+                       PERROR("Failed to init the supplementary group access list: "
+                                       "username = `%s`, primary gid = %d", username,
+                                       (int) primary_gid);
+                       goto end;
+               }
+
+               ret = seteuid(new_uid);
+               if (ret < 0) {
+                       PERROR("Failed to set effective user id: new_uid = %d",
+                                       (int) new_uid);
+                       goto end;
+               }
+       }
+end:
+       free(username);
+       return ret;
+}
+
+static int promote_creds(
+               uid_t prev_uid, gid_t prev_gid, uid_t new_uid, gid_t new_gid)
+{
+       int ret = 0;
+       gid_t primary_gid;
+       char *username = NULL;
+
+       /* Change the group id. */
+       if (prev_gid != new_gid) {
+               ret = setegid(new_gid);
+               if (ret < 0) {
+                       PERROR("Failed to set effective group id: new_gid = %d",
+                                       (int) new_gid);
+                       goto end;
+               }
+       }
+
+       /* Change the user id. */
+       if (prev_uid != new_uid) {
+               ret = get_user_infos_from_uid(new_uid, &username, &primary_gid);
+               if (ret < 0) {
+                       goto end;
+               }
+
+               /*
+                * seteuid call must be done before the initgroups call because
+                * we need to be privileged (CAP_SETGID) to call initgroups().
+                */
+               ret = seteuid(new_uid);
+               if (ret < 0) {
+                       PERROR("Failed to set effective user id: new_uid = %d",
+                                       (int) new_uid);
+                       goto end;
+               }
+
+               /*
+                * Initialize the supplementary group access list.
+                *
+                * There is a possibility the groups we set in the following
+                * initgroups() call are not exactly the same as the ones we
+                * had when we originally demoted. This can happen if the
+                * /etc/group file is modified after the runas process is
+                * forked. This is very unlikely.
+                */
+               ret = initgroups(username, primary_gid);
+               if (ret < 0) {
+                       PERROR("Failed to init the supplementary group access "
+                                       "list: username = `%s`, primary gid = %d",
+                                       username, (int) primary_gid)
+                       goto end;
+               }
+       }
+end:
+       free(username);
+       return ret;
+}
+
 /*
  * Return < 0 on error, 0 if OK, 1 on hangup.
  */
 static
 int handle_one_cmd(struct run_as_worker *worker)
 {
-       int ret = 0;
-        struct run_as_data data = {};
-        ssize_t readlen, writelen;
-        struct run_as_ret sendret = {};
-        run_as_fct cmd;
-       uid_t prev_euid;
+       int ret = 0, promote_ret;
+       struct run_as_data data = {};
+       ssize_t readlen, writelen;
+       struct run_as_ret sendret = {};
+       run_as_fct cmd;
+       uid_t prev_ruid;
+       gid_t prev_rgid;
 
        /*
         * Stage 1: Receive run_as_data struct from the master.
@@ -872,24 +1063,12 @@ int handle_one_cmd(struct run_as_worker *worker)
                goto end;
        }
 
-       prev_euid = getuid();
-       if (data.gid != getegid()) {
-               ret = setegid(data.gid);
-               if (ret < 0) {
-                       sendret._error = true;
-                       sendret._errno = errno;
-                       PERROR("setegid");
-                       goto write_return;
-               }
-       }
-       if (data.uid != prev_euid) {
-               ret = seteuid(data.uid);
-               if (ret < 0) {
-                       sendret._error = true;
-                       sendret._errno = errno;
-                       PERROR("seteuid");
-                       goto write_return;
-               }
+       prev_ruid = getuid();
+       prev_rgid = getgid();
+
+       ret = demote_creds(prev_ruid, prev_rgid, data.uid, data.gid);
+       if (ret < 0) {
+               goto write_return;
        }
 
        /*
@@ -909,7 +1088,7 @@ write_return:
        ret = cleanup_received_fds(&data);
        if (ret < 0) {
                ERR("Error cleaning up FD");
-               goto end;
+               goto promote_back;
        }
 
        /*
@@ -921,7 +1100,7 @@ write_return:
        if (writelen < sizeof(sendret)) {
                PERROR("lttcomm_send_unix_sock error");
                ret = -1;
-               goto end;
+               goto promote_back;
        }
 
        /*
@@ -930,15 +1109,17 @@ write_return:
        ret = send_fds_to_master(worker, data.cmd, &sendret);
        if (ret < 0) {
                DBG("Sending FD to master returned an error");
-               goto end;
        }
 
-       if (seteuid(prev_euid) < 0) {
-               PERROR("seteuid");
-               ret = -1;
-               goto end;
-       }
        ret = 0;
+
+promote_back:
+       /* Return to previous uid/gid. */
+       promote_ret = promote_creds(data.uid, data.gid, prev_ruid, prev_rgid);
+       if (promote_ret < 0) {
+               ERR("Failed to promote back to the initial credentials");
+       }
+
 end:
        return ret;
 }
This page took 0.030327 seconds and 4 git commands to generate.