diff --git a/DOWNLOADS.md b/DOWNLOADS.md index 1140aafad6..046ff5eabb 100644 --- a/DOWNLOADS.md +++ b/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. --> diff --git a/cmd/podman/common/create.go b/cmd/podman/common/create.go index 853e37b59a..42b6d28d56 100644 --- a/cmd/podman/common/create.go +++ b/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, - "read-only-tmpfs", cf.ReadOnlyTmpFS, + &cf.ReadWriteTmpFS, + "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" diff --git a/cmd/podman/common/create_opts.go b/cmd/podman/common/create_opts.go index d77df29edb..b67ee8f707 100644 --- a/cmd/podman/common/create_opts.go +++ b/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" diff --git a/cmd/podman/containers/create.go b/cmd/podman/containers/create.go index e15877023f..d9da303cf5 100644 --- a/cmd/podman/containers/create.go +++ b/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 diff --git a/pkg/api/handlers/compat/containers_create.go b/pkg/api/handlers/compat/containers_create.go index 234c101c81..107070f2c0 100644 --- a/pkg/api/handlers/compat/containers_create.go +++ b/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, diff --git a/pkg/domain/entities/pods.go b/pkg/domain/entities/pods.go index a70e3c8dca..36676d56d2 100644 --- a/pkg/domain/entities/pods.go +++ b/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 diff --git a/pkg/domain/infra/abi/play.go b/pkg/domain/infra/abi/play.go index 430ce7bd41..e3752c0a61 100644 --- a/pkg/domain/infra/abi/play.go +++ b/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, diff --git a/pkg/specgen/generate/kube/kube.go b/pkg/specgen/generate/kube/kube.go index 4ca74fc40e..29704e6149 100644 --- a/pkg/specgen/generate/kube/kube.go +++ b/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. diff --git a/pkg/specgen/generate/storage.go b/pkg/specgen/generate/storage.go index 6b131c1ddc..ffaaa0e1fb 100644 --- a/pkg/specgen/generate/storage.go +++ b/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 +} diff --git a/pkg/specgen/specgen.go b/pkg/specgen/specgen.go index 6daa4dc018..2e7078115e 100644 --- a/pkg/specgen/specgen.go +++ b/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. diff --git a/pkg/specgenutil/specgen.go b/pkg/specgenutil/specgen.go index 8c64508bef..695018f6fc 100644 --- a/pkg/specgenutil/specgen.go +++ b/pkg/specgenutil/specgen.go @@ -592,10 +592,11 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *entities.ContainerCreateOptions s.DependencyContainers = c.Requires } - // TODO - // outside of specgen and oci though - // defaults to true, check spec/storage - // s.readonly = c.ReadOnlyTmpFS + // Only add ReadWrite tmpfs mounts iff the container is + // being run ReadOnly and ReadWriteTmpFS is not disabled, + // (user specifying --read-only-tmpfs=false.) + 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 } diff --git a/pkg/specgenutil/volumes.go b/pkg/specgenutil/volumes.go index 6b6c0d91ce..c70206addf 100644 --- a/pkg/specgenutil/volumes.go +++ b/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 -} diff --git a/test/system/030-run.bats b/test/system/030-run.bats index fff16a1d59..1710083939 100644 --- a/test/system/030-run.bats +++ b/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 <$containersconf <