From 89a2a885c82bade2774f4b669b4e09ede8f840c3 Mon Sep 17 00:00:00 2001 From: Dave Tucker Date: Mon, 6 Mar 2017 14:46:15 +0000 Subject: [PATCH] Ignore built-in allowed build-args in image history Removes the build-args from the image history if they are in the BuiltinAllowedBuildArgs map unless they are explicitly defined in an ARG instruction. Signed-off-by: Dave Tucker --- builder/dockerfile/builder.go | 2 ++ builder/dockerfile/dispatchers.go | 20 +++++++++++++++ docs/reference/builder.md | 31 ++++++++++++++++++++++++ integration-cli/docker_cli_build_test.go | 30 +++++++++++++++++++++++ 4 files changed, 83 insertions(+) diff --git a/builder/dockerfile/builder.go b/builder/dockerfile/builder.go index 6cc4b139a7..b28c584419 100644 --- a/builder/dockerfile/builder.go +++ b/builder/dockerfile/builder.go @@ -38,6 +38,8 @@ var validCommitCommands = map[string]bool{ } // BuiltinAllowedBuildArgs is list of built-in allowed build args +// these args are considered transparent and are excluded from the image history. +// Filtering from history is implemented in dispatchers.go var BuiltinAllowedBuildArgs = map[string]bool{ "HTTP_PROXY": true, "http_proxy": true, diff --git a/builder/dockerfile/dispatchers.go b/builder/dockerfile/dispatchers.go index e10bb32594..9982a72264 100644 --- a/builder/dockerfile/dispatchers.go +++ b/builder/dockerfile/dispatchers.go @@ -411,6 +411,26 @@ func run(b *Builder, args []string, attributes map[string]bool, original string) // have the build-time env vars in it (if any) so that future cache look-ups // properly match it. b.runConfig.Env = env + + // remove BuiltinAllowedBuildArgs (see: builder.go) from the saveCmd + // these args are transparent so resulting image should be the same regardless of the value + if len(cmdBuildEnv) > 0 { + saveCmd = config.Cmd + tmpBuildEnv := make([]string, len(cmdBuildEnv)) + copy(tmpBuildEnv, cmdBuildEnv) + for i, env := range tmpBuildEnv { + key := strings.SplitN(env, "=", 2)[0] + if _, ok := BuiltinAllowedBuildArgs[key]; ok { + // If an built-in arg is explicitly added in the Dockerfile, don't prune it + if _, ok := b.allowedBuildArgs[key]; !ok { + tmpBuildEnv = append(tmpBuildEnv[:i], tmpBuildEnv[i+1:]...) + } + } + } + sort.Strings(tmpBuildEnv) + tmpEnv := append([]string{fmt.Sprintf("|%d", len(tmpBuildEnv))}, tmpBuildEnv...) + saveCmd = strslice.StrSlice(append(tmpEnv, saveCmd...)) + } b.runConfig.Cmd = saveCmd return b.commit(cID, cmd, "run") } diff --git a/docs/reference/builder.md b/docs/reference/builder.md index 75c7e0c0c1..fa8b3e138f 100644 --- a/docs/reference/builder.md +++ b/docs/reference/builder.md @@ -1396,6 +1396,35 @@ To use these, simply pass them on the command line using the flag: --build-arg = ``` +By default, these pre-defined variables are excluded from the output of +`docker history`. Excluding them reduces the risk of accidentally leaking +sensitive authentication information in an `HTTP_PROXY` variable. + +For example, consider building the following Dockerfile using +`--build-arg HTTP_PROXY=http://user:pass@proxy.lon.example.com` + +``` Dockerfile +FROM ubuntu +RUN echo "Hello World" +``` + +In this case, the value of the `HTTP_PROXY` variable is not available in the +`docker history` and is not cached. If you were to change location, and your +proxy server changed to `http://user:pass@proxy.sfo.example.com`, a subsequent +build does not result in a cache miss. + +If you need to override this behaviour then you may do so by adding an `ARG` +statement in the Dockerfile as follows: + +``` Dockerfile +FROM ubuntu +ARG HTTP_PROXY +RUN echo "Hello World" +``` + +When building this Dockerfile, the `HTTP_PROXY` is preserved in the +`docker history`, and changing its value invalidates the build cache. + ### Impact on build caching `ARG` variables are not persisted into the built image as `ENV` variables are. @@ -1404,6 +1433,8 @@ Dockerfile defines an `ARG` variable whose value is different from a previous build, then a "cache miss" occurs upon its first usage, not its definition. In particular, all `RUN` instructions following an `ARG` instruction use the `ARG` variable implicitly (as an environment variable), thus can cause a cache miss. +All predefined `ARG` variables are exempt from caching unless there is a +matching `ARG` statement in the `Dockerfile`. For example, consider these two Dockerfile: diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go index c4396d82c4..174e4ad4f5 100644 --- a/integration-cli/docker_cli_build_test.go +++ b/integration-cli/docker_cli_build_test.go @@ -4290,6 +4290,36 @@ func (s *DockerSuite) TestBuildBuildTimeArgHistory(c *check.C) { } } +func (s *DockerSuite) TestBuildTimeArgHistoryExclusions(c *check.C) { + imgName := "bldargtest" + envKey := "foo" + envVal := "bar" + proxy := "HTTP_PROXY=http://user:password@proxy.example.com" + explicitProxyKey := "http_proxy" + explicitProxyVal := "http://user:password@someproxy.example.com" + dockerfile := fmt.Sprintf(`FROM busybox + ARG %s + ARG %s + RUN echo "Testing Build Args!"`, envKey, explicitProxyKey) + buildImage(imgName, + withBuildFlags("--build-arg", fmt.Sprintf("%s=%s", envKey, envVal), + "--build-arg", fmt.Sprintf("%s=%s", explicitProxyKey, explicitProxyVal), + "--build-arg", proxy), + withDockerfile(dockerfile), + ).Assert(c, icmd.Success) + + out, _ := dockerCmd(c, "history", "--no-trunc", imgName) + if strings.Contains(out, proxy) { + c.Fatalf("failed to exclude proxy settings from history!") + } + if !strings.Contains(out, fmt.Sprintf("%s=%s", envKey, envVal)) { + c.Fatalf("explicitly defined ARG %s is not in output", explicitProxyKey) + } + if !strings.Contains(out, fmt.Sprintf("%s=%s", envKey, envVal)) { + c.Fatalf("missing build arguments from output") + } +} + func (s *DockerSuite) TestBuildBuildTimeArgCacheHit(c *check.C) { imgName := "bldargtest" envKey := "foo"