From bfcaf538bb000d7eb72975d234a95f566da54715 Mon Sep 17 00:00:00 2001 From: Aditya Rajan Date: Thu, 25 Nov 2021 17:10:50 +0530 Subject: api: allow build api to accept secrets Following commit makes sure that `build` api can accept external secret and allows currently `NOOP` `podman-remote build -t tag --secret id=mysecret,src=/path/on/remote` to become functional. Just like `docker` following api is a hidden field and only exposed to `podman-remote` but could document it if it needs exposed on `swagger`. Signed-off-by: Aditya Rajan --- pkg/api/handlers/compat/images_build.go | 12 ++++++++++++ pkg/bindings/images/build.go | 7 +++++++ 2 files changed, 19 insertions(+) diff --git a/pkg/api/handlers/compat/images_build.go b/pkg/api/handlers/compat/images_build.go index ac5934c13..18f9dc98b 100644 --- a/pkg/api/handlers/compat/images_build.go +++ b/pkg/api/handlers/compat/images_build.go @@ -122,6 +122,7 @@ func BuildImage(w http.ResponseWriter, r *http.Request) { Target string `schema:"target"` Timestamp int64 `schema:"timestamp"` Ulimits string `schema:"ulimits"` + Secrets string `schema:"secrets"` }{ Dockerfile: "Dockerfile", Registry: "docker.io", @@ -239,6 +240,16 @@ func BuildImage(w http.ResponseWriter, r *http.Request) { dnssearch = m } + var secrets = []string{} + if _, found := r.URL.Query()["secrets"]; found { + var m = []string{} + if err := json.Unmarshal([]byte(query.Secrets), &m); err != nil { + utils.BadRequest(w, "secrets", query.Secrets, err) + return + } + secrets = m + } + var output string if len(query.Tag) > 0 { output = query.Tag[0] @@ -447,6 +458,7 @@ func BuildImage(w http.ResponseWriter, r *http.Request) { SeccompProfilePath: seccomp, ShmSize: strconv.Itoa(query.ShmSize), Ulimit: ulimits, + Secrets: secrets, }, CNIConfigDir: rtc.Network.CNIPluginDirs[0], CNIPluginPath: util.DefaultCNIPluginPath, diff --git a/pkg/bindings/images/build.go b/pkg/bindings/images/build.go index 3b0bebe9f..9c2e3e6b2 100644 --- a/pkg/bindings/images/build.go +++ b/pkg/bindings/images/build.go @@ -116,6 +116,13 @@ func Build(ctx context.Context, containerFiles []string, options entities.BuildO } params.Add("dnsservers", c) } + if secrets := options.CommonBuildOpts.Secrets; len(secrets) > 0 { + c, err := jsoniter.MarshalToString(secrets) + if err != nil { + return nil, err + } + params.Add("secrets", c) + } if dnsoptions := options.CommonBuildOpts.DNSOptions; len(dnsoptions) > 0 { c, err := jsoniter.MarshalToString(dnsoptions) if err != nil { -- cgit v1.2.3-54-g00ecf From e7204178e175d8ad619faa626ba284c777886cd3 Mon Sep 17 00:00:00 2001 From: Aditya Rajan Date: Fri, 26 Nov 2021 18:18:30 +0530 Subject: podman-remote: copy secret to contextdir is absolute path on host Podman remote must treat build secrets as part of context directory. If secret path is absolute path on host copy it to tar file and pass it to remote server. Signed-off-by: Aditya Rajan --- pkg/api/handlers/compat/images_build.go | 23 +++++++++- pkg/bindings/images/build.go | 61 +++++++++++++++++++++++--- test/e2e/build/Dockerfile.with-multiple-secret | 3 ++ test/e2e/build/Dockerfile.with-secret | 2 + test/e2e/build/anothersecret.txt | 1 + test/e2e/build/secret.txt | 1 + test/e2e/build_test.go | 23 ++++++++++ 7 files changed, 106 insertions(+), 8 deletions(-) create mode 100644 test/e2e/build/Dockerfile.with-multiple-secret create mode 100644 test/e2e/build/Dockerfile.with-secret create mode 100644 test/e2e/build/anothersecret.txt create mode 100644 test/e2e/build/secret.txt diff --git a/pkg/api/handlers/compat/images_build.go b/pkg/api/handlers/compat/images_build.go index 18f9dc98b..200d72192 100644 --- a/pkg/api/handlers/compat/images_build.go +++ b/pkg/api/handlers/compat/images_build.go @@ -247,7 +247,28 @@ func BuildImage(w http.ResponseWriter, r *http.Request) { utils.BadRequest(w, "secrets", query.Secrets, err) return } - secrets = m + + // for podman-remote all secrets must be picked from context director + // hence modify src so contextdir is added as prefix + + for _, secret := range m { + secretOpt := strings.Split(secret, ",") + if len(secretOpt) > 0 { + modifiedOpt := []string{} + for _, token := range secretOpt { + arr := strings.SplitN(token, "=", 2) + if len(arr) > 1 { + if arr[0] == "src" { + modifiedSrc := fmt.Sprintf("src=%s", filepath.Join(contextDirectory, arr[1])) + modifiedOpt = append(modifiedOpt, modifiedSrc) + } else { + modifiedOpt = append(modifiedOpt, token) + } + } + } + secrets = append(secrets, strings.Join(modifiedOpt[:], ",")) + } + } } var output string diff --git a/pkg/bindings/images/build.go b/pkg/bindings/images/build.go index 9c2e3e6b2..25770fdfc 100644 --- a/pkg/bindings/images/build.go +++ b/pkg/bindings/images/build.go @@ -5,6 +5,7 @@ import ( "compress/gzip" "context" "encoding/json" + "fmt" "io" "io/ioutil" "net/http" @@ -116,13 +117,6 @@ func Build(ctx context.Context, containerFiles []string, options entities.BuildO } params.Add("dnsservers", c) } - if secrets := options.CommonBuildOpts.Secrets; len(secrets) > 0 { - c, err := jsoniter.MarshalToString(secrets) - if err != nil { - return nil, err - } - params.Add("secrets", c) - } if dnsoptions := options.CommonBuildOpts.DNSOptions; len(dnsoptions) > 0 { c, err := jsoniter.MarshalToString(dnsoptions) if err != nil { @@ -384,6 +378,59 @@ func Build(ctx context.Context, containerFiles []string, options entities.BuildO } params.Set("dockerfile", string(cFileJSON)) } + + // build secrets are usually absolute host path or relative to context dir on host + // in any case move secret to current context and ship the tar. + if secrets := options.CommonBuildOpts.Secrets; len(secrets) > 0 { + secretsForRemote := []string{} + + for _, secret := range secrets { + secretOpt := strings.Split(secret, ",") + if len(secretOpt) > 0 { + modifiedOpt := []string{} + for _, token := range secretOpt { + arr := strings.SplitN(token, "=", 2) + if len(arr) > 1 { + if arr[0] == "src" { + // read specified secret into a tmp file + // move tmp file to tar and change secret source to relative tmp file + tmpSecretFile, err := ioutil.TempFile(options.ContextDirectory, "podman-build-secret") + if err != nil { + return nil, err + } + defer os.Remove(tmpSecretFile.Name()) // clean up + defer tmpSecretFile.Close() + srcSecretFile, err := os.Open(arr[1]) + if err != nil { + return nil, err + } + defer srcSecretFile.Close() + _, err = io.Copy(tmpSecretFile, srcSecretFile) + if err != nil { + return nil, err + } + + //add tmp file to context dir + tarContent = append(tarContent, tmpSecretFile.Name()) + + modifiedSrc := fmt.Sprintf("src=%s", filepath.Base(tmpSecretFile.Name())) + modifiedOpt = append(modifiedOpt, modifiedSrc) + } else { + modifiedOpt = append(modifiedOpt, token) + } + } + } + secretsForRemote = append(secretsForRemote, strings.Join(modifiedOpt[:], ",")) + } + } + + c, err := jsoniter.MarshalToString(secretsForRemote) + if err != nil { + return nil, err + } + params.Add("secrets", c) + } + tarfile, err := nTar(append(excludes, dontexcludes...), tarContent...) if err != nil { logrus.Errorf("Cannot tar container entries %v error: %v", tarContent, err) diff --git a/test/e2e/build/Dockerfile.with-multiple-secret b/test/e2e/build/Dockerfile.with-multiple-secret new file mode 100644 index 000000000..f3478914f --- /dev/null +++ b/test/e2e/build/Dockerfile.with-multiple-secret @@ -0,0 +1,3 @@ +FROM alpine +RUN --mount=type=secret,id=mysecret cat /run/secrets/mysecret +RUN --mount=type=secret,id=mysecret2 cat /run/secrets/mysecret2 diff --git a/test/e2e/build/Dockerfile.with-secret b/test/e2e/build/Dockerfile.with-secret new file mode 100644 index 000000000..920663a92 --- /dev/null +++ b/test/e2e/build/Dockerfile.with-secret @@ -0,0 +1,2 @@ +FROM alpine +RUN --mount=type=secret,id=mysecret cat /run/secrets/mysecret diff --git a/test/e2e/build/anothersecret.txt b/test/e2e/build/anothersecret.txt new file mode 100644 index 000000000..bc5fdbd32 --- /dev/null +++ b/test/e2e/build/anothersecret.txt @@ -0,0 +1 @@ +anothersecret diff --git a/test/e2e/build/secret.txt b/test/e2e/build/secret.txt new file mode 100644 index 000000000..d9106c0af --- /dev/null +++ b/test/e2e/build/secret.txt @@ -0,0 +1 @@ +somesecret diff --git a/test/e2e/build_test.go b/test/e2e/build_test.go index 420ed929f..ad401ca83 100644 --- a/test/e2e/build_test.go +++ b/test/e2e/build_test.go @@ -59,6 +59,29 @@ var _ = Describe("Podman build", func() { Expect(session).Should(Exit(0)) }) + It("podman build with a secret from file", func() { + session := podmanTest.Podman([]string{"build", "-f", "build/Dockerfile.with-secret", "-t", "secret-test", "--secret", "id=mysecret,src=build/secret.txt", "build/"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + Expect(session.OutputToString()).To(ContainSubstring("somesecret")) + + session = podmanTest.Podman([]string{"rmi", "secret-test"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + }) + + It("podman build with multiple secrets from files", func() { + session := podmanTest.Podman([]string{"build", "-f", "build/Dockerfile.with-multiple-secret", "-t", "multiple-secret-test", "--secret", "id=mysecret,src=build/secret.txt", "--secret", "id=mysecret2,src=build/anothersecret.txt", "build/"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + Expect(session.OutputToString()).To(ContainSubstring("somesecret")) + Expect(session.OutputToString()).To(ContainSubstring("anothersecret")) + + session = podmanTest.Podman([]string{"rmi", "multiple-secret-test"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + }) + It("podman build with logfile", func() { logfile := filepath.Join(podmanTest.TempDir, "logfile") session := podmanTest.Podman([]string{"build", "--pull-never", "--tag", "test", "--logfile", logfile, "build/basicalpine"}) -- cgit v1.2.3-54-g00ecf From c80a2e4495f877bc0f6a522e99b511de6c0d525d Mon Sep 17 00:00:00 2001 From: Aditya Rajan Date: Tue, 30 Nov 2021 14:15:53 +0530 Subject: podman-remote: prevent leaking secret into image Prevents temp secrets leaking into image by moving it away from context directory to parent builder directory. Builder directory automatically gets cleaned up when we are done with the build. Signed-off-by: Aditya Rajan --- pkg/api/handlers/compat/images_build.go | 14 +++++++++++++- test/e2e/build/Dockerfile.with-secret-verify-leak | 3 +++ test/e2e/build_test.go | 16 ++++++++++++++++ 3 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 test/e2e/build/Dockerfile.with-secret-verify-leak diff --git a/pkg/api/handlers/compat/images_build.go b/pkg/api/handlers/compat/images_build.go index 200d72192..3aa1af71a 100644 --- a/pkg/api/handlers/compat/images_build.go +++ b/pkg/api/handlers/compat/images_build.go @@ -259,7 +259,19 @@ func BuildImage(w http.ResponseWriter, r *http.Request) { arr := strings.SplitN(token, "=", 2) if len(arr) > 1 { if arr[0] == "src" { - modifiedSrc := fmt.Sprintf("src=%s", filepath.Join(contextDirectory, arr[1])) + /* move secret away from contextDir */ + /* to make sure we dont accidentally commit temporary secrets to image*/ + builderDirectory, _ := filepath.Split(contextDirectory) + // following path is outside build context + newSecretPath := filepath.Join(builderDirectory, arr[1]) + oldSecretPath := filepath.Join(contextDirectory, arr[1]) + err := os.Rename(oldSecretPath, newSecretPath) + if err != nil { + utils.BadRequest(w, "secrets", query.Secrets, err) + return + } + + modifiedSrc := fmt.Sprintf("src=%s", newSecretPath) modifiedOpt = append(modifiedOpt, modifiedSrc) } else { modifiedOpt = append(modifiedOpt, token) diff --git a/test/e2e/build/Dockerfile.with-secret-verify-leak b/test/e2e/build/Dockerfile.with-secret-verify-leak new file mode 100644 index 000000000..0957ac6a6 --- /dev/null +++ b/test/e2e/build/Dockerfile.with-secret-verify-leak @@ -0,0 +1,3 @@ +FROM alpine +COPY * / +RUN --mount=type=secret,id=mysecret cat /run/secrets/mysecret diff --git a/test/e2e/build_test.go b/test/e2e/build_test.go index ad401ca83..c541f25ae 100644 --- a/test/e2e/build_test.go +++ b/test/e2e/build_test.go @@ -82,6 +82,22 @@ var _ = Describe("Podman build", func() { Expect(session).Should(Exit(0)) }) + It("podman build with a secret from file and verify if secret file is not leaked into image", func() { + session := podmanTest.Podman([]string{"build", "-f", "build/Dockerfile.with-secret-verify-leak", "-t", "secret-test-leak", "--secret", "id=mysecret,src=build/secret.txt", "build/"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + Expect(session.OutputToString()).To(ContainSubstring("somesecret")) + + session = podmanTest.Podman([]string{"run", "--rm", "secret-test-leak", "ls"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + Expect(session.OutputToString()).To(Not(ContainSubstring("podman-build-secret"))) + + session = podmanTest.Podman([]string{"rmi", "secret-test-leak"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + }) + It("podman build with logfile", func() { logfile := filepath.Join(podmanTest.TempDir, "logfile") session := podmanTest.Podman([]string{"build", "--pull-never", "--tag", "test", "--logfile", logfile, "build/basicalpine"}) -- cgit v1.2.3-54-g00ecf