Merge pull request #42648 from thaJeztah/seccomp_closer_to_oci

seccomp.Seccomp: embed oci-spec LinuxSeccomp, add support for seccomp flags

integration-cli/docker_cli_run_unix_test.go

@@ -1473,7 +1473,7 @@ func (s *DockerDaemonSuite) TestRunSeccompJSONNoNameAndNames(c *testing.T) {
 
 	out, err := s.d.Cmd("run", "--security-opt", "seccomp="+tmpFile.Name(), "busybox", "chmod", "777", ".")
 	assert.ErrorContains(c, err, "")
-	assert.Assert(c, strings.Contains(out, "'name' and 'names' were specified in the seccomp profile, use either 'name' or 'names'"))
+	assert.Assert(c, strings.Contains(out, "use either 'name' or 'names'"))
 }
 
 func (s *DockerDaemonSuite) TestRunSeccompJSONNoArchAndArchMap(c *testing.T) {
@@ -1510,7 +1510,7 @@ func (s *DockerDaemonSuite) TestRunSeccompJSONNoArchAndArchMap(c *testing.T) {
 
 	out, err := s.d.Cmd("run", "--security-opt", "seccomp="+tmpFile.Name(), "busybox", "chmod", "777", ".")
 	assert.ErrorContains(c, err, "")
-	assert.Assert(c, strings.Contains(out, "'architectures' and 'archMap' were specified in the seccomp profile, use either 'architectures' or 'archMap'"))
+	assert.Assert(c, strings.Contains(out, "use either 'architectures' or 'archMap'"))
 }
 
 func (s *DockerDaemonSuite) TestRunWithDaemonDefaultSeccompProfile(c *testing.T) {

profiles/seccomp/default_linux.go

@@ -740,8 +740,10 @@ func DefaultProfile() *Seccomp {
 	}
 
 	return &Seccomp{
-		DefaultAction: specs.ActErrno,
-		ArchMap:       arches(),
-		Syscalls:      syscalls,
+		LinuxSeccomp: specs.LinuxSeccomp{
+			DefaultAction: specs.ActErrno,
+		},
+		ArchMap:  arches(),
+		Syscalls: syscalls,
 	}
 }

profiles/seccomp/seccomp.go

@@ -10,17 +10,24 @@ import (
 )
 
 // Seccomp represents the config for a seccomp profile for syscall restriction.
+// It is used to marshal/unmarshal the JSON profiles as accepted by docker, and
+// extends the runtime-spec's specs.LinuxSeccomp, overriding some fields to
+// provide the ability to define conditional rules based on the host's kernel
+// version, architecture, and the container's capabilities.
 type Seccomp struct {
-	DefaultAction    specs.LinuxSeccompAction `json:"defaultAction"`
-	DefaultErrnoRet  *uint                    `json:"defaultErrnoRet,omitempty"`
-	ListenerPath     string                   `json:"listenerPath,omitempty"`
-	ListenerMetadata string                   `json:"listenerMetadata,omitempty"`
+	specs.LinuxSeccomp
 
-	// Architectures is kept to maintain backward compatibility with the old
-	// seccomp profile.
-	Architectures []specs.Arch   `json:"architectures,omitempty"`
-	ArchMap       []Architecture `json:"archMap,omitempty"`
-	Syscalls      []*Syscall     `json:"syscalls"`
+	// ArchMap contains a list of Architectures and Sub-architectures for the
+	// profile. When generating the profile, this list is expanded to a
+	// []specs.Arch, to propagate the Architectures field of the profile.
+	ArchMap []Architecture `json:"archMap,omitempty"`
+
+	// Syscalls contains lists of syscall rules. Rules can define conditions
+	// for them to be included or excluded in the resulting profile (based on
+	// on kernel version, architecture, capabilities, etc.). These lists are
+	// expanded to an specs.Syscall  When generating the profile, these lists
+	// are expanded to a []specs.LinuxSyscall.
+	Syscalls []*Syscall `json:"syscalls"`
 }
 
 // Architecture is used to represent a specific architecture

profiles/seccomp/seccomp_linux.go

@@ -82,21 +82,23 @@ func setupSeccomp(config *Seccomp, rs *specs.Spec) (*specs.LinuxSeccomp, error)
 		return nil, nil
 	}
 
