diff options
-rw-r--r-- | src/nsresourced/nsresourcework.c | 106 | ||||
-rw-r--r-- | src/nsresourced/userns-registry.c | 3 | ||||
-rw-r--r-- | src/shared/nsresource.c | 4 | ||||
-rw-r--r-- | src/shared/varlink-io.systemd.NamespaceResource.c | 18 | ||||
-rwxr-xr-x | test/units/TEST-13-NSPAWN.nspawn.sh | 5 |
5 files changed, 116 insertions, 20 deletions
diff --git a/src/nsresourced/nsresourcework.c b/src/nsresourced/nsresourcework.c index f7eb04b207..8c43581d8f 100644 --- a/src/nsresourced/nsresourcework.c +++ b/src/nsresourced/nsresourcework.c @@ -7,6 +7,7 @@ #include <sys/eventfd.h> #include <sys/stat.h> #include <sys/wait.h> +#include <utmpx.h> #include "sd-daemon.h" #include "sd-netlink.h" @@ -608,8 +609,34 @@ static int test_userns_api_support(sd_varlink *link) { return 0; } -static int validate_name(sd_varlink *link, const char *name, char **ret) { - _cleanup_free_ char *un = NULL; +static char* random_name(void) { + char *s = NULL; + + /* Make up a random name for this userns. Make sure the random name fits into utmpx even if prefixed + * with "ns-", the peer's UID, "-", and suffixed by "-65535". */ + assert_cc(STRLEN("ns-65535-") + 16 + STRLEN("-65535") < sizeof_field(struct utmpx, ut_user)); + + if (asprintf(&s, "%016" PRIx64, random_u64()) < 0) + return NULL; + + return s; +} + +static char *shorten_name(const char *name) { + + /* Shorten the specified name, so that it works as a userns name */ + + char *n = strdup(name); + if (!n) + return NULL; + + /* make sure the truncated name fits into utmpx even if prefixed with "ns-" and suffixed by "-65535" */ + strshorten(n, sizeof_field(struct utmpx, ut_user) - STRLEN("ns-") - STRLEN("-65536") - 1); + + return n; +} + +static int validate_name(sd_varlink *link, const char *name, bool mangle, char **ret) { int r; assert(link); @@ -621,22 +648,65 @@ static int validate_name(sd_varlink *link, const char *name, char **ret) { if (r < 0) return r; + _cleanup_free_ char *un = NULL; if (peer_uid == 0) { - if (!userns_name_is_valid(name)) - return sd_varlink_error_invalid_parameter_name(link, "name"); - - un = strdup(name); - if (!un) - return -ENOMEM; + /* If the client is root, we'll not prefix it, but we will make sure it's suitable for + * inclusion in a user name */ + if (userns_name_is_valid(name)) { + un = strdup(name); + if (!un) + return -ENOMEM; + } else { + if (!mangle) + return sd_varlink_error_invalid_parameter_name(link, "name"); + + un = shorten_name(name); + if (!un) + return -ENOMEM; + + /* Let's see if shortening was enough? (It might not be, for example because an empty + * string was provided – which no truncation would make valid.) */ + if (!userns_name_is_valid(un)) { + free(un); + + /* if not, make up a random name */ + un = random_name(); + if (!un) + return -ENOMEM; + } + } } else { - /* The the client is not root then prefix the name with the UID of the peer, so that they - * live in separate namespaces and cannot steal each other's names. */ + /* If the client is not root then prefix the name with the UID of the peer, so that they live + * in separate namespaces and cannot interfere with each other's names. */ if (asprintf(&un, UID_FMT "-%s", peer_uid, name) < 0) return -ENOMEM; - if (!userns_name_is_valid(un)) - return sd_varlink_error_invalid_parameter_name(link, "name"); + if (!userns_name_is_valid(un)) { + if (!mangle) + return sd_varlink_error_invalid_parameter_name(link, "name"); + + _cleanup_free_ char *c = shorten_name(un); + if (!c) + return -ENOMEM; + + /* Let's see if shortening was enough? */ + if (userns_name_is_valid(c)) + free_and_replace(un, c); + else { + free(c); + c = random_name(); + if (!c) + return -ENOMEM; + + un = mfree(un); + if (asprintf(&un, UID_FMT "-%s", peer_uid, c) < 0) + return -ENOMEM; + + if (!userns_name_is_valid(un)) + return sd_varlink_error_invalid_parameter_name(link, "name"); + } + } } *ret = TAKE_PTR(un); @@ -727,6 +797,7 @@ typedef struct AllocateParameters { unsigned size; unsigned target; unsigned userns_fd_idx; + bool mangle_name; } AllocateParameters; static int vl_method_allocate_user_range(sd_varlink *link, sd_json_variant *parameters, sd_varlink_method_flags_t flags, void *userdata) { @@ -736,6 +807,7 @@ static int vl_method_allocate_user_range(sd_varlink *link, sd_json_variant *para { "size", _SD_JSON_VARIANT_TYPE_INVALID, sd_json_dispatch_uint, offsetof(AllocateParameters, size), SD_JSON_MANDATORY }, { "target", _SD_JSON_VARIANT_TYPE_INVALID, sd_json_dispatch_uint, offsetof(AllocateParameters, target), 0 }, { "userNamespaceFileDescriptor", _SD_JSON_VARIANT_TYPE_INVALID, sd_json_dispatch_uint, offsetof(AllocateParameters, userns_fd_idx), SD_JSON_MANDATORY }, + { "mangleName", SD_JSON_VARIANT_BOOLEAN, sd_json_dispatch_stdbool, offsetof(AllocateParameters, mangle_name), 0 }, {} }; @@ -761,7 +833,7 @@ static int vl_method_allocate_user_range(sd_varlink *link, sd_json_variant *para if (r != 0) return r; - r = validate_name(link, p.name, &userns_name); + r = validate_name(link, p.name, p.mangle_name, &userns_name); if (r != 0) return r; @@ -856,7 +928,7 @@ static int vl_method_allocate_user_range(sd_varlink *link, sd_json_variant *para /* Note, we'll not return UID values from the host, since the child might not run in the same * user namespace as us. If they want to know the ranges they should read them off the userns fd, so * that they are translated into their PoV */ - return sd_varlink_replyb(link, SD_JSON_BUILD_EMPTY_OBJECT); + return sd_varlink_replybo(link, SD_JSON_BUILD_PAIR_STRING("name", userns_name)); fail: /* Note: we don't have to clean-up the BPF maps in the error path: the bpf map type used will @@ -928,6 +1000,7 @@ static int validate_userns_is_safe(sd_varlink *link, int userns_fd) { typedef struct RegisterParameters { const char *name; unsigned userns_fd_idx; + bool mangle_name; } RegisterParameters; static int vl_method_register_user_namespace(sd_varlink *link, sd_json_variant *parameters, sd_varlink_method_flags_t flags, void *userdata) { @@ -935,6 +1008,7 @@ static int vl_method_register_user_namespace(sd_varlink *link, sd_json_variant * static const sd_json_dispatch_field dispatch_table[] = { { "name", SD_JSON_VARIANT_STRING, sd_json_dispatch_const_string, offsetof(RegisterParameters, name), SD_JSON_MANDATORY }, { "userNamespaceFileDescriptor", _SD_JSON_VARIANT_TYPE_INVALID, sd_json_dispatch_uint, offsetof(RegisterParameters, userns_fd_idx), SD_JSON_MANDATORY }, + { "mangleName", SD_JSON_VARIANT_BOOLEAN, sd_json_dispatch_stdbool, offsetof(AllocateParameters, mangle_name), 0 }, {} }; @@ -959,7 +1033,7 @@ static int vl_method_register_user_namespace(sd_varlink *link, sd_json_variant * if (r != 0) return r; - r = validate_name(link, p.name, &userns_name); + r = validate_name(link, p.name, p.mangle_name, &userns_name); if (r != 0) return r; @@ -1044,7 +1118,7 @@ static int vl_method_register_user_namespace(sd_varlink *link, sd_json_variant * if (r < 0) goto fail; - return sd_varlink_replyb(link, SD_JSON_BUILD_EMPTY_OBJECT); + return sd_varlink_replybo(link, SD_JSON_BUILD_PAIR_STRING("name", userns_name)); fail: userns_registry_remove(registry_dir_fd, userns_info); diff --git a/src/nsresourced/userns-registry.c b/src/nsresourced/userns-registry.c index d69348aae7..b1754b51fe 100644 --- a/src/nsresourced/userns-registry.c +++ b/src/nsresourced/userns-registry.c @@ -592,6 +592,9 @@ bool userns_name_is_valid(const char *name) { /* Checks if the specified string is suitable as user namespace name. */ + if (isempty(name)) + return false; + if (strlen(name) > NAME_MAX) /* before we use alloca(), let's check for size */ return false; diff --git a/src/shared/nsresource.c b/src/shared/nsresource.c index 2475315315..79a2da45a7 100644 --- a/src/shared/nsresource.c +++ b/src/shared/nsresource.c @@ -23,7 +23,7 @@ static int make_pid_name(char **ret) { /* So the namespace name should be 16 chars at max (because we want that it is usable in usernames, * which have a limit of 31 chars effectively, and the nsresourced service wants to prefix/suffix * some bits). But it also should be unique if we are called multiple times in a row. Hence we take - * the "comm" name (which is 15 chars), and suffix it with the UID, possibly overriding the end. */ + * the "comm" name (which is 15 chars), and suffix it with the PID, possibly overriding the end. */ assert_cc(TASK_COMM_LEN == 15 + 1); char spid[DECIMAL_STR_MAX(pid_t)]; @@ -83,6 +83,7 @@ int nsresource_allocate_userns(const char *name, uint64_t size) { &reply, &error_id, SD_JSON_BUILD_PAIR("name", SD_JSON_BUILD_STRING(name)), + SD_JSON_BUILD_PAIR("mangleName", SD_JSON_BUILD_BOOLEAN(true)), SD_JSON_BUILD_PAIR("size", SD_JSON_BUILD_UNSIGNED(size)), SD_JSON_BUILD_PAIR("userNamespaceFileDescriptor", SD_JSON_BUILD_UNSIGNED(userns_fd_idx))); if (r < 0) @@ -139,6 +140,7 @@ int nsresource_register_userns(const char *name, int userns_fd) { &reply, &error_id, SD_JSON_BUILD_PAIR("name", SD_JSON_BUILD_STRING(name)), + SD_JSON_BUILD_PAIR("mangleName", SD_JSON_BUILD_BOOLEAN(true)), SD_JSON_BUILD_PAIR("userNamespaceFileDescriptor", SD_JSON_BUILD_UNSIGNED(userns_fd_idx))); if (r < 0) return log_debug_errno(r, "Failed to call RegisterUserNamespace() varlink call: %m"); diff --git a/src/shared/varlink-io.systemd.NamespaceResource.c b/src/shared/varlink-io.systemd.NamespaceResource.c index 83419acba4..145d706c1b 100644 --- a/src/shared/varlink-io.systemd.NamespaceResource.c +++ b/src/shared/varlink-io.systemd.NamespaceResource.c @@ -4,15 +4,29 @@ static SD_VARLINK_DEFINE_METHOD( AllocateUserRange, + SD_VARLINK_FIELD_COMMENT("The name for the user namespace, a short string that must be fit to be included in a file name and in a user name"), SD_VARLINK_DEFINE_INPUT(name, SD_VARLINK_STRING, 0), + SD_VARLINK_FIELD_COMMENT("Controls whether to mangle the provided name if needed so that it is suitable for naming a user namespace. If true this will shorten the name as necessary or randomize it if that's not sufficient. If null defaults to false."), + SD_VARLINK_DEFINE_INPUT(mangleName, SD_VARLINK_BOOL, SD_VARLINK_NULLABLE), + SD_VARLINK_FIELD_COMMENT("The number of UIDs to assign. Must be 1 or 65536"), SD_VARLINK_DEFINE_INPUT(size, SD_VARLINK_INT, 0), + SD_VARLINK_FIELD_COMMENT("The target UID inside the user namespace"), SD_VARLINK_DEFINE_INPUT(target, SD_VARLINK_INT, SD_VARLINK_NULLABLE), - SD_VARLINK_DEFINE_INPUT(userNamespaceFileDescriptor, SD_VARLINK_INT, 0)); + SD_VARLINK_FIELD_COMMENT("A file descriptor to an allocated userns with no current UID range assignments"), + SD_VARLINK_DEFINE_INPUT(userNamespaceFileDescriptor, SD_VARLINK_INT, 0), + SD_VARLINK_FIELD_COMMENT("The name assigned to the user namespace"), + SD_VARLINK_DEFINE_OUTPUT(name, SD_VARLINK_STRING, SD_VARLINK_NULLABLE)); static SD_VARLINK_DEFINE_METHOD( RegisterUserNamespace, + SD_VARLINK_FIELD_COMMENT("The name for the user namespce, a short string that must be fit to be included in a file name and in a user name"), SD_VARLINK_DEFINE_INPUT(name, SD_VARLINK_STRING, 0), - SD_VARLINK_DEFINE_INPUT(userNamespaceFileDescriptor, SD_VARLINK_INT, 0)); + SD_VARLINK_FIELD_COMMENT("Controls whether to mangle the provided name if needed so that it is suitable for naming a user namespace. If true this will shorten the name as necessary or randomize it if that's not sufficient. If null defaults to false."), + SD_VARLINK_DEFINE_INPUT(mangleName, SD_VARLINK_BOOL, SD_VARLINK_NULLABLE), + SD_VARLINK_FIELD_COMMENT("A user namespace file descriptor that is fully initialized"), + SD_VARLINK_DEFINE_INPUT(userNamespaceFileDescriptor, SD_VARLINK_INT, 0), + SD_VARLINK_FIELD_COMMENT("The name assigned to the user namespace"), + SD_VARLINK_DEFINE_OUTPUT(name, SD_VARLINK_STRING, SD_VARLINK_NULLABLE)); static SD_VARLINK_DEFINE_METHOD( AddMountToUserNamespace, diff --git a/test/units/TEST-13-NSPAWN.nspawn.sh b/test/units/TEST-13-NSPAWN.nspawn.sh index 1b0ff77889..cd37f4c65e 100755 --- a/test/units/TEST-13-NSPAWN.nspawn.sh +++ b/test/units/TEST-13-NSPAWN.nspawn.sh @@ -1131,7 +1131,10 @@ testcase_unpriv() { local tmpdir name tmpdir="$(mktemp -d /var/tmp/TEST-13-NSPAWN.unpriv.XXX)" - name="unprv-${tmpdir##*.}" + # Note: we pick the machine name short enough to be a valid machine name, + # but definitely longer than 16 chars, so that userns name mangling in the + # nsresourced userns allocation logic is triggered and tested. */ + name="unprv-${tmpdir##*.}-somelongsuffix" trap 'rm -fr ${tmpdir@Q} || true; rm -f /run/verity.d/test-13-nspawn-${name@Q} || true' RETURN ERR create_dummy_ddi "$tmpdir" "$name" chown --recursive testuser: "$tmpdir" |