diff --git a/api/client/commands.go b/api/client/commands.go index 3cd110b4ec..c4ce5e01f3 100644 --- a/api/client/commands.go +++ b/api/client/commands.go @@ -39,6 +39,7 @@ import ( "github.com/docker/docker/pkg/parsers/filters" "github.com/docker/docker/pkg/promise" "github.com/docker/docker/pkg/signal" + "github.com/docker/docker/pkg/symlink" "github.com/docker/docker/pkg/term" "github.com/docker/docker/pkg/timeutils" "github.com/docker/docker/pkg/units" @@ -147,36 +148,36 @@ func (cli *DockerCli) CmdBuild(args ...string) error { return err } - var filename string // path to Dockerfile - var origDockerfile string // used for error msg + filename := *dockerfileName // path to Dockerfile if *dockerfileName == "" { // No -f/--file was specified so use the default - origDockerfile = api.DefaultDockerfileName - *dockerfileName = origDockerfile + *dockerfileName = api.DefaultDockerfileName filename = path.Join(absRoot, *dockerfileName) - } else { - origDockerfile = *dockerfileName - if filename, err = filepath.Abs(*dockerfileName); err != nil { - return err - } - - // Verify that 'filename' is within the build context - if !strings.HasSuffix(absRoot, string(os.PathSeparator)) { - absRoot += string(os.PathSeparator) - } - if !strings.HasPrefix(filename, absRoot) { - return fmt.Errorf("The Dockerfile (%s) must be within the build context (%s)", *dockerfileName, root) - } - - // Now reset the dockerfileName to be relative to the build context - *dockerfileName = filename[len(absRoot):] } - if _, err = os.Stat(filename); os.IsNotExist(err) { - return fmt.Errorf("Can not locate Dockerfile: %s", origDockerfile) + origDockerfile := *dockerfileName // used for error msg + + if filename, err = filepath.Abs(filename); err != nil { + return err } - var includes []string = []string{"."} + + // Verify that 'filename' is within the build context + filename, err = symlink.FollowSymlinkInScope(filename, absRoot) + if err != nil { + return fmt.Errorf("The Dockerfile (%s) must be within the build context (%s)", origDockerfile, root) + } + + // Now reset the dockerfileName to be relative to the build context + *dockerfileName, err = filepath.Rel(absRoot, filename) + if err != nil { + return err + } + + if _, err = os.Lstat(filename); os.IsNotExist(err) { + return fmt.Errorf("Cannot locate Dockerfile: %s", origDockerfile) + } + var includes = []string{"."} excludes, err := utils.ReadDockerIgnore(path.Join(root, ".dockerignore")) if err != nil { diff --git a/builder/evaluator.go b/builder/evaluator.go index 3149bd0df7..b76c7f29bb 100644 --- a/builder/evaluator.go +++ b/builder/evaluator.go @@ -24,7 +24,7 @@ import ( "fmt" "io" "os" - "path" + "path/filepath" "strings" log "github.com/Sirupsen/logrus" @@ -32,6 +32,7 @@ import ( "github.com/docker/docker/daemon" "github.com/docker/docker/engine" "github.com/docker/docker/pkg/fileutils" + "github.com/docker/docker/pkg/symlink" "github.com/docker/docker/pkg/tarsum" "github.com/docker/docker/registry" "github.com/docker/docker/runconfig" @@ -169,12 +170,15 @@ func (b *Builder) Run(context io.Reader) (string, error) { // Reads a Dockerfile from the current context. It assumes that the // 'filename' is a relative path from the root of the context -func (b *Builder) readDockerfile(filename string) error { - filename = path.Join(b.contextPath, filename) +func (b *Builder) readDockerfile(origFile string) error { + filename, err := symlink.FollowSymlinkInScope(filepath.Join(b.contextPath, origFile), b.contextPath) + if err != nil { + return fmt.Errorf("The Dockerfile (%s) must be within the build context", origFile) + } - fi, err := os.Stat(filename) + fi, err := os.Lstat(filename) if os.IsNotExist(err) { - return fmt.Errorf("Cannot build a directory without a Dockerfile") + return fmt.Errorf("Cannot locate specified Dockerfile: %s", origFile) } if fi.Size() == 0 { return ErrDockerfileEmpty @@ -201,13 +205,13 @@ func (b *Builder) readDockerfile(filename string) error { // Note that this assumes the Dockerfile has been read into memory and // is now safe to be removed. - excludes, _ := utils.ReadDockerIgnore(path.Join(b.contextPath, ".dockerignore")) + excludes, _ := utils.ReadDockerIgnore(filepath.Join(b.contextPath, ".dockerignore")) if rm, _ := fileutils.Matches(".dockerignore", excludes); rm == true { - os.Remove(path.Join(b.contextPath, ".dockerignore")) + os.Remove(filepath.Join(b.contextPath, ".dockerignore")) b.context.(tarsum.BuilderContext).Remove(".dockerignore") } if rm, _ := fileutils.Matches(b.dockerfileName, excludes); rm == true { - os.Remove(path.Join(b.contextPath, b.dockerfileName)) + os.Remove(filepath.Join(b.contextPath, b.dockerfileName)) b.context.(tarsum.BuilderContext).Remove(b.dockerfileName) } diff --git a/integration-cli/docker_api_containers_test.go b/integration-cli/docker_api_containers_test.go index 4e945f5429..5dce388381 100644 --- a/integration-cli/docker_api_containers_test.go +++ b/integration-cli/docker_api_containers_test.go @@ -299,3 +299,70 @@ func TestGetContainerStats(t *testing.T) { } logDone("container REST API - check GET containers/stats") } + +func TestBuildApiDockerfilePath(t *testing.T) { + // Test to make sure we stop people from trying to leave the + // build context when specifying the path to the dockerfile + buffer := new(bytes.Buffer) + tw := tar.NewWriter(buffer) + defer tw.Close() + + dockerfile := []byte("FROM busybox") + if err := tw.WriteHeader(&tar.Header{ + Name: "Dockerfile", + Size: int64(len(dockerfile)), + }); err != nil { + t.Fatalf("failed to write tar file header: %v", err) + } + if _, err := tw.Write(dockerfile); err != nil { + t.Fatalf("failed to write tar file content: %v", err) + } + if err := tw.Close(); err != nil { + t.Fatalf("failed to close tar archive: %v", err) + } + + out, err := sockRequestRaw("POST", "/build?dockerfile=../Dockerfile", buffer, "application/x-tar") + if err == nil { + t.Fatalf("Build was supposed to fail: %s", out) + } + + if !strings.Contains(string(out), "must be within the build context") { + t.Fatalf("Didn't complain about leaving build context: %s", out) + } + + logDone("container REST API - check build w/bad Dockerfile path") +} + +func TestBuildApiDockerfileSymlink(t *testing.T) { + // Test to make sure we stop people from trying to leave the + // build context when specifying a symlink as the path to the dockerfile + buffer := new(bytes.Buffer) + tw := tar.NewWriter(buffer) + defer tw.Close() + + if err := tw.WriteHeader(&tar.Header{ + Name: "Dockerfile", + Typeflag: tar.TypeSymlink, + Linkname: "/etc/passwd", + }); err != nil { + t.Fatalf("failed to write tar file header: %v", err) + } + if err := tw.Close(); err != nil { + t.Fatalf("failed to close tar archive: %v", err) + } + + out, err := sockRequestRaw("POST", "/build", buffer, "application/x-tar") + if err == nil { + t.Fatalf("Build was supposed to fail: %s", out) + } + + // The reason the error is "Cannot locate specified Dockerfile" is because + // in the builder, the symlink is resolved within the context, therefore + // Dockerfile -> /etc/passwd becomes etc/passwd from the context which is + // a nonexistent file. + if !strings.Contains(string(out), "Cannot locate specified Dockerfile: Dockerfile") { + t.Fatalf("Didn't complain about leaving build context: %s", out) + } + + logDone("container REST API - check build w/bad Dockerfile symlink path") +} diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go index 2c04ba8f27..fad38317ef 100644 --- a/integration-cli/docker_cli_build_test.go +++ b/integration-cli/docker_cli_build_test.go @@ -4630,3 +4630,67 @@ func TestBuildFromOfficialNames(t *testing.T) { } logDone("build - from official names") } + +func TestBuildDockerfileOutsideContext(t *testing.T) { + name := "testbuilddockerfileoutsidecontext" + tmpdir, err := ioutil.TempDir("", name) + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tmpdir) + ctx := filepath.Join(tmpdir, "context") + if err := os.MkdirAll(ctx, 0755); err != nil { + t.Fatal(err) + } + if err := ioutil.WriteFile(filepath.Join(ctx, "Dockerfile"), []byte("FROM busybox"), 0644); err != nil { + t.Fatal(err) + } + wd, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + defer os.Chdir(wd) + if err := os.Chdir(ctx); err != nil { + t.Fatal(err) + } + if err := ioutil.WriteFile(filepath.Join(tmpdir, "outsideDockerfile"), []byte("FROM busbox"), 0644); err != nil { + t.Fatal(err) + } + if err := os.Symlink("../outsideDockerfile", filepath.Join(ctx, "dockerfile1")); err != nil { + t.Fatal(err) + } + if err := os.Symlink(filepath.Join(tmpdir, "outsideDockerfile"), filepath.Join(ctx, "dockerfile2")); err != nil { + t.Fatal(err) + } + if err := os.Link("../outsideDockerfile", filepath.Join(ctx, "dockerfile3")); err != nil { + t.Fatal(err) + } + if err := os.Link(filepath.Join(tmpdir, "outsideDockerfile"), filepath.Join(ctx, "dockerfile4")); err != nil { + t.Fatal(err) + } + for _, dockerfilePath := range []string{ + "../outsideDockerfile", + filepath.Join(ctx, "dockerfile1"), + filepath.Join(ctx, "dockerfile2"), + filepath.Join(ctx, "dockerfile3"), + filepath.Join(ctx, "dockerfile4"), + } { + out, _, err := runCommandWithOutput(exec.Command(dockerBinary, "build", "-t", name, "--no-cache", "-f", dockerfilePath, ".")) + if err == nil { + t.Fatalf("Expected error with %s. Out: %s", dockerfilePath, out) + } + deleteImages(name) + } + + os.Chdir(tmpdir) + + // Path to Dockerfile should be resolved relative to working directory, not relative to context. + // There is a Dockerfile in the context, but since there is no Dockerfile in the current directory, the following should fail + out, _, err := runCommandWithOutput(exec.Command(dockerBinary, "build", "-t", name, "--no-cache", "-f", "Dockerfile", ctx)) + if err == nil { + t.Fatalf("Expected error. Out: %s", out) + } + deleteImages(name) + + logDone("build - Dockerfile outside context") +} diff --git a/integration-cli/docker_utils.go b/integration-cli/docker_utils.go index 3d66e9948f..35a97feca7 100644 --- a/integration-cli/docker_utils.go +++ b/integration-cli/docker_utils.go @@ -274,6 +274,15 @@ func daemonHost() string { } func sockRequest(method, endpoint string, data interface{}) ([]byte, error) { + jsonData := bytes.NewBuffer(nil) + if err := json.NewEncoder(jsonData).Encode(data); err != nil { + return nil, err + } + + return sockRequestRaw(method, endpoint, jsonData, "application/json") +} + +func sockRequestRaw(method, endpoint string, data io.Reader, ct string) ([]byte, error) { daemon := daemonHost() daemonUrl, err := url.Parse(daemon) if err != nil { @@ -296,17 +305,16 @@ func sockRequest(method, endpoint string, data interface{}) ([]byte, error) { client := httputil.NewClientConn(c, nil) defer client.Close() - jsonData := bytes.NewBuffer(nil) - if err := json.NewEncoder(jsonData).Encode(data); err != nil { - return nil, err - } - - req, err := http.NewRequest(method, endpoint, jsonData) - req.Header.Set("Content-Type", "application/json") + req, err := http.NewRequest(method, endpoint, data) if err != nil { return nil, fmt.Errorf("could not create new request: %v", err) } + if ct == "" { + ct = "application/json" + } + req.Header.Set("Content-Type", ct) + resp, err := client.Do(req) if err != nil { return nil, fmt.Errorf("could not perform request: %v", err)