Zbigniew Jędrzejewski-Szmek 35bb94
From 699b9dcb4e1ab66703bc22c0e3eed16b650895c9 Mon Sep 17 00:00:00 2001
Zbigniew Jędrzejewski-Szmek 35bb94
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Zbigniew Jędrzejewski-Szmek 35bb94
Date: Fri, 20 Oct 2017 13:00:12 +0200
Zbigniew Jędrzejewski-Szmek 35bb94
Subject: [PATCH] core/dynamic-user: use gid from pwnam if a static user was
Zbigniew Jędrzejewski-Szmek 35bb94
 found
Zbigniew Jędrzejewski-Szmek 35bb94
Zbigniew Jędrzejewski-Szmek 35bb94
Fixes #7133.
Zbigniew Jędrzejewski-Szmek 35bb94
Zbigniew Jędrzejewski-Szmek 35bb94
v2:
Zbigniew Jędrzejewski-Szmek 35bb94
- update based on review
Zbigniew Jędrzejewski-Szmek 35bb94
Zbigniew Jędrzejewski-Szmek 35bb94
(cherry picked from commit c2983a7fdd027b6241b8c8814706c6ea9768a34c)
Zbigniew Jędrzejewski-Szmek 35bb94
---
Zbigniew Jędrzejewski-Szmek 35bb94
 src/core/dynamic-user.c | 53 +++++++++++++++++++++++++++++--------------------
Zbigniew Jędrzejewski-Szmek 35bb94
 1 file changed, 32 insertions(+), 21 deletions(-)
Zbigniew Jędrzejewski-Szmek 35bb94
Zbigniew Jędrzejewski-Szmek 35bb94
diff --git a/src/core/dynamic-user.c b/src/core/dynamic-user.c
Zbigniew Jędrzejewski-Szmek 35bb94
index 0f4ec15721..ce84f8d41a 100644
Zbigniew Jędrzejewski-Szmek 35bb94
--- a/src/core/dynamic-user.c
Zbigniew Jędrzejewski-Szmek 35bb94
+++ b/src/core/dynamic-user.c
Zbigniew Jędrzejewski-Szmek 35bb94
@@ -435,15 +435,22 @@ static void unlockfp(int *fd_lock) {
Zbigniew Jędrzejewski-Szmek 35bb94
         *fd_lock = -1;
Zbigniew Jędrzejewski-Szmek 35bb94
 }
Zbigniew Jędrzejewski-Szmek 35bb94
 
