The following pull request was submitted through Github. It can be accessed and reviewed at: https://github.com/lxc/lxc/pull/3575
This e-mail was sent by the LXC bot, direct replies will not reach the author unless they happen to be subscribed to this list. === Description (from pull-request) === Hi lxc team, I've implemented additional group ID handling for the init / attach process. The newly introduced config options are are `lxc.init.groups` for lxc config and `groups.size` / `groups.list` for attach options. Please review the changes carefully. Regards, Ruben
From a8213d477aa91fd770141c0acdd1e78e3fe82498 Mon Sep 17 00:00:00 2001 From: Ruben Jenster <r.jens...@drachenfels.de> Date: Fri, 30 Oct 2020 10:00:07 +0100 Subject: [PATCH 1/2] Introduce lxc.init.groups to keep additional group ID's. Signed-off-by: Ruben Jenster <r.jens...@drachenfels.de> --- src/lxc/conf.c | 1 + src/lxc/conf.h | 4 +++ src/lxc/confile.c | 79 ++++++++++++++++++++++++++++++++++++++++++++ src/lxc/start.c | 12 +++++-- src/lxc/utils.c | 7 +++- src/tests/get_item.c | 28 ++++++++++++++++ 6 files changed, 128 insertions(+), 3 deletions(-) diff --git a/src/lxc/conf.c b/src/lxc/conf.c index c258d0b4c5..ceffb8f6c9 100644 --- a/src/lxc/conf.c +++ b/src/lxc/conf.c @@ -3820,6 +3820,7 @@ void lxc_conf_free(struct lxc_conf *conf) free(conf->rcfile); free(conf->execute_cmd); free(conf->init_cmd); + free(conf->init_groups.list); free(conf->init_cwd); free(conf->unexpanded_config); free(conf->syslog); diff --git a/src/lxc/conf.h b/src/lxc/conf.h index 907cbdfa52..90d77e6eff 100644 --- a/src/lxc/conf.h +++ b/src/lxc/conf.h @@ -373,6 +373,10 @@ struct lxc_conf { * should run under when using lxc-execute */ uid_t init_uid; gid_t init_gid; + struct { + int size; + gid_t *list; + } init_groups; /* indicator if the container will be destroyed on shutdown */ unsigned int ephemeral; diff --git a/src/lxc/confile.c b/src/lxc/confile.c index 4f7621a900..103163280c 100644 --- a/src/lxc/confile.c +++ b/src/lxc/confile.c @@ -94,6 +94,7 @@ lxc_config_define(init_cmd); lxc_config_define(init_cwd); lxc_config_define(init_gid); lxc_config_define(init_uid); +lxc_config_define(init_groups); lxc_config_define(keyring_session); lxc_config_define(log_file); lxc_config_define(log_level); @@ -211,6 +212,7 @@ static struct lxc_config_t config_jump_table[] = { { "lxc.include", set_config_includefiles, get_config_includefiles, clr_config_includefiles, }, { "lxc.init.cmd", set_config_init_cmd, get_config_init_cmd, clr_config_init_cmd, }, { "lxc.init.gid", set_config_init_gid, get_config_init_gid, clr_config_init_gid, }, + { "lxc.init.groups", set_config_init_groups, get_config_init_groups, clr_config_init_groups, }, { "lxc.init.uid", set_config_init_uid, get_config_init_uid, clr_config_init_uid, }, { "lxc.init.cwd", set_config_init_cwd, get_config_init_cwd, clr_config_init_cwd, }, { "lxc.keyring.session", set_config_keyring_session, get_config_keyring_session, clr_config_keyring_session }, @@ -1217,6 +1219,53 @@ static int set_config_init_gid(const char *key, const char *value, return 0; } +static int set_config_init_groups(const char *key, const char *value, + struct lxc_conf *lxc_conf, void *data) +{ + char *value_dup, *token; + int num_groups = 0; + gid_t *init_groups; + int iter = 0; + + if (lxc_config_value_empty(value)) + return clr_config_init_groups(key, lxc_conf, NULL); + + value_dup = strdup(value); + if (!value_dup) + return -1; + + lxc_iterate_parts(token, value_dup, " \t") num_groups++; + + if (num_groups == 0) { + free(value_dup); + return clr_config_init_groups(key, lxc_conf, NULL); + } + + init_groups = malloc(sizeof(gid_t) * num_groups); + if (!init_groups) { + free(value_dup); + return -1; + } + + strcpy(value_dup, value); + lxc_iterate_parts(token, value_dup, " \t") + { + gid_t group; + if (lxc_safe_uint(token, &group) < 0) { + free(value_dup); + free(init_groups); + return -1; + } + init_groups[iter++] = group; + } + + lxc_conf->init_groups.size = num_groups; + lxc_conf->init_groups.list = init_groups; + + free(value_dup); + return 0; +} + static int set_config_hooks(const char *key, const char *value, struct lxc_conf *lxc_conf, void *data) { @@ -4441,6 +4490,27 @@ static int get_config_init_gid(const char *key, char *retv, int inlen, return lxc_get_conf_int(c, retv, inlen, c->init_gid); } +static int get_config_init_groups(const char *key, char *retv, int inlen, + struct lxc_conf *c, void *data) +{ + int fulllen = 0, len; + + if (!retv) + inlen = 0; + else + memset(retv, 0, inlen); + + if (c->init_groups.size == 0) { + return 0; + } + + for (int i = 0; i < c->init_groups.size; i++) + strprint(retv, inlen, "%s%d", (i > 0) ? " " : "", + c->init_groups.list[i]); + + return fulllen; +} + static int get_config_ephemeral(const char *key, char *retv, int inlen, struct lxc_conf *c, void *data) { @@ -5147,6 +5217,15 @@ static inline int clr_config_init_gid(const char *key, struct lxc_conf *c, return 0; } +static inline int clr_config_init_groups(const char *key, struct lxc_conf *c, + void *data) +{ + free(c->init_groups.list); + c->init_groups.list = NULL; + c->init_groups.size = 0; + return 0; +} + static inline int clr_config_ephemeral(const char *key, struct lxc_conf *c, void *data) { diff --git a/src/lxc/start.c b/src/lxc/start.c index 7bf7f8a2fb..c650367885 100644 --- a/src/lxc/start.c +++ b/src/lxc/start.c @@ -1414,8 +1414,16 @@ static int do_start(void *data) #if HAVE_LIBCAP if (lxc_proc_cap_is_set(CAP_SETGID, CAP_EFFECTIVE)) #endif - if (!lxc_setgroups(0, NULL)) - goto out_warn_father; + { + if (handler->conf->init_groups.size > 0) { + if (!lxc_setgroups(handler->conf->init_groups.size, + handler->conf->init_groups.list)) + goto out_warn_father; + } else { + if (!lxc_setgroups(0, NULL)) + goto out_warn_father; + } + } if (!lxc_switch_uid_gid(new_uid, new_gid)) goto out_warn_father; diff --git a/src/lxc/utils.c b/src/lxc/utils.c index baf80b7f5c..75ad3137d6 100644 --- a/src/lxc/utils.c +++ b/src/lxc/utils.c @@ -1412,7 +1412,12 @@ bool lxc_setgroups(int size, gid_t list[]) SYSERROR("Failed to setgroups()"); return false; } - NOTICE("Dropped additional groups"); + if (size == 0) + NOTICE("Dropped additional groups"); + else { + for(int i = 0; i < size; i++) + NOTICE("Kept additional group with GID %d", list[i]); + } return true; } diff --git a/src/tests/get_item.c b/src/tests/get_item.c index 11db5f6738..5ceea9ec49 100644 --- a/src/tests/get_item.c +++ b/src/tests/get_item.c @@ -136,6 +136,34 @@ int main(int argc, char *argv[]) } printf("lxc.init_gid returned %d %s\n", ret, v2); + if (!c->set_config_item(c, "lxc.init.groups", "")) { + fprintf(stderr, "%d: failed to set init_groups\n", __LINE__); + goto out; + } + + if (!c->set_config_item(c, "lxc.init.groups", "10 20 foo 40")) { + printf("%d: failed to set init_groups\n", __LINE__); + } else { + goto out; + } + + if (!c->set_config_item(c, "lxc.init.groups", "10 20 30 40")) { + fprintf(stderr, "%d: failed to set init_groups\n", __LINE__); + goto out; + } + + ret = c->get_config_item(c, "lxc.init.groups", v2, 255); + if (ret < 0) { + fprintf(stderr, "%d: get_config_item(lxc.init_gid) returned %d\n", + __LINE__, ret); + goto out; + } + ret = strcmp("10 20 30 40", v2); + printf("lxc.init_groups returned %d %s\n", ret, v2); + if (ret != 0) { + goto out; + } + #define HNAME "hostname1" // demonstrate proper usage: char *alloced; From 29de94987e8908210caa450afdd49d671f708e75 Mon Sep 17 00:00:00 2001 From: Ruben Jenster <r.jens...@drachenfels.de> Date: Thu, 5 Nov 2020 10:56:19 +0100 Subject: [PATCH 2/2] attach: Add groups option to keep additional group IDs. Signed-off-by: Ruben Jenster <r.jens...@drachenfels.de> --- src/lxc/attach.c | 13 +++++++------ src/lxc/attach_options.h | 12 ++++++++++++ 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/lxc/attach.c b/src/lxc/attach.c index acbffa238d..477308c047 100644 --- a/src/lxc/attach.c +++ b/src/lxc/attach.c @@ -552,10 +552,6 @@ static void lxc_attach_get_init_uidgid(uid_t *init_uid, gid_t *init_gid) if (gid != LXC_INVALID_GID) *init_gid = gid; - - /* TODO: we should also parse supplementary groups and use - * setgroups() to set them. - */ } static bool fetch_seccomp(struct lxc_container *c, lxc_attach_options_t *options) @@ -740,8 +736,13 @@ static int attach_child_main(struct attach_clone_payload *payload) goto on_error; } - if (!lxc_setgroups(0, NULL) && errno != EPERM) - goto on_error; + if (options->groups.size > 0) { + if (!lxc_setgroups(options->groups.size, options->groups.list)) + goto on_error; + } else { + if (!lxc_setgroups(0, NULL) && errno != EPERM) + goto on_error; + } if (options->namespaces & CLONE_NEWUSER) { /* Check whether nsuid 0 has a mapping. */ diff --git a/src/lxc/attach_options.h b/src/lxc/attach_options.h index 80fe439103..c07c13cd85 100644 --- a/src/lxc/attach_options.h +++ b/src/lxc/attach_options.h @@ -52,6 +52,11 @@ enum { */ typedef int (*lxc_attach_exec_t)(void* payload); +typedef struct lxc_groups_t { + int size; + gid_t *list; +} lxc_groups_t; + /*! * LXC attach options for \ref lxc_container \c attach(). */ @@ -88,6 +93,12 @@ typedef struct lxc_attach_options_t { */ gid_t gid; + /*! The additional group GIDs to run with. + * + * If unset all additional groups are dropped. + */ + lxc_groups_t groups; + /*! Environment policy */ lxc_attach_env_policy_t env_policy; @@ -128,6 +139,7 @@ typedef struct lxc_attach_options_t { /* .initial_cwd = */ NULL, \ /* .uid = */ (uid_t)-1, \ /* .gid = */ (gid_t)-1, \ + /* .groups = */ { 0, NULL}, \ /* .env_policy = */ LXC_ATTACH_KEEP_ENV, \ /* .extra_env_vars = */ NULL, \ /* .extra_keep_env = */ NULL, \
_______________________________________________ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel