config-linux: Make linux.seccomp.syscalls OPTIONAL

wking · wking · commit bb4519c25845 · 2017-04-25T13:40:32.000-07:00
Before this commit, linux.seccomp.sycalls was required, but we didn't require an entry in the array. That means '"syscalls": []' would be technically valid, and I'm pretty sure that's not what we want. If it makes sense to have a seccomp property that does not need syscalls entries, then syscalls should be optional (which is what this commit is doing). If it does not makes sense to have an empty/unset syscalls then it should be required and have a minimum length of one. Before 652323c (improve seccomp format to be more expressive, 2017-01-13, #657), syscalls was omitempty (and therefore more optional-feeling, although there was no real Markdown spec for seccomp before 3ca5c6c, config-linux.md: fix seccomp, 2017-03-02, #706, so it's hard to know). This commit has gone with OPTIONAL, because a seccomp config which only sets defaultAction seems potentially valid. The SCMP_ACT_KILL example is prompted by: On Tue, Apr 25, 2017 at 01:32:26PM -0700, David Lyle wrote [1]: > Technically, OPTIONAL is the right value, but unless you specify the > default action for seccomp to be SCMP_ACT_ALLOW the result will be > an error at run time. > > I would suggest an additional clarification to this fact in > config-linux.md would be very helpful if marking syscall as > OPTIONAL. I've phrased the example more conservatively, because I'm not sure that SCMP_ACT_ALLOW is the only possible value to avoid an error. For example, perhaps a SCMP_ACT_TRACE default with an empty syscalls array would not die on the first syscall. The point of the example is to remind config authors that without a useful syscalls array, the default value is very important ;). Also add the previously-missing 'required' property to the seccomp JSON Schema entry. [1]: #768 (comment) Signed-off-by: W. Trevor King <wking@tremily.us>
diff --git a/config-linux.md b/config-linux.md
@@ -610,7 +610,9 @@ The following parameters can be specified to setup seccomp:
     * `SCMP_ARCH_PARISC`
     * `SCMP_ARCH_PARISC64`
 
-* **`syscalls`** *(array of objects, REQUIRED)* - match a syscall in seccomp.
+* **`syscalls`** *(array of objects, OPTIONAL)* - match a syscall in seccomp.
+
+    Note that if `defaultAction` is `SCMP_ACT_KILL` and `syscalls` is empty or unset, the kernel will kill the container process on its first syscall.
 
     Each entry has the following structure:
 
diff --git a/schema/config-linux.json b/schema/config-linux.json
@@ -251,7 +251,10 @@
                             "$ref": "defs-linux.json#/definitions/Syscall"
                         }
                     }
-                }
+                },
+                "required": [
+                    "defaultAction"
+                ]
             },
             "sysctl": {
                 "id": "https://opencontainers.org/schema/bundle/linux/sysctl",
diff --git a/specs-go/config.go b/specs-go/config.go
@@ -484,7 +484,7 @@ type WindowsNetworkResources struct {
 type LinuxSeccomp struct {
 	DefaultAction LinuxSeccompAction `json:"defaultAction"`
 	Architectures []Arch             `json:"architectures,omitempty"`
-	Syscalls      []LinuxSyscall     `json:"syscalls"`
+	Syscalls      []LinuxSyscall     `json:"syscalls,omitempty"`
 }
 
 // Arch used for additional architectures

Original file line number	Diff line number	Diff line change
`@@ -251,7 +251,10 @@`
`251`	`251`	`"$ref": "defs-linux.json#/definitions/Syscall"`
`252`	`252`	`}`
`253`	`253`	`}`
`254`		`- }`
	`254`	`+ },`
	`255`	`+ "required": [`
	`256`	`+ "defaultAction"`
	`257`	`+ ]`
`255`	`258`	`},`
`256`	`259`	`"sysctl": {`
`257`	`260`	`"id": "https://opencontainers.org/schema/bundle/linux/sysctl",`
Original file line number	Diff line number	Diff line change
`@@ -484,7 +484,7 @@ type WindowsNetworkResources struct {`
`484`	`484`	`type LinuxSeccomp struct {`
`485`	`485`	DefaultAction LinuxSeccompAction `json:"defaultAction"`
`486`	`486`	Architectures []Arch `json:"architectures,omitempty"`
`487`		- Syscalls []LinuxSyscall `json:"syscalls"`
	`487`	+ Syscalls []LinuxSyscall `json:"syscalls,omitempty"`
`488`	`488`	`}`
`489`	`489`
`490`	`490`	`// Arch used for additional architectures`