-	newConfig := &specs.LinuxSeccomp{}
-
 	if len(config.Architectures) != 0 && len(config.ArchMap) != 0 {
-		return nil, errors.New("'architectures' and 'archMap' were specified in the seccomp profile, use either 'architectures' or 'archMap'")
+		return nil, errors.New("both 'architectures' and 'archMap' are specified in the seccomp profile, use either 'architectures' or 'archMap'")
 	}
 
-	// if config.Architectures == 0 then libseccomp will figure out the architecture to use
-	if len(config.Architectures) != 0 {
-		newConfig.Architectures = config.Architectures
+	if len(config.LinuxSeccomp.Syscalls) != 0 {
+		// The Seccomp type overrides the LinuxSeccomp.Syscalls field,
+		// so 'this should never happen' when loaded from JSON, but could
+		// happen if someone constructs the Config from source.
+		return nil, errors.New("the LinuxSeccomp.Syscalls field should be empty")
 	}
 
-	arch := goToNative[runtime.GOARCH]
-	seccompArch, archExists := nativeToSeccomp[arch]
-
-	if len(config.ArchMap) != 0 && archExists {
+	var (
+		// Copy all common / standard properties to the output profile
+		newConfig = &config.LinuxSeccomp
+		arch      = goToNative[runtime.GOARCH]
+	)
+	if seccompArch, ok := nativeToSeccomp[arch]; ok {
 		for _, a := range config.ArchMap {
 			if a.Arch == seccompArch {
 				newConfig.Architectures = append(newConfig.Architectures, a.Arch)
@@ -106,14 +108,15 @@ func setupSeccomp(config *Seccomp, rs *specs.Spec) (*specs.LinuxSeccomp, error)
 		}
 	}
 
-	newConfig.DefaultAction = config.DefaultAction
-	newConfig.DefaultErrnoRet = config.DefaultErrnoRet
-	newConfig.ListenerPath = config.ListenerPath
-	newConfig.ListenerMetadata = config.ListenerMetadata
-
 Loop:
-	// Loop through all syscall blocks and convert them to libcontainer format after filtering them
+	// Convert Syscall to OCI runtimes-spec specs.LinuxSyscall after filtering them.
 	for _, call := range config.Syscalls {
+		if call.Name != "" {
+			if len(call.Names) != 0 {
+				return nil, errors.New("both 'name' and 'names' are specified in the seccomp profile, use either 'name' or 'names'")
+			}
+			call.Names = []string{call.Name}
+		}
 		if call.Excludes != nil {
 			if len(call.Excludes.Arches) > 0 {
 				if inSlice(call.Excludes.Arches, arch) {
@@ -156,14 +159,6 @@ Loop:
 				}
 			}
 		}
-
-		if call.Name != "" {
-			if len(call.Names) != 0 {
-				return nil, errors.New("'name' and 'names' were specified in the seccomp profile, use either 'name' or 'names'")
-			}
-			call.Names = append(call.Names, call.Name)
-		}
-
 		newConfig.Syscalls = append(newConfig.Syscalls, call.LinuxSyscall)
 	}
 

profiles/seccomp/seccomp_test.go

@@ -24,32 +24,32 @@ func TestLoadProfile(t *testing.T) {
 	}
 	var expectedErrno uint = 12345
 	expected := specs.LinuxSeccomp{
-		DefaultAction: "SCMP_ACT_ERRNO",
+		DefaultAction: specs.ActErrno,
 		Syscalls: []specs.LinuxSyscall{
 			{
 				Names:  []string{"clone"},
-				Action: "SCMP_ACT_ALLOW",
+				Action: specs.ActAllow,
 				Args: []specs.LinuxSeccompArg{{
 					Index:    0,
 					Value:    2114060288,
 					ValueTwo: 0,
-					Op:       "SCMP_CMP_MASKED_EQ",
+					Op:       specs.OpMaskedEqual,
 				}},
 			},
 			{
 
 				Names:  []string{"open"},
-				Action: "SCMP_ACT_ALLOW",
+				Action: specs.ActAllow,
 				Args:   []specs.LinuxSeccompArg{},
 			},
 			{
 				Names:  []string{"close"},
-				Action: "SCMP_ACT_ALLOW",
+				Action: specs.ActAllow,
 				Args:   []specs.LinuxSeccompArg{},
 			},
 			{
 				Names:    []string{"syslog"},
-				Action:   "SCMP_ACT_ERRNO",
+				Action:   specs.ActErrno,
 				ErrnoRet: &expectedErrno,
 				Args:     []specs.LinuxSeccompArg{},
 			},
@@ -72,7 +72,7 @@ func TestLoadProfileWithDefaultErrnoRet(t *testing.T) {
 
 	expectedErrnoRet := uint(6)
 	expected := specs.LinuxSeccomp{
-		DefaultAction:   "SCMP_ACT_ERRNO",
+		DefaultAction:   specs.ActErrno,
 		DefaultErrnoRet: &expectedErrnoRet,
 	}
 
@@ -92,7 +92,7 @@ func TestLoadProfileWithListenerPath(t *testing.T) {
 	}
 
 	expected := specs.LinuxSeccomp{
-		DefaultAction:    "SCMP_ACT_ERRNO",
+		DefaultAction:    specs.ActErrno,
 		ListenerPath:     "/var/run/seccompaget.sock",
 		ListenerMetadata: "opaque-metadata",
 	}
@@ -100,6 +100,46 @@ func TestLoadProfileWithListenerPath(t *testing.T) {
 	assert.DeepEqual(t, expected, *p)
 }
 
+func TestLoadProfileWithFlag(t *testing.T) {
+	profile := `{"defaultAction": "SCMP_ACT_ERRNO", "flags": ["SECCOMP_FILTER_FLAG_SPEC_ALLOW", "SECCOMP_FILTER_FLAG_LOG"]}`
+	expected := specs.LinuxSeccomp{
+		DefaultAction: specs.ActErrno,
+		Flags:         []specs.LinuxSeccompFlag{"SECCOMP_FILTER_FLAG_SPEC_ALLOW", "SECCOMP_FILTER_FLAG_LOG"},
+	}
+	rs := createSpec()
+	p, err := LoadProfile(profile, &rs)
+	assert.NilError(t, err)
+	assert.DeepEqual(t, expected, *p)
+}
+
+// TestLoadProfileValidation tests that invalid profiles produce the correct error.
+func TestLoadProfileValidation(t *testing.T) {
+	tests := []struct {
+		doc      string
+		profile  string
+		expected string
+	}{
+		{
+			doc:      "conflicting architectures and archMap",
+			profile:  `{"defaultAction": "SCMP_ACT_ERRNO", "architectures": ["A", "B", "C"], "archMap": [{"architecture": "A", "subArchitectures": ["B", "C"]}]}`,
+			expected: `use either 'architectures' or 'archMap'`,
+		},
+		{
+			doc:      "conflicting syscall.name and syscall.names",
+			profile:  `{"defaultAction": "SCMP_ACT_ERRNO", "syscalls": [{"name": "accept", "names": ["accept"], "action": "SCMP_ACT_ALLOW"}]}`,
+			expected: `use either 'name' or 'names'`,
+		},
+	}
+	for _, tc := range tests {
+		tc := tc
+		rs := createSpec()
+		t.Run(tc.doc, func(t *testing.T) {
+			_, err := LoadProfile(tc.profile, &rs)
+			assert.ErrorContains(t, err, tc.expected)
+		})
+	}
+}
+
 // TestLoadLegacyProfile tests loading a seccomp profile in the old format
 // (before https://github.com/docker/docker/pull/24510)
 func TestLoadLegacyProfile(t *testing.T) {
@@ -108,9 +148,17 @@ func TestLoadLegacyProfile(t *testing.T) {
 		t.Fatal(err)
 	}
 	rs := createSpec()
-	if _, err := LoadProfile(string(f), &rs); err != nil {
-		t.Fatal(err)
+	p, err := LoadProfile(string(f), &rs)
+	assert.NilError(t, err)
+	assert.Equal(t, p.DefaultAction, specs.ActErrno)
+	assert.DeepEqual(t, p.Architectures, []specs.Arch{"SCMP_ARCH_X86_64", "SCMP_ARCH_X86", "SCMP_ARCH_X32"})
+	assert.Equal(t, len(p.Syscalls), 311)
+	expected := specs.LinuxSyscall{
+		Names:  []string{"accept"},
+		Action: specs.ActAllow,
+		Args:   []specs.LinuxSeccompArg{},
 	}
+	assert.DeepEqual(t, p.Syscalls[0], expected)
 }
 
 func TestLoadDefaultProfile(t *testing.T) {