daemon: add a flag to override the default seccomp profile #26276

runcom · 2016-09-02T13:23:04Z

Supersedes #24221

Introduce /etc/docker/seccomp.json as per discussion in #24221

/cc @justincormack @cpuguy83 @LK4D4 @jfrazelle @cyphar @rhatdan

Signed-off-by: Antonio Murdaca runcom@redhat.com

justincormack · 2016-09-02T13:43:27Z

@runcom accidentally typing systemd in the issue title!

I will summarise the discussion in the previous issue.

runcom · 2016-09-02T13:44:41Z

@runcom accidentally typing systemd in the issue title!

I swear it was a mistake! 😄

runcom · 2016-09-02T13:58:14Z

/cc @rhatdan @jeremyeder

runcom · 2016-09-02T14:04:10Z

daemon/daemon_unix.go

+func (daemon *Daemon) setupSeccompProfile() error {
+	b, err := ioutil.ReadFile(seccompProfilePath)
+	if err != nil {
+		// TODO(runcom): generate a default seccomp profile if system wide isn't present?


@justincormack wdyt? Will be okay to bail out if no seccomp json in there?

If we are going to have an internal one, then we should check if the path is "" and use the default. If the admin specified seccomppath and the path does not exist, we should error out.

I'm not sure we're going to override the default with a flag in the daemon given people/admin/distros can already override the default at /etc/docker/seccomp.json

I believe this was the intention of @justincormack as well.

Currently I don't think we provide ways to override a lot of the file locations for things. I would think that if we provided a way to override the /etc/docker location in future, this would then look in that directory, but I don't think it needs to be explicitly overrideable other than that.

Alright, what about my question in the comment in the code instead? @justincormack

This is also causing CI to fail because on integration tests start the daemon doesn't come up (not /etc/docker/seccomp.json)

I don't think we should generate one either, as that would mean embedding the json in the binary which seems gross.

Does the CI use make install? In which case can fix that.

gotcha, I'll fix it by installing a profile under /etc/docker/seccomp.json

it doesn't seem to be using make install, I hacked up something to install the profile in /etc/docker, maybe @tianon can guide me :)

we can probably copy the default.json in bundles/VERSION/bundle as we do now for the binaries, I don't know really, it's working now but waiting for any input

runcom · 2016-09-03T14:21:54Z

hack/make/.integration-daemon-start

@@ -16,6 +16,10 @@ if [ "$(go env GOOS)" = 'windows' ]; then
       return
 fi

+if [ ! -x "/etc/docker/seccomp.json" ]; then
+	install_seccomp "$base/../../profiles/seccomp/default.json"


@tianon PTAL this is a hack to make tests running - not sure where should this go honestly..

also @jhowardmsft win2lin fails but I wasn't be able to understand why...

Yeah, this isn't safe to include here (since these scripts are supposed to be runnable on your host, too). Couldn't/shouldn't the default profile path be a daemon flag/config option? (esp. so that the tests could point directly to the copy in the repo)

Couldn't/shouldn't the default profile path be a daemon flag/config option? (esp. so that the tests could point directly to the copy in the repo)

There has been some discussion around this in #24221 and that was how this feature was initially proposed. This PR basically follows this specific comment #24221 (comment) and so, I think it isn't really ok to have a daemon flag/config. I'm still open to have this though. /cc @justincormack

runcom · 2016-09-06T11:24:49Z

@justincormack PTAL

thaJeztah · 2016-11-04T16:12:09Z

ping @runcom looks like the API change https://github.com/docker/docker/pull/26276/files#r82990210 was not documented (think it's an API change 😄)

Can you add it to the API changes (https://github.com/docker/docker/blob/master/docs/reference/api/docker_remote_api.md#v125-api-changes), and update the API reference docs (https://github.com/docker/docker/blob/master/docs/reference/api/docker_remote_api_v1.25.md) if needed?

also ping @albers @sdurrheimer for the completion scripts :)

runcom · 2016-11-04T16:13:02Z

@thaJeztah sure

vikstrous · 2016-11-06T21:49:11Z

FYI This is a backwards incompatible API change that broke installing UCP 2.0.0 on docker master.

runcom · 2016-11-06T22:11:38Z

@vikstrous could you open an issue? I'll take care of fixing the backwards compatibility, though, if you use api version 1.24 everything should work

vikstrous · 2016-11-07T03:35:48Z

Sure: #28113

daemon: add a flag to override the default seccomp profile

Commit b237189 implemented an option to set the default seccomp profile in the daemon configuration. When that PR was reviewed, it was discussed to have the option accept the path to a custom profile JSON file; moby#26276 (comment) However, in the implementation, the special "unconfined" value was not taken into account. The "unconfined" value is meant to disable seccomp (more factually: run with an empty profile). While it's likely possible to achieve this by creating a file with an an empty (`{}`) profile, and passing the path to that file, it's inconsistent with the `--security-opt seccomp=unconfined` option on `docker run` and `docker create`, which is both confusing, and makes it harder to use (especially on Docker Desktop, where there's no direct access to the VM's filesystem). This patch adds the missing check for the special "unconfined" value. Co-authored-by: Tianon Gravi <admwiggin@gmail.com> Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Commit b237189 implemented an option to set the default seccomp profile in the daemon configuration. When that PR was reviewed, it was discussed to have the option accept the path to a custom profile JSON file; moby/moby#26276 (comment) However, in the implementation, the special "unconfined" value was not taken into account. The "unconfined" value is meant to disable seccomp (more factually: run with an empty profile). While it's likely possible to achieve this by creating a file with an an empty (`{}`) profile, and passing the path to that file, it's inconsistent with the `--security-opt seccomp=unconfined` option on `docker run` and `docker create`, which is both confusing, and makes it harder to use (especially on Docker Desktop, where there's no direct access to the VM's filesystem). This patch adds the missing check for the special "unconfined" value. Co-authored-by: Tianon Gravi <admwiggin@gmail.com> Signed-off-by: Sebastiaan van Stijn <github@gone.nl> Upstream-commit: 68e96f88ee1598563a66a1f53b8844291423fc88 Component: engine

GordonTheTurtle added the status/0-triage label Sep 2, 2016

runcom force-pushed the seccomp-conf branch from e22e8ae to 041170e Compare September 2, 2016 13:38

runcom added status/1-design-review area/security/seccomp and removed status/0-triage labels Sep 2, 2016

runcom changed the title ~~[WIP] systemd wide default seccomp profile~~ [WIP] system wide default seccomp profile Sep 2, 2016

runcom force-pushed the seccomp-conf branch from 041170e to 1aa1c46 Compare September 2, 2016 13:45

runcom reviewed Sep 2, 2016
View reviewed changes

runcom force-pushed the seccomp-conf branch 6 times, most recently from fbd486b to 58bc5d5 Compare September 3, 2016 11:04

runcom added the status/needs-vendoring label Sep 3, 2016

runcom force-pushed the seccomp-conf branch from 58bc5d5 to 52edeb6 Compare September 3, 2016 11:07

runcom added this to the 1.13.0 milestone Sep 3, 2016

runcom force-pushed the seccomp-conf branch 4 times, most recently from 90bae56 to 02a6ee0 Compare September 3, 2016 14:21

runcom reviewed Sep 3, 2016
View reviewed changes

runcom force-pushed the seccomp-conf branch from 02a6ee0 to a7491b8 Compare September 6, 2016 08:46

runcom changed the title ~~[WIP] system wide default seccomp profile~~ [RFC] system wide default seccomp profile Sep 6, 2016

runcom force-pushed the seccomp-conf branch from a7491b8 to 45ffa57 Compare September 8, 2016 13:56

runcom deleted the seccomp-conf branch November 4, 2016 15:46

thaJeztah added impact/api impact/changelog impact/cli labels Nov 4, 2016

vikstrous mentioned this pull request Nov 7, 2016

add backwards compatibility for docker info api change from #26276 #28113

Closed

sdurrheimer mentioned this pull request Nov 10, 2016

Add zsh completion for 'docker daemon --seccomp-profile' #28255

Merged

albers mentioned this pull request Nov 11, 2016

Add bash completion for docker daemon --seccomp-profile #28307

Merged

jlhawn mentioned this pull request Nov 16, 2016

docker info unmarshal error (1.12.1 client talking to 1.13-rc1 server) #28449

Closed

stevvooe mentioned this pull request Nov 17, 2016

api: types: do not remove old SecurityOptions struct #28504

Merged

dnephin pushed a commit to dnephin/docker that referenced this pull request Apr 17, 2017

Merge pull request moby#26276 from runcom/seccomp-conf

4758d72

daemon: add a flag to override the default seccomp profile

cyphar mentioned this pull request May 7, 2017

Impossible to see contents of AppArmor Profile "docker-default" #33060

Open

thaJeztah mentioned this pull request Jun 7, 2021

Fix daemon.json and daemon --seccomp-profile not accepting "unconfined", and rename default profile to "builtin" #42481

Merged

thaJeztah mentioned this pull request Jan 20, 2024

info: remove printSecurityOptionsWarnings, printServerWarnings docker/cli#4809

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

daemon: add a flag to override the default seccomp profile #26276

daemon: add a flag to override the default seccomp profile #26276

runcom commented Sep 2, 2016 •

edited

Loading

justincormack commented Sep 2, 2016

runcom commented Sep 2, 2016

runcom commented Sep 2, 2016

runcom Sep 2, 2016

rhatdan Sep 2, 2016

runcom Sep 2, 2016 •

edited

Loading

justincormack Sep 2, 2016

runcom Sep 2, 2016 •

edited

Loading

runcom Sep 2, 2016

justincormack Sep 2, 2016

runcom Sep 2, 2016

runcom Sep 3, 2016

runcom Sep 3, 2016

runcom Sep 3, 2016 •

edited

Loading

tianon Sep 8, 2016

runcom Sep 9, 2016

runcom commented Sep 6, 2016

thaJeztah commented Nov 4, 2016

runcom commented Nov 4, 2016

vikstrous commented Nov 6, 2016

runcom commented Nov 6, 2016 •

edited

Loading

vikstrous commented Nov 7, 2016

daemon: add a flag to override the default seccomp profile #26276

daemon: add a flag to override the default seccomp profile #26276

Conversation

runcom commented Sep 2, 2016 • edited Loading

justincormack commented Sep 2, 2016

runcom commented Sep 2, 2016

runcom commented Sep 2, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

runcom Sep 2, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

runcom Sep 2, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

runcom Sep 3, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

runcom commented Sep 6, 2016

thaJeztah commented Nov 4, 2016

runcom commented Nov 4, 2016

vikstrous commented Nov 6, 2016

runcom commented Nov 6, 2016 • edited Loading

vikstrous commented Nov 7, 2016

runcom commented Sep 2, 2016 •

edited

Loading

runcom Sep 2, 2016 •

edited

Loading

runcom Sep 2, 2016 •

edited

Loading

runcom Sep 3, 2016 •

edited

Loading

runcom commented Nov 6, 2016 •

edited

Loading