]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
seccomp: rework how the S[UG]ID filter is installed 12202/head
authorZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Wed, 3 Apr 2019 11:11:00 +0000 (13:11 +0200)
committerZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Wed, 3 Apr 2019 11:33:06 +0000 (13:33 +0200)
If we know that a syscall is undefined on the given architecture, don't
even try to add it.

Try to install the filter even if some syscalls fail. Also use a helper
function to make the whole a bit less magic.

This allows the S[UG]ID test to pass on arm64.

src/shared/seccomp-util.c

index 7a179998bdbd8cf63dd1fe37b810026d1ae57d6a..65d800c9145ea681b6c0ddae659bd217baedf69b 100644 (file)
@@ -1803,9 +1803,139 @@ int seccomp_protect_hostname(void) {
         return 0;
 }
 
+static int seccomp_restrict_sxid(scmp_filter_ctx seccomp, mode_t m) {
+        /* Checks the mode_t parameter of the following system calls:
+         *
+         *       → chmod() + fchmod() + fchmodat()
+         *       → open() + creat() + openat()
+         *       → mkdir() + mkdirat()
+         *       → mknod() + mknodat()
+         *
+         * Returns error if *everything* failed, and 0 otherwise.
+         */
+        int r = 0;
+        bool any = false;
+
+        r = seccomp_rule_add_exact(
+                        seccomp,
+                        SCMP_ACT_ERRNO(EPERM),
+                        SCMP_SYS(chmod),
+                        1,
+                        SCMP_A1(SCMP_CMP_MASKED_EQ, m, m));
+        if (r < 0)
+                log_debug_errno(r, "Failed to add filter for chmod: %m");
+        else
+                any = true;
+
+        r = seccomp_rule_add_exact(
+                        seccomp,
+                        SCMP_ACT_ERRNO(EPERM),
+                        SCMP_SYS(fchmod),
+                        1,
+                        SCMP_A1(SCMP_CMP_MASKED_EQ, m, m));
+        if (r < 0)
+                log_debug_errno(r, "Failed to add filter for fchmod: %m");
+        else
+                any = true;
+
+        r = seccomp_rule_add_exact(
+                        seccomp,
+                        SCMP_ACT_ERRNO(EPERM),
+                        SCMP_SYS(fchmodat),
+                        1,
+                        SCMP_A2(SCMP_CMP_MASKED_EQ, m, m));
+        if (r < 0)
+                log_debug_errno(r, "Failed to add filter for fchmodat: %m");
+        else
+                any = true;
+
+        r = seccomp_rule_add_exact(
+                        seccomp,
+                        SCMP_ACT_ERRNO(EPERM),
+                        SCMP_SYS(mkdir),
+                        1,
+                        SCMP_A1(SCMP_CMP_MASKED_EQ, m, m));
+        if (r < 0)
+                log_debug_errno(r, "Failed to add filter for mkdir: %m");
+        else
+                any = true;
+
+        r = seccomp_rule_add_exact(
+                        seccomp,
+                        SCMP_ACT_ERRNO(EPERM),
+                        SCMP_SYS(mkdirat),
+                        1,
+                        SCMP_A2(SCMP_CMP_MASKED_EQ, m, m));
+        if (r < 0)
+                log_debug_errno(r, "Failed to add filter for mkdirat: %m");
+        else
+                any = true;
+
+        r = seccomp_rule_add_exact(
+                        seccomp,
+                        SCMP_ACT_ERRNO(EPERM),
+                        SCMP_SYS(mknod),
+                        1,
+                        SCMP_A1(SCMP_CMP_MASKED_EQ, m, m));
+        if (r < 0)
+                log_debug_errno(r, "Failed to add filter for mknod: %m");
+        else
+                any = true;
+
+        r = seccomp_rule_add_exact(
+                        seccomp,
+                        SCMP_ACT_ERRNO(EPERM),
+                        SCMP_SYS(mknodat),
+                        1,
+                        SCMP_A2(SCMP_CMP_MASKED_EQ, m, m));
+        if (r < 0)
+                log_debug_errno(r, "Failed to add filter for mknodat: %m");
+        else
+                any = true;
+
+#if SCMP_SYS(open) > 0
+        r = seccomp_rule_add_exact(
+                        seccomp,
+                        SCMP_ACT_ERRNO(EPERM),
+                        SCMP_SYS(open),
+                        2,
+                        SCMP_A1(SCMP_CMP_MASKED_EQ, O_CREAT, O_CREAT),
+                        SCMP_A2(SCMP_CMP_MASKED_EQ, m, m));
+        if (r < 0)
+                log_debug_errno(r, "Failed to add filter for open: %m");
+        else
+                any = true;
+#endif
+
+        r = seccomp_rule_add_exact(
+                        seccomp,
+                        SCMP_ACT_ERRNO(EPERM),
+                        SCMP_SYS(openat),
+                        2,
+                        SCMP_A2(SCMP_CMP_MASKED_EQ, O_CREAT, O_CREAT),
+                        SCMP_A3(SCMP_CMP_MASKED_EQ, m, m));
+        if (r < 0)
+                log_debug_errno(r, "Failed to add filter for openat: %m");
+        else
+                any = true;
+
+        r = seccomp_rule_add_exact(
+                        seccomp,
+                        SCMP_ACT_ERRNO(EPERM),
+                        SCMP_SYS(creat),
+                        1,
+                        SCMP_A1(SCMP_CMP_MASKED_EQ, m, m));
+        if (r < 0)
+                log_debug_errno(r, "Failed to add filter for creat: %m");
+        else
+                any = true;
+
+        return any ? 0 : r;
+}
+
 int seccomp_restrict_suid_sgid(void) {
         uint32_t arch;
-        int r;
+        int r, k;
 
         SECCOMP_FOREACH_LOCAL_ARCH(arch) {
                 _cleanup_(seccomp_releasep) scmp_filter_ctx seccomp = NULL;
@@ -1814,114 +1944,16 @@ int seccomp_restrict_suid_sgid(void) {
                 if (r < 0)
                         return r;
 
-                /* Checks the mode_t parameter of the following system calls:
-                 *
-                 *       → chmod() + fchmod() + fchmodat()
-                 *       → open() + creat() + openat()
-                 *       → mkdir() + mkdirat()
-                 *       → mknod() + mknodat()
-                 */
-
-                for (unsigned bit = 0; bit < 2; bit ++) {
-                        /* Block S_ISUID in the first iteration, S_ISGID in the second */
-                        mode_t m = bit == 0 ? S_ISUID : S_ISGID;
-
-                        r = seccomp_rule_add_exact(
-                                        seccomp,
-                                        SCMP_ACT_ERRNO(EPERM),
-                                        SCMP_SYS(chmod),
-                                        1,
-                                        SCMP_A1(SCMP_CMP_MASKED_EQ, m, m));
-                        if (r < 0)
-                                break;
-
-                        r = seccomp_rule_add_exact(
-                                        seccomp,
-                                        SCMP_ACT_ERRNO(EPERM),
-                                        SCMP_SYS(fchmod),
-                                        1,
-                                        SCMP_A1(SCMP_CMP_MASKED_EQ, m, m));
-                        if (r < 0)
-                                break;
-
-                        r = seccomp_rule_add_exact(
-                                        seccomp,
-                                        SCMP_ACT_ERRNO(EPERM),
-                                        SCMP_SYS(fchmodat),
-                                        1,
-                                        SCMP_A2(SCMP_CMP_MASKED_EQ, m, m));
-                        if (r < 0)
-                                break;
-
-                        r = seccomp_rule_add_exact(
-                                        seccomp,
-                                        SCMP_ACT_ERRNO(EPERM),
-                                        SCMP_SYS(mkdir),
-                                        1,
-                                        SCMP_A1(SCMP_CMP_MASKED_EQ, m, m));
-                        if (r < 0)
-                                break;
-
-                        r = seccomp_rule_add_exact(
-                                        seccomp,
-                                        SCMP_ACT_ERRNO(EPERM),
-                                        SCMP_SYS(mkdirat),
-                                        1,
-                                        SCMP_A2(SCMP_CMP_MASKED_EQ, m, m));
-                        if (r < 0)
-                                break;
-
-                        r = seccomp_rule_add_exact(
-                                        seccomp,
-                                        SCMP_ACT_ERRNO(EPERM),
-                                        SCMP_SYS(mknod),
-                                        1,
-                                        SCMP_A1(SCMP_CMP_MASKED_EQ, m, m));
-                        if (r < 0)
-                                break;
-
-                        r = seccomp_rule_add_exact(
-                                        seccomp,
-                                        SCMP_ACT_ERRNO(EPERM),
-                                        SCMP_SYS(mknodat),
-                                        1,
-                                        SCMP_A2(SCMP_CMP_MASKED_EQ, m, m));
-                        if (r < 0)
-                                break;
-
-                        r = seccomp_rule_add_exact(
-                                        seccomp,
-                                        SCMP_ACT_ERRNO(EPERM),
-                                        SCMP_SYS(open),
-                                        2,
-                                        SCMP_A1(SCMP_CMP_MASKED_EQ, O_CREAT, O_CREAT),
-                                        SCMP_A2(SCMP_CMP_MASKED_EQ, m, m));
-                        if (r < 0)
-                                break;
+                r = seccomp_restrict_sxid(seccomp, S_ISUID);
+                if (r < 0)
+                        log_debug_errno(r, "Failed to add suid rule for architecture %s, ignoring: %m", seccomp_arch_to_string(arch));
 
-                        r = seccomp_rule_add_exact(
-                                        seccomp,
-                                        SCMP_ACT_ERRNO(EPERM),
-                                        SCMP_SYS(openat),
-                                        2,
-                                        SCMP_A2(SCMP_CMP_MASKED_EQ, O_CREAT, O_CREAT),
-                                        SCMP_A3(SCMP_CMP_MASKED_EQ, m, m));
-                        if (r < 0)
-                                break;
+                k = seccomp_restrict_sxid(seccomp, S_ISGID);
+                if (k < 0)
+                        log_debug_errno(r, "Failed to add sgid rule for architecture %s, ignoring: %m", seccomp_arch_to_string(arch));
 
-                        r = seccomp_rule_add_exact(
-                                        seccomp,
-                                        SCMP_ACT_ERRNO(EPERM),
-                                        SCMP_SYS(creat),
-                                        1,
-                                        SCMP_A1(SCMP_CMP_MASKED_EQ, m, m));
-                        if (r < 0)
-                                break;
-                }
-                if (r < 0) {
-                        log_debug_errno(r, "Failed to add suid/sgid rule for architecture %s, skipping: %m", seccomp_arch_to_string(arch));
+                if (r < 0 && k < 0)
                         continue;
-                }
 
                 r = seccomp_load(seccomp);
                 if (IN_SET(r, -EPERM, -EACCES))