Add support for env var secret sources

Run secrets can now be created from an environment variable. The
environment variable is read and is briefly stored as a file on /dev/shm
when it's being used, and the file is removed after the RUN command is
finished.

Fixes: #3524

Signed-off-by: Ashley Cui <[email protected]>

define/types.go

@@ -93,6 +93,13 @@ type IDMappingOptions struct {
  GIDMap []specs.LinuxIDMapping
 }
 
+// Secret is a secret source that can be used in a RUN
+type Secret struct {
+ ID string
+ Source string
+ SourceType string
+}
+
 // TempDirForURL checks if the passed-in string looks like a URL or -. If it is,
 // TempDirForURL creates a temporary directory, arranges for its contents to be
 // the contents of that URL, and returns the temporary directory's path, along

imagebuildah/executor.go

@@ -123,7 +123,7 @@ type Executor struct {
  imageInfoCache map[string]imageTypeAndHistoryAndDiffIDs
  fromOverride string
  manifest string
- secrets map[string]string
+ secrets map[string]define.Secret
  sshsources map[string]*sshagent.Source
  logPrefix string
 }

pkg/parse/parse.go

@@ -1210,35 +1210,60 @@ func GetTempDir() string {
 }
 
 // Secrets parses the --secret flag
-func Secrets(secrets []string) (map[string]string, error) {
- parsed := make(map[string]string)
- invalidSyntax := errors.Errorf("incorrect secret flag format: should be --secret id=foo,src=bar")
+func Secrets(secrets []string) (map[string]define.Secret, error) {
+ invalidSyntax := errors.Errorf("incorrect secret flag format: should be --secret id=foo,src=bar[,env=ENV,type=file|env]")
+ parsed := make(map[string]define.Secret)
  for _, secret := range secrets {
- split := strings.Split(secret, ",")
- if len(split) > 2 {
+ tokens := strings.Split(secret, ",")
+ var id, src, typ string
+ for _, val := range tokens {
+ kv := strings.SplitN(val, "=", 2)
+ switch kv[0] {
+ case "id":
+ id = kv[1]
+ case "src":
+ src = kv[1]
+ case "env":
+ src = kv[1]
+ typ = "env"
+ case "type":
+ if kv[1] != "file" && kv[1] != "env" {
+ return nil, errors.New("invalid secret type, must be file or env")
+ }
+ typ = kv[1]
+ }
+ }
+ if id == "" {
  return nil, invalidSyntax
  }
- if len(split) == 2 {
- id := strings.Split(split[0], "=")
- src := strings.Split(split[1], "=")
- if len(split) == 2 && strings.ToLower(id[0]) == "id" && strings.ToLower(src[0]) == "src" {
- fullPath, err := filepath.Abs(src[1])
- if err != nil {
- return nil, err
- }
- _, err = os.Stat(fullPath)
- if err == nil {
- parsed[id[1]] = fullPath
- }
- if err != nil {
- return nil, errors.Wrap(err, "could not parse secrets")
- }
+ if src == "" {
+ src = id
+ }
+ if typ == "" {
+ if _, ok := os.LookupEnv(id); ok {
+ typ = "env"
  } else {
- return nil, invalidSyntax
+ typ = "file"
  }
- } else {
- return nil, invalidSyntax
  }
+
+ if typ == "file" {
+ fullPath, err := filepath.Abs(src)
+ if err != nil {
+ return nil, errors.Wrap(err, "could not parse secrets")
+ }
+ _, err = os.Stat(fullPath)
+ if err != nil {
+ return nil, errors.Wrap(err, "could not parse secrets")
+ }
+ src = fullPath
+ }
+ newSecret := define.Secret{
+ Source: src,
+ SourceType: typ,
+ }
+ parsed[id] = newSecret
+
  }
  return parsed, nil
 }

run.go

@@ -141,7 +141,7 @@ type RunOptions struct {
  // Devices are the additional devices to add to the containers
  Devices define.ContainerDevices
  // Secrets are the available secrets to use in a RUN
- Secrets map[string]string
+ Secrets map[string]define.Secret
  // SSHSources is the available ssh agents to use in a RUN
  SSHSources map[string]*sshagent.Source `json:"-"`
  // RunMounts are mounts for this run. RunMounts for this run
@@ -153,6 +153,8 @@ type RunOptions struct {
 type runMountArtifacts struct {
  // RunMountTargets are the run mount targets inside the container
  RunMountTargets []string
+ // TmpFiles are artifacts that need to be removed outside the container
+ TmpFiles []string
  // Agents are the ssh agents started
  Agents []*sshagent.AgentServer
  // SSHAuthSock is the path to the ssh auth sock inside the container

run_linux.go

@@ -415,7 +415,7 @@ func runSetupBuiltinVolumes(mountLabel, mountPoint, containerDir string, builtin
  return mounts, nil
 }
 
-func (b *Builder) setupMounts(mountPoint string, spec *specs.Spec, bundlePath string, optionMounts []specs.Mount, bindFiles map[string]string, builtinVolumes, volumeMounts []string, shmSize string, namespaceOptions define.NamespaceOptions, secrets map[string]string, sshSources map[string]*sshagent.Source, runFileMounts []string, contextDir string) (*runMountArtifacts, error) {
+func (b *Builder) setupMounts(mountPoint string, spec *specs.Spec, bundlePath string, optionMounts []specs.Mount, bindFiles map[string]string, builtinVolumes, volumeMounts []string, shmSize string, namespaceOptions define.NamespaceOptions, secrets map[string]define.Secret, sshSources map[string]*sshagent.Source, runFileMounts []string, contextDir string) (*runMountArtifacts, error) {
  // Start building a new list of mounts.
  var mounts []specs.Mount
  haveMount := func(destination string) bool {
@@ -2366,8 +2366,9 @@ func init() {
 }
 
 // runSetupRunMounts sets up mounts that exist only in this RUN, not in subsequent runs
-func (b *Builder) runSetupRunMounts(mounts []string, secrets map[string]string, sshSources map[string]*sshagent.Source, containerWorkingDir string, contextDir string, uidmap []spec.LinuxIDMapping, gidmap []spec.LinuxIDMapping, rootUID int, rootGID int, processUID int, processGID int) ([]spec.Mount, *runMountArtifacts, error) {
- mountTargets := make([]string, 0, 10)
+func (b *Builder) runSetupRunMounts(mounts []string, secrets map[string]define.Secret, sshSources map[string]*sshagent.Source, containerWorkingDir string, contextDir string, uidmap []spec.LinuxIDMapping, gidmap []spec.LinuxIDMapping, rootUID int, rootGID int, processUID int, processGID int) ([]spec.Mount, *runMountArtifacts, error) {
+ mountTargets := make([]string, 0, len(mounts))
+ tmpFiles := make([]string, 0, len(mounts))
  finalMounts := make([]specs.Mount, 0, len(mounts))
  agents := make([]*sshagent.AgentServer, 0, len(mounts))
  sshCount := 0
@@ -2386,14 +2387,16 @@ func (b *Builder) runSetupRunMounts(mounts []string, secrets map[string]string,
  // For now, we only support type secret.
  switch kv[1] {
  case "secret":
- mount, err := getSecretMount(tokens, secrets, b.MountLabel, containerWorkingDir, uidmap, gidmap)
+ mount, envFile, err := getSecretMount(tokens, secrets, b.MountLabel, containerWorkingDir, uidmap, gidmap)
  if err != nil {
  return nil, nil, err
  }
  if mount != nil {
  finalMounts = append(finalMounts, *mount)
  mountTargets = append(mountTargets, mount.Destination)
-
+ if envFile != "" {
+ tmpFiles = append(tmpFiles, envFile)
+ }
  }
  case "ssh":
  mount, agent, err := b.getSSHMount(tokens, sshCount, sshSources, b.MountLabel, uidmap, gidmap, b.ProcessLabel)
@@ -2437,6 +2440,7 @@ func (b *Builder) runSetupRunMounts(mounts []string, secrets map[string]string,
  }
  artifacts := &runMountArtifacts{
  RunMountTargets: mountTargets,
+ TmpFiles: tmpFiles,
  Agents: agents,
  SSHAuthSock: defaultSSHSock,
  }
@@ -2488,10 +2492,10 @@ func (b *Builder) getCacheMount(tokens []string, rootUID, rootGID, processUID, p
  return &volumes[0], nil
 }
 
-func getSecretMount(tokens []string, secrets map[string]string, mountlabel string, containerWorkingDir string, uidmap []spec.LinuxIDMapping, gidmap []spec.LinuxIDMapping) (*spec.Mount, error) {
+func getSecretMount(tokens []string, secrets map[string]define.Secret, mountlabel string, containerWorkingDir string, uidmap []spec.LinuxIDMapping, gidmap []spec.LinuxIDMapping) (*spec.Mount, string, error) {
  errInvalidSyntax := errors.New("secret should have syntax id=id[,target=path,required=bool,mode=uint,uid=uint,gid=uint")
  if len(tokens) == 0 {
- return nil, errInvalidSyntax
+ return nil, "", errInvalidSyntax
  }
  var err error
  var id, target string
@@ -2508,84 +2512,102 @@ func getSecretMount(tokens []string, secrets map[string]string, mountlabel strin
  case "required":
  required, err = strconv.ParseBool(kv[1])
  if err != nil {
- return nil, errInvalidSyntax
+ return nil, "", errInvalidSyntax
  }
  case "mode":
  mode64, err := strconv.ParseUint(kv[1], 8, 32)
  if err != nil {
- return nil, errInvalidSyntax
+ return nil, "", errInvalidSyntax
  }
  mode = uint32(mode64)
  case "uid":
  uid64, err := strconv.ParseUint(kv[1], 10, 32)
  if err != nil {
- return nil, errInvalidSyntax
+ return nil, "", errInvalidSyntax
  }
  uid = uint32(uid64)
  case "gid":
  gid64, err := strconv.ParseUint(kv[1], 10, 32)
  if err != nil {
- return nil, errInvalidSyntax
+ return nil, "", errInvalidSyntax
  }
  gid = uint32(gid64)
  default:
- return nil, errInvalidSyntax
+ return nil, "", errInvalidSyntax
  }
  }
 
  if id == "" {
- return nil, errInvalidSyntax
+ return nil, "", errInvalidSyntax
  }
  // Default location for secretis is /run/secrets/id
  if target == "" {
  target = "/run/secrets/" + id
  }
 
- src, ok := secrets[id]
+ secr, ok := secrets[id]
  if !ok {
  if required {
- return nil, errors.Errorf("secret required but no secret with id %s found", id)
+ return nil, "", errors.Errorf("secret required but no secret with id %s found", id)
  }
- return nil, nil
+ return nil, "", nil
  }
+ var data []byte
+ var envFile string
+ var ctrFileOnHost string
 
- // Copy secrets to container working dir, since we need to chmod, chown and relabel it
- // for the container user and we don't want to mess with the original file
- ctrFileOnHost := filepath.Join(containerWorkingDir, "secrets", id)
- _, err = os.Stat(ctrFileOnHost)
- if os.IsNotExist(err) {
- data, err := ioutil.ReadFile(src)
+ switch secr.SourceType {
+ case "env":
+ data = []byte(os.Getenv(secr.Source))
+ tmpFile, err := ioutil.TempFile("/dev/shm", "buildah*")
  if err != nil {
- return nil, err
+ return nil, "", err
  }
- if err := os.MkdirAll(filepath.Dir(ctrFileOnHost), 0644); err != nil {
- return nil, err
+ envFile = tmpFile.Name()
+ ctrFileOnHost = tmpFile.Name()
+ case "file":
+ data, err = ioutil.ReadFile(secr.Source)
+ if err != nil {
+ return nil, "", err
  }
- if err := ioutil.WriteFile(ctrFileOnHost, data, 0644); err != nil {
- return nil, err
+ ctrFileOnHost = filepath.Join(containerWorkingDir, "secrets", id)
+ _, err = os.Stat(ctrFileOnHost)
+ if !os.IsNotExist(err) {
+ return nil, "", err
  }
+ default:
+ return nil, "", errors.New("invalid source secret type")
+ }
+
+ // Copy secrets to container working dir (or tmp dir if it's an env), since we need to chmod,
+ // chown and relabel it for the container user and we don't want to mess with the original file
+ if err := os.MkdirAll(filepath.Dir(ctrFileOnHost), 0644); err != nil {
+ return nil, "", err
+ }
+ if err := ioutil.WriteFile(ctrFileOnHost, data, 0644); err != nil {
+ return nil, "", err
  }
 
  if err := label.Relabel(ctrFileOnHost, mountlabel, false); err != nil {
- return nil, err
+ return nil, "", err
  }
  hostUID, hostGID, err := util.GetHostIDs(uidmap, gidmap, uid, gid)
  if err != nil {
- return nil, err
+ return nil, "", err
  }
  if err := os.Lchown(ctrFileOnHost, int(hostUID), int(hostGID)); err != nil {
- return nil, err
+ return nil, "", err
  }
  if err := os.Chmod(ctrFileOnHost, os.FileMode(mode)); err != nil {
- return nil, err
+ return nil, "", err
  }
  newMount := specs.Mount{
  Destination: target,
  Type: "bind",
  Source: ctrFileOnHost,
  Options: []string{"bind", "rprivate", "ro"},
  }
- return &newMount, nil
+ return &newMount, envFile, nil
 }
 
 // getSSHMount parses the --mount type=ssh flag in the Containerfile, checks if there's an ssh source provided, and creates and starts an ssh-agent to be forwarded into the container
@@ -2721,5 +2743,15 @@ func cleanupRunMounts(mountpoint string, artifacts *runMountArtifacts) error {
  return err
  }
  }
- return nil
+ var prevErr error
+ for _, path := range artifacts.TmpFiles {
+ err := os.Remove(path)
+ if !os.IsNotExist(err) {
+ if prevErr != nil {
+ logrus.Error(prevErr)
+ }
+ prevErr = err
+ }
+ }
+ return prevErr
 }

tests/bud.bats

@@ -3329,6 +3329,38 @@ _EOF
  expect_output --substring "secret required but no secret with id mysecret found"
 }
 
+@test "bud with containerfile env secret" {
+ export MYSECRET=SOMESECRETDATA
+ run_buildah build --secret=id=mysecret,src=MYSECRET,type=env --signature-policy ${TESTSDIR}/policy.json -t secretimg -f ${TESTSDIR}/bud/run-mounts/Dockerfile.secret ${TESTSDIR}/bud/run-mounts
+ expect_output --substring "SOMESECRETDATA"
+
+ run_buildah from secretimg
+ run_buildah 1 run secretimg-working-container cat /run/secrets/mysecret
+ expect_output --substring "cat: can't open '/run/secrets/mysecret': No such file or directory"
+ run_buildah rm -a
+
+ run_buildah build --secret=id=mysecret,env=MYSECRET --signature-policy ${TESTSDIR}/policy.json -t secretimg -f ${TESTSDIR}/bud/run-mounts/Dockerfile.secret ${TESTSDIR}/bud/run-mounts
+ expect_output --substring "SOMESECRETDATA"
+
+ run_buildah from secretimg
+ run_buildah 1 run secretimg-working-container cat /run/secrets/mysecret
+ expect_output --substring "cat: can't open '/run/secrets/mysecret': No such file or directory"
+ run_buildah rm -a
+}
+
+@test "bud with containerfile env secret priority" {
+ _prefetch alpine
+ mytmpdir=${TESTDIR}/my-dir1
+ mkdir -p ${mytmpdir}
+ cat > $mytmpdir/mysecret << _EOF
+SOMESECRETDATA
+_EOF
+
+ export mysecret=ENVDATA
+ run_buildah build --secret=id=mysecret --signature-policy ${TESTSDIR}/policy.json -t secretimg -f ${TESTSDIR}/bud/run-mounts/Dockerfile.secret ${TESTSDIR}/bud/run-mounts
+ expect_output --substring "ENVDATA"
+}
+
 @test "bud-multiple-platform-values" {
  outputlist=testlist
  # check if we can run a couple of 32-bit versions of an image, and if we can,