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

If possible, apply seccomp rules immediately before exec #789

Merged
merged 1 commit into from
Apr 28, 2016

Conversation

justincormack
Copy link
Contributor

See moby/moby#22252

Previously we would apply seccomp rules before applying
capabilities, because it requires CAP_SYS_ADMIN. This
however means that a seccomp profile needs to allow
operations such as setcap() and setuid() which you
might reasonably want to disallow.

If prctl(PR_SET_NO_NEW_PRIVS) has been applied however
setting a seccomp filter is an unprivileged operation.
Therefore if this has been set, apply the seccomp
filter as late as possible, after capabilities have
been dropped and the uid set.

Note a small number of syscalls will take place
after the filter is applied, such as futex,
stat and execve, so these still need to be allowed
in addition to any the program itself needs.

Signed-off-by: Justin Cormack justin.cormack@docker.com

// Without NoNewPrivileges seccomp is a privileged operation, so we need to
// do this before dropping capabilities; otherwise do it as late as possible
// just before execve so as few syscalls take place after it as possible.
if l.config.Config.Seccomp != nil && l.config.NoNewPrivileges == false {
Copy link
Contributor

Choose a reason for hiding this comment

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

!l.config.NoNewPrivileges

See moby/moby#22252

Previously we would apply seccomp rules before applying
capabilities, because it requires CAP_SYS_ADMIN. This
however means that a seccomp profile needs to allow
operations such as setcap() and setuid() which you
might reasonably want to disallow.

If prctl(PR_SET_NO_NEW_PRIVS) has been applied however
setting a seccomp filter is an unprivileged operation.
Therefore if this has been set, apply the seccomp
filter as late as possible, after capabilities have
been dropped and the uid set.

Note a small number of syscalls will take place
after the filter is applied, such as `futex`,
`stat` and `execve`, so these still need to be allowed
in addition to any the program itself needs.

Signed-off-by: Justin Cormack <justin.cormack@docker.com>
@mrunalp
Copy link
Contributor

mrunalp commented Apr 27, 2016

LGTM

1 similar comment
@crosbymichael
Copy link
Member

LGTM

@crosbymichael crosbymichael merged commit 7d23639 into opencontainers:master Apr 28, 2016
justincormack added a commit to justincormack/docker that referenced this pull request Jul 13, 2016
The change to runc in opencontainers/runc#789
was not documented previously. Also say what this affects and clean
up layout of initial table as there was some miscolouration of the
continuation lines.

Signed-off-by: Justin Cormack <justin.cormack@docker.com>
tiborvass pushed a commit to tiborvass/docker that referenced this pull request Jul 26, 2016
The change to runc in opencontainers/runc#789
was not documented previously. Also say what this affects and clean
up layout of initial table as there was some miscolouration of the
continuation lines.

Signed-off-by: Justin Cormack <justin.cormack@docker.com>
(cherry picked from commit 3050d9a)
Signed-off-by: Tibor Vass <tibor@docker.com>
rchicoli pushed a commit to rchicoli/docker that referenced this pull request Nov 12, 2016
The change to runc in opencontainers/runc#789
was not documented previously. Also say what this affects and clean
up layout of initial table as there was some miscolouration of the
continuation lines.

Signed-off-by: Justin Cormack <justin.cormack@docker.com>
(cherry picked from commit 3050d9a)
Signed-off-by: Tibor Vass <tibor@docker.com>
andrewhsu pushed a commit to docker-archive/docker-ce that referenced this pull request Jun 5, 2017
The change to runc in opencontainers/runc#789
was not documented previously. Also say what this affects and clean
up layout of initial table as there was some miscolouration of the
continuation lines.

Signed-off-by: Justin Cormack <justin.cormack@docker.com>
Upstream-commit: 8bc84934fbb2974f9bf79afc503b5e07eb2b07d2
Component: cli
stefanberger pushed a commit to stefanberger/runc that referenced this pull request Sep 8, 2017
config: Shift oomScoreAdj from linux.resources to process
stefanberger pushed a commit to stefanberger/runc that referenced this pull request Sep 8, 2017
This should have happened in 4b49c64 (config: Shift oomScoreAdj from
linux.resources to process, 2017-05-09, opencontainers#789) as part of moving the
property from a Linux-specific type to a cross-platform type.

Signed-off-by: W. Trevor King <wking@tremily.us>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants