--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: #12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>

cmd/podman/common/create.go

@@ -381,8 +381,8 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions,
 			"Make containers root filesystem read-only",
 		)
 		createFlags.BoolVar(
-			&cf.ReadOnlyTmpFS,
-			"read-only-tmpfs", cf.ReadOnlyTmpFS,
+			&cf.ReadWriteTmpFS,
+			"read-write-tmpfs", cf.ReadWriteTmpFS,
 			"When running containers in read-only mode mount a read-write tmpfs on /run, /tmp and /var/tmp",
 		)
 		requiresFlagName := "requires"

cmd/podman/common/create_opts.go

@@ -83,7 +83,7 @@ func DefineCreateDefaults(opts *entities.ContainerCreateOptions) {
 	opts.MemorySwappiness = -1
 	opts.ImageVolume = podmanConfig.ContainersConfDefaultsRO.Engine.ImageVolumeMode
 	opts.Pull = policy()
-	opts.ReadOnlyTmpFS = true
+	opts.ReadWriteTmpFS = true
 	opts.SdNotifyMode = define.SdNotifyModeContainer
 	opts.StopTimeout = podmanConfig.ContainersConfDefaultsRO.Engine.StopTimeout
 	opts.Systemd = "true"

cmd/podman/containers/create.go

@@ -121,6 +121,11 @@ func commonFlags(cmd *cobra.Command) error {
 	if cmd.Flags().Changed("image-volume") {
 		cliVals.ImageVolume = cmd.Flag("image-volume").Value.String()
 	}
+
+	if cmd.Flags().Changed("read-write-tmpfs") && !cliVals.ReadOnly {
+		return errors.New("--read-write-tmpfs is valid only when --read-only=true")
+	}
+
 	return nil
 }
 

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)
 }

docs/source/markdown/options/read-write-tmpfs.md

@@ -0,0 +1,11 @@
+####> This option file is used in:
+####>   podman create, run
+####> If you edit this file, make sure your changes
+####> are applicable to all of those.
+#### **--read-write-tmpfs**
+
+If container is running in --read-only mode, when true Podman mounts a
+read-write tmpfs on _/run_, _/tmp_, and _/var/tmp_. When false, Podman sets
+the entire container such that no tmpfs can be written to unless specified on
+the command line.  Podman mounts _/dev_, _/dev/mqueue_, _/dev/pts_, _/dev/shm_
+as read only. The default is *true*

docs/source/markdown/podman-create.1.md.in

@@ -305,7 +305,7 @@ Suppress output information when pulling images
 
 @@option read-only
 
-@@option read-only-tmpfs
+@@option read-write-tmpfs
 
 @@option replace
 

docs/source/markdown/podman-run.1.md.in

@@ -329,7 +329,7 @@ Suppress output information when pulling images
 
 @@option read-only
 
-@@option read-only-tmpfs
+@@option read-write-tmpfs
 
 @@option replace
 
@@ -455,12 +455,12 @@ made more secure by running them in read-only mode using the **--read-only** swi
 This protects the container's image from modification. By default read-only
 containers can write to temporary data. Podman mounts a tmpfs on _/run_ and
 _/tmp_ within the container. If the container should not write to any file
-system within the container, including tmpfs, set --read-only-tmpfs=false.
+system within the container, including tmpfs, set --read-write-tmpfs=false.
 
 ```
 $ 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

pkg/api/handlers/compat/containers_create.go

@@ -400,7 +400,7 @@ func cliOpts(cc handlers.CreateContainerConfig, rtc *config.Config) (*entities.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,

pkg/domain/entities/pods.go

@@ -249,7 +249,7 @@ type ContainerCreateOptions struct {
 	Pull               string
 	Quiet              bool
 	ReadOnly           bool
-	ReadOnlyTmpFS      bool
+	ReadWriteTmpFS     bool
 	Restart            string
 	Replace            bool
 	Requires           []string
@@ -308,6 +308,7 @@ func NewInfraContainerCreateOptions() ContainerCreateOptions {
 		IsInfra:          true,
 		ImageVolume:      "bind",
 		MemorySwappiness: -1,
+		ReadWriteTmpFS:   true,
 	}
 	return options
 }

pkg/specgen/specgen.go

@@ -384,6 +384,9 @@ type ContainerSecurityConfig struct {
 	// ReadOnlyFilesystem indicates that everything will be mounted
 	// as read-only
 	ReadOnlyFilesystem bool `json:"read_only_filesystem,omitempty"`
+	// ReadWriteTmpfs indicates that tmpfs will be mounted
+	// as read-only
+	ReadWriteTmpFS bool `json:"read_write_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.

