From dcf2632945b87acedeea989a5aa36c084a20ae88 Mon Sep 17 00:00:00 2001 From: Justin Cormack Date: Tue, 9 May 2017 14:21:19 +0100 Subject: [PATCH] Revert "Block obsolete socket families in the default seccomp profile" This reverts commit 7e3a596a63fd8d0ab958132901b6ded81f8b44c0. Unfortunately, it was pointed out in https://github.com/moby/moby/pull/29076#commitcomment-21831387 that the `socketcall` syscall takes a pointer to a struct so it is not possible to use seccomp profiles to filter it. This means these cannot be blocked as you can use `socketcall` to call them regardless, as we currently allow 32 bit syscalls. Users who wish to block these should use a seccomp profile that blocks all 32 bit syscalls and then just block the non socketcall versions. Signed-off-by: Justin Cormack --- contrib/syscall-test/Dockerfile | 3 +- contrib/syscall-test/appletalk.c | 12 - integration-cli/docker_cli_run_unix_test.go | 12 - integration-cli/fixtures_linux_daemon_test.go | 2 +- profiles/seccomp/default.json | 219 +----------------- profiles/seccomp/seccomp_default.go | 149 +----------- 6 files changed, 6 insertions(+), 391 deletions(-) delete mode 100644 contrib/syscall-test/appletalk.c diff --git a/contrib/syscall-test/Dockerfile b/contrib/syscall-test/Dockerfile index fcf5892be4..f95f1758c0 100644 --- a/contrib/syscall-test/Dockerfile +++ b/contrib/syscall-test/Dockerfile @@ -10,7 +10,6 @@ RUN gcc -g -Wall -static userns.c -o /usr/bin/userns-test \ && gcc -g -Wall -static setuid.c -o /usr/bin/setuid-test \ && gcc -g -Wall -static setgid.c -o /usr/bin/setgid-test \ && gcc -g -Wall -static socket.c -o /usr/bin/socket-test \ - && gcc -g -Wall -static raw.c -o /usr/bin/raw-test \ - && gcc -g -Wall -static appletalk.c -o /usr/bin/appletalk-test + && gcc -g -Wall -static raw.c -o /usr/bin/raw-test RUN [ "$(uname -m)" = "x86_64" ] && gcc -s -m32 -nostdlib exit32.s -o /usr/bin/exit32-test || true diff --git a/contrib/syscall-test/appletalk.c b/contrib/syscall-test/appletalk.c deleted file mode 100644 index 0001dd4247..0000000000 --- a/contrib/syscall-test/appletalk.c +++ /dev/null @@ -1,12 +0,0 @@ -#include -#include - -int main() { - - if (socket(AF_APPLETALK, SOCK_DGRAM, 0) != -1) { - fprintf(stderr, "Opening Appletalk socket worked, should be blocked\n"); - return 1; - } - - return 0; -} diff --git a/integration-cli/docker_cli_run_unix_test.go b/integration-cli/docker_cli_run_unix_test.go index bb5913a126..b3d1b07218 100644 --- a/integration-cli/docker_cli_run_unix_test.go +++ b/integration-cli/docker_cli_run_unix_test.go @@ -1015,18 +1015,6 @@ func (s *DockerSuite) TestRunSeccompProfileDenyUnshareUserns(c *check.C) { }) } -// TestRunSeccompProfileDenyUnusualSocketFamilies checks that rarely used socket families such as Appletalk are blocked by the default profile -func (s *DockerSuite) TestRunSeccompProfileDenyUnusualSocketFamilies(c *check.C) { - testRequires(c, SameHostDaemon, seccompEnabled) - ensureSyscallTest(c) - - runCmd := exec.Command(dockerBinary, "run", "syscall-test", "appletalk-test") - _, _, err := runCommandWithOutput(runCmd) - if err != nil { - c.Fatal("expected opening appletalk socket family to fail") - } -} - // TestRunSeccompProfileDenyCloneUserns checks that 'docker run syscall-test' // with a the default seccomp profile exits with operation not permitted. func (s *DockerSuite) TestRunSeccompProfileDenyCloneUserns(c *check.C) { diff --git a/integration-cli/fixtures_linux_daemon_test.go b/integration-cli/fixtures_linux_daemon_test.go index 4968514701..895f976a18 100644 --- a/integration-cli/fixtures_linux_daemon_test.go +++ b/integration-cli/fixtures_linux_daemon_test.go @@ -60,7 +60,7 @@ func ensureSyscallTest(c *check.C) { gcc, err := exec.LookPath("gcc") c.Assert(err, checker.IsNil, check.Commentf("could not find gcc")) - tests := []string{"userns", "ns", "acct", "setuid", "setgid", "socket", "raw", "appletalk"} + tests := []string{"userns", "ns", "acct", "setuid", "setgid", "socket", "raw"} for _, test := range tests { out, err := exec.Command(gcc, "-g", "-Wall", "-static", fmt.Sprintf("../contrib/syscall-test/%s.c", test), "-o", fmt.Sprintf("%s/%s-test", tmp, test)).CombinedOutput() c.Assert(err, checker.IsNil, check.Commentf(string(out))) diff --git a/profiles/seccomp/default.json b/profiles/seccomp/default.json index 364505090d..0d43e60795 100755 --- a/profiles/seccomp/default.json +++ b/profiles/seccomp/default.json @@ -314,6 +314,8 @@ "signalfd", "signalfd4", "sigreturn", + "socket", + "socketcall", "socketpair", "splice", "stat", @@ -449,223 +451,6 @@ "includes": {}, "excludes": {} }, - { - "names": [ - "socket" - ], - "action": "SCMP_ACT_ALLOW", - "args": [ - { - "index": 0, - "value": 1, - "valueTwo": 0, - "op": "SCMP_CMP_EQ" - } - ], - "comment": "", - "includes": {}, - "excludes": {} - }, - { - "names": [ - "socket" - ], - "action": "SCMP_ACT_ALLOW", - "args": [ - { - "index": 0, - "value": 2, - "valueTwo": 0, - "op": "SCMP_CMP_EQ" - } - ], - "comment": "", - "includes": {}, - "excludes": {} - }, - { - "names": [ - "socket" - ], - "action": "SCMP_ACT_ALLOW", - "args": [ - { - "index": 0, - "value": 10, - "valueTwo": 0, - "op": "SCMP_CMP_EQ" - } - ], - "comment": "", - "includes": {}, - "excludes": {} - }, - { - "names": [ - "socket" - ], - "action": "SCMP_ACT_ALLOW", - "args": [ - { - "index": 0, - "value": 16, - "valueTwo": 0, - "op": "SCMP_CMP_EQ" - } - ], - "comment": "", - "includes": {}, - "excludes": {} - }, - { - "names": [ - "socket" - ], - "action": "SCMP_ACT_ALLOW", - "args": [ - { - "index": 0, - "value": 17, - "valueTwo": 0, - "op": "SCMP_CMP_EQ" - } - ], - "comment": "", - "includes": {}, - "excludes": {} - }, - { - "names": [ - "socketcall" - ], - "action": "SCMP_ACT_ALLOW", - "args": [ - { - "index": 0, - "value": 1, - "valueTwo": 0, - "op": "SCMP_CMP_GT" - } - ], - "comment": "", - "includes": {}, - "excludes": {} - }, - { - "names": [ - "socketcall" - ], - "action": "SCMP_ACT_ALLOW", - "args": [ - { - "index": 0, - "value": 1, - "valueTwo": 0, - "op": "SCMP_CMP_EQ" - }, - { - "index": 1, - "value": 1, - "valueTwo": 0, - "op": "SCMP_CMP_EQ" - } - ], - "comment": "", - "includes": {}, - "excludes": {} - }, - { - "names": [ - "socketcall" - ], - "action": "SCMP_ACT_ALLOW", - "args": [ - { - "index": 0, - "value": 1, - "valueTwo": 0, - "op": "SCMP_CMP_EQ" - }, - { - "index": 1, - "value": 2, - "valueTwo": 0, - "op": "SCMP_CMP_EQ" - } - ], - "comment": "", - "includes": {}, - "excludes": {} - }, - { - "names": [ - "socketcall" - ], - "action": "SCMP_ACT_ALLOW", - "args": [ - { - "index": 0, - "value": 1, - "valueTwo": 0, - "op": "SCMP_CMP_EQ" - }, - { - "index": 1, - "value": 10, - "valueTwo": 0, - "op": "SCMP_CMP_EQ" - } - ], - "comment": "", - "includes": {}, - "excludes": {} - }, - { - "names": [ - "socketcall" - ], - "action": "SCMP_ACT_ALLOW", - "args": [ - { - "index": 0, - "value": 1, - "valueTwo": 0, - "op": "SCMP_CMP_EQ" - }, - { - "index": 1, - "value": 16, - "valueTwo": 0, - "op": "SCMP_CMP_EQ" - } - ], - "comment": "", - "includes": {}, - "excludes": {} - }, - { - "names": [ - "socketcall" - ], - "action": "SCMP_ACT_ALLOW", - "args": [ - { - "index": 0, - "value": 1, - "valueTwo": 0, - "op": "SCMP_CMP_EQ" - }, - { - "index": 1, - "value": 17, - "valueTwo": 0, - "op": "SCMP_CMP_EQ" - } - ], - "comment": "", - "includes": {}, - "excludes": {} - }, { "names": [ "sync_file_range2" diff --git a/profiles/seccomp/seccomp_default.go b/profiles/seccomp/seccomp_default.go index 6a8dc4ed3b..8cf0df2698 100644 --- a/profiles/seccomp/seccomp_default.go +++ b/profiles/seccomp/seccomp_default.go @@ -308,6 +308,8 @@ func DefaultProfile() *types.Seccomp { "signalfd", "signalfd4", "sigreturn", + "socket", + "socketcall", "socketpair", "splice", "stat", @@ -410,153 +412,6 @@ func DefaultProfile() *types.Seccomp { }, }, }, - { - Names: []string{"socket"}, - Action: types.ActAllow, - Args: []*types.Arg{ - { - Index: 0, - Value: syscall.AF_UNIX, - Op: types.OpEqualTo, - }, - }, - }, - { - Names: []string{"socket"}, - Action: types.ActAllow, - Args: []*types.Arg{ - { - Index: 0, - Value: syscall.AF_INET, - Op: types.OpEqualTo, - }, - }, - }, - { - Names: []string{"socket"}, - Action: types.ActAllow, - Args: []*types.Arg{ - { - Index: 0, - Value: syscall.AF_INET6, - Op: types.OpEqualTo, - }, - }, - }, - { - Names: []string{"socket"}, - Action: types.ActAllow, - Args: []*types.Arg{ - { - Index: 0, - Value: syscall.AF_NETLINK, - Op: types.OpEqualTo, - }, - }, - }, - { - Names: []string{"socket"}, - Action: types.ActAllow, - Args: []*types.Arg{ - { - Index: 0, - Value: syscall.AF_PACKET, - Op: types.OpEqualTo, - }, - }, - }, - // socketcall(1, ...) is equivalent to socket(...) on some architectures eg i386 - { - Names: []string{"socketcall"}, - Action: types.ActAllow, - Args: []*types.Arg{ - { - Index: 0, - Value: 1, - Op: types.OpGreaterThan, - }, - }, - }, - { - Names: []string{"socketcall"}, - Action: types.ActAllow, - Args: []*types.Arg{ - { - Index: 0, - Value: 1, - Op: types.OpEqualTo, - }, - { - Index: 1, - Value: syscall.AF_UNIX, - Op: types.OpEqualTo, - }, - }, - }, - { - Names: []string{"socketcall"}, - Action: types.ActAllow, - Args: []*types.Arg{ - { - Index: 0, - Value: 1, - Op: types.OpEqualTo, - }, - { - Index: 1, - Value: syscall.AF_INET, - Op: types.OpEqualTo, - }, - }, - }, - { - Names: []string{"socketcall"}, - Action: types.ActAllow, - Args: []*types.Arg{ - { - Index: 0, - Value: 1, - Op: types.OpEqualTo, - }, - { - Index: 1, - Value: syscall.AF_INET6, - Op: types.OpEqualTo, - }, - }, - }, - { - Names: []string{"socketcall"}, - Action: types.ActAllow, - Args: []*types.Arg{ - { - Index: 0, - Value: 1, - Op: types.OpEqualTo, - }, - { - Index: 1, - Value: syscall.AF_NETLINK, - Op: types.OpEqualTo, - }, - }, - }, - { - Names: []string{"socketcall"}, - Action: types.ActAllow, - Args: []*types.Arg{ - { - Index: 0, - Value: 1, - Op: types.OpEqualTo, - }, - { - Index: 1, - Value: syscall.AF_PACKET, - Op: types.OpEqualTo, - }, - }, - }, { Names: []string{ "sync_file_range2",