From 42984e8d3cbe741cb9c70bc240d2e915c38ecceb Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Wed, 12 Apr 2017 10:03:10 -0700 Subject: [PATCH] config-linux: Make linux.seccomp.syscalls OPTIONAL 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]: https://github.com/opencontainers/runtime-spec/pull/768#issuecomment-297156102 Signed-off-by: W. Trevor King --- config-linux.md | 5 ++++- schema/config-linux.json | 5 ++++- specs-go/config.go | 2 +- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/config-linux.md b/config-linux.md index 873982fc2..0813fbb30 100644 --- a/config-linux.md +++ b/config-linux.md @@ -610,7 +610,10 @@ 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. + + While this property is OPTIONAL, some values of `defaultAction` are not useful without `syscalls` entries. + For example, 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 index d51e5b5dd..c12101d00 100644 --- 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 index 70d708d23..ded283428 100644 --- 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