Skip to content

Commit

Permalink
config-linux: Make linux.seccomp.syscalls OPTIONAL
Browse files Browse the repository at this point in the history
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, opencontainers#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, opencontainers#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]: opencontainers#768 (comment)

Signed-off-by: W. Trevor King <wking@tremily.us>
  • Loading branch information
wking committed Apr 25, 2017
1 parent cfc95a5 commit bb4519c
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 3 deletions.
4 changes: 3 additions & 1 deletion config-linux.md
Expand Up @@ -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:

Expand Down
5 changes: 4 additions & 1 deletion schema/config-linux.json
Expand Up @@ -251,7 +251,10 @@
"$ref": "defs-linux.json#/definitions/Syscall"
}
}
}
},
"required": [
"defaultAction"
]
},
"sysctl": {
"id": "https://opencontainers.org/schema/bundle/linux/sysctl",
Expand Down
2 changes: 1 addition & 1 deletion specs-go/config.go
Expand Up @@ -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
Expand Down

0 comments on commit bb4519c

Please sign in to comment.