Add containers.conf read-only flag support

If you are running temporary containers within podman play kube
we should really be running these in read-only mode. For automotive
they plan on running all of their containers in read-only temporal
mode. Adding this option guarantees that the container image is not
being modified during the running of the container.

The containers can only write to tmpfs mounted directories.

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

DOWNLOADS.md

@@ -30,7 +30,7 @@ don't take them beyond that.
 
 WARNING:  The items linked below all come from scripts in the `artifacts_task`
 map of `.cirrus.yml`.  When adding or updating any item below, please ensure it
-matches cooresponding changes in the artifacts task.
+matches corresponding changes in the artifacts task.
 
 -->
 

cmd/podman/common/create.go

@@ -377,12 +377,12 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions,
 		)
 		createFlags.BoolVar(
 			&cf.ReadOnly,
-			"read-only", false,
+			"read-only", podmanConfig.ContainersConfDefaultsRO.Containers.ReadOnly,
 			"Make containers root filesystem read-only",
 		)
 		createFlags.BoolVar(
-			&cf.ReadOnlyTmpFS,
+			&cf.ReadWriteTmpFS,
-			"read-only-tmpfs", cf.ReadOnlyTmpFS,
+			"read-only-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

@@ -409,6 +409,8 @@ func createPodIfNecessary(cmd *cobra.Command, s *specgen.SpecGenerator, netOpts
 	infraOpts := entities.NewInfraContainerCreateOptions()
 	infraOpts.Net = netOpts
 	infraOpts.Quiet = true
+	infraOpts.ReadOnly = true
+	infraOpts.ReadWriteTmpFS = false
 	infraOpts.Hostname, err = cmd.Flags().GetString("hostname")
 	if err != nil {
 		return err

pkg/api/handlers/compat/containers_create.go

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

pkg/domain/infra/abi/play.go

@@ -68,6 +68,8 @@ func (ic *ContainerEngine) createServiceContainer(ctx context.Context, name stri
 		ImageVolume:      "bind",
 		IsInfra:          false,
 		MemorySwappiness: -1,
+		ReadOnly:         true,
+		ReadWriteTmpFS:   false,
 		// No need to spin up slirp etc.
 		Net: &entities.NetOptions{Network: specgen.Namespace{NSMode: specgen.NoNetwork}},
 	}
@@ -560,6 +562,8 @@ func (ic *ContainerEngine) playKubePod(ctx context.Context, podName string, podY
 		infraImage := util.DefaultContainerConfig().Engine.InfraImage
 		infraOptions := entities.NewInfraContainerCreateOptions()
 		infraOptions.Hostname = podSpec.PodSpecGen.PodBasicConfig.Hostname
+		infraOptions.ReadOnly = true
+		infraOptions.ReadWriteTmpFS = false
 		infraOptions.UserNS = options.Userns
 		podSpec.PodSpecGen.InfraImage = infraImage
 		podSpec.PodSpecGen.NoInfra = false
@@ -605,6 +609,16 @@ func (ic *ContainerEngine) playKubePod(ctx context.Context, podName string, podY
 		}
 	}
 
+	cfg, err := ic.Libpod.GetConfigNoCopy()
+	if err != nil {
+		return nil, nil, err
+	}
+
+	var readOnly types.OptionalBool
+	if cfg.Containers.ReadOnly {
+		readOnly = types.NewOptionalBool(cfg.Containers.ReadOnly)
+	}
+
 	ctrNames := make(map[string]string)
 	for _, initCtr := range podYAML.Spec.InitContainers {
 		// Error out if same name is used for more than one container
@@ -643,6 +657,7 @@ func (ic *ContainerEngine) playKubePod(ctx context.Context, podName string, podY
 			PodInfraID:         podInfraID,
 			PodName:            podName,
 			PodSecurityContext: podYAML.Spec.SecurityContext,
+			ReadOnly:           readOnly,
 			RestartPolicy:      define.RestartPolicyNo,
 			SeccompPaths:       seccompPaths,
 			SecretsManager:     secretsManager,
@@ -697,6 +712,7 @@ func (ic *ContainerEngine) playKubePod(ctx context.Context, podName string, podY
 			PodInfraID:         podInfraID,
 			PodName:            podName,
 			PodSecurityContext: podYAML.Spec.SecurityContext,
+			ReadOnly:           readOnly,
 			RestartPolicy:      ctrRestartPolicy,
 			SeccompPaths:       seccompPaths,
 			SecretsManager:     secretsManager,

pkg/specgen/generate/kube/kube.go

@@ -472,6 +472,8 @@ func ToSpecGen(ctx context.Context, opts *CtrSpecGenOptions) (*specgen.SpecGener
 	if ro := opts.ReadOnly; ro != itypes.OptionalBoolUndefined {
 		s.ReadOnlyFilesystem = (ro == itypes.OptionalBoolTrue)
 	}
+	// This should default to true for kubernetes yaml
+	s.ReadWriteTmpfs = true
 
 	// Make sure the container runs in a systemd unit which is
 	// stored as a label at container creation.

pkg/specgen/generate/storage.go

@@ -159,14 +159,19 @@ func finalizeMounts(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Ru
 	// Check for conflicts between named volumes and mounts
 	for dest := range baseMounts {
 		if _, ok := baseVolumes[dest]; ok {
-			return nil, nil, nil, fmt.Errorf("conflict at mount destination %v: %w", dest, specgen.ErrDuplicateDest)
+			return nil, nil, nil, fmt.Errorf("baseMounts conflict at mount destination %v: %w", dest, specgen.ErrDuplicateDest)
 		}
 	}
 	for dest := range baseVolumes {
 		if _, ok := baseMounts[dest]; ok {
-			return nil, nil, nil, fmt.Errorf("conflict at mount destination %v: %w", dest, specgen.ErrDuplicateDest)
+			return nil, nil, nil, fmt.Errorf("baseVolumes conflict at mount destination %v: %w", dest, specgen.ErrDuplicateDest)
 		}
 	}
+
+	if s.ReadWriteTmpfs {
+		baseMounts = addReadWriteTmpfsMounts(baseMounts, s.Volumes)
+	}
+
 	// Final step: maps to arrays
 	finalMounts := make([]spec.Mount, 0, len(baseMounts))
 	for _, mount := range baseMounts {
@@ -427,3 +432,29 @@ func InitFSMounts(mounts []spec.Mount) error {
 	}
 	return nil
 }
+
+func addReadWriteTmpfsMounts(mounts map[string]spec.Mount, volumes []*specgen.NamedVolume) map[string]spec.Mount {
+	readonlyTmpfs := []string{"/tmp", "/var/tmp", "/run"}
+	options := []string{"rw", "rprivate", "nosuid", "nodev", "tmpcopyup"}
+	for _, dest := range readonlyTmpfs {
+		if _, ok := mounts[dest]; ok {
+			continue
+		}
+		for _, m := range volumes {
+			if m.Dest == dest {
+				continue
+			}
+		}
+		mnt := spec.Mount{
+			Destination: dest,
+			Type:        define.TypeTmpfs,
+			Source:      define.TypeTmpfs,
+			Options:     options,
+		}
+		if dest != "/run" {
+			mnt.Options = append(mnt.Options, "noexec")
+		}
+		mounts[dest] = mnt
+	}
+	return mounts
+}

pkg/specgen/specgen.go

@@ -384,6 +384,10 @@ type ContainerSecurityConfig struct {
 	// ReadOnlyFilesystem indicates that everything will be mounted
 	// as read-only
 	ReadOnlyFilesystem bool `json:"read_only_filesystem,omitempty"`
+	// ReadWriteTmpfs indicates that when running with a ReadOnlyFilesystem
+	// mount temporary file systems
+	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

@@ -592,10 +592,11 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *entities.ContainerCreateOptions
 		s.DependencyContainers = c.Requires
 	}
 
-	// TODO
+	// Only add ReadWrite tmpfs mounts iff the container is
-	// outside of specgen and oci though
+	// being run ReadOnly and ReadWriteTmpFS is not disabled,
-	// defaults to true, check spec/storage
+	// (user specifying --read-only-tmpfs=false.)
-	// s.readonly = c.ReadOnlyTmpFS
+	s.ReadWriteTmpfs = c.ReadOnly && c.ReadWriteTmpFS
+
 	//  TODO convert to map?
 	// check if key=value and convert
 	sysmap := make(map[string]string)
@@ -853,10 +854,6 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *entities.ContainerCreateOptions
 		s.PasswdEntry = c.PasswdEntry
 	}
 
-	if c.ReadOnly && c.ReadOnlyTmpFS {
-		s.Mounts = addReadOnlyMounts(s.Mounts)
-	}
-
 	return nil
 }
 

pkg/specgenutil/volumes.go

@@ -703,26 +703,3 @@ func validChownFlag(flag string) (bool, error) {
 func unixPathClean(p string) string {
 	return path.Clean(p)
 }
-
-func addReadOnlyMounts(mounts []spec.Mount) []spec.Mount {
-	readonlyTmpfs := []string{"/tmp", "/var/tmp", "/run"}
-	options := []string{"rw", "rprivate", "nosuid", "nodev", "tmpcopyup"}
-	for _, dest := range readonlyTmpfs {
-		for _, m := range mounts {
-			if m.Destination == dest {
-				continue
-			}
-		}
-		mnt := spec.Mount{
-			Destination: dest,
-			Type:        define.TypeTmpfs,
-			Source:      define.TypeTmpfs,
-			Options:     options,
-		}
-		if dest != "/run" {
-			mnt.Options = append(mnt.Options, "noexec")
-		}
-		mounts = append(mounts, mnt)
-	}
-	return mounts
-}

test/system/030-run.bats

@@ -951,4 +951,17 @@ $IMAGE--c_ok" \
     run_podman stop -t 0 $cid
 }
 
+@test "podman run read-only from containers.conf" {
+    containersconf=$PODMAN_TMPDIR/containers.conf
+    cat >$containersconf <<EOF
+[containers]
+read_only=true
+EOF
+
+    CONTAINERS_CONF="$containersconf" run_podman 1 run --rm $IMAGE touch /testro
+    CONTAINERS_CONF="$containersconf" run_podman run --rm --read-only=false $IMAGE touch /testrw
+    CONTAINERS_CONF="$containersconf" run_podman run --rm $IMAGE touch /tmp/testrw
+    CONTAINERS_CONF="$containersconf" run_podman 1 run --rm --read-only-tmpfs=false $IMAGE touch /tmp/testro
+}
+
 # vim: filetype=sh

test/system/700-play.bats

@@ -238,6 +238,41 @@ EOF
     run_podman 1 container exists pod1-test3
 }
 
+@test "podman kube play read-only from containers.conf" {
+    containersconf=$PODMAN_TMPDIR/containers.conf
+    cat >$containersconf <<EOF
+[containers]
+read_only=true
+EOF
+
+    YAML=$PODMAN_TMPDIR/test.yml
+    CONTAINERS_CONF="$containersconf" run_podman create --pod new:pod1 --read-only=false --name test1 $IMAGE touch /testrw
+    CONTAINERS_CONF="$containersconf" run_podman create --pod pod1 --name test2 $IMAGE touch /testro
+    CONTAINERS_CONF="$containersconf" run_podman create --pod pod1 --name test3 $IMAGE touch /tmp/testtmp
+    CONTAINERS_CONF="$containersconf" run_podman container inspect --format '{{.HostConfig.ReadonlyRootfs}}' test1 test2 test3
+    is "$output" "false.*true.*true" "Rootfs should be read/only"
+
+    # Now generate and run kube.yaml on a machine without the defaults set
+    CONTAINERS_CONF="$containersconf" run_podman kube generate pod1 -f $YAML
+    cat $YAML
+
+    run_podman kube play --replace $YAML
+    run_podman container inspect --format '{{.HostConfig.ReadonlyRootfs}}' pod1-test1 pod1-test2 pod1-test3
+    is "$output" "false.*true.*true" "Rootfs should be read/only"
+
+    run_podman inspect --format "{{.State.ExitCode}}" pod1-test1
+    is "$output" "0" "Container / should be read/write"
+    run_podman inspect --format "{{.State.ExitCode}}" pod1-test2
+    is "$output" "1" "Container / should be read/only"
+    run_podman inspect --format "{{.State.ExitCode}}" pod1-test3
+    is "$output" "0" "/tmp in a read-only container should be read/write"
+
+    run_podman kube down - < $YAML
+    run_podman 1 container exists pod1-test1
+    run_podman 1 container exists pod1-test2
+    run_podman 1 container exists pod1-test3
+}
+
 @test "podman play with user from image" {
     TESTDIR=$PODMAN_TMPDIR/testdir
     mkdir -p $TESTDIR