From 23268a96f3f321827f55fc06a4bcfe92305a73c0 Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Tue, 22 Feb 2022 16:30:56 -0500 Subject: [PATCH] --read-write-tmpfs=false should set /dev/* tmpfs to readonly Change the --read-only-tmpfs option to --read-write-tmpfs since this makes sense and the old name was doing the exact opposite. Fixes: https://github.com/containers/podman/issues/12937 Signed-off-by: Daniel J Walsh --- cmd/podman/common/create.go | 4 ++-- cmd/podman/common/create_opts.go | 2 +- cmd/podman/containers/create.go | 3 +++ cmd/podman/utils/alias.go | 2 ++ docs/source/markdown/podman-create.1.md | 4 ++-- docs/source/markdown/podman-run.1.md | 6 +++--- pkg/domain/entities/pods.go | 2 +- pkg/specgen/generate/oci.go | 15 +++++++++++++-- pkg/specgen/specgen.go | 3 +++ pkg/specgenutil/specgen.go | 7 ++----- pkg/specgenutil/volumes.go | 8 ++++---- test/system/030-run.bats | 14 ++++++++++++++ 12 files changed, 50 insertions(+), 20 deletions(-) diff --git a/cmd/podman/common/create.go b/cmd/podman/common/create.go index 634b49db7bcf..536d3a5d842b 100644 --- a/cmd/podman/common/create.go +++ b/cmd/podman/common/create.go @@ -439,8 +439,8 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions, "Make containers root filesystem read-only", ) createFlags.BoolVar( - &cf.ReadOnlyTmpFS, - "read-only-tmpfs", true, + &cf.ReadWriteTmpFS, + "read-write-tmpfs", true, "When running containers in read-only mode mount a read-write tmpfs on /run, /tmp and /var/tmp", ) requiresFlagName := "requires" diff --git a/cmd/podman/common/create_opts.go b/cmd/podman/common/create_opts.go index a4f94616c7d9..cb3e766b5f32 100644 --- a/cmd/podman/common/create_opts.go +++ b/cmd/podman/common/create_opts.go @@ -287,7 +287,7 @@ func ContainerCreateToContainerCLIOpts(cc handlers.CreateContainerConfig, rtc *c PublishAll: cc.HostConfig.PublishAllPorts, Quiet: false, ReadOnly: cc.HostConfig.ReadonlyRootfs, - ReadOnlyTmpFS: true, // podman default + ReadWriteTmpFS: true, // podman default Rm: cc.HostConfig.AutoRemove, SecurityOpt: cc.HostConfig.SecurityOpt, StopSignal: cc.Config.StopSignal, diff --git a/cmd/podman/containers/create.go b/cmd/podman/containers/create.go index de1a41e25335..40c9fde905a2 100644 --- a/cmd/podman/containers/create.go +++ b/cmd/podman/containers/create.go @@ -111,6 +111,9 @@ func create(cmd *cobra.Command, args []string) error { return err } + if cmd.Flags().Changed("read-write-tmpfs") && !cliVals.ReadOnly { + return errors.New("--read-write-tmpfs is only used if --read-write=true") + } // Check if initctr is used with --pod and the value is correct if initctr := InitContainerType; cmd.Flags().Changed("init-ctr") { if !cmd.Flags().Changed("pod") { diff --git a/cmd/podman/utils/alias.go b/cmd/podman/utils/alias.go index 4d5b625d02b9..b04d476c229e 100644 --- a/cmd/podman/utils/alias.go +++ b/cmd/podman/utils/alias.go @@ -31,6 +31,8 @@ func AliasFlags(f *pflag.FlagSet, name string) pflag.NormalizedName { name = "os" case "override-variant": name = "variant" + case "read-only-tmpfs": + name = "read-write-tmpfs" } return pflag.NormalizedName(name) } diff --git a/docs/source/markdown/podman-create.1.md b/docs/source/markdown/podman-create.1.md index 2a0f3b738a5f..0904558aa931 100644 --- a/docs/source/markdown/podman-create.1.md +++ b/docs/source/markdown/podman-create.1.md @@ -851,9 +851,9 @@ By default a container will have its root filesystem writable allowing processes to write files anywhere. By specifying the `--read-only` flag the container will have its root filesystem mounted as read only prohibiting any writes. -#### **--read-only-tmpfs** +#### **--read-write-tmpfs** -If container is running in --read-only mode, then mount a read-write tmpfs on /run, /tmp, and /var/tmp. The default is *true* +If container is running in --read-only mode, then mount a read-write tmpfs on /run, /tmp, and /var/tmp. When false, Podman the entire container has no tmpfs that are read/write unless specified on the command line. Podman mounts /dev, /dev/mqueue, /dev/pts, /dev/shm as read only. The default is *true* #### **--replace** diff --git a/docs/source/markdown/podman-run.1.md b/docs/source/markdown/podman-run.1.md index 239cf3b832c0..90519000cef7 100644 --- a/docs/source/markdown/podman-run.1.md +++ b/docs/source/markdown/podman-run.1.md @@ -891,9 +891,9 @@ By default a container will have its root filesystem writable allowing processes to write files anywhere. By specifying the **--read-only** flag, the container will have its root filesystem mounted as read only prohibiting any writes. -#### **--read-only-tmpfs** +#### **--read-write-tmpfs** -If container is running in **--read-only** mode, then mount a read-write tmpfs on _/run_, _/tmp_, and _/var/tmp_. The default is **true**. +If container is running in --read-only mode, then mount a read-write tmpfs on /run, /tmp, and /var/tmp. When false, Podman the entire container has no tmpfs that are read/write unless specified on the command line. Podman mounts /dev, /dev/mqueue, /dev/pts, /dev/shm as read only. The default is *true* #### **--replace** @@ -1574,7 +1574,7 @@ tmpfs directories on _/run_ and _/tmp_. ``` $ podman run --read-only -i -t fedora /bin/bash -$ podman run --read-only --read-only-tmpfs=false --tmpfs /run -i -t fedora /bin/bash +$ podman run --read-only --read-write-tmpfs=false --tmpfs /run -i -t fedora /bin/bash ``` ### Exposing log messages from the container to the host's log diff --git a/pkg/domain/entities/pods.go b/pkg/domain/entities/pods.go index 6fb3db1b5cc7..69ac36fac623 100644 --- a/pkg/domain/entities/pods.go +++ b/pkg/domain/entities/pods.go @@ -226,7 +226,7 @@ type ContainerCreateOptions struct { Pull string Quiet bool ReadOnly bool - ReadOnlyTmpFS bool + ReadWriteTmpFS bool Restart string Replace bool Requires []string diff --git a/pkg/specgen/generate/oci.go b/pkg/specgen/generate/oci.go index 8b3550e36a52..53a0923349f2 100644 --- a/pkg/specgen/generate/oci.go +++ b/pkg/specgen/generate/oci.go @@ -182,8 +182,6 @@ func SpecGenToOCI(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Runt if err != nil { return nil, err } - // Remove the default /dev/shm mount to ensure we overwrite it - g.RemoveMount("/dev/shm") g.HostSpecific = true addCgroup := true @@ -437,5 +435,18 @@ func SpecGenToOCI(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Runt } setProcOpts(s, &g) + if s.ReadOnlyTmpFS { + for n, m := range configSpec.Mounts { + switch m.Destination { + case "/dev", "/dev/shm", "/dev/mqueue", "/dev/pts": + m.Options = append(m.Options, "ro") + configSpec.Mounts[n] = m + } + } + } else { + // Remove the default /dev/shm mount to ensure we overwrite it + g.RemoveMount("/dev/shm") + } + return configSpec, nil } diff --git a/pkg/specgen/specgen.go b/pkg/specgen/specgen.go index 7f6f79b8739a..229949301593 100644 --- a/pkg/specgen/specgen.go +++ b/pkg/specgen/specgen.go @@ -367,6 +367,9 @@ type ContainerSecurityConfig struct { // ReadOnlyFilesystem indicates that everything will be mounted // as read-only ReadOnlyFilesystem bool `json:"read_only_filesystem,omitempty"` + // ReadOnlyTmpfs indicates that tmpfs will be mounted + // as read-only + ReadOnlyTmpFS bool `json:"read_only_tmpfs,omitempty"` // Umask is the umask the init process of the container will be run with. Umask string `json:"umask,omitempty"` // ProcOpts are the options used for the proc mount. diff --git a/pkg/specgenutil/specgen.go b/pkg/specgenutil/specgen.go index b037e14cc57e..e32642ec397e 100644 --- a/pkg/specgenutil/specgen.go +++ b/pkg/specgenutil/specgen.go @@ -584,10 +584,10 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *entities.ContainerCreateOptions if !s.ReadOnlyFilesystem { s.ReadOnlyFilesystem = c.ReadOnly } + s.ReadOnlyTmpFS = !c.ReadWriteTmpFS if len(s.ConmonPidFile) == 0 || len(c.ConmonPIDFile) != 0 { s.ConmonPidFile = c.ConmonPIDFile } - if len(s.DependencyContainers) == 0 || len(c.Requires) != 0 { s.DependencyContainers = c.Requires } @@ -595,9 +595,6 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *entities.ContainerCreateOptions // TODO // outside of specgen and oci though // defaults to true, check spec/storage - // s.readonly = c.ReadOnlyTmpFS - // TODO convert to map? - // check if key=value and convert sysmap := make(map[string]string) for _, ctl := range c.Sysctl { splitCtl := strings.SplitN(ctl, "=", 2) @@ -662,7 +659,7 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *entities.ContainerCreateOptions // Only add read-only tmpfs mounts in case that we are read-only and the // read-only tmpfs flag has been set. - mounts, volumes, overlayVolumes, imageVolumes, err := parseVolumes(c.Volume, c.Mount, c.TmpFS, c.ReadOnlyTmpFS && c.ReadOnly) + mounts, volumes, overlayVolumes, imageVolumes, err := parseVolumes(c.Volume, c.Mount, c.TmpFS, c.ReadWriteTmpFS && c.ReadOnly) if err != nil { return err } diff --git a/pkg/specgenutil/volumes.go b/pkg/specgenutil/volumes.go index 2bd79b186ab9..bb0f966e0016 100644 --- a/pkg/specgenutil/volumes.go +++ b/pkg/specgenutil/volumes.go @@ -26,7 +26,7 @@ var ( // Does not handle image volumes, init, and --volumes-from flags. // Can also add tmpfs mounts from read-only tmpfs. // TODO: handle options parsing/processing via containers/storage/pkg/mount -func parseVolumes(volumeFlag, mountFlag, tmpfsFlag []string, addReadOnlyTmpfs bool) ([]spec.Mount, []*specgen.NamedVolume, []*specgen.OverlayVolume, []*specgen.ImageVolume, error) { +func parseVolumes(volumeFlag, mountFlag, tmpfsFlag []string, readWriteTmpfs bool) ([]spec.Mount, []*specgen.NamedVolume, []*specgen.OverlayVolume, []*specgen.ImageVolume, error) { // Get mounts from the --mounts flag. unifiedMounts, unifiedVolumes, unifiedImageVolumes, err := getMounts(mountFlag) if err != nil { @@ -68,10 +68,10 @@ func parseVolumes(volumeFlag, mountFlag, tmpfsFlag []string, addReadOnlyTmpfs bo } // If requested, add tmpfs filesystems for read-only containers. - if addReadOnlyTmpfs { - readonlyTmpfs := []string{"/tmp", "/var/tmp", "/run"} + if readWriteTmpfs { + tmpfs := []string{"/tmp", "/var/tmp", "/run"} options := []string{"rw", "rprivate", "nosuid", "nodev", "tmpcopyup"} - for _, dest := range readonlyTmpfs { + for _, dest := range tmpfs { if _, ok := unifiedMounts[dest]; ok { continue } diff --git a/test/system/030-run.bats b/test/system/030-run.bats index ec85ef16672e..910601db421d 100644 --- a/test/system/030-run.bats +++ b/test/system/030-run.bats @@ -815,4 +815,18 @@ EOF run_podman run --uidmap 0:10001:10002 --rm --hostname ${HOST} $IMAGE grep ${HOST} /etc/hosts is "${lines[0]}" ".*${HOST}.*" } + +@test "podman run --read-only" { + HOST=$(random_string 25) + run_podman 1 run --rm --read-only $IMAGE touch /foo + is "$output" "touch: /foo: Read-only file system" "Should fail with read only file system" + + for dir in /run /tmp /var/tmp /dev /dev/shm ; do + run_podman run --rm --read-only $IMAGE touch $dir/foo + run_podman run --rm --read-only --read-write-tmpfs=true $IMAGE touch $dir/foo + run_podman 1 run --rm --read-only --read-write-tmpfs=false $IMAGE touch $dir/foo + is "$output" "touch: $dir/foo: Read-only file system" "Should fail with $dir/foo read only file system" + done +} + # vim: filetype=sh