Skip to content

Commit

Permalink
--read-write-tmpfs=false should set /dev/* tmpfs to readonly
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
rhatdan committed Feb 21, 2022
1 parent 4ad98b9 commit ecc6563
Show file tree
Hide file tree
Showing 11 changed files with 35 additions and 18 deletions.
4 changes: 2 additions & 2 deletions cmd/podman/common/create.go
Expand Up @@ -493,8 +493,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"
Expand Down
2 changes: 1 addition & 1 deletion cmd/podman/common/create_opts.go
Expand Up @@ -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,
Expand Down
3 changes: 3 additions & 0 deletions cmd/podman/containers/create.go
Expand Up @@ -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") {
Expand Down
2 changes: 2 additions & 0 deletions cmd/podman/utils/alias.go
Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions docs/source/markdown/podman-create.1.md
Expand Up @@ -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**

Expand Down
4 changes: 2 additions & 2 deletions docs/source/markdown/podman-run.1.md
Expand Up @@ -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**

Expand Down
2 changes: 1 addition & 1 deletion pkg/domain/entities/pods.go
Expand Up @@ -226,7 +226,7 @@ type ContainerCreateOptions struct {
Pull string
Quiet bool
ReadOnly bool
ReadOnlyTmpFS bool
ReadWriteTmpFS bool
Restart string
Replace bool
Requires []string
Expand Down
15 changes: 13 additions & 2 deletions pkg/specgen/generate/oci.go
Expand Up @@ -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

Expand Down Expand Up @@ -430,5 +428,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
}
3 changes: 3 additions & 0 deletions pkg/specgen/specgen.go
Expand Up @@ -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.
Expand Down
6 changes: 2 additions & 4 deletions pkg/specgenutil/specgen.go
Expand Up @@ -522,16 +522,14 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *entities.ContainerCreateOptions
s.CapDrop = c.CapDrop
s.Privileged = c.Privileged
s.ReadOnlyFilesystem = c.ReadOnly
s.ReadOnlyTmpFS = !c.ReadWriteTmpFS
s.ConmonPidFile = c.ConmonPIDFile

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)
Expand Down Expand Up @@ -590,7 +588,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
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/specgenutil/volumes.go
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Expand Down

0 comments on commit ecc6563

Please sign in to comment.