Support uid,gid,mode options for secrets

Support UID, GID, Mode options for mount type secrets. Also, change
default secret permissions to 444 so all users can read secret.

Signed-off-by: Ashley Cui <acui@redhat.com>

cmd/podman/common/specgen.go

@@ -778,20 +778,30 @@ func parseThrottleIOPsDevices(iopsDevices []string) (map[string]specs.LinuxThrot
 	return td, nil
 }
 
-func parseSecrets(secrets []string) ([]string, map[string]string, error) {
+func parseSecrets(secrets []string) ([]specgen.Secret, map[string]string, error) {
 	secretParseError := errors.New("error parsing secret")
-	var mount []string
+	var mount []specgen.Secret
 	envs := make(map[string]string)
 	for _, val := range secrets {
+		// mount only tells if user has set an option that can only be used with mount secret type
+		mountOnly := false
 		source := ""
 		secretType := ""
 		target := ""
+		var uid, gid uint32
+		// default mode 444 octal = 292 decimal
+		var mode uint32 = 292
 		split := strings.Split(val, ",")
 
 		// --secret mysecret
 		if len(split) == 1 {
-			source = val
-			mount = append(mount, source)
+			mountSecret := specgen.Secret{
+				Source: val,
+				UID:    uid,
+				GID:    gid,
+				Mode:   mode,
+			}
+			mount = append(mount, mountSecret)
 			continue
 		}
 		// --secret mysecret,opt=opt
@@ -799,7 +809,7 @@ func parseSecrets(secrets []string) ([]string, map[string]string, error) {
 			source = split[0]
 			split = split[1:]
 		}
-		// TODO: implement other secret options
+
 		for _, val := range split {
 			kv := strings.SplitN(val, "=", 2)
 			if len(kv) < 2 {
@@ -818,6 +828,28 @@ func parseSecrets(secrets []string) ([]string, map[string]string, error) {
 				secretType = kv[1]
 			case "target":
 				target = kv[1]
+			case "mode":
+				mountOnly = true
+				mode64, err := strconv.ParseUint(kv[1], 8, 32)
+				if err != nil {
+					return nil, nil, errors.Wrapf(secretParseError, "mode %s invalid", kv[1])
+				}
+				mode = uint32(mode64)
+			case "uid", "UID":
+				mountOnly = true
+				uid64, err := strconv.ParseUint(kv[1], 10, 32)
+				if err != nil {
+					return nil, nil, errors.Wrapf(secretParseError, "UID %s invalid", kv[1])
+				}
+				uid = uint32(uid64)
+			case "gid", "GID":
+				mountOnly = true
+				gid64, err := strconv.ParseUint(kv[1], 10, 32)
+				if err != nil {
+					return nil, nil, errors.Wrapf(secretParseError, "GID %s invalid", kv[1])
+				}
+				gid = uint32(gid64)
+
 			default:
 				return nil, nil, errors.Wrapf(secretParseError, "option %s invalid", val)
 			}
@@ -833,9 +865,18 @@ func parseSecrets(secrets []string) ([]string, map[string]string, error) {
 			if target != "" {
 				return nil, nil, errors.Wrapf(secretParseError, "target option is invalid for mounted secrets")
 			}
-			mount = append(mount, source)
+			mountSecret := specgen.Secret{
+				Source: source,
+				UID:    uid,
+				GID:    gid,
+				Mode:   mode,
+			}
+			mount = append(mount, mountSecret)
 		}
 		if secretType == "env" {
+			if mountOnly {
+				return nil, nil, errors.Wrap(secretParseError, "UID, GID, Mode options cannot be set with secret type env")
+			}
 			if target == "" {
 				target = source
 			}

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

@@ -859,6 +859,9 @@ Secret Options
 
 - `type=mount|env`    : How the secret will be exposed to the container. Default mount.
 - `target=target`     : Target of secret. Defauts to secret name.
+- `uid=0`             : UID of secret. Defaults to 0. Mount secret type only.
+- `gid=0`             : GID of secret. Defaults to 0. Mount secret type only.
+- `mode=0`            : Mode of secret. Defaults to 0444. Mount secret type only.
 
 #### **--security-opt**=*option*
 

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

@@ -911,6 +911,9 @@ Secret Options
 
 - `type=mount|env`    : How the secret will be exposed to the container. Default mount.
 - `target=target`     : Target of secret. Defauts to secret name.
+- `uid=0`             : UID of secret. Defaults to 0. Mount secret type only.
+- `gid=0`             : GID of secret. Defaults to 0. Mount secret type only.
+- `mode=0`            : Mode of secret. Defaults to 0444. Mount secret type only.
 
 #### **--security-opt**=*option*
 

libpod/container.go

@@ -235,6 +235,18 @@ type ContainerImageVolume struct {
 	ReadWrite bool `json:"rw"`
 }
 
+// ContainerSecret is a secret that is mounted in a container
+type ContainerSecret struct {
+	// Secret is the secret
+	*secrets.Secret
+	// UID is tbe UID of the secret file
+	UID uint32
+	// GID is the GID of the secret file
+	GID uint32
+	// Mode is the mode of the secret file
+	Mode uint32
+}
+
 // ContainerNetworkDescriptions describes the relationship between the CNI
 // network and the ethN where N is an integer
 type ContainerNetworkDescriptions map[string]int
@@ -1124,7 +1136,7 @@ func (c *Container) Umask() string {
 }
 
 //Secrets return the secrets in the container
-func (c *Container) Secrets() []*secrets.Secret {
+func (c *Container) Secrets() []*ContainerSecret {
 	return c.config.Secrets
 }
 

libpod/container_config.go

@@ -148,7 +148,7 @@ type ContainerRootFSConfig struct {
 	// default, but others do not.
 	CreateWorkingDir bool `json:"createWorkingDir,omitempty"`
 	// Secrets lists secrets to mount into the container
-	Secrets []*secrets.Secret `json:"secrets,omitempty"`
+	Secrets []*ContainerSecret `json:"secrets,omitempty"`
 	// SecretPath is the secrets location in storage
 	SecretsPath string `json:"secretsPath"`
 	// Volatile specifies whether the container storage can be optimized

libpod/container_inspect.go

@@ -343,11 +343,13 @@ func (c *Container) generateInspectContainerConfig(spec *spec.Spec) *define.Insp
 	ctrConfig.CreateCommand = c.config.CreateCommand
 
 	ctrConfig.Timezone = c.config.Timezone
-
 	for _, secret := range c.config.Secrets {
 		newSec := define.InspectSecret{}
 		newSec.Name = secret.Name
 		newSec.ID = secret.ID
+		newSec.UID = secret.UID
+		newSec.GID = secret.GID
+		newSec.Mode = secret.Mode
 		ctrConfig.Secrets = append(ctrConfig.Secrets, &newSec)
 	}
 

libpod/container_internal.go

@@ -15,7 +15,7 @@ import (
 
 	metadata "github.com/checkpoint-restore/checkpointctl/lib"
 	"github.com/containers/buildah/copier"
-	"github.com/containers/common/pkg/secrets"
+	butil "github.com/containers/buildah/util"
 	"github.com/containers/podman/v3/libpod/define"
 	"github.com/containers/podman/v3/libpod/events"
 	"github.com/containers/podman/v3/pkg/cgroups"
@@ -24,6 +24,7 @@ import (
 	"github.com/containers/podman/v3/pkg/hooks/exec"
 	"github.com/containers/podman/v3/pkg/rootless"
 	"github.com/containers/podman/v3/pkg/selinux"
+	"github.com/containers/podman/v3/pkg/util"
 	"github.com/containers/storage"
 	"github.com/containers/storage/pkg/archive"
 	"github.com/containers/storage/pkg/idtools"
@@ -2235,21 +2236,31 @@ func (c *Container) hasNamespace(namespace spec.LinuxNamespaceType) bool {
 }
 
 // extractSecretToStorage copies a secret's data from the secrets manager to the container's static dir
-func (c *Container) extractSecretToCtrStorage(name string) error {
-	manager, err := secrets.NewManager(c.runtime.GetSecretsStorageDir())
+func (c *Container) extractSecretToCtrStorage(secr *ContainerSecret) error {
+	manager, err := c.runtime.SecretsManager()
 	if err != nil {
 		return err
 	}
-	secr, data, err := manager.LookupSecretData(name)
+	_, data, err := manager.LookupSecretData(secr.Name)
 	if err != nil {
 		return err
 	}
 	secretFile := filepath.Join(c.config.SecretsPath, secr.Name)
 
+	hostUID, hostGID, err := butil.GetHostIDs(util.IDtoolsToRuntimeSpec(c.config.IDMappings.UIDMap), util.IDtoolsToRuntimeSpec(c.config.IDMappings.GIDMap), secr.UID, secr.GID)
+	if err != nil {
+		return errors.Wrap(err, "unable to extract secret")
+	}
 	err = ioutil.WriteFile(secretFile, data, 0644)
 	if err != nil {
 		return errors.Wrapf(err, "unable to create %s", secretFile)
 	}
+	if err := os.Lchown(secretFile, int(hostUID), int(hostGID)); err != nil {
+		return err
+	}
+	if err := os.Chmod(secretFile, os.FileMode(secr.Mode)); err != nil {
+		return err
+	}
 	if err := label.Relabel(secretFile, c.config.MountLabel, false); err != nil {
 		return err
 	}

libpod/container_internal_linux.go

@@ -29,7 +29,6 @@ import (
 	"github.com/containers/common/pkg/apparmor"
 	"github.com/containers/common/pkg/chown"
 	"github.com/containers/common/pkg/config"
-	"github.com/containers/common/pkg/secrets"
 	"github.com/containers/common/pkg/subscriptions"
 	"github.com/containers/common/pkg/umask"
 	"github.com/containers/podman/v3/libpod/define"
@@ -759,7 +758,10 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) {
 		return nil, errors.Wrapf(err, "error setting up OCI Hooks")
 	}
 	if len(c.config.EnvSecrets) > 0 {
-		manager, err := secrets.NewManager(c.runtime.GetSecretsStorageDir())
+		manager, err := c.runtime.SecretsManager()
+		if err != nil {
+			return nil, err
+		}
 		if err != nil {
 			return nil, err
 		}
@@ -2339,7 +2341,7 @@ func (c *Container) createSecretMountDir() error {
 		oldUmask := umask.Set(0)
 		defer umask.Set(oldUmask)
 
-		if err := os.MkdirAll(src, 0644); err != nil {
+		if err := os.MkdirAll(src, 0755); err != nil {
 			return err
 		}
 		if err := label.Relabel(src, c.config.MountLabel, false); err != nil {

libpod/define/container_inspect.go

@@ -713,13 +713,16 @@ type DriverData struct {
 	Data map[string]string `json:"Data"`
 }
 
-// InspectHostPort provides information on a port on the host that a container's
-// port is bound to.
+// InspectSecret contains information on secrets mounted inside the container
 type InspectSecret struct {
-	// IP on the host we are bound to. "" if not specified (binding to all
-	// IPs).
+	// Name is the name of the secret
 	Name string `json:"Name"`
-	// Port on the host we are bound to. No special formatting - just an
-	// integer stuffed into a string.
+	// ID is the ID of the secret
 	ID string `json:"ID"`
+	// ID is the UID of the mounted secret file
+	UID uint32 `json:"UID"`
+	// ID is the GID of the mounted secret file
+	GID uint32 `json:"GID"`
+	// ID is the ID of the mode of the mounted secret file
+	Mode uint32 `json:"Mode"`
 }

libpod/options.go

@@ -1695,23 +1695,12 @@ func WithUmask(umask string) CtrCreateOption {
 }
 
 // WithSecrets adds secrets to the container
-func WithSecrets(secretNames []string) CtrCreateOption {
+func WithSecrets(containerSecrets []*ContainerSecret) CtrCreateOption {
 	return func(ctr *Container) error {
 		if ctr.valid {
 			return define.ErrCtrFinalized
 		}
-		manager, err := secrets.NewManager(ctr.runtime.GetSecretsStorageDir())
-		if err != nil {
-			return err
-		}
-		for _, name := range secretNames {
-			secr, err := manager.Lookup(name)
-			if err != nil {
-				return err
-			}
-			ctr.config.Secrets = append(ctr.config.Secrets, secr)
-		}
-
+		ctr.config.Secrets = containerSecrets
 		return nil
 	}
 }
@@ -1723,7 +1712,7 @@ func WithEnvSecrets(envSecrets map[string]string) CtrCreateOption {
 		if ctr.valid {
 			return define.ErrCtrFinalized
 		}
-		manager, err := secrets.NewManager(ctr.runtime.GetSecretsStorageDir())
+		manager, err := ctr.runtime.SecretsManager()
 		if err != nil {
 			return err
 		}

libpod/runtime.go

@@ -16,6 +16,7 @@ import (
 
 	"github.com/containers/common/libimage"
 	"github.com/containers/common/pkg/config"
+	"github.com/containers/common/pkg/secrets"
 	"github.com/containers/image/v5/pkg/sysregistriesv2"
 	is "github.com/containers/image/v5/storage"
 	"github.com/containers/image/v5/types"
@@ -103,6 +104,8 @@ type Runtime struct {
 
 	// noStore indicates whether we need to interact with a store or not
 	noStore bool
+	// secretsManager manages secrets
+	secretsManager *secrets.SecretsManager
 }
 
 // SetXdgDirs ensures the XDG_RUNTIME_DIR env and XDG_CONFIG_HOME variables are set.
@@ -1022,6 +1025,18 @@ func (r *Runtime) GetSecretsStorageDir() string {
 	return filepath.Join(r.store.GraphRoot(), "secrets")
 }
 
+// SecretsManager returns the directory that the secrets manager should take
+func (r *Runtime) SecretsManager() (*secrets.SecretsManager, error) {
+	if r.secretsManager == nil {
+		manager, err := secrets.NewManager(r.GetSecretsStorageDir())
+		if err != nil {
+			return nil, err
+		}
+		r.secretsManager = manager
+	}
+	return r.secretsManager, nil
+}
+
 func graphRootMounted() bool {
 	f, err := os.OpenFile("/run/.containerenv", os.O_RDONLY, os.ModePerm)
 	if err != nil {

libpod/runtime_ctr.go

@@ -366,7 +366,7 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Contai
 		return nil, err
 	}
 	for _, secr := range ctr.config.Secrets {
-		err = ctr.extractSecretToCtrStorage(secr.Name)
+		err = ctr.extractSecretToCtrStorage(secr)
 		if err != nil {
 			return nil, err
 		}

pkg/domain/infra/abi/play.go

@@ -12,7 +12,6 @@ import (
 
 	"github.com/containers/common/libimage"
 	"github.com/containers/common/pkg/config"
-	"github.com/containers/common/pkg/secrets"
 	"github.com/containers/image/v5/types"
 	"github.com/containers/podman/v3/libpod"
 	"github.com/containers/podman/v3/libpod/define"
@@ -161,7 +160,7 @@ func (ic *ContainerEngine) playKubePod(ctx context.Context, podName string, podY
 	)
 
 	// Create the secret manager before hand
-	secretsManager, err := secrets.NewManager(ic.Libpod.GetSecretsStorageDir())
+	secretsManager, err := ic.Libpod.SecretsManager()
 	if err != nil {
 		return nil, err
 	}

pkg/domain/infra/abi/secrets.go

@@ -6,7 +6,6 @@ import (
 	"io/ioutil"
 	"path/filepath"
 
-	"github.com/containers/common/pkg/secrets"
 	"github.com/containers/podman/v3/pkg/domain/entities"
 	"github.com/pkg/errors"
 )
@@ -14,7 +13,7 @@ import (
 func (ic *ContainerEngine) SecretCreate(ctx context.Context, name string, reader io.Reader, options entities.SecretCreateOptions) (*entities.SecretCreateReport, error) {
 	data, _ := ioutil.ReadAll(reader)
 	secretsPath := ic.Libpod.GetSecretsStorageDir()
-	manager, err := secrets.NewManager(secretsPath)
+	manager, err := ic.Libpod.SecretsManager()
 	if err != nil {
 		return nil, err
 	}
@@ -36,8 +35,7 @@ func (ic *ContainerEngine) SecretCreate(ctx context.Context, name string, reader
 }
 
 func (ic *ContainerEngine) SecretInspect(ctx context.Context, nameOrIDs []string) ([]*entities.SecretInfoReport, []error, error) {
-	secretsPath := ic.Libpod.GetSecretsStorageDir()
-	manager, err := secrets.NewManager(secretsPath)
+	manager, err := ic.Libpod.SecretsManager()
 	if err != nil {
 		return nil, nil, err
 	}
@@ -71,8 +69,7 @@ func (ic *ContainerEngine) SecretInspect(ctx context.Context, nameOrIDs []string
 }
 
 func (ic *ContainerEngine) SecretList(ctx context.Context) ([]*entities.SecretInfoReport, error) {
-	secretsPath := ic.Libpod.GetSecretsStorageDir()
-	manager, err := secrets.NewManager(secretsPath)
+	manager, err := ic.Libpod.SecretsManager()
 	if err != nil {
 		return nil, err
 	}
@@ -105,8 +102,7 @@ func (ic *ContainerEngine) SecretRm(ctx context.Context, nameOrIDs []string, opt
 		toRemove []string
 		reports  = []*entities.SecretRmReport{}
 	)
-	secretsPath := ic.Libpod.GetSecretsStorageDir()
-	manager, err := secrets.NewManager(secretsPath)
+	manager, err := ic.Libpod.SecretsManager()
 	if err != nil {
 		return nil, err
 	}

pkg/specgen/generate/container_create.go

@@ -400,7 +400,24 @@ func createContainerOptions(ctx context.Context, rt *libpod.Runtime, s *specgen.
 	}
 
 	if len(s.Secrets) != 0 {
-		options = append(options, libpod.WithSecrets(s.Secrets))
+		manager, err := rt.SecretsManager()
+		if err != nil {
+			return nil, err
+		}
+		var secrs []*libpod.ContainerSecret
+		for _, s := range s.Secrets {
+			secr, err := manager.Lookup(s.Source)
+			if err != nil {
+				return nil, err
+			}
+			secrs = append(secrs, &libpod.ContainerSecret{
+				Secret: secr,
+				UID:    s.UID,
+				GID:    s.GID,
+				Mode:   s.Mode,
+			})
+		}
+		options = append(options, libpod.WithSecrets(secrs))
 	}
 
 	if len(s.EnvSecrets) != 0 {

pkg/specgen/specgen.go

@@ -258,7 +258,7 @@ type ContainerStorageConfig struct {
 	RootfsPropagation string `json:"rootfs_propagation,omitempty"`
 	// Secrets are the secrets that will be added to the container
 	// Optional.
-	Secrets []string `json:"secrets,omitempty"`
+	Secrets []Secret `json:"secrets,omitempty"`
 	// Volatile specifies whether the container storage can be optimized
 	// at the cost of not syncing all the dirty files in memory.
 	Volatile bool `json:"volatile,omitempty"`
@@ -521,6 +521,13 @@ type PortMapping struct {
 	Protocol string `json:"protocol,omitempty"`
 }
 
+type Secret struct {
+	Source string
+	UID    uint32
+	GID    uint32
+	Mode   uint32
+}
+
 var (
 	// ErrNoStaticIPRootless is used when a rootless user requests to assign a static IP address
 	// to a pod or container

test/e2e/run_test.go

@@ -1669,6 +1669,49 @@ WORKDIR /madethis`, BB)
 		Expect(session.OutputToString()).To(Equal(secretsString))
 	})
 
+	It("podman run --secret mount with uid, gid, mode options", func() {
+		secretsString := "somesecretdata"
+		secretFilePath := filepath.Join(podmanTest.TempDir, "secret")
+		err := ioutil.WriteFile(secretFilePath, []byte(secretsString), 0755)
+		Expect(err).To(BeNil())
+
+		session := podmanTest.Podman([]string{"secret", "create", "mysecret", secretFilePath})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+
+		// check default permissions
+		session = podmanTest.Podman([]string{"run", "--secret", "mysecret", "--name", "secr", ALPINE, "ls", "-l", "/run/secrets/mysecret"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		output := session.OutputToString()
+		Expect(output).To(ContainSubstring("-r--r--r--"))
+		Expect(output).To(ContainSubstring("root"))
+
+		session = podmanTest.Podman([]string{"run", "--secret", "source=mysecret,type=mount,uid=1000,gid=1001,mode=777", "--name", "secr2", ALPINE, "ls", "-ln", "/run/secrets/mysecret"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		output = session.OutputToString()
+		Expect(output).To(ContainSubstring("-rwxrwxrwx"))
+		Expect(output).To(ContainSubstring("1000"))
+		Expect(output).To(ContainSubstring("1001"))
+	})
+
+	It("podman run --secret with --user", func() {
+		secretsString := "somesecretdata"
+		secretFilePath := filepath.Join(podmanTest.TempDir, "secret")
+		err := ioutil.WriteFile(secretFilePath, []byte(secretsString), 0755)
+		Expect(err).To(BeNil())
+
+		session := podmanTest.Podman([]string{"secret", "create", "mysecret", secretFilePath})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+
+		session = podmanTest.Podman([]string{"run", "--secret", "mysecret", "--name", "nonroot", "--user", "200:200", ALPINE, "cat", "/run/secrets/mysecret"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(session.OutputToString()).To(Equal(secretsString))
+	})
+
 	It("podman run invalid secret option", func() {
 		secretsString := "somesecretdata"
 		secretFilePath := filepath.Join(podmanTest.TempDir, "secret")
@@ -1694,6 +1737,11 @@ WORKDIR /madethis`, BB)
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Not(Equal(0)))
 
+		// mount option with env type
+		session = podmanTest.Podman([]string{"run", "--secret", "source=mysecret,type=env,uid=1000", "--name", "secr", ALPINE, "printenv", "mysecret"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Not(Equal(0)))
+
 		// No source given
 		session = podmanTest.Podman([]string{"run", "--secret", "type=env", "--name", "secr", ALPINE, "printenv", "mysecret"})
 		session.WaitWithDefaultTimeout()