-
Notifications
You must be signed in to change notification settings - Fork 553
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Architecture field to Seccomp configuration in Linux runtime #200
Conversation
By default, Seccomp filters will only permit syscalls to be made using the native architecture of the kernel. This is fine for most use cases, but breaks others (such as running 32-bit code in a container on a host with a 64-bit kernel). This patch adds a field to specify additional architectures which may make syscalls. Signed-off-by: Matthew Heon <mheon@redhat.com>
Ping @crosbymichael |
Syscalls []*Syscall `json:"syscalls"` | ||
} | ||
|
||
// Additional architectures permitted to be used for system calls | ||
// By default only the native architecture of the kernel is permitted | ||
type Arch string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a string? ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, there are 3 other types here which are just string... Didn't want this one to stand out :-)
Wouldn't mind changing all of them at once, though that might be worthy of a separate PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only other one that i see is Operator. Namespaces is a type because it has constants defined for it so its ok that it stays a type. Maybe if we define operators for type Operator string
it would be better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was also thinking of Action, which is another just-a-string.
I agree that adding the relevant constants to the config is a good idea - I'll see about doing that
Signed-off-by: Matthew Heon <mheon@redhat.com>
Signed-off-by: Matthew Heon <mheon@redhat.com>
@crosbymichael Added the relevant constants into the runtime config. I think they're valuable to have in the Go version of the spec, probably less so in the markdown description - considering converting that to a table so I can show descriptions |
LGTM |
@@ -319,11 +319,44 @@ For more information about Apparmor, see [Apparmor documentation](https://wiki.u | |||
Seccomp provides application sandboxing mechanism in the Linux kernel. | |||
Seccomp configuration allows one to configure actions to take for matched syscalls and furthermore also allows matching on values passed as arguments to syscalls. | |||
For more information about Seccomp, see [Seccomp kernel documentation](https://www.kernel.org/doc/Documentation/prctl/seccomp_filter.txt) | |||
The actions and operators are strings that match the definitions in seccomp.h from [libseccomp](/~https://github.com/seccomp/libseccomp) and are translated to corresponding values. | |||
The actions, architectures, and operators are strings that match the definitions in seccomp.h from [libseccomp](/~https://github.com/seccomp/libseccomp) and are translated to corresponding values. | |||
A valid list of constants as of Libseccomp v2.2.3 is contained below. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have a clear picture of how this sort of thing is going to get versioned. If I create a bundle against the v0.2.0 spec (assuming this PR lands in that version), am I always going to be allowed to use the architectures listed below? I.e. are runtimes compiled against earlier versions of libseccomp (that don't support all of these) nonconformant with v0.2.0? Will future versions of libseccomp always support these strings? I.e. are runtimes compiled against later versions of libseccomp (e.g. that no longer support SCMP_ARCH_X86
) nonconformant with v0.2.0? Those seem like difficult requirements to enforce without binding the spec to a particular (range) of libseccomp versions, but without them I'm not sure how the bundle author should know the valid strings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mheon maybe we should not use the actual values that libseccomp expects because it would be hard to version and have spec stability if some low level detail like what strings libseccomp expects leaks out via the spec.
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Wed, Sep 23, 2015 at 01:34:39PM -0700, Michael Crosby wrote:
@mheon maybe we should not use the actual values that libseccomp
expects because it would be hard to version and have spec stability
if some low level detail like what strings libseccomp expects leaks
out via the spec.
The bundle author will need to know what to use, regardless of whether
we list these in the spec ;). So one way is to code the allowed
values into the spec, so it's a contract between the spec and bundle
author. Another way is to allow contract directly between the
runtime's libseccomp and the bundle author (so the spec just provides
a way for the author to specify the libseccomp version they're writing
to). Maybe this is a use-case for my proposed version structure 1?
The bundle author would use something like:
"version": {
"openContainerSpecification": "0.2.0",
"libseccomp": "2.2.0",
}
And then be compatible with runtimes that support spec versions <0.3.0
which are compiled against libseccomp versions <3.0.0 (assuming
libseccomp semantically versions their header).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wking No, the goal is to abstract away some of these lower level details and dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Wed, Sep 23, 2015 at 02:00:38PM -0700, Michael Crosby wrote:
@wking No, the goal is to abstract away some of these lower level
details and dependencies.
If the goal is to make this spec the full bundle-author contract, then
I think you need an explicit list of allowed constants (although that
might be a link to libseccomp's v2.2.3 header. So long as there's no
ambiguity about whether a given constant is valid for this version of
the OCI spec or not). Then conformant runtimes will have to support
all of those values (although they could optionally support more).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Wed, Sep 23, 2015 at 02:13:29PM -0700, W. Trevor King wrote:
Wed, Sep 23, 2015 at 02:00:38PM -0700, Michael Crosby:
@wking No, the goal is to abstract away some of these lower level
details and dependencies.If the goal is to make this spec the full bundle-author contract, then
I think you need an explicit list of allowed constants…
This PR landed, so I've spun off my suggestion for more explicit (and
less libseccomp-dependent) phrasing into #201.
LGTM |
Add Architecture field to Seccomp configuration in Linux runtime
By default, Seccomp filters will only permit syscalls to be made using the
native architecture of the kernel. This is fine for most use cases, but breaks
others (such as running 32-bit code in a container on a host with a 64-bit
kernel). This PR adds a field to specify additional architectures which may
make syscalls in a container secured by Seccomp.
As with other the other aspects of Seccomp, Libseccomp's seccomp.h is used to provide string constants which can be used to represent these additional architectures.