seccomp: add new seccomp_init_conservative() helper

This adds a new seccomp_init_conservative() helper call that is mostly just a
wrapper around seccomp_init(), but turns off NNP and adds in all secondary
archs, for best compatibility with everything else.

Pretty much all of our code used the very same constructs for these three
steps, hence unifying this in one small function makes things a lot shorter.

This also changes incorrect usage of the "scmp_filter_ctx" type at various
places. libseccomp defines it as typedef to "void*", i.e. it is a pointer type
(pretty poor choice already!) that casts implicitly to and from all other
pointer types (even poorer choice: you defined a confusing type now, and don't
even gain any bit of type safety through it...). A lot of the code assumed the
type would refer to a structure, and hence aded additional "*" here and there.
Remove that.

diff --git a/src/core/execute.c b/src/core/execute.c
index f435a079c7629fa33cccabc11aecb93d532c0085..668504c5cf33115bc533e9740aa16d688c020e82 100644 (file)
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -1197,7 +1197,7 @@ static bool skip_seccomp_unavailable(const Unit* u, const char* msg) {
 
 static int apply_seccomp(const Unit* u, const ExecContext *c) {
         uint32_t negative_action, action;
-        scmp_filter_ctx *seccomp;
+        scmp_filter_ctx seccomp;
         Iterator i;
         void *id;
         int r;
@@ -1248,7 +1248,7 @@ finish:
 }
 
 static int apply_address_families(const Unit* u, const ExecContext *c) {
-        scmp_filter_ctx *seccomp;
+        scmp_filter_ctx seccomp;
         Iterator i;
         int r;
 
@@ -1257,13 +1257,9 @@ static int apply_address_families(const Unit* u, const ExecContext *c) {
         if (skip_seccomp_unavailable(u, "RestrictAddressFamilies="))
                 return 0;
 
-        seccomp = seccomp_init(SCMP_ACT_ALLOW);
-        if (!seccomp)
-                return -ENOMEM;
-
-        r = seccomp_add_secondary_archs(seccomp);
+        r = seccomp_init_conservative(&seccomp, SCMP_ACT_ALLOW);
         if (r < 0)
-                goto finish;
+                return r;
 
         if (c->address_families_whitelist) {
                 int af, first = 0, last = 0;
@@ -1360,10 +1356,6 @@ static int apply_address_families(const Unit* u, const ExecContext *c) {
                 }
         }
 
-        r = seccomp_attr_set(seccomp, SCMP_FLTATR_CTL_NNP, 0);
-        if (r < 0)
-                goto finish;
-
         r = seccomp_load(seccomp);
 
 finish:
@@ -1372,7 +1364,7 @@ finish:
 }
 
 static int apply_memory_deny_write_execute(const Unit* u, const ExecContext *c) {
-        scmp_filter_ctx *seccomp;
+        scmp_filter_ctx seccomp;
         int r;
 
         assert(c);
@@ -1380,13 +1372,9 @@ static int apply_memory_deny_write_execute(const Unit* u, const ExecContext *c)
         if (skip_seccomp_unavailable(u, "MemoryDenyWriteExecute="))
                 return 0;
 
-        seccomp = seccomp_init(SCMP_ACT_ALLOW);
-        if (!seccomp)
-                return -ENOMEM;
-
-        r = seccomp_add_secondary_archs(seccomp);
+        r = seccomp_init_conservative(&seccomp, SCMP_ACT_ALLOW);
         if (r < 0)
-                goto finish;
+                return r;
 
         r = seccomp_rule_add(
                         seccomp,
@@ -1406,10 +1394,6 @@ static int apply_memory_deny_write_execute(const Unit* u, const ExecContext *c)
         if (r < 0)
                 goto finish;
 
-        r = seccomp_attr_set(seccomp, SCMP_FLTATR_CTL_NNP, 0);
-        if (r < 0)
-                goto finish;
-
         r = seccomp_load(seccomp);
 
 finish:
@@ -1424,7 +1408,7 @@ static int apply_restrict_realtime(const Unit* u, const ExecContext *c) {
                 SCHED_IDLE,
         };
 
-        scmp_filter_ctx *seccomp;
+        scmp_filter_ctx seccomp;
         unsigned i;
         int r, p, max_policy = 0;
 
@@ -1433,13 +1417,9 @@ static int apply_restrict_realtime(const Unit* u, const ExecContext *c) {
         if (skip_seccomp_unavailable(u, "RestrictRealtime="))
                 return 0;
 
-        seccomp = seccomp_init(SCMP_ACT_ALLOW);
-        if (!seccomp)
-                return -ENOMEM;
-
-        r = seccomp_add_secondary_archs(seccomp);
+        r = seccomp_init_conservative(&seccomp, SCMP_ACT_ALLOW);
         if (r < 0)
-                goto finish;
+                return r;
 
         /* Determine the highest policy constant we want to allow */
         for (i = 0; i < ELEMENTSOF(permitted_policies); i++)
@@ -1483,10 +1463,6 @@ static int apply_restrict_realtime(const Unit* u, const ExecContext *c) {
         if (r < 0)
                 goto finish;
 
-        r = seccomp_attr_set(seccomp, SCMP_FLTATR_CTL_NNP, 0);
-        if (r < 0)
-                goto finish;
-
         r = seccomp_load(seccomp);
 
 finish:
@@ -1495,7 +1471,7 @@ finish:
 }
 
 static int apply_protect_sysctl(Unit *u, const ExecContext *c) {
-        scmp_filter_ctx *seccomp;
+        scmp_filter_ctx seccomp;
         int r;
 
         assert(c);
@@ -1506,13 +1482,9 @@ static int apply_protect_sysctl(Unit *u, const ExecContext *c) {
         if (skip_seccomp_unavailable(u, "ProtectKernelTunables="))
                 return 0;
 
-        seccomp = seccomp_init(SCMP_ACT_ALLOW);
-        if (!seccomp)
-                return -ENOMEM;
-
-        r = seccomp_add_secondary_archs(seccomp);
+        r = seccomp_init_conservative(&seccomp, SCMP_ACT_ALLOW);
         if (r < 0)
-                goto finish;
+                return r;
 
         r = seccomp_rule_add(
                         seccomp,
@@ -1522,10 +1494,6 @@ static int apply_protect_sysctl(Unit *u, const ExecContext *c) {
         if (r < 0)
                 goto finish;
 
-        r = seccomp_attr_set(seccomp, SCMP_FLTATR_CTL_NNP, 0);
-        if (r < 0)
-                goto finish;
-
         r = seccomp_load(seccomp);
 
 finish:
@@ -1534,9 +1502,7 @@ finish:
 }
 
 static int apply_protect_kernel_modules(Unit *u, const ExecContext *c) {
-
-        scmp_filter_ctx *seccomp;
-        const char *sys;
+        scmp_filter_ctx seccomp;
         int r;
 
         assert(c);
@@ -1546,22 +1512,14 @@ static int apply_protect_kernel_modules(Unit *u, const ExecContext *c) {
         if (skip_seccomp_unavailable(u, "ProtectKernelModules="))
                 return 0;
 
-        seccomp = seccomp_init(SCMP_ACT_ALLOW);
-        if (!seccomp)
-                return -ENOMEM;
-
-        r = seccomp_add_secondary_archs(seccomp);
+        r = seccomp_init_conservative(&seccomp, SCMP_ACT_ALLOW);
         if (r < 0)
-                goto finish;
+                return r;
 
         r = seccomp_add_syscall_filter_set(seccomp, syscall_filter_sets + SYSCALL_FILTER_SET_MODULE, SCMP_ACT_ERRNO(EPERM));
         if (r < 0)
                 goto finish;
 
-        r = seccomp_attr_set(seccomp, SCMP_FLTATR_CTL_NNP, 0);
-        if (r < 0)
-                goto finish;
-
         r = seccomp_load(seccomp);
 
 finish:
@@ -1570,7 +1528,7 @@ finish:
 }
 
 static int apply_private_devices(Unit *u, const ExecContext *c) {
-        scmp_filter_ctx *seccomp;
+        scmp_filter_ctx seccomp;
         int r;
 
         assert(c);
@@ -1580,22 +1538,14 @@ static int apply_private_devices(Unit *u, const ExecContext *c) {
         if (skip_seccomp_unavailable(u, "PrivateDevices="))
                 return 0;
 
-        seccomp = seccomp_init(SCMP_ACT_ALLOW);
-        if (!seccomp)
-                return -ENOMEM;
-
-        r = seccomp_add_secondary_archs(seccomp);
+        r = seccomp_init_conservative(&seccomp, SCMP_ACT_ALLOW);
         if (r < 0)
-                goto finish;
+                return r;
 
         r = seccomp_add_syscall_filter_set(seccomp, syscall_filter_sets + SYSCALL_FILTER_SET_RAW_IO, SCMP_ACT_ERRNO(EPERM));
         if (r < 0)
                 goto finish;
 
-        r = seccomp_attr_set(seccomp, SCMP_FLTATR_CTL_NNP, 0);
-        if (r < 0)
-                goto finish;
-
         r = seccomp_load(seccomp);
 
 finish:

diff --git a/src/nspawn/nspawn-seccomp.c b/src/nspawn/nspawn-seccomp.c
index 44a0b397abf9493cfd7eb4ce411b7bb3637f1690..03a397d30ce6065254796bd7a0a3cdb7bf5e2c79 100644 (file)
--- a/src/nspawn/nspawn-seccomp.c
+++ b/src/nspawn/nspawn-seccomp.c
@@ -135,15 +135,9 @@ int setup_seccomp(uint64_t cap_list_retain) {
                 return 0;
         }
 
-        seccomp = seccomp_init(SCMP_ACT_ALLOW);
-        if (!seccomp)
-                return log_oom();
-
-        r = seccomp_add_secondary_archs(seccomp);
-        if (r < 0) {
-                log_error_errno(r, "Failed to add secondary archs to seccomp filter: %m");
-                goto finish;
-        }
+        r = seccomp_init_conservative(&seccomp, SCMP_ACT_ALLOW);
+        if (r < 0)
+                return log_error_errno(r, "Failed to allocate seccomp object: %m");
 
         r = seccomp_add_default_syscall_filter(seccomp, cap_list_retain);
         if (r < 0)
@@ -171,12 +165,6 @@ int setup_seccomp(uint64_t cap_list_retain) {
                 goto finish;
         }
 
-        r = seccomp_attr_set(seccomp, SCMP_FLTATR_CTL_NNP, 0);
-        if (r < 0) {
-                log_error_errno(r, "Failed to unset NO_NEW_PRIVS: %m");
-                goto finish;
-        }
-
         r = seccomp_load(seccomp);
         if (r < 0) {
                 log_error_errno(r, "Failed to install seccomp audit filter: %m");

diff --git a/src/shared/seccomp-util.c b/src/shared/seccomp-util.c
index 1d51f3fd1fd2a56ea7ded7c747fc89183c35f310..0b9fa47c4403af7fcfb32e2425b6ac0f9e9d0357 100644 (file)
--- a/src/shared/seccomp-util.c
+++ b/src/shared/seccomp-util.c
@@ -74,7 +74,34 @@ int seccomp_arch_from_string(const char *n, uint32_t *ret) {
         return 0;
 }
 
-int seccomp_add_secondary_archs(scmp_filter_ctx *c) {
+int seccomp_init_conservative(scmp_filter_ctx *ret, uint32_t default_action) {
+        scmp_filter_ctx seccomp;
+        int r;
+
+        /* Much like seccomp_init(), but tries to be a bit more conservative in its defaults: all secondary archs are
+         * added by default, and NNP is turned off. */
+
+        seccomp = seccomp_init(default_action);
+        if (!seccomp)
+                return -ENOMEM;
+
+        r = seccomp_add_secondary_archs(seccomp);
+        if (r < 0)
+                goto finish;
+
+        r = seccomp_attr_set(seccomp, SCMP_FLTATR_CTL_NNP, 0);
+        if (r < 0)
+                goto finish;
+
+        *ret = seccomp;
+        return 0;
+
+finish:
+        seccomp_release(seccomp);
+        return r;
+}
+
+int seccomp_add_secondary_archs(scmp_filter_ctx c) {
 
 #if defined(__i386__) || defined(__x86_64__)
         int r;
@@ -111,7 +138,6 @@ int seccomp_add_secondary_archs(scmp_filter_ctx *c) {
 #endif
 
         return 0;
-
 }
 
 static bool is_basic_seccomp_available(void) {

diff --git a/src/shared/seccomp-util.h b/src/shared/seccomp-util.h
index 34fd49c12256aa599668f6c447d6eaa972862ca7..2de429a772345239acf5347143481a931ed90298 100644 (file)
--- a/src/shared/seccomp-util.h
+++ b/src/shared/seccomp-util.h
@@ -25,7 +25,9 @@
 const char* seccomp_arch_to_string(uint32_t c);
 int seccomp_arch_from_string(const char *n, uint32_t *ret);
 
-int seccomp_add_secondary_archs(scmp_filter_ctx *c);
+int seccomp_init_conservative(scmp_filter_ctx *ret, uint32_t default_action);
+
+int seccomp_add_secondary_archs(scmp_filter_ctx c);
 
 bool is_seccomp_available(void);