diff --git a/define/types.go b/define/types.go index f8c1d4e3fa..1d7dd5fc62 100644 --- a/define/types.go +++ b/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 diff --git a/imagebuildah/executor.go b/imagebuildah/executor.go index ea81cab15a..0930b4d5eb 100644 --- a/imagebuildah/executor.go +++ b/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 } diff --git a/pkg/parse/parse.go b/pkg/parse/parse.go index 1b2bc2abec..b9dfb33d9a 100644 --- a/pkg/parse/parse.go +++ b/pkg/parse/parse.go @@ -1212,35 +1212,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 } diff --git a/run.go b/run.go index f0ac474e8e..8264273e10 100644 --- a/run.go +++ b/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 diff --git a/run_linux.go b/run_linux.go index a7618b963e..9e88f10a60 100644 --- a/run_linux.go +++ b/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 { @@ -2375,8 +2375,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 @@ -2395,14 +2396,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) @@ -2446,6 +2449,7 @@ func (b *Builder) runSetupRunMounts(mounts []string, secrets map[string]string, } artifacts := &runMountArtifacts{ RunMountTargets: mountTargets, + TmpFiles: tmpFiles, Agents: agents, SSHAuthSock: defaultSSHSock, } @@ -2497,10 +2501,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 @@ -2517,76 +2521,94 @@ 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, @@ -2594,7 +2616,7 @@ func getSecretMount(tokens []string, secrets map[string]string, mountlabel strin 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 @@ -2730,5 +2752,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 } diff --git a/tests/bud.bats b/tests/bud.bats index f1f7cc578b..bb0c4efabe 100644 --- a/tests/bud.bats +++ b/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,