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

@@ -114,6 +114,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

@@ -33,6 +33,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

@@ -293,7 +293,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

@@ -317,7 +317,7 @@ Suppress output information when pulling images
 
 @@option read-only
 
-@@option read-only-tmpfs
+@@option read-write-tmpfs
 
 @@option replace
 
@@ -470,14 +470,12 @@ content. Installing packages into _/usr_, for example. In production,
 applications seldom need to write to the image.  Container applications write
 to volumes if they need to write to file systems at all. Applications can be
 made more secure by running them in read-only mode using the **--read-only** switch.
-This protects the container's image from modification. Read-only containers may
-still need to write temporary data. The best way to handle this is to mount
-tmpfs directories on _/run_ and _/tmp_.
+This protects the container's image from modification.
 
 ```
 $ 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

@@ -399,7 +399,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
@@ -303,6 +303,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

@@ -566,20 +566,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)
@@ -657,7 +654,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

@@ -832,6 +832,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