From 20f35c191dc8a367def8a553f0b7756cb6a58dfc Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 7 Jun 2021 13:50:00 +0200 Subject: [PATCH] Fix daemon.json and daemon --seccomp-profile not accepting "unconfined" Commit b237189e6c8a4f97be59f08c63cdcb1f2f4680a8 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; https://github.com/moby/moby/pull/26276#issuecomment-253546966 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 Signed-off-by: Sebastiaan van Stijn Upstream-commit: 68e96f88ee1598563a66a1f53b8844291423fc88 Component: engine --- components/engine/cmd/dockerd/config_unix.go | 2 +- components/engine/daemon/daemon_unix.go | 10 ++-- components/engine/daemon/seccomp_linux.go | 2 + .../engine/integration/daemon/daemon_test.go | 47 ++++++++++++++++++- 4 files changed, 54 insertions(+), 7 deletions(-) diff --git a/components/engine/cmd/dockerd/config_unix.go b/components/engine/cmd/dockerd/config_unix.go index 7ae58281d1c..68bd81af43e 100644 --- a/components/engine/cmd/dockerd/config_unix.go +++ b/components/engine/cmd/dockerd/config_unix.go @@ -58,7 +58,7 @@ func installConfigFlags(conf *config.Config, flags *pflag.FlagSet) error { flags.StringVar(&conf.InitPath, "init-path", "", "Path to the docker-init binary") flags.Int64Var(&conf.CPURealtimePeriod, "cpu-rt-period", 0, "Limit the CPU real-time period in microseconds for the parent cgroup for all containers") flags.Int64Var(&conf.CPURealtimeRuntime, "cpu-rt-runtime", 0, "Limit the CPU real-time runtime in microseconds for the parent cgroup for all containers") - flags.StringVar(&conf.SeccompProfile, "seccomp-profile", "", "Path to seccomp profile") + flags.StringVar(&conf.SeccompProfile, "seccomp-profile", config.SeccompProfileDefault, `Path to seccomp profile. Use "unconfined" to disable the default seccomp profile`) flags.Var(&conf.ShmSize, "default-shm-size", "Default shm size for containers") flags.BoolVar(&conf.NoNewPrivileges, "no-new-privileges", false, "Set no-new-privileges by default for new containers") flags.StringVar(&conf.IpcMode, "default-ipc-mode", config.DefaultIpcMode, `Default mode for containers ipc ("shareable" | "private")`) diff --git a/components/engine/daemon/daemon_unix.go b/components/engine/daemon/daemon_unix.go index 44429d36929..05f790ee2c6 100644 --- a/components/engine/daemon/daemon_unix.go +++ b/components/engine/daemon/daemon_unix.go @@ -1708,11 +1708,13 @@ func maybeCreateCPURealTimeFile(configValue int64, file string, path string) err func (daemon *Daemon) setupSeccompProfile() error { if daemon.configStore.SeccompProfile != "" { daemon.seccompProfilePath = daemon.configStore.SeccompProfile - b, err := ioutil.ReadFile(daemon.configStore.SeccompProfile) - if err != nil { - return fmt.Errorf("opening seccomp profile (%s) failed: %v", daemon.configStore.SeccompProfile, err) + if daemon.configStore.SeccompProfile != config.SeccompProfileUnconfined { + b, err := ioutil.ReadFile(daemon.configStore.SeccompProfile) + if err != nil { + return fmt.Errorf("opening seccomp profile (%s) failed: %v", daemon.configStore.SeccompProfile, err) + } + daemon.seccompProfile = b } - daemon.seccompProfile = b } return nil } diff --git a/components/engine/daemon/seccomp_linux.go b/components/engine/daemon/seccomp_linux.go index 7b07a53d0a3..d871e43ae80 100644 --- a/components/engine/daemon/seccomp_linux.go +++ b/components/engine/daemon/seccomp_linux.go @@ -39,6 +39,8 @@ func WithSeccomp(daemon *Daemon, c *container.Container) coci.SpecOpts { s.Linux.Seccomp, err = seccomp.LoadProfile(c.SeccompProfile, s) case daemon.seccompProfile != nil: s.Linux.Seccomp, err = seccomp.LoadProfile(string(daemon.seccompProfile), s) + case daemon.seccompProfilePath == dconfig.SeccompProfileUnconfined: + c.SeccompProfile = dconfig.SeccompProfileUnconfined default: s.Linux.Seccomp, err = seccomp.GetDefaultProfile(s) } diff --git a/components/engine/integration/daemon/daemon_test.go b/components/engine/integration/daemon/daemon_test.go index 661f1ae91e1..4be41e04728 100644 --- a/components/engine/integration/daemon/daemon_test.go +++ b/components/engine/integration/daemon/daemon_test.go @@ -8,11 +8,11 @@ import ( "runtime" "testing" + "github.com/docker/docker/daemon/config" "github.com/docker/docker/testutil/daemon" "gotest.tools/v3/assert" - "gotest.tools/v3/skip" - is "gotest.tools/v3/assert/cmp" + "gotest.tools/v3/skip" ) func TestConfigDaemonLibtrustID(t *testing.T) { @@ -99,3 +99,46 @@ func TestDaemonConfigValidation(t *testing.T) { }) } } + +func TestConfigDaemonSeccompProfiles(t *testing.T) { + skip.If(t, runtime.GOOS != "linux") + + d := daemon.New(t) + defer d.Stop(t) + + tests := []struct { + doc string + profile string + expectedProfile string + }{ + { + doc: "empty profile set", + profile: "", + expectedProfile: config.SeccompProfileDefault, + }, + { + doc: "unconfined profile", + profile: config.SeccompProfileUnconfined, + expectedProfile: config.SeccompProfileUnconfined, + }, + } + + for _, tc := range tests { + tc := tc + t.Run(tc.doc, func(t *testing.T) { + d.Start(t, "--seccomp-profile="+tc.profile) + info := d.Info(t) + assert.Assert(t, is.Contains(info.SecurityOptions, "name=seccomp,profile="+tc.expectedProfile)) + d.Stop(t) + + cfg := filepath.Join(d.RootDir(), "daemon.json") + err := ioutil.WriteFile(cfg, []byte(`{"seccomp-profile": "`+tc.profile+`"}`), 0644) + assert.NilError(t, err) + + d.Start(t, "--config-file", cfg) + info = d.Info(t) + assert.Assert(t, is.Contains(info.SecurityOptions, "name=seccomp,profile="+tc.expectedProfile)) + d.Stop(t) + }) + } +}