Merge pull request #7006 from ashley-cui/umask

Add --umask flag for create, run

cmd/podman/common/create.go

@@ -463,6 +463,11 @@ func GetCreateFlags(cf *ContainerCLIOpts) *pflag.FlagSet {
 		"tz", containerConfig.TZ(),
 		"Set timezone in container",
 	)
+	createFlags.StringVar(
+		&cf.Umask,
+		"umask", containerConfig.Umask(),
+		"Set umask in container",
+	)
 	createFlags.StringSliceVar(
 		&cf.UIDMap,
 		"uidmap", []string{},

cmd/podman/common/create_opts.go

@@ -93,6 +93,7 @@ type ContainerCLIOpts struct {
 	TmpFS             []string
 	TTY               bool
 	Timezone          string
+	Umask             string
 	UIDMap            []string
 	Ulimit            []string
 	User              string

cmd/podman/common/specgen.go

@@ -613,6 +613,7 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *ContainerCLIOpts, args []string
 	s.Remove = c.Rm
 	s.StopTimeout = &c.StopTimeout
 	s.Timezone = c.Timezone
+	s.Umask = c.Umask
 
 	return nil
 }

completions/bash/podman

@@ -2119,12 +2119,13 @@ _podman_container_run() {
 		--shm-size
 		--stop-signal
 		--stop-timeout
-		--tmpfs
-		--tz
 		--subgidname
 		--subuidname
 		--sysctl
 		--systemd
+		--tmpfs
+		--tz
+		--umask
 		--uidmap
 		--ulimit
 		--user -u

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

@@ -839,6 +839,10 @@ standard input.
 
 Set timezone in container. This flag takes area-based timezones, GMT time, as well as `local`, which sets the timezone in the container to match the host machine. See `/usr/share/zoneinfo/` for valid timezones.
 
+**--umask**=*umask*
+
+Set the umask inside the container. Defaults to `0022`.
+
 **--uidmap**=*container_uid:host_uid:amount*
 
 UID map for the user namespace.  Using this flag will run the container with user namespace enabled.  It conflicts with the `--userns` and `--subuidname` flags.

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

@@ -880,6 +880,10 @@ standard input.
 
 Set timezone in container. This flag takes area-based timezones, GMT time, as well as `local`, which sets the timezone in the container to match the host machine. See `/usr/share/zoneinfo/` for valid timezones.
 
+**--umask**=*umask*
+
+Set the umask inside the container. Defaults to `0022`.
+
 **--uidmap**=*container_uid*:*host_uid*:*amount*
 
 Run the container in a new user namespace using the supplied mapping. This option conflicts

go.mod

@@ -41,7 +41,7 @@ require (
 	github.com/opencontainers/image-spec v1.0.2-0.20190823105129-775207bd45b6
 	github.com/opencontainers/runc v1.0.0-rc91.0.20200708210054-ce54a9d4d79b
 	github.com/opencontainers/runtime-spec v1.0.3-0.20200520003142-237cc4f519e2
-	github.com/opencontainers/runtime-tools v0.9.0
+	github.com/opencontainers/runtime-tools v0.9.1-0.20200714183735-07406c5828aa
 	github.com/opencontainers/selinux v1.6.0
 	github.com/opentracing/opentracing-go v1.2.0
 	github.com/pkg/errors v0.9.1

go.sum

@@ -342,6 +342,8 @@ github.com/opencontainers/runtime-spec v1.0.3-0.20200520003142-237cc4f519e2/go.m
 github.com/opencontainers/runtime-tools v0.0.0-20181011054405-1d69bd0f9c39/go.mod h1:r3f7wjNzSs2extwzU3Y+6pKfobzPh+kKFJ3ofN+3nfs=
 github.com/opencontainers/runtime-tools v0.9.0 h1:FYgwVsKRI/H9hU32MJ/4MLOzXWodKK5zsQavY8NPMkU=
 github.com/opencontainers/runtime-tools v0.9.0/go.mod h1:r3f7wjNzSs2extwzU3Y+6pKfobzPh+kKFJ3ofN+3nfs=
+github.com/opencontainers/runtime-tools v0.9.1-0.20200714183735-07406c5828aa h1:iyj+fFHVBn0xOalz9UChYzSU1K0HJ+d75b4YqShBRhI=
+github.com/opencontainers/runtime-tools v0.9.1-0.20200714183735-07406c5828aa/go.mod h1:r3f7wjNzSs2extwzU3Y+6pKfobzPh+kKFJ3ofN+3nfs=
 github.com/opencontainers/selinux v1.3.0/go.mod h1:+BLncwf63G4dgOzykXAxcmnFlUaOlkDdmw/CqsW6pjs=
 github.com/opencontainers/selinux v1.5.1/go.mod h1:yTcKuYAh6R95iDpefGLQaPaRwJFwyzAJufJyiTt7s0g=
 github.com/opencontainers/selinux v1.5.2/go.mod h1:yTcKuYAh6R95iDpefGLQaPaRwJFwyzAJufJyiTt7s0g=

libpod/container.go

@@ -437,6 +437,9 @@ type ContainerConfig struct {
 	// Timezone is the timezone inside the container.
 	// Local means it has the same timezone as the host machine
 	Timezone string `json:"timezone,omitempty"`
+
+	// Umask is the umask inside the container.
+	Umask string `json:"umask,omitempty"`
 }
 
 // ContainerNamedVolume is a named volume that will be mounted into the
@@ -1276,5 +1279,8 @@ func (c *Container) AutoRemove() bool {
 
 func (c *Container) Timezone() string {
 	return c.config.Timezone
-
+}
+
+func (c *Container) Umask() string {
+	return c.config.Umask
 }

libpod/container_inspect.go

@@ -325,6 +325,14 @@ func (c *Container) generateInspectContainerConfig(spec *spec.Spec) *define.Insp
 
 	ctrConfig.Timezone = c.config.Timezone
 
+	// Pad Umask to 4 characters
+	if len(c.config.Umask) < 4 {
+		pad := strings.Repeat("0", 4-len(c.config.Umask))
+		ctrConfig.Umask = pad + c.config.Umask
+	} else {
+		ctrConfig.Umask = c.config.Umask
+	}
+
 	return ctrConfig
 }
 

libpod/container_internal_linux.go

@@ -355,6 +355,14 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) {
 		g.SetProcessGID(uint32(execUser.Gid))
 	}
 
+	if c.config.Umask != "" {
+		decVal, err := strconv.ParseUint(c.config.Umask, 8, 32)
+		if err != nil {
+			return nil, errors.Wrapf(err, "Invalid Umask Value")
+		}
+		g.SetProcessUmask(uint32(decVal))
+	}
+
 	// Add addition groups if c.config.GroupAdd is not empty
 	if len(c.config.Groups) > 0 {
 		gids, err := lookup.GetContainerGroups(c.config.Groups, c.state.Mountpoint, overrides)

libpod/define/config.go

@@ -20,6 +20,8 @@ var (
 	NameRegex = regexp.MustCompile("^[a-zA-Z0-9][a-zA-Z0-9_.-]*$")
 	// RegexError is thrown in presence of an invalid container/pod name.
 	RegexError = errors.Wrapf(ErrInvalidArg, "names must match [a-zA-Z0-9][a-zA-Z0-9_.-]*")
+	// UmaskRegex is a regular expression to validate Umask.
+	UmaskRegex = regexp.MustCompile(`^[0-7]{1,4}$`)
 )
 
 const (

libpod/define/container_inspect.go

@@ -61,6 +61,8 @@ type InspectContainerConfig struct {
 	// systemd mode, the container configuration is customized to optimize
 	// running systemd in the container.
 	SystemdMode bool `json:"SystemdMode,omitempty"`
+	// Umask is the umask inside the container.
+	Umask string `json:"Umask,omitempty"`
 }
 
 // InspectRestartPolicy holds information about the container's restart policy.

libpod/options.go

@@ -1607,6 +1607,20 @@ func WithTimezone(path string) CtrCreateOption {
 	}
 }
 
+// WithUmask sets the umask in the container
+func WithUmask(umask string) CtrCreateOption {
+	return func(ctr *Container) error {
+		if ctr.valid {
+			return define.ErrCtrFinalized
+		}
+		if !define.UmaskRegex.MatchString(umask) {
+			return errors.Wrapf(define.ErrInvalidArg, "Invalid umask string %s", umask)
+		}
+		ctr.config.Umask = umask
+		return nil
+	}
+}
+
 // Pod Creation Options
 
 // WithPodName sets the name of the pod.

pkg/specgen/generate/container_create.go

@@ -145,6 +145,9 @@ func createContainerOptions(ctx context.Context, rt *libpod.Runtime, s *specgen.
 	if s.Timezone != "" {
 		options = append(options, libpod.WithTimezone(s.Timezone))
 	}
+	if s.Umask != "" {
+		options = append(options, libpod.WithUmask(s.Umask))
+	}
 
 	useSystemd := false
 	switch s.Systemd {

pkg/specgen/specgen.go

@@ -287,6 +287,8 @@ type ContainerSecurityConfig struct {
 	// ReadOnlyFilesystem indicates that everything will be mounted
 	// as read-only
 	ReadOnlyFilesystem bool `json:"read_only_filesystem,omittempty"`
+	// Umask is the umask the init process of the container will be run with.
+	Umask string `json:"umask,omitempty"`
 }
 
 // ContainerCgroupConfig contains configuration information about a container's

test/e2e/config/containers.conf

@@ -50,3 +50,5 @@ dns_servers=[ "1.2.3.4", ]
 dns_options=[ "debug", ]
 
 tz = "Pacific/Honolulu"
+
+umask = "0002"

test/e2e/containers_conf_test.go

@@ -218,6 +218,17 @@ var _ = Describe("Podman run", func() {
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 		Expect(session.OutputToString()).To(ContainSubstring("HST"))
-
 	})
+	It("podman run containers.conf umask", func() {
+		//containers.conf umask set to 0002
+		if !strings.Contains(podmanTest.OCIRuntime, "crun") {
+			Skip("Test only works on crun")
+		}
+
+		session := podmanTest.Podman([]string{"run", "--rm", ALPINE, "sh", "-c", "umask"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(session.OutputToString()).To(Equal("0002"))
+	})
+
 })

test/e2e/create_test.go

@@ -5,6 +5,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"strings"
 
 	. "github.com/containers/libpod/v2/test/utils"
 	. "github.com/onsi/ginkgo"
@@ -499,4 +500,46 @@ var _ = Describe("Podman create", func() {
 		Expect(data[0].Config.Timezone).To(Equal("local"))
 	})
 
+	It("podman create --umask", func() {
+		if !strings.Contains(podmanTest.OCIRuntime, "crun") {
+			Skip("Test only works on crun")
+		}
+
+		session := podmanTest.Podman([]string{"create", "--name", "default", ALPINE})
+		session.WaitWithDefaultTimeout()
+		inspect := podmanTest.Podman([]string{"inspect", "default"})
+		inspect.WaitWithDefaultTimeout()
+		data := inspect.InspectContainerToJSON()
+		Expect(len(data)).To(Equal(1))
+		Expect(data[0].Config.Umask).To(Equal("0022"))
+
+		session = podmanTest.Podman([]string{"create", "--umask", "0002", "--name", "umask", ALPINE})
+		session.WaitWithDefaultTimeout()
+		inspect = podmanTest.Podman([]string{"inspect", "umask"})
+		inspect.WaitWithDefaultTimeout()
+		data = inspect.InspectContainerToJSON()
+		Expect(len(data)).To(Equal(1))
+		Expect(data[0].Config.Umask).To(Equal("0002"))
+
+		session = podmanTest.Podman([]string{"create", "--umask", "0077", "--name", "fedora", fedoraMinimal})
+		session.WaitWithDefaultTimeout()
+		inspect = podmanTest.Podman([]string{"inspect", "fedora"})
+		inspect.WaitWithDefaultTimeout()
+		data = inspect.InspectContainerToJSON()
+		Expect(len(data)).To(Equal(1))
+		Expect(data[0].Config.Umask).To(Equal("0077"))
+
+		session = podmanTest.Podman([]string{"create", "--umask", "22", "--name", "umask-short", ALPINE})
+		session.WaitWithDefaultTimeout()
+		inspect = podmanTest.Podman([]string{"inspect", "umask-short"})
+		inspect.WaitWithDefaultTimeout()
+		data = inspect.InspectContainerToJSON()
+		Expect(len(data)).To(Equal(1))
+		Expect(data[0].Config.Umask).To(Equal("0022"))
+
+		session = podmanTest.Podman([]string{"create", "--umask", "9999", "--name", "bad", ALPINE})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Not(Equal(0)))
+		Expect(session.ErrorToString()).To(ContainSubstring("Invalid umask"))
+	})
 })

test/e2e/run_test.go

@@ -1081,4 +1081,35 @@ USER mail`
 		Expect(session.ExitCode()).To(Equal(0))
 		Expect(session.OutputToString()).To(ContainSubstring(limit))
 	})
+
+	It("podman run umask", func() {
+		if !strings.Contains(podmanTest.OCIRuntime, "crun") {
+			Skip("Test only works on crun")
+		}
+
+		session := podmanTest.Podman([]string{"run", "--rm", ALPINE, "sh", "-c", "umask"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(session.OutputToString()).To(Equal("0022"))
+
+		session = podmanTest.Podman([]string{"run", "--umask", "0002", "--rm", ALPINE, "sh", "-c", "umask"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(session.OutputToString()).To(Equal("0002"))
+
+		session = podmanTest.Podman([]string{"run", "--umask", "0077", "--rm", fedoraMinimal, "umask"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(session.OutputToString()).To(Equal("0077"))
+
+		session = podmanTest.Podman([]string{"run", "--umask", "22", "--rm", ALPINE, "sh", "-c", "umask"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(session.OutputToString()).To(Equal("0022"))
+
+		session = podmanTest.Podman([]string{"run", "--umask", "9999", "--rm", ALPINE, "sh", "-c", "umask"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Not(Equal(0)))
+		Expect(session.ErrorToString()).To(ContainSubstring("Invalid umask"))
+	})
 })

vendor/github.com/opencontainers/runtime-tools/generate/generate.go

@@ -29,6 +29,9 @@ var (
 type Generator struct {
 	Config       *rspec.Spec
 	HostSpecific bool
+	// This is used to keep a cache of the ENVs added to improve
+	// performance when adding a huge number of ENV variables
+	envMap map[string]int
 }
 
 // ExportOptions have toggles for exporting only certain parts of the specification
@@ -236,7 +239,12 @@ func New(os string) (generator Generator, err error) {
 		}
 	}
 
-	return Generator{Config: &config}, nil
+	envCache := map[string]int{}
+	if config.Process != nil {
+		envCache = createEnvCacheMap(config.Process.Env)
+	}
+
+	return Generator{Config: &config, envMap: envCache}, nil
 }
 
 // NewFromSpec creates a configuration Generator from a given
@@ -246,8 +254,14 @@ func New(os string) (generator Generator, err error) {
 //
 //   generator := Generator{Config: config}
 func NewFromSpec(config *rspec.Spec) Generator {
+	envCache := map[string]int{}
+	if config != nil && config.Process != nil {
+		envCache = createEnvCacheMap(config.Process.Env)
+	}
+
 	return Generator{
 		Config: config,
+		envMap: envCache,
 	}
 }
 
@@ -273,11 +287,27 @@ func NewFromTemplate(r io.Reader) (Generator, error) {
 	if err := json.NewDecoder(r).Decode(&config); err != nil {
 		return Generator{}, err
 	}
+
+	envCache := map[string]int{}
+	if config.Process != nil {
+		envCache = createEnvCacheMap(config.Process.Env)
+	}
+
 	return Generator{
 		Config: &config,
+		envMap: envCache,
 	}, nil
 }
 
+// createEnvCacheMap creates a hash map with the ENV variables given by the config
+func createEnvCacheMap(env []string) map[string]int {
+	envMap := make(map[string]int, len(env))
+	for i, val := range env {
+		envMap[val] = i
+	}
+	return envMap
+}
+
 // SetSpec sets the configuration in the Generator g.
 //
 // Deprecated: Replace with:
@@ -414,6 +444,12 @@ func (g *Generator) SetProcessUsername(username string) {
 	g.Config.Process.User.Username = username
 }
 
+// SetProcessUmask sets g.Config.Process.User.Umask.
+func (g *Generator) SetProcessUmask(umask uint32) {
+	g.initConfigProcess()
+	g.Config.Process.User.Umask = umask
+}
+
 // SetProcessGID sets g.Config.Process.User.GID.
 func (g *Generator) SetProcessGID(gid uint32) {
 	g.initConfigProcess()
@@ -456,21 +492,44 @@ func (g *Generator) ClearProcessEnv() {
 		return
 	}
 	g.Config.Process.Env = []string{}
+	// Clear out the env cache map as well
+	g.envMap = map[string]int{}
 }
 
 // AddProcessEnv adds name=value into g.Config.Process.Env, or replaces an
 // existing entry with the given name.
 func (g *Generator) AddProcessEnv(name, value string) {
+	if name == "" {
+		return
+	}
+
+	g.initConfigProcess()
+	g.addEnv(fmt.Sprintf("%s=%s", name, value), name)
+}
+
+// AddMultipleProcessEnv adds multiple name=value into g.Config.Process.Env, or replaces
+// existing entries with the given name.
+func (g *Generator) AddMultipleProcessEnv(envs []string) {
 	g.initConfigProcess()
 
-	env := fmt.Sprintf("%s=%s", name, value)
-	for idx := range g.Config.Process.Env {
-		if strings.HasPrefix(g.Config.Process.Env[idx], name+"=") {
-			g.Config.Process.Env[idx] = env
-			return
-		}
+	for _, val := range envs {
+		split := strings.SplitN(val, "=", 2)
+		g.addEnv(val, split[0])
+	}
+}
+
+// addEnv looks through adds ENV to the Process and checks envMap for
+// any duplicates
+// This is called by both AddMultipleProcessEnv and AddProcessEnv
+func (g *Generator) addEnv(env, key string) {
+	if idx, ok := g.envMap[key]; ok {
+		// The ENV exists in the cache, so change its value in g.Config.Process.Env
+		g.Config.Process.Env[idx] = env
+	} else {
+		// else the env doesn't exist, so add it and add it's index to g.envMap
+		g.Config.Process.Env = append(g.Config.Process.Env, env)
+		g.envMap[key] = len(g.Config.Process.Env) - 1
 	}
-	g.Config.Process.Env = append(g.Config.Process.Env, env)
 }
 
 // AddProcessRlimits adds rlimit into g.Config.Process.Rlimits.
@@ -1443,7 +1502,7 @@ func (g *Generator) AddDevice(device rspec.LinuxDevice) {
 			return
 		}
 		if dev.Type == device.Type && dev.Major == device.Major && dev.Minor == device.Minor {
-			fmt.Fprintln(os.Stderr, "WARNING: The same type, major and minor should not be used for multiple devices.")
+			fmt.Fprintf(os.Stderr, "WARNING: Creating device %q with same type, major and minor as existing %q.\n", device.Path, dev.Path)
 		}
 	}
 

vendor/github.com/opencontainers/runtime-tools/generate/seccomp/seccomp_default.go

@@ -566,6 +566,20 @@ func DefaultProfile(rs *specs.Spec) *rspec.LinuxSeccomp {
 			},
 		}...)
 		/* Flags parameter of the clone syscall is the 2nd on s390 */
+		syscalls = append(syscalls, []rspec.LinuxSyscall{
+			{
+				Names:  []string{"clone"},
+				Action: rspec.ActAllow,
+				Args: []rspec.LinuxSeccompArg{
+					{
+						Index:    1,
+						Value:    2080505856,
+						ValueTwo: 0,
+						Op:       rspec.OpMaskedEqual,
+					},
+				},
+			},
+		}...)
 	}
 
 	return &rspec.LinuxSeccomp{

vendor/modules.txt

@@ -421,7 +421,7 @@ github.com/opencontainers/runc/libcontainer/user
 github.com/opencontainers/runc/libcontainer/utils
 # github.com/opencontainers/runtime-spec v1.0.3-0.20200520003142-237cc4f519e2
 github.com/opencontainers/runtime-spec/specs-go
-# github.com/opencontainers/runtime-tools v0.9.0
+# github.com/opencontainers/runtime-tools v0.9.1-0.20200714183735-07406c5828aa
 github.com/opencontainers/runtime-tools/error
 github.com/opencontainers/runtime-tools/filepath
 github.com/opencontainers/runtime-tools/generate