Merge pull request #8817 from yuwata/cleanup-nsflags

core: allow to specify RestrictNamespaces= multiple times

10 files changed, 79 insertions, 93 deletions

diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c
index f7e8798327..977ef22c94 100644
--- a/src/core/dbus-execute.c
+++ b/src/core/dbus-execute.c
@@ -1032,7 +1032,7 @@ static BUS_DEFINE_SET_TRANSIENT_PARSE_PTR(personality, unsigned long, parse_pers
 static BUS_DEFINE_SET_TRANSIENT_TO_STRING_ALLOC(secure_bits, "i", int32_t, int, "%" PRIi32, secure_bits_to_string_alloc_with_check);
 static BUS_DEFINE_SET_TRANSIENT_TO_STRING_ALLOC(capability, "t", uint64_t, uint64_t, "%" PRIu64, capability_set_to_string_alloc);
 static BUS_DEFINE_SET_TRANSIENT_TO_STRING_ALLOC(sched_policy, "i", int32_t, int, "%" PRIi32, sched_policy_to_string_alloc_with_check);
-static BUS_DEFINE_SET_TRANSIENT_TO_STRING_ALLOC(namespace_flag, "t", uint64_t, unsigned long, "%" PRIu64, namespace_flag_to_string_many_with_check);
+static BUS_DEFINE_SET_TRANSIENT_TO_STRING_ALLOC(namespace_flag, "t", uint64_t, unsigned long, "%" PRIu64, namespace_flags_to_string);
 static BUS_DEFINE_SET_TRANSIENT_TO_STRING(mount_flags, "t", uint64_t, unsigned long, "%" PRIu64, mount_propagation_flags_to_string_with_check);
 
 int bus_exec_context_set_transient_property(
diff --git a/src/core/execute.c b/src/core/execute.c
index 8cb16eb49b..939bc12b56 100644
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -3548,7 +3548,8 @@ void exec_context_init(ExecContext *c) {
         for (i = 0; i < _EXEC_DIRECTORY_TYPE_MAX; i++)
                 c->directories[i].mode = 0755;
         c->capability_bounding_set = CAP_ALL;
-        c->restrict_namespaces = NAMESPACE_FLAGS_ALL;
+        assert_cc(NAMESPACE_FLAGS_INITIAL != NAMESPACE_FLAGS_ALL);
+        c->restrict_namespaces = NAMESPACE_FLAGS_INITIAL;
         c->log_level_max = -1;
 }
 
@@ -4250,7 +4251,7 @@ void exec_context_dump(const ExecContext *c, FILE* f, const char *prefix) {
         if (exec_context_restrict_namespaces_set(c)) {
                 _cleanup_free_ char *s = NULL;
 
-                r = namespace_flag_to_string_many(c->restrict_namespaces, &s);
+                r = namespace_flags_to_string(c->restrict_namespaces, &s);
                 if (r >= 0)
                         fprintf(f, "%sRestrictNamespaces: %s\n",
                                 prefix, s);
diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
index b9b23bb278..2a11e4bbd0 100644
--- a/src/core/load-fragment.c
+++ b/src/core/load-fragment.c
@@ -3065,11 +3065,22 @@ int config_parse_restrict_namespaces(
                 void *userdata) {
 
         ExecContext *c = data;
+        unsigned long flags;
         bool invert = false;
         int r;
 
         if (isempty(rvalue)) {
                 /* Reset to the default. */
+                c->restrict_namespaces = NAMESPACE_FLAGS_INITIAL;
+                return 0;
+        }
+
+        /* Boolean parameter ignores the previous settings */
+        r = parse_boolean(rvalue);
+        if (r > 0) {
+                c->restrict_namespaces = 0;
+                return 0;
+        } else if (r == 0) {
                 c->restrict_namespaces = NAMESPACE_FLAGS_ALL;
                 return 0;
         }
@@ -3079,23 +3090,19 @@ int config_parse_restrict_namespaces(
                 rvalue++;
         }
 
-        r = parse_boolean(rvalue);
-        if (r > 0)
-                c->restrict_namespaces = 0;
-        else if (r == 0)
-                c->restrict_namespaces = NAMESPACE_FLAGS_ALL;
-        else {
-                /* Not a boolean argument, in this case it's a list of namespace types. */
-
-                r = namespace_flag_from_string_many(rvalue, &c->restrict_namespaces);
-                if (r < 0) {
-                        log_syntax(unit, LOG_ERR, filename, line, r, "Failed to parse namespace type string, ignoring: %s", rvalue);
-                        return 0;
-                }
+        /* Not a boolean argument, in this case it's a list of namespace types. */
+        r = namespace_flags_from_string(rvalue, &flags);
+        if (r < 0) {
+                log_syntax(unit, LOG_ERR, filename, line, r, "Failed to parse namespace type string, ignoring: %s", rvalue);
+                return 0;
         }
 
-        if (invert)
-                c->restrict_namespaces = (~c->restrict_namespaces) & NAMESPACE_FLAGS_ALL;
+        if (c->restrict_namespaces == NAMESPACE_FLAGS_INITIAL)
+                /* Initial assignment. Just set the value. */
+                c->restrict_namespaces = invert ? (~flags) & NAMESPACE_FLAGS_ALL : flags;
+        else
+                /* Merge the value with the previous one. */
+                SET_FLAG(c->restrict_namespaces, flags, !invert);
 
         return 0;
 }
diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c
index fc035e796e..539a7b4d9d 100644
--- a/src/shared/bus-unit-util.c
+++ b/src/shared/bus-unit-util.c
@@ -1008,12 +1008,7 @@ static int bus_append_execute_property(sd_bus_message *m, const char *field, con
 
         if (streq(field, "RestrictNamespaces")) {
                 bool invert = false;
-                unsigned long flags = 0;
-
-                if (eq[0] == '~') {
-                        invert = true;
-                        eq++;
-                }
+                unsigned long flags;
 
                 r = parse_boolean(eq);
                 if (r > 0)
@@ -1021,7 +1016,12 @@ static int bus_append_execute_property(sd_bus_message *m, const char *field, con
                 else if (r == 0)
                         flags = NAMESPACE_FLAGS_ALL;
                 else {
-                        r = namespace_flag_from_string_many(eq, &flags);
+                        if (eq[0] == '~') {
+                                invert = true;
+                                eq++;
+                        }
+
+                        r = namespace_flags_from_string(eq, &flags);
                         if (r < 0)
                                 return log_error_errno(r, "Failed to parse %s value %s.", field, eq);
                 }
diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c
index 1564966e52..ed30edfb60 100644
--- a/src/shared/bus-util.c
+++ b/src/shared/bus-util.c
@@ -716,7 +716,7 @@ int bus_print_property(const char *name, sd_bus_message *m, bool value, bool all
                         else if ((u & NAMESPACE_FLAGS_ALL) == NAMESPACE_FLAGS_ALL)
                                 result = "no";
                         else {
-                                r = namespace_flag_to_string_many(u, &s);
+                                r = namespace_flags_to_string(u, &s);
                                 if (r < 0)
                                         return r;
 
diff --git a/src/shared/nsflags.c b/src/shared/nsflags.c
index 4e01976d58..baac16bbb4 100644
--- a/src/shared/nsflags.c
+++ b/src/shared/nsflags.c
@@ -25,32 +25,7 @@ const struct namespace_flag_map namespace_flag_map[] = {
         {}
 };
 
-const char* namespace_flag_to_string(unsigned long flag) {
-        unsigned i;
-
-        flag &= NAMESPACE_FLAGS_ALL;
-
-        for (i = 0; namespace_flag_map[i].name; i++)
-                if (flag == namespace_flag_map[i].flag)
-                        return namespace_flag_map[i].name;
-
-        return NULL; /* either unknown namespace flag, or a combination of many. This call supports neither. */
-}
-
-unsigned long namespace_flag_from_string(const char *name) {
-        unsigned i;
-
-        if (isempty(name))
-                return 0;
-
-        for (i = 0; namespace_flag_map[i].name; i++)
-                if (streq(name, namespace_flag_map[i].name))
-                        return namespace_flag_map[i].flag;
-
-        return 0;
-}
-
-int namespace_flag_from_string_many(const char *name, unsigned long *ret) {
+int namespace_flags_from_string(const char *name, unsigned long *ret) {
         unsigned long flags = 0;
         int r;
 
@@ -58,7 +33,8 @@ int namespace_flag_from_string_many(const char *name, unsigned long *ret) {
 
         for (;;) {
                 _cleanup_free_ char *word = NULL;
-                unsigned long f;
+                unsigned long f = 0;
+                unsigned i;
 
                 r = extract_first_word(&name, &word, NULL, 0);
                 if (r < 0)
@@ -66,7 +42,12 @@ int namespace_flag_from_string_many(const char *name, unsigned long *ret) {
                 if (r == 0)
                         break;
 
-                f = namespace_flag_from_string(word);
+                for (i = 0; namespace_flag_map[i].name; i++)
+                        if (streq(word, namespace_flag_map[i].name)) {
+                                 f = namespace_flag_map[i].flag;
+                                 break;
+                        }
+
                 if (f == 0)
                         return -EINVAL;
 
@@ -77,7 +58,7 @@ int namespace_flag_from_string_many(const char *name, unsigned long *ret) {
         return 0;
 }
 
-int namespace_flag_to_string_many(unsigned long flags, char **ret) {
+int namespace_flags_to_string(unsigned long flags, char **ret) {
         _cleanup_free_ char *s = NULL;
         unsigned i;
 
@@ -85,14 +66,8 @@ int namespace_flag_to_string_many(unsigned long flags, char **ret) {
                 if ((flags & namespace_flag_map[i].flag) != namespace_flag_map[i].flag)
                         continue;
 
-                if (!s) {
-                        s = strdup(namespace_flag_map[i].name);
-                        if (!s)
-                                return -ENOMEM;
-                } else {
-                        if (!strextend(&s, " ", namespace_flag_map[i].name, NULL))
-                                return -ENOMEM;
-                }
+                if (!strextend_with_separator(&s, " ", namespace_flag_map[i].name, NULL))
+                        return -ENOMEM;
         }
 
         if (!s) {
diff --git a/src/shared/nsflags.h b/src/shared/nsflags.h
index c5bc83e723..2133a6c1be 100644
--- a/src/shared/nsflags.h
+++ b/src/shared/nsflags.h
@@ -24,17 +24,10 @@
                           CLONE_NEWUSER|                                \
                           CLONE_NEWUTS))
 
-const char* namespace_flag_to_string(unsigned long flag);
-unsigned long namespace_flag_from_string(const char *name);
-int namespace_flag_from_string_many(const char *name, unsigned long *ret);
-int namespace_flag_to_string_many(unsigned long flags, char **ret);
+#define NAMESPACE_FLAGS_INITIAL  ((unsigned long) -1)
 
-static inline int namespace_flag_to_string_many_with_check(unsigned long n, char **s) {
-        if ((n & NAMESPACE_FLAGS_ALL) != n)
-                return -EINVAL;
-
-        return namespace_flag_to_string_many(n, s);
-}
+int namespace_flags_from_string(const char *name, unsigned long *ret);
+int namespace_flags_to_string(unsigned long flags, char **ret);
 
 struct namespace_flag_map {
         unsigned long flag;
diff --git a/src/shared/seccomp-util.c b/src/shared/seccomp-util.c
index 8332942002..cddfc05bd3 100644
--- a/src/shared/seccomp-util.c
+++ b/src/shared/seccomp-util.c
@@ -1007,7 +1007,7 @@ int seccomp_restrict_namespaces(unsigned long retain) {
         if (DEBUG_LOGGING) {
                 _cleanup_free_ char *s = NULL;
 
-                (void) namespace_flag_to_string_many(retain, &s);
+                (void) namespace_flags_to_string(retain, &s);
                 log_debug("Restricting namespace to: %s.", strna(s));
         }
 
diff --git a/src/test/test-execute.c b/src/test/test-execute.c
index 20202c9421..7095dd2a2e 100644
--- a/src/test/test-execute.c
+++ b/src/test/test-execute.c
@@ -359,6 +359,9 @@ static void test_exec_restrictnamespaces(Manager *m) {
         test(m, "exec-restrictnamespaces-yes.service", 1, CLD_EXITED);
         test(m, "exec-restrictnamespaces-mnt.service", 0, CLD_EXITED);
         test(m, "exec-restrictnamespaces-mnt-blacklist.service", 1, CLD_EXITED);
+        test(m, "exec-restrictnamespaces-merge-and.service", 0, CLD_EXITED);
+        test(m, "exec-restrictnamespaces-merge-or.service", 0, CLD_EXITED);
+        test(m, "exec-restrictnamespaces-merge-all.service", 0, CLD_EXITED);
 #endif
 }
 
diff --git a/src/test/test-seccomp.c b/src/test/test-seccomp.c
index 155ce5cfbf..6062ca1baf 100644
--- a/src/test/test-seccomp.c
+++ b/src/test/test-seccomp.c
@@ -133,29 +133,36 @@ static void test_filter_sets(void) {
 }
 
 static void test_restrict_namespace(void) {
-        _cleanup_free_ char *s = NULL;
+        char *s = NULL;
         unsigned long ul;
         pid_t pid;
 
-        assert_se(namespace_flag_to_string(0) == NULL);
-        assert_se(streq(namespace_flag_to_string(CLONE_NEWNS), "mnt"));
-        assert_se(namespace_flag_to_string(CLONE_NEWNS|CLONE_NEWIPC) == NULL);
-        assert_se(streq(namespace_flag_to_string(CLONE_NEWCGROUP), "cgroup"));
-
-        assert_se(namespace_flag_from_string("mnt") == CLONE_NEWNS);
-        assert_se(namespace_flag_from_string(NULL) == 0);
-        assert_se(namespace_flag_from_string("") == 0);
-        assert_se(namespace_flag_from_string("uts") == CLONE_NEWUTS);
-        assert_se(namespace_flag_from_string(namespace_flag_to_string(CLONE_NEWUTS)) == CLONE_NEWUTS);
-        assert_se(streq(namespace_flag_to_string(namespace_flag_from_string("ipc")), "ipc"));
-
-        assert_se(namespace_flag_from_string_many(NULL, &ul) == 0 && ul == 0);
-        assert_se(namespace_flag_from_string_many("", &ul) == 0 && ul == 0);
-        assert_se(namespace_flag_from_string_many("mnt uts ipc", &ul) == 0 && ul == (CLONE_NEWNS|CLONE_NEWUTS|CLONE_NEWIPC));
-
-        assert_se(namespace_flag_to_string_many(NAMESPACE_FLAGS_ALL, &s) == 0);
+        assert_se(namespace_flags_to_string(0, &s) == 0 && streq(s, ""));
+        s = mfree(s);
+        assert_se(namespace_flags_to_string(CLONE_NEWNS, &s) == 0 && streq(s, "mnt"));
+        s = mfree(s);
+        assert_se(namespace_flags_to_string(CLONE_NEWNS|CLONE_NEWIPC, &s) == 0 && streq(s, "ipc mnt"));
+        s = mfree(s);
+        assert_se(namespace_flags_to_string(CLONE_NEWCGROUP, &s) == 0 && streq(s, "cgroup"));
+        s = mfree(s);
+
+        assert_se(namespace_flags_from_string("mnt", &ul) == 0 && ul == CLONE_NEWNS);
+        assert_se(namespace_flags_from_string(NULL, &ul) == 0 && ul == 0);
+        assert_se(namespace_flags_from_string("", &ul) == 0 && ul == 0);
+        assert_se(namespace_flags_from_string("uts", &ul) == 0 && ul == CLONE_NEWUTS);
+        assert_se(namespace_flags_from_string("mnt uts ipc", &ul) == 0 && ul == (CLONE_NEWNS|CLONE_NEWUTS|CLONE_NEWIPC));
+
+        assert_se(namespace_flags_to_string(CLONE_NEWUTS, &s) == 0 && streq(s, "uts"));
+        assert_se(namespace_flags_from_string(s, &ul) == 0 && ul == CLONE_NEWUTS);
+        s = mfree(s);
+        assert_se(namespace_flags_from_string("ipc", &ul) == 0 && ul == CLONE_NEWIPC);
+        assert_se(namespace_flags_to_string(ul, &s) == 0 && streq(s, "ipc"));
+        s = mfree(s);
+
+        assert_se(namespace_flags_to_string(NAMESPACE_FLAGS_ALL, &s) == 0);
         assert_se(streq(s, "cgroup ipc net mnt pid user uts"));
-        assert_se(namespace_flag_from_string_many(s, &ul) == 0 && ul == NAMESPACE_FLAGS_ALL);
+        assert_se(namespace_flags_from_string(s, &ul) == 0 && ul == NAMESPACE_FLAGS_ALL);
+        s = mfree(s);
 
         if (!is_seccomp_available())
                 return;