Skip to content
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 ambient and bounding capability support #675

Merged
merged 1 commit into from
Feb 22, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 55 additions & 11 deletions config.md
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,13 @@ For Windows, see links for details about [mountvol](http://ss64.com/nt/mountvol.
* **`env`** (array of strings, OPTIONAL) with the same semantics as [IEEE Std 1003.1-2001's `environ`][ieee-1003.1-2001-xbd-c8.1].
* **`args`** (array of strings, REQUIRED) with similar semantics to [IEEE Std 1003.1-2001 `execvp`'s *argv*][ieee-1003.1-2001-xsh-exec].
This specification extends the IEEE standard in that at least one entry is REQUIRED, and that entry is used with the same semantics as `execvp`'s *file*.
* **`capabilities`** (array of strings, OPTIONAL) is an array that specifies the set of capabilities of the process(es) inside the container. Valid values are platform-specific. For example, valid values for Linux are defined in the [CAPABILITIES(7)](http://man7.org/linux/man-pages/man7/capabilities.7.html) man page.
* **`capabilities`** (object, OPTIONAL) is an object containing arrays that specifies the sets of capabilities for the process(es) inside the container. Valid values are platform-specific. For example, valid values for Linux are defined in the [CAPABILITIES(7)](http://man7.org/linux/man-pages/man7/capabilities.7.html) man page.
capabilities contains the following properties:
* **`effective`** (array of strings, OPTIONAL) - the `effective` field is an array of effective capabilities that are kept for the process.
* **`bounding`** (array of strings, OPTIONAL) - the `bounding` field is an array of bounding capabilities that are kept for the process.
* **`inheritable`** (array of strings, OPTIONAL) - the `inheritable` field is an array of inheritable capabilities that are kept for the process.
* **`permitted`** (array of strings, OPTIONAL) - the `permitted` field is an array of permitted capabilities that are kept for the process.
* **`ambient`** (array of strings, OPTIONAL) - the `ambient` field is an array of ambient capabilities that are kept for the process.
* **`rlimits`** (array of objects, OPTIONAL) allows setting resource limits for a process inside the container.
Each entry has the following structure:

Expand Down Expand Up @@ -191,11 +197,30 @@ _Note: symbolic name for uid and gid, such as uname and gname respectively, are
"apparmorProfile": "acme_secure_profile",
"selinuxLabel": "system_u:system_r:svirt_lxc_net_t:s0:c124,c675",
"noNewPrivileges": true,
"capabilities": [
"CAP_AUDIT_WRITE",
"CAP_KILL",
"CAP_NET_BIND_SERVICE"
],
"capabilities": {
"bounding": [
"CAP_AUDIT_WRITE",
"CAP_KILL",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the example should have CAP_NET_BIND_SERVICE in the bounding set or it cannot be applied as an ambient capability.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CAP_NET_BIND_SERVICE also needs to be added to permitted/inherited.
From capabilities(7)

The ambient capability set obeys the invariant that no capability can ever be ambient if it is not both permitted and inheritable.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mrunalp does that mean we should or should not have it in bounding?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@crosbymichael We should have it in bounding, permitted and inheritable.

"CAP_NET_BIND_SERVICE"
],
"permitted": [
"CAP_AUDIT_WRITE",
"CAP_KILL",
"CAP_NET_BIND_SERVICE"
],
"inheritable": [
"CAP_AUDIT_WRITE",
"CAP_KILL",
"CAP_NET_BIND_SERVICE"
],
"effective": [
"CAP_AUDIT_WRITE",
"CAP_KILL",
],
"ambient": [
"CAP_NET_BIND_SERVICE"
]
},
"rlimits": [
{
"type": "RLIMIT_NOFILE",
Expand Down Expand Up @@ -446,11 +471,30 @@ Here is a full example `config.json` for reference.
"TERM=xterm"
],
"cwd": "/",
"capabilities": [
"CAP_AUDIT_WRITE",
"CAP_KILL",
"CAP_NET_BIND_SERVICE"
],
"capabilities": {
"bounding": [
"CAP_AUDIT_WRITE",
"CAP_KILL",
"CAP_NET_BIND_SERVICE"
],
"permitted": [
"CAP_AUDIT_WRITE",
"CAP_KILL",
"CAP_NET_BIND_SERVICE"
],
"inheritable": [
"CAP_AUDIT_WRITE",
"CAP_KILL",
"CAP_NET_BIND_SERVICE"
],
"effective": [
"CAP_AUDIT_WRITE",
"CAP_KILL",
],
"ambient": [
"CAP_NET_BIND_SERVICE"
]
},
"rlimits": [
{
"type": "RLIMIT_CORE",
Expand Down
40 changes: 37 additions & 3 deletions schema/config-schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,43 @@
},
"capabilities": {
"id": "https://opencontainers.org/schema/bundle/process/linux/capabilities",
"type": "array",
"items": {
"$ref": "defs-linux.json#/definitions/Capability"
"type": "object",
"properties": {
"bounding": {
"id": "https://opencontainers.org/schema/bundle/process/linux/capabilities/bounding",
"type": "array",
"items": {
"$ref": "defs-linux.json#/definitions/Capability"
}
},
"permitted": {
"id": "https://opencontainers.org/schema/bundle/process/linux/capabilities/permitted",
"type": "array",
"items": {
"$ref": "defs-linux.json#/definitions/Capability"
}
},
"effective": {
"id": "https://opencontainers.org/schema/bundle/process/linux/capabilities/effective",
"type": "array",
"items": {
"$ref": "defs-linux.json#/definitions/Capability"
}
},
"inheritable": {
"id": "https://opencontainers.org/schema/bundle/process/linux/capabilities/inheritable",
"type": "array",
"items": {
"$ref": "defs-linux.json#/definitions/Capability"
}
},
"ambient": {
"id": "https://opencontainers.org/schema/bundle/process/linux/capabilities/ambient",
"type": "array",
"items": {
"$ref": "defs-linux.json#/definitions/Capability"
}
}
}
},
"apparmorProfile": {
Expand Down
2 changes: 1 addition & 1 deletion schema/defs-linux.json
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
}
},
"Capability": {
"description": "Linux process permissions",
"description": "Linux process capabilities",
"type": "string",
"pattern": "^CAP_([A-Z]|_)+$"
},
Expand Down
19 changes: 17 additions & 2 deletions specs-go/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ type Process struct {
// Cwd is the current working directory for the process and must be
// relative to the container's root.
Cwd string `json:"cwd"`
// Capabilities are Linux capabilities that are kept for the container.
Capabilities []string `json:"capabilities,omitempty" platform:"linux"`
// Capabilities are Linux capabilities that are kept for the process.
Capabilities *LinuxCapabilities `json:"capabilities,omitempty" platform:"linux"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jlbutler just removed the Linux-specific-ness of process.capabilities in #673 (at least in the Markdown spec). With this change, it becomes even less clear to me how other OSes are supposed to configure their capabilities (although I wasn't clear on that since #673, so it's not a lot worse ;).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No other systems have capabilities; there was a withdrawn Posix draft that the Linux implementation was based on, and then customised for Linux. They should have remained Linux specific.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No other systems have capabilities…

capabilies(7) references this for the withdrawn POSIX.1e draft, and that has:

Why Posix.1e was abandoned is difficult to understand from today's (July 2014) point of view. Solaris, Irix, Linux, and probably other Unices seemed to recognize the standard. On the other hand the FreeBSD project found counter arguments and didn't integrate capabilities ('fine grained privileges') by default.

I don't have access to Solaris or Irix for testing, or enough interest in them to find relevant docs, but I'd caution against making this Linux-specific (again) without consulting someone with Solaris access. On the other hand, maybe the policy is “unless you document your valid values, we may drop support for this property on your system”, and I'd be +1 for that, giving the Solaris folks some reasonable period to provide the valid-values docs, and then dropping Solaris capability support if they don't.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But here we are adding the Linux specific types such as ambient. Solaris capabilities are a bit similar by the look of it (they have E, I, P but different capabilities) but that doesn't mean they should be at the top level.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solaris capabilities are a bit similar by the look of it (they have E, I, P but different capabilities) but that doesn't mean they should be at the top level.

Is that “I don't think there is enough overlap to be worth sharing the same namespace”? If the Solaris folks are ok using bounding instead of their limited, we could just make ambient Linux-specific. In other situations where only the values are different (e.g. mounts entries), the maintainer preference seems to be “share the same property, but define the different per-platform values”.

On the other hand, if Windows has nothing equivalent, the pro-namespacing policy here suggests namespacing the whole capabilities structure to something POSIX-ish-specific. I tried to raise the “how will this be handled on Windows?” issue in #673, but am still not clear on if/whether Windows supports something capability-like.

// Rlimits specifies rlimit options to apply to the process.
Rlimits []LinuxRlimit `json:"rlimits,omitempty" platform:"linux"`
// NoNewPrivileges controls whether additional privileges could be gained by processes in the container.
Expand All @@ -56,6 +56,21 @@ type Process struct {
SelinuxLabel string `json:"selinuxLabel,omitempty" platform:"linux"`
}

// LinuxCapabilities specifies the whitelist of capabilities that are kept for a process.
// http://man7.org/linux/man-pages/man7/capabilities.7.html
type LinuxCapabilities struct {
// Bounding is the set of capabilities checked by the kernel.
Bounding []string `json:"bounding,omitempty" platform:"linux"`
// Effective is the set of capabilities checked by the kernel.
Effective []string `json:"effective,omitempty" platform:"linux"`
// Inheritable is the capabilities preserved across execve.
Inheritable []string `json:"inheritable,omitempty" platform:"linux"`
// Permitted is the limiting superset for effective capabilities.
Permitted []string `json:"permitted,omitempty" platform:"linux"`
// Ambient is the ambient set of capabilities that are kept.
Ambient []string `json:"ambient,omitempty" platform:"linux"`
}

// Box specifies dimensions of a rectangle. Used for specifying the size of a console.
type Box struct {
// Height is the vertical dimension of a box.
Expand Down