pkg/specgenutil/specgen.go

@@ -585,20 +585,17 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *entities.ContainerCreateOptions
 	if !s.ReadOnlyFilesystem {
 		s.ReadOnlyFilesystem = c.ReadOnly
 	}
+	s.ReadWriteTmpFS = 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
 	}
 
 	// 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)
@@ -676,7 +673,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
 	}

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, addReadWriteTmpfs bool) ([]spec.Mount, []*specgen.NamedVolume, []*specgen.OverlayVolume, []*specgen.ImageVolume, error) {
 	// Get mounts from the --mounts flag.
 	unifiedMounts, unifiedVolumes, unifiedImageVolumes, err := Mounts(mountFlag)
 	if err != nil {
@@ -79,10 +79,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 addReadWriteTmpfs {
+		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
 			}

test/e2e/container_clone_test.go

@@ -2,6 +2,7 @@ package integration
 
 import (
 	"os"
+	"strings"
 
 	. "github.com/containers/podman/v4/test/utils"
 	. "github.com/onsi/ginkgo"
@@ -51,6 +52,11 @@ var _ = Describe("Podman container clone", func() {
 		Expect(ctrInspect).To(Exit(0))
 		Expect(ctrInspect.InspectContainerToJSON()[0].Name).To(ContainSubstring("-clone1"))
 
+		// FIXME when new crun reaches Ubuntu
+		if podmanTest.Host.Distribution == "ubuntu" &&
+			strings.Contains(podmanTest.OCIRuntime, "crun") {
+			Skip("The version of crun shipped by ubuntu is too old. Needs crun-1.4.2 or greater")
+		}
 		ctrStart := podmanTest.Podman([]string{"container", "start", clone.OutputToString()})
 		ctrStart.WaitWithDefaultTimeout()
 		Expect(ctrStart).To(Exit(0))
@@ -164,12 +170,18 @@ var _ = Describe("Podman container clone", func() {
 
 	It("podman container clone in a pod", func() {
 		SkipIfRootlessCgroupsV1("starting a container with the memory limits not supported")
+		// FIXME when new crun reaches Ubuntu
+		if podmanTest.Host.Distribution == "ubuntu" &&
+			strings.Contains(podmanTest.OCIRuntime, "crun") {
+			Skip("The version of crun shipped by ubuntu is too old. Needs crun-1.4.2 or greater")
+		}
 		run := podmanTest.Podman([]string{"run", "-dt", "--pod", "new:1234", ALPINE, "sleep", "20"})
 		run.WaitWithDefaultTimeout()
 		Expect(run).To(Exit(0))
 		clone := podmanTest.Podman([]string{"container", "clone", run.OutputToString()})
 		clone.WaitWithDefaultTimeout()
 		Expect(clone).To(Exit(0))
+
 		ctrStart := podmanTest.Podman([]string{"container", "start", clone.OutputToString()})
 		ctrStart.WaitWithDefaultTimeout()
 		Expect(ctrStart).To(Exit(0))

test/system/030-run.bats

@@ -836,6 +836,25 @@ EOF
     is "${lines[0]}" ".*${HOST}.*"
 }
 
+@test "podman run --read-write-tmpfs" {
+    # FIXME once ubuntu gets a newer crun
+    if is_ubuntu; then
+        if [ $(podman_runtime) == "crun" ]; then
+	   skip "The version of crun shipped by ubuntu is too old. Needs crun-1.4.2 or greater"
+	fi
+    fi
+    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 rwdir in /run /tmp /var/tmp /dev /dev/shm; do
+        run_podman run --rm --read-only $IMAGE touch ${rwdir}/foo
+        run_podman run --rm --read-only --read-write-tmpfs=true $IMAGE touch ${rwdir}/foo
+	run_podman 1 run --rm --read-only --read-write-tmpfs=false $IMAGE touch ${rwdir}/foo
+	is "$output" "touch: ${rwdir}/foo: Read-only file system" "Should fail with ${rwdir}/foo read only file system"
+     done
+}
+
 @test "podman run doesn't override oom-score-adj" {
     current_oom_score_adj=$(cat /proc/self/oom_score_adj)
     run_podman run --rm $IMAGE cat /proc/self/oom_score_adj