From 4fd1965ab4d1395b5cc4a0e03526ef9c43f794ec Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Sat, 1 May 2021 05:50:31 -0400 Subject: Add filepath glob support to --security-opt unmask Want to allow users to specify --security-opt unmask=/proc/*. This allows us to run podman within podman more securely, then specifing umask=all, also gives the user more flexibilty. Signed-off-by: Daniel J Walsh --- cmd/podman/common/specgen.go | 2 +- docs/source/markdown/podman-create.1.md | 3 +- docs/source/markdown/podman-run.1.md | 10 ++++-- pkg/specgen/generate/config_linux.go | 56 ++++++++++++++++++------------- pkg/specgen/generate/config_linux_test.go | 28 ++++++++++++++++ test/e2e/run_test.go | 12 +++++-- 6 files changed, 81 insertions(+), 30 deletions(-) create mode 100644 pkg/specgen/generate/config_linux_test.go diff --git a/cmd/podman/common/specgen.go b/cmd/podman/common/specgen.go index f889a0169..80a8985e9 100644 --- a/cmd/podman/common/specgen.go +++ b/cmd/podman/common/specgen.go @@ -540,7 +540,7 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *ContainerCLIOpts, args []string return fmt.Errorf("invalid systempaths option %q, only `unconfined` is supported", con[1]) } case "unmask": - s.ContainerSecurityConfig.Unmask = append(s.ContainerSecurityConfig.Unmask, strings.Split(con[1], ":")...) + s.ContainerSecurityConfig.Unmask = append(s.ContainerSecurityConfig.Unmask, con[1:]...) default: return fmt.Errorf("invalid --security-opt 2: %q", opt) } diff --git a/docs/source/markdown/podman-create.1.md b/docs/source/markdown/podman-create.1.md index ff3f86ec9..25b2fe11a 100644 --- a/docs/source/markdown/podman-create.1.md +++ b/docs/source/markdown/podman-create.1.md @@ -882,8 +882,7 @@ Note: Labeling can be disabled for all containers by setting label=false in the - `proc-opts=OPTIONS` : Comma-separated list of options to use for the /proc mount. More details for the possible mount options are specified in the **proc(5)** man page. -- `unmask=ALL or /path/1:/path/2` : Paths to unmask separated by a colon. If set to **ALL**, it will - unmask all the paths that are masked or made read only by default. +- **unmask**=_ALL_ or _/path/1:/path/2_, or shell expanded paths (/proc/*): Paths to unmask separated by a colon. If set to **ALL**, it will unmask all the paths that are masked or made read only by default. The default masked paths are **/proc/acpi, /proc/kcore, /proc/keys, /proc/latency_stats, /proc/sched_debug, /proc/scsi, /proc/timer_list, /proc/timer_stats, /sys/firmware, and /sys/fs/selinux.** The default paths that are read only are **/proc/asound, /proc/bus, /proc/fs, /proc/irq, /proc/sys, /proc/sysrq-trigger, /sys/fs/cgroup**. Note: Labeling can be disabled for all containers by setting label=false in the **containers.conf** (`/etc/containers/containers.conf` or `$HOME/.config/containers/containers.conf`) file. diff --git a/docs/source/markdown/podman-run.1.md b/docs/source/markdown/podman-run.1.md index a41938ff6..8689014c0 100644 --- a/docs/source/markdown/podman-run.1.md +++ b/docs/source/markdown/podman-run.1.md @@ -934,8 +934,7 @@ Note: Labeling can be disabled for all containers by setting label=false in the - **proc-opts**=_OPTIONS_ : Comma-separated list of options to use for the /proc mount. More details for the possible mount options are specified in the **proc(5)** man page. -- **unmask**=_ALL_ or _/path/1:/path/2_: Paths to unmask separated by a colon. If set to **ALL**, it will - unmask all the paths that are masked or made read only by default. +- **unmask**=_ALL_ or _/path/1:/path/2_, or shell expanded paths (/proc/*): Paths to unmask separated by a colon. If set to **ALL**, it will unmask all the paths that are masked or made read only by default. The default masked paths are **/proc/acpi, /proc/kcore, /proc/keys, /proc/latency_stats, /proc/sched_debug, /proc/scsi, /proc/timer_list, /proc/timer_stats, /sys/firmware, and /sys/fs/selinux.**. The default paths that are read only are **/proc/asound**, **/proc/bus**, **/proc/fs**, **/proc/irq**, **/proc/sys**, **/proc/sysrq-trigger**, **/sys/fs/cgroup**. Note: Labeling can be disabled for all containers by setting **label=false** in the **containers.conf**(5) file. @@ -1644,6 +1643,13 @@ the **mask** option. $ podman run --security-opt unmask=ALL fedora bash ``` +To unmask all the paths that start with /proc, set the **unmask** option to +**/proc/***. + +``` +$ podman run --security-opt unmask=/proc/* fedora bash +``` + ``` $ podman run --security-opt unmask=/foo/bar:/sys/firmware fedora bash ``` diff --git a/pkg/specgen/generate/config_linux.go b/pkg/specgen/generate/config_linux.go index 5c945cff3..6b9e9c4bf 100644 --- a/pkg/specgen/generate/config_linux.go +++ b/pkg/specgen/generate/config_linux.go @@ -10,7 +10,6 @@ import ( "github.com/containers/podman/v3/libpod/define" "github.com/containers/podman/v3/pkg/rootless" - "github.com/containers/podman/v3/pkg/util" spec "github.com/opencontainers/runtime-spec/specs-go" "github.com/opencontainers/runtime-tools/generate" "github.com/pkg/errors" @@ -151,30 +150,23 @@ func BlockAccessToKernelFilesystems(privileged, pidModeIsHost bool, mask, unmask "/sys/dev/block", } - unmaskAll := false - if unmask != nil && unmask[0] == "ALL" { - unmaskAll = true - } - if !privileged { - if !unmaskAll { - for _, mp := range defaultMaskPaths { - // check that the path to mask is not in the list of paths to unmask - if !util.StringInSlice(mp, unmask) { - g.AddLinuxMaskedPaths(mp) - } + for _, mp := range defaultMaskPaths { + // check that the path to mask is not in the list of paths to unmask + if shouldMask(mp, unmask) { + g.AddLinuxMaskedPaths(mp) } - for _, rp := range []string{ - "/proc/asound", - "/proc/bus", - "/proc/fs", - "/proc/irq", - "/proc/sys", - "/proc/sysrq-trigger", - } { - if !util.StringInSlice(rp, unmask) { - g.AddLinuxReadonlyPaths(rp) - } + } + for _, rp := range []string{ + "/proc/asound", + "/proc/bus", + "/proc/fs", + "/proc/irq", + "/proc/sys", + "/proc/sysrq-trigger", + } { + if shouldMask(rp, unmask) { + g.AddLinuxReadonlyPaths(rp) } } @@ -376,3 +368,21 @@ func supportAmbientCapabilities() bool { err := unix.Prctl(unix.PR_CAP_AMBIENT, unix.PR_CAP_AMBIENT_IS_SET, 0, 0, 0) return err == nil } + +func shouldMask(mask string, unmask []string) bool { + for _, m := range unmask { + if strings.ToLower(m) == "all" { + return false + } + for _, m1 := range strings.Split(m, ":") { + match, err := filepath.Match(m1, mask) + if err != nil { + logrus.Errorf(err.Error()) + } + if match { + return false + } + } + } + return true +} diff --git a/pkg/specgen/generate/config_linux_test.go b/pkg/specgen/generate/config_linux_test.go new file mode 100644 index 000000000..39973324b --- /dev/null +++ b/pkg/specgen/generate/config_linux_test.go @@ -0,0 +1,28 @@ +package generate + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestShouldMask(t *testing.T) { + tests := []struct { + mask string + unmask []string + shouldMask bool + }{ + {"/proc/foo", []string{"all"}, false}, + {"/proc/foo", []string{"ALL"}, false}, + {"/proc/foo", []string{"/proc/foo"}, false}, + {"/proc/foo", []string{"/proc/*"}, false}, + {"/proc/foo", []string{"/proc/bar", "all"}, false}, + {"/proc/foo", []string{"/proc/f*"}, false}, + {"/proc/foo", []string{"/proc/b*"}, true}, + {"/proc/foo", []string{}, true}, + } + for _, test := range tests { + val := shouldMask(test.mask, test.unmask) + assert.Equal(t, val, test.shouldMask) + } +} diff --git a/test/e2e/run_test.go b/test/e2e/run_test.go index 74bdfce2c..d8d7dab07 100644 --- a/test/e2e/run_test.go +++ b/test/e2e/run_test.go @@ -299,9 +299,17 @@ var _ = Describe("Podman run", func() { session = podmanTest.Podman([]string{"run", "-d", "--name=maskCtr5", "--security-opt", "systempaths=unconfined", ALPINE, "grep", "/proc", "/proc/self/mounts"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) - stdoutLines := session.OutputToStringArray() - Expect(stdoutLines).Should(HaveLen(1)) + Expect(session.OutputToStringArray()).Should(HaveLen(1)) + + session = podmanTest.Podman([]string{"run", "-d", "--security-opt", "unmask=/proc/*", ALPINE, "grep", "/proc", "/proc/self/mounts"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(session.OutputToStringArray()).Should(HaveLen(1)) + session = podmanTest.Podman([]string{"run", "--security-opt", "unmask=/proc/a*", ALPINE, "ls", "/proc/acpi"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(session.OutputToString()).To(Not(BeEmpty())) }) It("podman run security-opt unmask on /sys/fs/cgroup", func() { -- cgit v1.2.3-54-g00ecf