Zbigniew Jędrzejewski-Szmek 35bb94
-static int dynamic_user_realize(DynamicUser *d, char **suggested_dirs, uid_t *ret, bool is_user) {
Zbigniew Jędrzejewski-Szmek 35bb94
+static int dynamic_user_realize(
Zbigniew Jędrzejewski-Szmek 35bb94
+                DynamicUser *d,
Zbigniew Jędrzejewski-Szmek 35bb94
+                char **suggested_dirs,
Zbigniew Jędrzejewski-Szmek 35bb94
+                uid_t *ret_uid, gid_t *ret_gid,
Zbigniew Jędrzejewski-Szmek 35bb94
+                bool is_user) {
Zbigniew Jędrzejewski-Szmek 35bb94
 
Zbigniew Jędrzejewski-Szmek 35bb94
         _cleanup_(unlockfp) int storage_socket0_lock = -1;
Zbigniew Jędrzejewski-Szmek 35bb94
         _cleanup_close_ int uid_lock_fd = -1;
Zbigniew Jędrzejewski-Szmek 35bb94
         _cleanup_close_ int etc_passwd_lock_fd = -1;
Zbigniew Jędrzejewski-Szmek 35bb94
-        uid_t uid = UID_INVALID;
Zbigniew Jędrzejewski-Szmek 35bb94
+        uid_t num = UID_INVALID; /* a uid if is_user, and a gid otherwise */
Zbigniew Jędrzejewski-Szmek 35bb94
+        gid_t gid = GID_INVALID; /* a gid if is_user, ignored otherwise */
Zbigniew Jędrzejewski-Szmek 35bb94
         int r;
Zbigniew Jędrzejewski-Szmek 35bb94
 
Zbigniew Jędrzejewski-Szmek 35bb94
         assert(d);
Zbigniew Jędrzejewski-Szmek 35bb94
+        assert(is_user == !!ret_uid);
Zbigniew Jędrzejewski-Szmek 35bb94
+        assert(ret_gid);
Zbigniew Jędrzejewski-Szmek 35bb94
 
Zbigniew Jędrzejewski-Szmek 35bb94
         /* Acquire a UID for the user name. This will allocate a UID for the user name if the user doesn't exist
Zbigniew Jędrzejewski-Szmek 35bb94
          * yet. If it already exists its existing UID/GID will be reused. */
Zbigniew Jędrzejewski-Szmek 35bb94
@@ -452,7 +459,7 @@ static int dynamic_user_realize(DynamicUser *d, char **suggested_dirs, uid_t *re
Zbigniew Jędrzejewski-Szmek 35bb94
         if (r < 0)
Zbigniew Jędrzejewski-Szmek 35bb94
                 return r;
Zbigniew Jędrzejewski-Szmek 35bb94
 
Zbigniew Jędrzejewski-Szmek 35bb94
-        r = dynamic_user_pop(d, &uid, &uid_lock_fd);
Zbigniew Jędrzejewski-Szmek 35bb94
+        r = dynamic_user_pop(d, &num, &uid_lock_fd);
Zbigniew Jędrzejewski-Szmek 35bb94
         if (r < 0) {
Zbigniew Jędrzejewski-Szmek 35bb94
                 int new_uid_lock_fd;
Zbigniew Jędrzejewski-Szmek 35bb94
                 uid_t new_uid;
Zbigniew Jędrzejewski-Szmek 35bb94
@@ -472,7 +479,7 @@ static int dynamic_user_realize(DynamicUser *d, char **suggested_dirs, uid_t *re
Zbigniew Jędrzejewski-Szmek 35bb94
                         return etc_passwd_lock_fd;
Zbigniew Jędrzejewski-Szmek 35bb94
 
Zbigniew Jędrzejewski-Szmek 35bb94
                 /* First, let's parse this as numeric UID */
Zbigniew Jędrzejewski-Szmek 35bb94
-                r = parse_uid(d->name, &uid);
Zbigniew Jędrzejewski-Szmek 35bb94
+                r = parse_uid(d->name, &num);
Zbigniew Jędrzejewski-Szmek 35bb94
                 if (r < 0) {
Zbigniew Jędrzejewski-Szmek 35bb94
                         struct passwd *p;
Zbigniew Jędrzejewski-Szmek 35bb94
                         struct group *g;
Zbigniew Jędrzejewski-Szmek 35bb94
@@ -480,9 +487,10 @@ static int dynamic_user_realize(DynamicUser *d, char **suggested_dirs, uid_t *re
Zbigniew Jędrzejewski-Szmek 35bb94
                         if (is_user) {
Zbigniew Jędrzejewski-Szmek 35bb94
                                 /* OK, this is not a numeric UID. Let's see if there's a user by this name */
Zbigniew Jędrzejewski-Szmek 35bb94
                                 p = getpwnam(d->name);
Zbigniew Jędrzejewski-Szmek 35bb94
-                                if (p)
Zbigniew Jędrzejewski-Szmek 35bb94
-                                        uid = p->pw_uid;
Zbigniew Jędrzejewski-Szmek 35bb94
-                                else {
Zbigniew Jędrzejewski-Szmek 35bb94
+                                if (p) {
Zbigniew Jędrzejewski-Szmek 35bb94
+                                        num = p->pw_uid;
Zbigniew Jędrzejewski-Szmek 35bb94
+                                        gid = p->pw_gid;
Zbigniew Jędrzejewski-Szmek 35bb94
+                                } else {
Zbigniew Jędrzejewski-Szmek 35bb94
                                         /* if the user does not exist but the group with the same name exists, refuse operation */
Zbigniew Jędrzejewski-Szmek 35bb94
                                         g = getgrnam(d->name);
Zbigniew Jędrzejewski-Szmek 35bb94
                                         if (g)
Zbigniew Jędrzejewski-Szmek 35bb94
@@ -492,7 +500,7 @@ static int dynamic_user_realize(DynamicUser *d, char **suggested_dirs, uid_t *re
Zbigniew Jędrzejewski-Szmek 35bb94
                                 /* Let's see if there's a group by this name */
Zbigniew Jędrzejewski-Szmek 35bb94
                                 g = getgrnam(d->name);
Zbigniew Jędrzejewski-Szmek 35bb94
                                 if (g)
Zbigniew Jędrzejewski-Szmek 35bb94
-                                        uid = (uid_t) g->gr_gid;
Zbigniew Jędrzejewski-Szmek 35bb94
+                                        num = (uid_t) g->gr_gid;
Zbigniew Jędrzejewski-Szmek 35bb94
                                 else {
Zbigniew Jędrzejewski-Szmek 35bb94
                                         /* if the group does not exist but the user with the same name exists, refuse operation */
Zbigniew Jędrzejewski-Szmek 35bb94
                                         p = getpwnam(d->name);
Zbigniew Jędrzejewski-Szmek 35bb94
@@ -502,10 +510,10 @@ static int dynamic_user_realize(DynamicUser *d, char **suggested_dirs, uid_t *re
Zbigniew Jędrzejewski-Szmek 35bb94
                         }
Zbigniew Jędrzejewski-Szmek 35bb94
                 }
Zbigniew Jędrzejewski-Szmek 35bb94
 
Zbigniew Jędrzejewski-Szmek 35bb94
-                if (uid == UID_INVALID) {
Zbigniew Jędrzejewski-Szmek 35bb94
+                if (num == UID_INVALID) {
Zbigniew Jędrzejewski-Szmek 35bb94
                         /* No static UID assigned yet, excellent. Let's pick a new dynamic one, and lock it. */
Zbigniew Jędrzejewski-Szmek 35bb94
 
Zbigniew Jędrzejewski-Szmek 35bb94
-                        uid_lock_fd = pick_uid(suggested_dirs, d->name, &uid);
Zbigniew Jędrzejewski-Szmek 35bb94
+                        uid_lock_fd = pick_uid(suggested_dirs, d->name, &num);
Zbigniew Jędrzejewski-Szmek 35bb94
                         if (uid_lock_fd < 0)
Zbigniew Jędrzejewski-Szmek 35bb94
                                 return uid_lock_fd;
Zbigniew Jędrzejewski-Szmek 35bb94
                 }
Zbigniew Jędrzejewski-Szmek 35bb94
@@ -513,7 +521,7 @@ static int dynamic_user_realize(DynamicUser *d, char **suggested_dirs, uid_t *re
Zbigniew Jędrzejewski-Szmek 35bb94
                 /* So, we found a working UID/lock combination. Let's see if we actually still need it. */
Zbigniew Jędrzejewski-Szmek 35bb94
                 r = lockfp(d->storage_socket[0], &storage_socket0_lock);
Zbigniew Jędrzejewski-Szmek 35bb94
                 if (r < 0) {
Zbigniew Jędrzejewski-Szmek 35bb94
-                        unlink_uid_lock(uid_lock_fd, uid, d->name);
Zbigniew Jędrzejewski-Szmek 35bb94
+                        unlink_uid_lock(uid_lock_fd, num, d->name);
Zbigniew Jędrzejewski-Szmek 35bb94
                         return r;
Zbigniew Jędrzejewski-Szmek 35bb94
                 }
Zbigniew Jędrzejewski-Szmek 35bb94
 
Zbigniew Jędrzejewski-Szmek 35bb94
@@ -521,7 +529,7 @@ static int dynamic_user_realize(DynamicUser *d, char **suggested_dirs, uid_t *re
Zbigniew Jędrzejewski-Szmek 35bb94
                 if (r < 0) {
Zbigniew Jędrzejewski-Szmek 35bb94
                         if (r != -EAGAIN) {
Zbigniew Jędrzejewski-Szmek 35bb94
                                 /* OK, something bad happened, let's get rid of the bits we acquired. */
Zbigniew Jędrzejewski-Szmek 35bb94
-                                unlink_uid_lock(uid_lock_fd, uid, d->name);
Zbigniew Jędrzejewski-Szmek 35bb94
+                                unlink_uid_lock(uid_lock_fd, num, d->name);
Zbigniew Jędrzejewski-Szmek 35bb94
                                 return r;
Zbigniew Jędrzejewski-Szmek 35bb94
                         }
Zbigniew Jędrzejewski-Szmek 35bb94
 
Zbigniew Jędrzejewski-Szmek 35bb94
@@ -530,10 +538,10 @@ static int dynamic_user_realize(DynamicUser *d, char **suggested_dirs, uid_t *re
Zbigniew Jędrzejewski-Szmek 35bb94
                         /* Hmm, so as it appears there's now something stored in the storage socket. Throw away what we
Zbigniew Jędrzejewski-Szmek 35bb94
                          * acquired, and use what's stored now. */
Zbigniew Jędrzejewski-Szmek 35bb94
 
Zbigniew Jędrzejewski-Szmek 35bb94
-                        unlink_uid_lock(uid_lock_fd, uid, d->name);
Zbigniew Jędrzejewski-Szmek 35bb94
+                        unlink_uid_lock(uid_lock_fd, num, d->name);
Zbigniew Jędrzejewski-Szmek 35bb94
                         safe_close(uid_lock_fd);
Zbigniew Jędrzejewski-Szmek 35bb94
 
Zbigniew Jędrzejewski-Szmek 35bb94
-                        uid = new_uid;
Zbigniew Jędrzejewski-Szmek 35bb94
+                        num = new_uid;
Zbigniew Jędrzejewski-Szmek 35bb94
                         uid_lock_fd = new_uid_lock_fd;
Zbigniew Jędrzejewski-Szmek 35bb94
                 }
Zbigniew Jędrzejewski-Szmek 35bb94
         }
Zbigniew Jędrzejewski-Szmek 35bb94
@@ -541,11 +549,16 @@ static int dynamic_user_realize(DynamicUser *d, char **suggested_dirs, uid_t *re
Zbigniew Jędrzejewski-Szmek 35bb94
         /* If the UID/GID was already allocated dynamically, push the data we popped out back in. If it was already
Zbigniew Jędrzejewski-Szmek 35bb94
          * allocated statically, push the UID back too, but do not push the lock fd in. If we allocated the UID
Zbigniew Jędrzejewski-Szmek 35bb94
          * dynamically right here, push that in along with the lock fd for it. */
Zbigniew Jędrzejewski-Szmek 35bb94
-        r = dynamic_user_push(d, uid, uid_lock_fd);
Zbigniew Jędrzejewski-Szmek 35bb94
+        r = dynamic_user_push(d, num, uid_lock_fd);
Zbigniew Jędrzejewski-Szmek 35bb94
         if (r < 0)
Zbigniew Jędrzejewski-Szmek 35bb94
                 return r;
Zbigniew Jędrzejewski-Szmek 35bb94
 
Zbigniew Jędrzejewski-Szmek 35bb94
-        *ret = uid;
Zbigniew Jędrzejewski-Szmek 35bb94
+        if (is_user) {
Zbigniew Jędrzejewski-Szmek 35bb94
+                *ret_uid = num;
Zbigniew Jędrzejewski-Szmek 35bb94
+                *ret_gid = gid != GID_INVALID ? gid : num;
Zbigniew Jędrzejewski-Szmek 35bb94
+        } else
Zbigniew Jędrzejewski-Szmek 35bb94
+                *ret_gid = num;
Zbigniew Jędrzejewski-Szmek 35bb94
+
Zbigniew Jędrzejewski-Szmek 35bb94
         return 0;
Zbigniew Jędrzejewski-Szmek 35bb94
 }
Zbigniew Jędrzejewski-Szmek 35bb94
 
Zbigniew Jędrzejewski-Szmek 35bb94
@@ -831,21 +844,19 @@ int dynamic_creds_realize(DynamicCreds *creds, char **suggested_paths, uid_t *ui
Zbigniew Jędrzejewski-Szmek 35bb94
         /* Realize both the referenced user and group */
Zbigniew Jędrzejewski-Szmek 35bb94
 
Zbigniew Jędrzejewski-Szmek 35bb94
         if (creds->user) {
Zbigniew Jędrzejewski-Szmek 35bb94
-                r = dynamic_user_realize(creds->user, suggested_paths, &u, true);
Zbigniew Jędrzejewski-Szmek 35bb94
+                r = dynamic_user_realize(creds->user, suggested_paths, &u, &g, true);
Zbigniew Jędrzejewski-Szmek 35bb94
                 if (r < 0)
Zbigniew Jędrzejewski-Szmek 35bb94
                         return r;
Zbigniew Jędrzejewski-Szmek 35bb94
         }
Zbigniew Jędrzejewski-Szmek 35bb94
 
Zbigniew Jędrzejewski-Szmek 35bb94
         if (creds->group && creds->group != creds->user) {
Zbigniew Jędrzejewski-Szmek 35bb94
-                r = dynamic_user_realize(creds->group, suggested_paths, &g, false);
Zbigniew Jędrzejewski-Szmek 35bb94
+                r = dynamic_user_realize(creds->group, suggested_paths, NULL, &g, false);
Zbigniew Jędrzejewski-Szmek 35bb94
                 if (r < 0)
Zbigniew Jędrzejewski-Szmek 35bb94
                         return r;
Zbigniew Jędrzejewski-Szmek 35bb94
-        } else
Zbigniew Jędrzejewski-Szmek 35bb94
-                g = u;
Zbigniew Jędrzejewski-Szmek 35bb94
+        }
Zbigniew Jędrzejewski-Szmek 35bb94
 
Zbigniew Jędrzejewski-Szmek 35bb94
         *uid = u;
Zbigniew Jędrzejewski-Szmek 35bb94
         *gid = g;
Zbigniew Jędrzejewski-Szmek 35bb94
-
Zbigniew Jędrzejewski-Szmek 35bb94
         return 0;
Zbigniew Jędrzejewski-Szmek 35bb94
 }
Zbigniew Jędrzejewski-Szmek 35bb94