diff --git a/CHANGELOG.md b/CHANGELOG.md index f958bbd48..2cddc6c90 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,21 @@ # Changelog +## 1.3.2 (2014-11-20) + +#### Security +- Fix tar breakout vulnerability +* Extractions are now sandboxed chroot +- Security options are no longer committed to images + +#### Runtime +- Fix deadlock in `docker ps -f exited=1` +- Fix a bug when `--volumes-from` references a container that failed to start + +#### Registry ++ `--insecure-registry` now accepts CIDR notation such as 10.1.0.0/16 +* Private registries whose IPs fall in the 127.0.0.0/8 range do no need the `--insecure-registry` flag +- Skip the experimental registry v2 API when mirroring is enabled + ## 1.3.1 (2014-10-28) #### Security diff --git a/Dockerfile b/Dockerfile index 34b0460c0..d9c58a921 100644 --- a/Dockerfile +++ b/Dockerfile @@ -78,7 +78,7 @@ RUN cd /usr/local/go/src && bash -xc 'for platform in $DOCKER_CROSSPLATFORMS; do RUN go get code.google.com/p/go.tools/cmd/cover # TODO replace FPM with some very minimal debhelper stuff -RUN gem install --no-rdoc --no-ri fpm --version 1.0.2 +RUN gem install --no-rdoc --no-ri fpm --version 1.3.2 # Install man page generator RUN mkdir -p /go/src/github.com/cpuguy83 \ diff --git a/VERSION b/VERSION index 3a3cd8cc8..1892b9267 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.3.1 +1.3.2 diff --git a/builder/internals.go b/builder/internals.go index 5fd03f774..fa8b9f703 100644 --- a/builder/internals.go +++ b/builder/internals.go @@ -22,6 +22,7 @@ import ( "github.com/docker/docker/daemon" imagepkg "github.com/docker/docker/image" "github.com/docker/docker/pkg/archive" + "github.com/docker/docker/pkg/chrootarchive" "github.com/docker/docker/pkg/log" "github.com/docker/docker/pkg/parsers" "github.com/docker/docker/pkg/promise" @@ -46,7 +47,8 @@ func (b *Builder) readContext(context io.Reader) error { if b.context, err = tarsum.NewTarSum(decompressedStream, true, tarsum.Version0); err != nil { return err } - if err := archive.Untar(b.context, tmpdirPath, nil); err != nil { + + if err := chrootarchive.Untar(b.context, tmpdirPath, nil); err != nil { return err } @@ -620,7 +622,7 @@ func (b *Builder) addContext(container *daemon.Container, orig, dest string, dec } // try to successfully untar the orig - if err := archive.UntarPath(origPath, tarDest); err == nil { + if err := chrootarchive.UntarPath(origPath, tarDest); err == nil { return nil } else if err != io.EOF { log.Debugf("Couldn't untar %s to %s: %s", origPath, tarDest, err) @@ -630,7 +632,7 @@ func (b *Builder) addContext(container *daemon.Container, orig, dest string, dec if err := os.MkdirAll(path.Dir(destPath), 0755); err != nil { return err } - if err := archive.CopyWithTar(origPath, destPath); err != nil { + if err := chrootarchive.CopyWithTar(origPath, destPath); err != nil { return err } @@ -643,7 +645,7 @@ func (b *Builder) addContext(container *daemon.Container, orig, dest string, dec } func copyAsDirectory(source, destination string, destinationExists bool) error { - if err := archive.CopyWithTar(source, destination); err != nil { + if err := chrootarchive.CopyWithTar(source, destination); err != nil { return err } diff --git a/daemon/config.go b/daemon/config.go index 9e8d08e2a..e45e73b99 100644 --- a/daemon/config.go +++ b/daemon/config.go @@ -56,7 +56,7 @@ func (config *Config) InstallFlags() { flag.StringVar(&config.BridgeIP, []string{"#bip", "-bip"}, "", "Use this CIDR notation address for the network bridge's IP, not compatible with -b") flag.StringVar(&config.BridgeIface, []string{"b", "-bridge"}, "", "Attach containers to a pre-existing network bridge\nuse 'none' to disable container networking") flag.StringVar(&config.FixedCIDR, []string{"-fixed-cidr"}, "", "IPv4 subnet for fixed IPs (ex: 10.20.0.0/16)\nthis subnet must be nested in the bridge subnet (which is defined by -b or --bip)") - opts.ListVar(&config.InsecureRegistries, []string{"-insecure-registry"}, "Enable insecure communication with specified registries (no certificate verification for HTTPS and enable HTTP fallback)") + opts.ListVar(&config.InsecureRegistries, []string{"-insecure-registry"}, "Enable insecure communication with specified registries (no certificate verification for HTTPS and enable HTTP fallback) (e.g., localhost:5000 or 10.20.0.0/16)") flag.BoolVar(&config.InterContainerCommunication, []string{"#icc", "-icc"}, true, "Enable inter-container communication") flag.StringVar(&config.GraphDriver, []string{"s", "-storage-driver"}, "", "Force the Docker runtime to use a specific storage driver") flag.StringVar(&config.ExecDriver, []string{"e", "-exec-driver"}, "native", "Force the Docker runtime to use a specific exec driver") @@ -68,6 +68,14 @@ func (config *Config) InstallFlags() { opts.IPListVar(&config.Dns, []string{"#dns", "-dns"}, "Force Docker to use specific DNS servers") opts.DnsSearchListVar(&config.DnsSearch, []string{"-dns-search"}, "Force Docker to use specific DNS search domains") opts.MirrorListVar(&config.Mirrors, []string{"-registry-mirror"}, "Specify a preferred Docker registry mirror") + + // Localhost is by default considered as an insecure registry + // This is a stop-gap for people who are running a private registry on localhost (especially on Boot2docker). + // + // TODO: should we deprecate this once it is easier for people to set up a TLS registry or change + // daemon flags on boot2docker? + // If so, do not forget to check the TODO in TestIsSecure + config.InsecureRegistries = append(config.InsecureRegistries, "127.0.0.0/8") } func GetDefaultNetworkMtu() int { diff --git a/daemon/daemon.go b/daemon/daemon.go index d069797ac..9f90643c4 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -528,10 +528,10 @@ func (daemon *Daemon) getEntrypointAndArgs(configEntrypoint, configCmd []string) return entrypoint, args } -func parseSecurityOpt(container *Container, config *runconfig.Config) error { +func parseSecurityOpt(container *Container, config *runconfig.HostConfig) error { var ( - label_opts []string - err error + labelOpts []string + err error ) for _, opt := range config.SecurityOpt { @@ -541,7 +541,7 @@ func parseSecurityOpt(container *Container, config *runconfig.Config) error { } switch con[0] { case "label": - label_opts = append(label_opts, con[1]) + labelOpts = append(labelOpts, con[1]) case "apparmor": container.AppArmorProfile = con[1] default: @@ -549,7 +549,7 @@ func parseSecurityOpt(container *Container, config *runconfig.Config) error { } } - container.ProcessLabel, container.MountLabel, err = label.InitLabels(label_opts) + container.ProcessLabel, container.MountLabel, err = label.InitLabels(labelOpts) return err } @@ -583,7 +583,6 @@ func (daemon *Daemon) newContainer(name string, config *runconfig.Config, img *i execCommands: newExecStore(), } container.root = daemon.containerRoot(container.ID) - err = parseSecurityOpt(container, config) return container, err } diff --git a/daemon/daemon_unit_test.go b/daemon/daemon_unit_test.go index f3b899ec8..fbc3302aa 100644 --- a/daemon/daemon_unit_test.go +++ b/daemon/daemon_unit_test.go @@ -8,7 +8,7 @@ import ( func TestParseSecurityOpt(t *testing.T) { container := &Container{} - config := &runconfig.Config{} + config := &runconfig.HostConfig{} // test apparmor config.SecurityOpt = []string{"apparmor:test_profile"} diff --git a/daemon/execdriver/lxc/init.go b/daemon/execdriver/lxc/init.go index 2a91bbb5f..680f53e1a 100644 --- a/daemon/execdriver/lxc/init.go +++ b/daemon/execdriver/lxc/init.go @@ -13,7 +13,7 @@ import ( "strings" "syscall" - "github.com/docker/docker/reexec" + "github.com/docker/docker/pkg/reexec" "github.com/docker/libcontainer/netlink" ) diff --git a/daemon/execdriver/native/exec.go b/daemon/execdriver/native/exec.go index 0f7e5c07b..84ad09672 100644 --- a/daemon/execdriver/native/exec.go +++ b/daemon/execdriver/native/exec.go @@ -11,7 +11,7 @@ import ( "runtime" "github.com/docker/docker/daemon/execdriver" - "github.com/docker/docker/reexec" + "github.com/docker/docker/pkg/reexec" "github.com/docker/libcontainer" "github.com/docker/libcontainer/namespaces" ) diff --git a/daemon/execdriver/native/init.go b/daemon/execdriver/native/init.go index 7021eeb67..c1c988d93 100644 --- a/daemon/execdriver/native/init.go +++ b/daemon/execdriver/native/init.go @@ -10,7 +10,7 @@ import ( "path/filepath" "runtime" - "github.com/docker/docker/reexec" + "github.com/docker/docker/pkg/reexec" "github.com/docker/libcontainer" "github.com/docker/libcontainer/namespaces" "github.com/docker/libcontainer/syncpipe" diff --git a/daemon/graphdriver/aufs/aufs.go b/daemon/graphdriver/aufs/aufs.go index 8e3ae0b18..bbaad08e3 100644 --- a/daemon/graphdriver/aufs/aufs.go +++ b/daemon/graphdriver/aufs/aufs.go @@ -32,6 +32,7 @@ import ( "github.com/docker/docker/daemon/graphdriver" "github.com/docker/docker/pkg/archive" + "github.com/docker/docker/pkg/chrootarchive" "github.com/docker/docker/pkg/log" mountpk "github.com/docker/docker/pkg/mount" "github.com/docker/docker/utils" @@ -304,7 +305,7 @@ func (a *Driver) Diff(id, parent string) (archive.Archive, error) { } func (a *Driver) applyDiff(id string, diff archive.ArchiveReader) error { - return archive.Untar(diff, path.Join(a.rootPath(), "diff", id), nil) + return chrootarchive.Untar(diff, path.Join(a.rootPath(), "diff", id), nil) } // DiffSize calculates the changes between the specified id diff --git a/daemon/graphdriver/aufs/aufs_test.go b/daemon/graphdriver/aufs/aufs_test.go index cc5b3a203..0c940bc9e 100644 --- a/daemon/graphdriver/aufs/aufs_test.go +++ b/daemon/graphdriver/aufs/aufs_test.go @@ -4,18 +4,24 @@ import ( "crypto/sha256" "encoding/hex" "fmt" - "github.com/docker/docker/daemon/graphdriver" - "github.com/docker/docker/pkg/archive" "io/ioutil" "os" "path" "testing" + + "github.com/docker/docker/daemon/graphdriver" + "github.com/docker/docker/pkg/archive" + "github.com/docker/docker/pkg/reexec" ) var ( tmp = path.Join(os.TempDir(), "aufs-tests", "aufs") ) +func init() { + reexec.Init() +} + func testInit(dir string, t *testing.T) graphdriver.Driver { d, err := Init(dir, nil) if err != nil { diff --git a/daemon/graphdriver/fsdiff.go b/daemon/graphdriver/fsdiff.go index 5e9d32c1c..14e27d60c 100644 --- a/daemon/graphdriver/fsdiff.go +++ b/daemon/graphdriver/fsdiff.go @@ -5,6 +5,7 @@ import ( "time" "github.com/docker/docker/pkg/archive" + "github.com/docker/docker/pkg/chrootarchive" "github.com/docker/docker/pkg/ioutils" "github.com/docker/docker/pkg/log" "github.com/docker/docker/utils" @@ -120,7 +121,7 @@ func (gdw *naiveDiffDriver) ApplyDiff(id, parent string, diff archive.ArchiveRea start := time.Now().UTC() log.Debugf("Start untar layer") - if err = archive.ApplyLayer(layerFs, diff); err != nil { + if err = chrootarchive.ApplyLayer(layerFs, diff); err != nil { return } log.Debugf("Untar time: %vs", time.Now().UTC().Sub(start).Seconds()) diff --git a/daemon/graphdriver/vfs/driver.go b/daemon/graphdriver/vfs/driver.go index a186060d0..aa104500b 100644 --- a/daemon/graphdriver/vfs/driver.go +++ b/daemon/graphdriver/vfs/driver.go @@ -8,6 +8,7 @@ import ( "path" "github.com/docker/docker/daemon/graphdriver" + "github.com/docker/docker/pkg/chrootarchive" "github.com/docker/libcontainer/label" ) @@ -46,21 +47,6 @@ func isGNUcoreutils() bool { return false } -func copyDir(src, dst string) error { - argv := make([]string, 0, 4) - - if isGNUcoreutils() { - argv = append(argv, "-aT", "--reflink=auto", src, dst) - } else { - argv = append(argv, "-a", src+"/.", dst+"/.") - } - - if output, err := exec.Command("cp", argv...).CombinedOutput(); err != nil { - return fmt.Errorf("Error VFS copying directory: %s (%s)", err, output) - } - return nil -} - func (d *Driver) Create(id, parent string) error { dir := d.dir(id) if err := os.MkdirAll(path.Dir(dir), 0700); err != nil { @@ -80,7 +66,7 @@ func (d *Driver) Create(id, parent string) error { if err != nil { return fmt.Errorf("%s: %s", parent, err) } - if err := copyDir(parentDir, dir); err != nil { + if err := chrootarchive.CopyWithTar(parentDir, dir); err != nil { return err } return nil diff --git a/daemon/graphdriver/vfs/vfs_test.go b/daemon/graphdriver/vfs/vfs_test.go index eaf70f59d..1ee6ae4a9 100644 --- a/daemon/graphdriver/vfs/vfs_test.go +++ b/daemon/graphdriver/vfs/vfs_test.go @@ -1,10 +1,17 @@ package vfs import ( - "github.com/docker/docker/daemon/graphdriver/graphtest" "testing" + + "github.com/docker/docker/daemon/graphdriver/graphtest" + + "github.com/docker/docker/pkg/reexec" ) +func init() { + reexec.Init() +} + // This avoids creating a new driver for each test if all tests are run // Make sure to put new tests between TestVfsSetup and TestVfsTeardown func TestVfsSetup(t *testing.T) { diff --git a/daemon/inspect.go b/daemon/inspect.go index 396ca0227..cf2ed644d 100644 --- a/daemon/inspect.go +++ b/daemon/inspect.go @@ -47,6 +47,7 @@ func (daemon *Daemon) ContainerInspect(job *engine.Job) engine.Status { out.Set("ProcessLabel", container.ProcessLabel) out.SetJson("Volumes", container.Volumes) out.SetJson("VolumesRW", container.VolumesRW) + out.SetJson("AppArmorProfile", container.AppArmorProfile) if children, err := daemon.Children(container.Name); err == nil { for linkAlias, child := range children { diff --git a/daemon/list.go b/daemon/list.go index 35effa434..25360b679 100644 --- a/daemon/list.go +++ b/daemon/list.go @@ -93,7 +93,7 @@ func (daemon *Daemon) Containers(job *engine.Job) engine.Status { if len(filt_exited) > 0 && !container.Running { should_skip := true for _, code := range filt_exited { - if code == container.GetExitCode() { + if code == container.ExitCode { should_skip = false break } diff --git a/daemon/networkdriver/portmapper/proxy.go b/daemon/networkdriver/portmapper/proxy.go index 341f0605e..af20469ed 100644 --- a/daemon/networkdriver/portmapper/proxy.go +++ b/daemon/networkdriver/portmapper/proxy.go @@ -14,7 +14,7 @@ import ( "time" "github.com/docker/docker/pkg/proxy" - "github.com/docker/docker/reexec" + "github.com/docker/docker/pkg/reexec" ) const userlandProxyCommandName = "docker-proxy" diff --git a/daemon/start.go b/daemon/start.go index f2c375ddc..f72407e3f 100644 --- a/daemon/start.go +++ b/daemon/start.go @@ -44,6 +44,9 @@ func (daemon *Daemon) ContainerStart(job *engine.Job) engine.Status { } func (daemon *Daemon) setHostConfig(container *Container, hostConfig *runconfig.HostConfig) error { + if err := parseSecurityOpt(container, hostConfig); err != nil { + return err + } // Validate the HostConfig binds. Make sure that: // the source exists for _, bind := range hostConfig.Binds { diff --git a/daemon/volumes.go b/daemon/volumes.go index b34d9678c..f98baa1c7 100644 --- a/daemon/volumes.go +++ b/daemon/volumes.go @@ -10,7 +10,7 @@ import ( "syscall" "github.com/docker/docker/daemon/execdriver" - "github.com/docker/docker/pkg/archive" + "github.com/docker/docker/pkg/chrootarchive" "github.com/docker/docker/pkg/log" "github.com/docker/docker/pkg/symlink" "github.com/docker/docker/volumes" @@ -191,15 +191,20 @@ func parseBindMountSpec(spec string) (string, string, bool, error) { func (container *Container) applyVolumesFrom() error { volumesFrom := container.hostConfig.VolumesFrom + mountGroups := make([]map[string]*Mount, 0, len(volumesFrom)) + for _, spec := range volumesFrom { - mounts, err := parseVolumesFromSpec(container.daemon, spec) + mountGroup, err := parseVolumesFromSpec(container.daemon, spec) if err != nil { return err } + mountGroups = append(mountGroups, mountGroup) + } + for _, mounts := range mountGroups { for _, mnt := range mounts { mnt.container = container - if err = mnt.initialize(); err != nil { + if err := mnt.initialize(); err != nil { return err } } @@ -302,7 +307,7 @@ func copyExistingContents(source, destination string) error { if len(srcList) == 0 { // If the source volume is empty copy files from the root into the volume - if err := archive.CopyWithTar(source, destination); err != nil { + if err := chrootarchive.CopyWithTar(source, destination); err != nil { return err } } diff --git a/docker/docker.go b/docker/docker.go index f0cbb6f6a..12900ecb1 100644 --- a/docker/docker.go +++ b/docker/docker.go @@ -13,7 +13,7 @@ import ( "github.com/docker/docker/api/client" "github.com/docker/docker/dockerversion" flag "github.com/docker/docker/pkg/mflag" - "github.com/docker/docker/reexec" + "github.com/docker/docker/pkg/reexec" "github.com/docker/docker/utils" ) diff --git a/dockerinit/dockerinit.go b/dockerinit/dockerinit.go index c5bba782b..a6754b05a 100644 --- a/dockerinit/dockerinit.go +++ b/dockerinit/dockerinit.go @@ -3,7 +3,7 @@ package main import ( _ "github.com/docker/docker/daemon/execdriver/lxc" _ "github.com/docker/docker/daemon/execdriver/native" - "github.com/docker/docker/reexec" + "github.com/docker/docker/pkg/reexec" ) func main() { diff --git a/docs/sources/reference/commandline/cli.md b/docs/sources/reference/commandline/cli.md index a57b4cc28..6e6ccd69a 100644 --- a/docs/sources/reference/commandline/cli.md +++ b/docs/sources/reference/commandline/cli.md @@ -70,7 +70,7 @@ expect an integer, and they can only be specified once. -g, --graph="/var/lib/docker" Path to use as the root of the Docker runtime -H, --host=[] The socket(s) to bind to in daemon mode or connect to in client mode, specified using one or more tcp://host:port, unix:///path/to/socket, fd://* or fd://socketfd. --icc=true Enable inter-container communication - --insecure-registry=[] Enable insecure communication with specified registries (no certificate verification for HTTPS and enable HTTP fallback) + --insecure-registry=[] Enable insecure communication with specified registries (disables certificate verification for HTTPS and enables HTTP fallback) (e.g., localhost:5000 or 10.20.0.0/16) --ip=0.0.0.0 Default IP address to use when binding container ports --ip-forward=true Enable net.ipv4.ip_forward --ip-masq=true Enable IP masquerading for bridge's IP range @@ -193,24 +193,44 @@ To set the DNS server for all Docker containers, use To set the DNS search domain for all Docker containers, use `docker -d --dns-search example.com`. +### Insecure registries + +Docker considers a private registry either secure or insecure. +In the rest of this section, *registry* is used for *private registry*, and `myregistry:5000` +is a placeholder example for a private registry. + +A secure registry uses TLS and a copy of its CA certificate is placed on the Docker host at +`/etc/docker/certs.d/myregistry:5000/ca.crt`. +An insecure registry is either not using TLS (i.e., listening on plain text HTTP), or is using +TLS with a CA certificate not known by the Docker daemon. The latter can happen when the +certificate was not found under `/etc/docker/certs.d/myregistry:5000/`, or if the certificate +verification failed (i.e., wrong CA). + +By default, Docker assumes all, but local (see local registries below), registries are secure. +Communicating with an insecure registry is not possible if Docker assumes that registry is secure. +In order to communicate with an insecure registry, the Docker daemon requires `--insecure-registry` +in one of the following two forms: + +* `--insecure-registry myregistry:5000` tells the Docker daemon that myregistry:5000 should be considered insecure. +* `--insecure-registry 10.1.0.0/16` tells the Docker daemon that all registries whose domain resolve to an IP address is part +of the subnet described by the CIDR syntax, should be considered insecure. + +The flag can be used multiple times to allow multiple registries to be marked as insecure. + +If an insecure registry is not marked as insecure, `docker pull`, `docker push`, and `docker search` +will result in an error message prompting the user to either secure or pass the `--insecure-registry` +flag to the Docker daemon as described above. + +Local registries, whose IP address falls in the 127.0.0.0/8 range, are automatically marked as insecure +as of Docker 1.3.2. It is not recommended to rely on this, as it may change in the future. + + ### Miscellaneous options IP masquerading uses address translation to allow containers without a public IP to talk to other machines on the Internet. This may interfere with some network topologies and can be disabled with --ip-masq=false. - -By default, Docker will assume all registries are secured via TLS with certificate verification -enabled. Prior versions of Docker used an auto fallback if a registry did not support TLS -(or if the TLS connection failed). This introduced the opportunity for Man In The Middle (MITM) -attacks, so as of Docker 1.3.1, the user must now specify the `--insecure-registry` daemon flag -for each insecure registry. An insecure registry is either not using TLS (i.e. plain text HTTP), -or is using TLS with a CA certificate not known by the Docker daemon (i.e. certification -verification disabled). For example, if there is a registry listening for HTTP at 127.0.0.1:5000, -as of Docker 1.3.1 you are required to specify `--insecure-registry 127.0.0.1:5000` when starting -the Docker daemon. - - Docker supports softlinks for the Docker data directory (`/var/lib/docker`) and for `/var/lib/docker/tmp`. The `DOCKER_TMPDIR` and the data directory can be set like this: diff --git a/docs/sources/release-notes.md b/docs/sources/release-notes.md index 6b2f0fd57..a19eb71b6 100644 --- a/docs/sources/release-notes.md +++ b/docs/sources/release-notes.md @@ -4,6 +4,35 @@ understanding, release #Release Notes +##Version 1.3.2 +(2014-11-24) + +This release fixes some bugs and addresses some security issues. We have also +made improvements to aspects of `docker run`. + +*Security fixes* + +Patches and changes were made to address CVE-2014-6407 and CVE-2014-6408. +Specifically, changes were made in order to: + +* Prevent host privilege escalation from an image extraction vulnerability (CVE-2014-6407). + +* Prevent container escalation from malicious security options applied to images (CVE-2014-6408). + +*Daemon fixes* + +The `--insecure-registry` flag of the `docker run` command has undergone +several refinements and additions. For details, please see the +[command-line reference](http://docs.docker.com/reference/commandline/cli/#run). + +* You can now specify a sub-net in order to set a range of registries which the Docker daemon will consider insecure. + +* By default, Docker now defines `localhost` as an insecure registry. + +* Registries can now be referenced using the Classless Inter-Domain Routing (CIDR) format. + +* When mirroring is enabled, the experimental registry v2 API is skipped. + ##Version 1.3.1 (2014-10-28) diff --git a/graph/load.go b/graph/load.go index 753f31d2c..fcbeef619 100644 --- a/graph/load.go +++ b/graph/load.go @@ -10,6 +10,7 @@ import ( "github.com/docker/docker/engine" "github.com/docker/docker/image" "github.com/docker/docker/pkg/archive" + "github.com/docker/docker/pkg/chrootarchive" "github.com/docker/docker/pkg/log" ) @@ -53,7 +54,7 @@ func (s *TagStore) CmdLoad(job *engine.Job) engine.Status { excludes[i] = k i++ } - if err := archive.Untar(repoFile, repoDir, &archive.TarOptions{Excludes: excludes}); err != nil { + if err := chrootarchive.Untar(repoFile, repoDir, &archive.TarOptions{Excludes: excludes}); err != nil { return job.Error(err) } diff --git a/graph/pools_test.go b/graph/pools_test.go index 785a4bd12..129a5e1fe 100644 --- a/graph/pools_test.go +++ b/graph/pools_test.go @@ -1,6 +1,14 @@ package graph -import "testing" +import ( + "testing" + + "github.com/docker/docker/pkg/reexec" +) + +func init() { + reexec.Init() +} func TestPools(t *testing.T) { s := &TagStore{ diff --git a/graph/pull.go b/graph/pull.go index 897e0813a..05d5ec765 100644 --- a/graph/pull.go +++ b/graph/pull.go @@ -113,9 +113,7 @@ func (s *TagStore) CmdPull(job *engine.Job) engine.Status { return job.Error(err) } - secure := registry.IsSecure(hostname, s.insecureRegistries) - - endpoint, err := registry.NewEndpoint(hostname, secure) + endpoint, err := registry.NewEndpoint(hostname, s.insecureRegistries) if err != nil { return job.Error(err) } @@ -139,7 +137,7 @@ func (s *TagStore) CmdPull(job *engine.Job) engine.Status { mirrors = s.mirrors } - if isOfficial || endpoint.Version == registry.APIVersion2 { + if len(mirrors) == 0 && (isOfficial || endpoint.Version == registry.APIVersion2) { j := job.Eng.Job("trust_update_base") if err = j.Run(); err != nil { return job.Errorf("error updating trust base graph: %s", err) diff --git a/graph/push.go b/graph/push.go index 1d2bf7242..165b580f2 100644 --- a/graph/push.go +++ b/graph/push.go @@ -214,9 +214,7 @@ func (s *TagStore) CmdPush(job *engine.Job) engine.Status { return job.Error(err) } - secure := registry.IsSecure(hostname, s.insecureRegistries) - - endpoint, err := registry.NewEndpoint(hostname, secure) + endpoint, err := registry.NewEndpoint(hostname, s.insecureRegistries) if err != nil { return job.Error(err) } diff --git a/integration-cli/docker_cli_ps_test.go b/integration-cli/docker_cli_ps_test.go index 4bcf95bf7..f2a7b2ab4 100644 --- a/integration-cli/docker_cli_ps_test.go +++ b/integration-cli/docker_cli_ps_test.go @@ -282,3 +282,81 @@ func TestPsListContainersFilterStatus(t *testing.T) { logDone("ps - test ps filter status") } + +func TestPsListContainersFilterExited(t *testing.T) { + deleteAllContainers() + defer deleteAllContainers() + runCmd := exec.Command(dockerBinary, "run", "--name", "zero1", "busybox", "true") + out, _, err := runCommandWithOutput(runCmd) + if err != nil { + t.Fatal(out, err) + } + firstZero, err := getIDByName("zero1") + if err != nil { + t.Fatal(err) + } + + runCmd = exec.Command(dockerBinary, "run", "--name", "zero2", "busybox", "true") + out, _, err = runCommandWithOutput(runCmd) + if err != nil { + t.Fatal(out, err) + } + secondZero, err := getIDByName("zero2") + if err != nil { + t.Fatal(err) + } + + runCmd = exec.Command(dockerBinary, "run", "--name", "nonzero1", "busybox", "false") + out, _, err = runCommandWithOutput(runCmd) + if err == nil { + t.Fatal("Should fail.", out, err) + } + firstNonZero, err := getIDByName("nonzero1") + if err != nil { + t.Fatal(err) + } + + runCmd = exec.Command(dockerBinary, "run", "--name", "nonzero2", "busybox", "false") + out, _, err = runCommandWithOutput(runCmd) + if err == nil { + t.Fatal("Should fail.", out, err) + } + secondNonZero, err := getIDByName("nonzero2") + if err != nil { + t.Fatal(err) + } + + // filter containers by exited=0 + runCmd = exec.Command(dockerBinary, "ps", "-a", "-q", "--no-trunc", "--filter=exited=0") + out, _, err = runCommandWithOutput(runCmd) + if err != nil { + t.Fatal(out, err) + } + ids := strings.Split(strings.TrimSpace(out), "\n") + if len(ids) != 2 { + t.Fatalf("Should be 2 zero exited containerst got %d", len(ids)) + } + if ids[0] != secondZero { + t.Fatalf("First in list should be %q, got %q", secondZero, ids[0]) + } + if ids[1] != firstZero { + t.Fatalf("Second in list should be %q, got %q", firstZero, ids[1]) + } + + runCmd = exec.Command(dockerBinary, "ps", "-a", "-q", "--no-trunc", "--filter=exited=1") + out, _, err = runCommandWithOutput(runCmd) + if err != nil { + t.Fatal(out, err) + } + ids = strings.Split(strings.TrimSpace(out), "\n") + if len(ids) != 2 { + t.Fatalf("Should be 2 zero exited containerst got %d", len(ids)) + } + if ids[0] != secondNonZero { + t.Fatalf("First in list should be %q, got %q", secondNonZero, ids[0]) + } + if ids[1] != firstNonZero { + t.Fatalf("Second in list should be %q, got %q", firstNonZero, ids[1]) + } + logDone("ps - test ps filter exited") +} diff --git a/integration-cli/docker_cli_start_test.go b/integration-cli/docker_cli_start_test.go index 18ad96aef..addc781ca 100644 --- a/integration-cli/docker_cli_start_test.go +++ b/integration-cli/docker_cli_start_test.go @@ -2,6 +2,7 @@ package main import ( "os/exec" + "strings" "testing" "time" ) @@ -36,3 +37,31 @@ func TestStartAttachReturnsOnError(t *testing.T) { logDone("start - error on start with attach exits") } + +// gh#8726: a failed Start() breaks --volumes-from on subsequent Start()'s +func TestStartVolumesFromFailsCleanly(t *testing.T) { + defer deleteAllContainers() + + // Create the first data volume + cmd(t, "run", "-d", "--name", "data_before", "-v", "/foo", "busybox") + + // Expect this to fail because the data test after contaienr doesn't exist yet + if _, err := runCommand(exec.Command(dockerBinary, "run", "-d", "--name", "consumer", "--volumes-from", "data_before", "--volumes-from", "data_after", "busybox")); err == nil { + t.Fatal("Expected error but got none") + } + + // Create the second data volume + cmd(t, "run", "-d", "--name", "data_after", "-v", "/bar", "busybox") + + // Now, all the volumes should be there + cmd(t, "start", "consumer") + + // Check that we have the volumes we want + out, _, _ := cmd(t, "inspect", "--format='{{ len .Volumes }}'", "consumer") + n_volumes := strings.Trim(out, " \r\n'") + if n_volumes != "2" { + t.Fatalf("Missing volumes: expected 2, got %s", n_volumes) + } + + logDone("start - missing containers in --volumes-from did not affect subsequent runs") +} diff --git a/integration/runtime_test.go b/integration/runtime_test.go index 6720485e2..0a9a74cf9 100644 --- a/integration/runtime_test.go +++ b/integration/runtime_test.go @@ -22,7 +22,7 @@ import ( "github.com/docker/docker/nat" "github.com/docker/docker/pkg/ioutils" "github.com/docker/docker/pkg/log" - "github.com/docker/docker/reexec" + "github.com/docker/docker/pkg/reexec" "github.com/docker/docker/runconfig" "github.com/docker/docker/utils" ) diff --git a/pkg/archive/archive.go b/pkg/archive/archive.go index 7d9103e10..155145f5d 100644 --- a/pkg/archive/archive.go +++ b/pkg/archive/archive.go @@ -35,10 +35,22 @@ type ( Compression Compression NoLchown bool } + + // Archiver allows the reuse of most utility functions of this package + // with a pluggable Untar function. + Archiver struct { + Untar func(io.Reader, string, *TarOptions) error + } + + // breakoutError is used to differentiate errors related to breaking out + // When testing archive breakout in the unit tests, this error is expected + // in order for the test to pass. + breakoutError error ) var ( ErrNotImplemented = errors.New("Function not implemented") + defaultArchiver = &Archiver{Untar} ) const ( @@ -263,11 +275,25 @@ func createTarFile(path, extractDir string, hdr *tar.Header, reader io.Reader, L } case tar.TypeLink: - if err := os.Link(filepath.Join(extractDir, hdr.Linkname), path); err != nil { + targetPath := filepath.Join(extractDir, hdr.Linkname) + // check for hardlink breakout + if !strings.HasPrefix(targetPath, extractDir) { + return breakoutError(fmt.Errorf("invalid hardlink %q -> %q", targetPath, hdr.Linkname)) + } + if err := os.Link(targetPath, path); err != nil { return err } case tar.TypeSymlink: + // path -> hdr.Linkname = targetPath + // e.g. /extractDir/path/to/symlink -> ../2/file = /extractDir/path/2/file + targetPath := filepath.Join(filepath.Dir(path), hdr.Linkname) + + // the reason we don't need to check symlinks in the path (with FollowSymlinkInScope) is because + // that symlink would first have to be created, which would be caught earlier, at this very check: + if !strings.HasPrefix(targetPath, extractDir) { + return breakoutError(fmt.Errorf("invalid symlink %q -> %q", path, hdr.Linkname)) + } if err := os.Symlink(hdr.Linkname, path); err != nil { return err } @@ -412,6 +438,8 @@ func TarWithOptions(srcPath string, options *TarOptions) (io.ReadCloser, error) // identity (uncompressed), gzip, bzip2, xz. // FIXME: specify behavior when target path exists vs. doesn't exist. func Untar(archive io.Reader, dest string, options *TarOptions) error { + dest = filepath.Clean(dest) + if options == nil { options = &TarOptions{} } @@ -449,6 +477,7 @@ loop: } // Normalize name, for safety and for a simple is-root check + // This keeps "../" as-is, but normalizes "/../" to "/" hdr.Name = filepath.Clean(hdr.Name) for _, exclude := range options.Excludes { @@ -469,7 +498,11 @@ loop: } } + // Prevent symlink breakout path := filepath.Join(dest, hdr.Name) + if !strings.HasPrefix(path, dest) { + return breakoutError(fmt.Errorf("%q is outside of %q", path, dest)) + } // If path exits we almost always just want to remove and replace it // The only exception is when it is a directory *and* the file from @@ -508,45 +541,47 @@ loop: return nil } -// TarUntar is a convenience function which calls Tar and Untar, with -// the output of one piped into the other. If either Tar or Untar fails, -// TarUntar aborts and returns the error. -func TarUntar(src string, dst string) error { +func (archiver *Archiver) TarUntar(src, dst string) error { log.Debugf("TarUntar(%s %s)", src, dst) archive, err := TarWithOptions(src, &TarOptions{Compression: Uncompressed}) if err != nil { return err } defer archive.Close() - return Untar(archive, dst, nil) + return archiver.Untar(archive, dst, nil) } -// UntarPath is a convenience function which looks for an archive -// at filesystem path `src`, and unpacks it at `dst`. -func UntarPath(src, dst string) error { +// TarUntar is a convenience function which calls Tar and Untar, with the output of one piped into the other. +// If either Tar or Untar fails, TarUntar aborts and returns the error. +func TarUntar(src, dst string) error { + return defaultArchiver.TarUntar(src, dst) +} + +func (archiver *Archiver) UntarPath(src, dst string) error { archive, err := os.Open(src) if err != nil { return err } defer archive.Close() - if err := Untar(archive, dst, nil); err != nil { + if err := archiver.Untar(archive, dst, nil); err != nil { return err } return nil } -// CopyWithTar creates a tar archive of filesystem path `src`, and -// unpacks it at filesystem path `dst`. -// The archive is streamed directly with fixed buffering and no -// intermediary disk IO. -// -func CopyWithTar(src, dst string) error { +// UntarPath is a convenience function which looks for an archive +// at filesystem path `src`, and unpacks it at `dst`. +func UntarPath(src, dst string) error { + return defaultArchiver.UntarPath(src, dst) +} + +func (archiver *Archiver) CopyWithTar(src, dst string) error { srcSt, err := os.Stat(src) if err != nil { return err } if !srcSt.IsDir() { - return CopyFileWithTar(src, dst) + return archiver.CopyFileWithTar(src, dst) } // Create dst, copy src's content into it log.Debugf("Creating dest directory: %s", dst) @@ -554,16 +589,18 @@ func CopyWithTar(src, dst string) error { return err } log.Debugf("Calling TarUntar(%s, %s)", src, dst) - return TarUntar(src, dst) + return archiver.TarUntar(src, dst) } -// CopyFileWithTar emulates the behavior of the 'cp' command-line -// for a single file. It copies a regular file from path `src` to -// path `dst`, and preserves all its metadata. -// -// If `dst` ends with a trailing slash '/', the final destination path -// will be `dst/base(src)`. -func CopyFileWithTar(src, dst string) (err error) { +// CopyWithTar creates a tar archive of filesystem path `src`, and +// unpacks it at filesystem path `dst`. +// The archive is streamed directly with fixed buffering and no +// intermediary disk IO. +func CopyWithTar(src, dst string) error { + return defaultArchiver.CopyWithTar(src, dst) +} + +func (archiver *Archiver) CopyFileWithTar(src, dst string) (err error) { log.Debugf("CopyFileWithTar(%s, %s)", src, dst) srcSt, err := os.Stat(src) if err != nil { @@ -611,7 +648,17 @@ func CopyFileWithTar(src, dst string) (err error) { err = er } }() - return Untar(r, filepath.Dir(dst), nil) + return archiver.Untar(r, filepath.Dir(dst), nil) +} + +// CopyFileWithTar emulates the behavior of the 'cp' command-line +// for a single file. It copies a regular file from path `src` to +// path `dst`, and preserves all its metadata. +// +// If `dst` ends with a trailing slash '/', the final destination path +// will be `dst/base(src)`. +func CopyFileWithTar(src, dst string) (err error) { + return defaultArchiver.CopyFileWithTar(src, dst) } // CmdStream executes a command, and returns its stdout as a stream. diff --git a/pkg/archive/archive_test.go b/pkg/archive/archive_test.go index b46f95322..7c9db4443 100644 --- a/pkg/archive/archive_test.go +++ b/pkg/archive/archive_test.go @@ -8,6 +8,7 @@ import ( "os" "os/exec" "path" + "path/filepath" "testing" "time" @@ -169,7 +170,12 @@ func TestTarWithOptions(t *testing.T) { // Failing prevents the archives from being uncompressed during ADD func TestTypeXGlobalHeaderDoesNotFail(t *testing.T) { hdr := tar.Header{Typeflag: tar.TypeXGlobalHeader} - err := createTarFile("pax_global_header", "some_dir", &hdr, nil, true) + tmpDir, err := ioutil.TempDir("", "docker-test-archive-pax-test") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tmpDir) + err = createTarFile(filepath.Join(tmpDir, "pax_global_header"), tmpDir, &hdr, nil, true) if err != nil { t.Fatal(err) } @@ -242,3 +248,201 @@ func BenchmarkTarUntar(b *testing.B) { os.RemoveAll(target) } } + +func TestUntarInvalidFilenames(t *testing.T) { + for i, headers := range [][]*tar.Header{ + { + { + Name: "../victim/dotdot", + Typeflag: tar.TypeReg, + Mode: 0644, + }, + }, + { + { + // Note the leading slash + Name: "/../victim/slash-dotdot", + Typeflag: tar.TypeReg, + Mode: 0644, + }, + }, + } { + if err := testBreakout("untar", "docker-TestUntarInvalidFilenames", headers); err != nil { + t.Fatalf("i=%d. %v", i, err) + } + } +} + +func TestUntarInvalidHardlink(t *testing.T) { + for i, headers := range [][]*tar.Header{ + { // try reading victim/hello (../) + { + Name: "dotdot", + Typeflag: tar.TypeLink, + Linkname: "../victim/hello", + Mode: 0644, + }, + }, + { // try reading victim/hello (/../) + { + Name: "slash-dotdot", + Typeflag: tar.TypeLink, + // Note the leading slash + Linkname: "/../victim/hello", + Mode: 0644, + }, + }, + { // try writing victim/file + { + Name: "loophole-victim", + Typeflag: tar.TypeLink, + Linkname: "../victim", + Mode: 0755, + }, + { + Name: "loophole-victim/file", + Typeflag: tar.TypeReg, + Mode: 0644, + }, + }, + { // try reading victim/hello (hardlink, symlink) + { + Name: "loophole-victim", + Typeflag: tar.TypeLink, + Linkname: "../victim", + Mode: 0755, + }, + { + Name: "symlink", + Typeflag: tar.TypeSymlink, + Linkname: "loophole-victim/hello", + Mode: 0644, + }, + }, + { // Try reading victim/hello (hardlink, hardlink) + { + Name: "loophole-victim", + Typeflag: tar.TypeLink, + Linkname: "../victim", + Mode: 0755, + }, + { + Name: "hardlink", + Typeflag: tar.TypeLink, + Linkname: "loophole-victim/hello", + Mode: 0644, + }, + }, + { // Try removing victim directory (hardlink) + { + Name: "loophole-victim", + Typeflag: tar.TypeLink, + Linkname: "../victim", + Mode: 0755, + }, + { + Name: "loophole-victim", + Typeflag: tar.TypeReg, + Mode: 0644, + }, + }, + } { + if err := testBreakout("untar", "docker-TestUntarInvalidHardlink", headers); err != nil { + t.Fatalf("i=%d. %v", i, err) + } + } +} + +func TestUntarInvalidSymlink(t *testing.T) { + for i, headers := range [][]*tar.Header{ + { // try reading victim/hello (../) + { + Name: "dotdot", + Typeflag: tar.TypeSymlink, + Linkname: "../victim/hello", + Mode: 0644, + }, + }, + { // try reading victim/hello (/../) + { + Name: "slash-dotdot", + Typeflag: tar.TypeSymlink, + // Note the leading slash + Linkname: "/../victim/hello", + Mode: 0644, + }, + }, + { // try writing victim/file + { + Name: "loophole-victim", + Typeflag: tar.TypeSymlink, + Linkname: "../victim", + Mode: 0755, + }, + { + Name: "loophole-victim/file", + Typeflag: tar.TypeReg, + Mode: 0644, + }, + }, + { // try reading victim/hello (symlink, symlink) + { + Name: "loophole-victim", + Typeflag: tar.TypeSymlink, + Linkname: "../victim", + Mode: 0755, + }, + { + Name: "symlink", + Typeflag: tar.TypeSymlink, + Linkname: "loophole-victim/hello", + Mode: 0644, + }, + }, + { // try reading victim/hello (symlink, hardlink) + { + Name: "loophole-victim", + Typeflag: tar.TypeSymlink, + Linkname: "../victim", + Mode: 0755, + }, + { + Name: "hardlink", + Typeflag: tar.TypeLink, + Linkname: "loophole-victim/hello", + Mode: 0644, + }, + }, + { // try removing victim directory (symlink) + { + Name: "loophole-victim", + Typeflag: tar.TypeSymlink, + Linkname: "../victim", + Mode: 0755, + }, + { + Name: "loophole-victim", + Typeflag: tar.TypeReg, + Mode: 0644, + }, + }, + { // try writing to victim/newdir/newfile with a symlink in the path + { + // this header needs to be before the next one, or else there is an error + Name: "dir/loophole", + Typeflag: tar.TypeSymlink, + Linkname: "../../victim", + Mode: 0755, + }, + { + Name: "dir/loophole/newdir/newfile", + Typeflag: tar.TypeReg, + Mode: 0644, + }, + }, + } { + if err := testBreakout("untar", "docker-TestUntarInvalidSymlink", headers); err != nil { + t.Fatalf("i=%d. %v", i, err) + } + } +} diff --git a/pkg/archive/diff.go b/pkg/archive/diff.go index 215f62ec0..5ed1a1d90 100644 --- a/pkg/archive/diff.go +++ b/pkg/archive/diff.go @@ -24,6 +24,8 @@ func mkdev(major int64, minor int64) uint32 { // ApplyLayer parses a diff in the standard layer format from `layer`, and // applies it to the directory `dest`. func ApplyLayer(dest string, layer ArchiveReader) error { + dest = filepath.Clean(dest) + // We need to be able to set any perms oldmask := syscall.Umask(0) defer syscall.Umask(oldmask) @@ -93,6 +95,12 @@ func ApplyLayer(dest string, layer ArchiveReader) error { path := filepath.Join(dest, hdr.Name) base := filepath.Base(path) + + // Prevent symlink breakout + if !strings.HasPrefix(path, dest) { + return breakoutError(fmt.Errorf("%q is outside of %q", path, dest)) + } + if strings.HasPrefix(base, ".wh.") { originalBase := base[len(".wh."):] originalPath := filepath.Join(filepath.Dir(path), originalBase) diff --git a/pkg/archive/diff_test.go b/pkg/archive/diff_test.go new file mode 100644 index 000000000..758c4115d --- /dev/null +++ b/pkg/archive/diff_test.go @@ -0,0 +1,191 @@ +package archive + +import ( + "testing" + + "github.com/docker/docker/vendor/src/code.google.com/p/go/src/pkg/archive/tar" +) + +func TestApplyLayerInvalidFilenames(t *testing.T) { + for i, headers := range [][]*tar.Header{ + { + { + Name: "../victim/dotdot", + Typeflag: tar.TypeReg, + Mode: 0644, + }, + }, + { + { + // Note the leading slash + Name: "/../victim/slash-dotdot", + Typeflag: tar.TypeReg, + Mode: 0644, + }, + }, + } { + if err := testBreakout("applylayer", "docker-TestApplyLayerInvalidFilenames", headers); err != nil { + t.Fatalf("i=%d. %v", i, err) + } + } +} + +func TestApplyLayerInvalidHardlink(t *testing.T) { + for i, headers := range [][]*tar.Header{ + { // try reading victim/hello (../) + { + Name: "dotdot", + Typeflag: tar.TypeLink, + Linkname: "../victim/hello", + Mode: 0644, + }, + }, + { // try reading victim/hello (/../) + { + Name: "slash-dotdot", + Typeflag: tar.TypeLink, + // Note the leading slash + Linkname: "/../victim/hello", + Mode: 0644, + }, + }, + { // try writing victim/file + { + Name: "loophole-victim", + Typeflag: tar.TypeLink, + Linkname: "../victim", + Mode: 0755, + }, + { + Name: "loophole-victim/file", + Typeflag: tar.TypeReg, + Mode: 0644, + }, + }, + { // try reading victim/hello (hardlink, symlink) + { + Name: "loophole-victim", + Typeflag: tar.TypeLink, + Linkname: "../victim", + Mode: 0755, + }, + { + Name: "symlink", + Typeflag: tar.TypeSymlink, + Linkname: "loophole-victim/hello", + Mode: 0644, + }, + }, + { // Try reading victim/hello (hardlink, hardlink) + { + Name: "loophole-victim", + Typeflag: tar.TypeLink, + Linkname: "../victim", + Mode: 0755, + }, + { + Name: "hardlink", + Typeflag: tar.TypeLink, + Linkname: "loophole-victim/hello", + Mode: 0644, + }, + }, + { // Try removing victim directory (hardlink) + { + Name: "loophole-victim", + Typeflag: tar.TypeLink, + Linkname: "../victim", + Mode: 0755, + }, + { + Name: "loophole-victim", + Typeflag: tar.TypeReg, + Mode: 0644, + }, + }, + } { + if err := testBreakout("applylayer", "docker-TestApplyLayerInvalidHardlink", headers); err != nil { + t.Fatalf("i=%d. %v", i, err) + } + } +} + +func TestApplyLayerInvalidSymlink(t *testing.T) { + for i, headers := range [][]*tar.Header{ + { // try reading victim/hello (../) + { + Name: "dotdot", + Typeflag: tar.TypeSymlink, + Linkname: "../victim/hello", + Mode: 0644, + }, + }, + { // try reading victim/hello (/../) + { + Name: "slash-dotdot", + Typeflag: tar.TypeSymlink, + // Note the leading slash + Linkname: "/../victim/hello", + Mode: 0644, + }, + }, + { // try writing victim/file + { + Name: "loophole-victim", + Typeflag: tar.TypeSymlink, + Linkname: "../victim", + Mode: 0755, + }, + { + Name: "loophole-victim/file", + Typeflag: tar.TypeReg, + Mode: 0644, + }, + }, + { // try reading victim/hello (symlink, symlink) + { + Name: "loophole-victim", + Typeflag: tar.TypeSymlink, + Linkname: "../victim", + Mode: 0755, + }, + { + Name: "symlink", + Typeflag: tar.TypeSymlink, + Linkname: "loophole-victim/hello", + Mode: 0644, + }, + }, + { // try reading victim/hello (symlink, hardlink) + { + Name: "loophole-victim", + Typeflag: tar.TypeSymlink, + Linkname: "../victim", + Mode: 0755, + }, + { + Name: "hardlink", + Typeflag: tar.TypeLink, + Linkname: "loophole-victim/hello", + Mode: 0644, + }, + }, + { // try removing victim directory (symlink) + { + Name: "loophole-victim", + Typeflag: tar.TypeSymlink, + Linkname: "../victim", + Mode: 0755, + }, + { + Name: "loophole-victim", + Typeflag: tar.TypeReg, + Mode: 0644, + }, + }, + } { + if err := testBreakout("applylayer", "docker-TestApplyLayerInvalidSymlink", headers); err != nil { + t.Fatalf("i=%d. %v", i, err) + } + } +} diff --git a/pkg/archive/utils_test.go b/pkg/archive/utils_test.go new file mode 100644 index 000000000..3624fe5af --- /dev/null +++ b/pkg/archive/utils_test.go @@ -0,0 +1,166 @@ +package archive + +import ( + "bytes" + "fmt" + "io" + "io/ioutil" + "os" + "path/filepath" + "time" + + "github.com/docker/docker/vendor/src/code.google.com/p/go/src/pkg/archive/tar" +) + +var testUntarFns = map[string]func(string, io.Reader) error{ + "untar": func(dest string, r io.Reader) error { + return Untar(r, dest, nil) + }, + "applylayer": func(dest string, r io.Reader) error { + return ApplyLayer(dest, ArchiveReader(r)) + }, +} + +// testBreakout is a helper function that, within the provided `tmpdir` directory, +// creates a `victim` folder with a generated `hello` file in it. +// `untar` extracts to a directory named `dest`, the tar file created from `headers`. +// +// Here are the tested scenarios: +// - removed `victim` folder (write) +// - removed files from `victim` folder (write) +// - new files in `victim` folder (write) +// - modified files in `victim` folder (write) +// - file in `dest` with same content as `victim/hello` (read) +// +// When using testBreakout make sure you cover one of the scenarios listed above. +func testBreakout(untarFn string, tmpdir string, headers []*tar.Header) error { + tmpdir, err := ioutil.TempDir("", tmpdir) + if err != nil { + return err + } + defer os.RemoveAll(tmpdir) + + dest := filepath.Join(tmpdir, "dest") + if err := os.Mkdir(dest, 0755); err != nil { + return err + } + + victim := filepath.Join(tmpdir, "victim") + if err := os.Mkdir(victim, 0755); err != nil { + return err + } + hello := filepath.Join(victim, "hello") + helloData, err := time.Now().MarshalText() + if err != nil { + return err + } + if err := ioutil.WriteFile(hello, helloData, 0644); err != nil { + return err + } + helloStat, err := os.Stat(hello) + if err != nil { + return err + } + + reader, writer := io.Pipe() + go func() { + t := tar.NewWriter(writer) + for _, hdr := range headers { + t.WriteHeader(hdr) + } + t.Close() + }() + + untar := testUntarFns[untarFn] + if untar == nil { + return fmt.Errorf("could not find untar function %q in testUntarFns", untarFn) + } + if err := untar(dest, reader); err != nil { + if _, ok := err.(breakoutError); !ok { + // If untar returns an error unrelated to an archive breakout, + // then consider this an unexpected error and abort. + return err + } + // Here, untar detected the breakout. + // Let's move on verifying that indeed there was no breakout. + fmt.Printf("breakoutError: %v\n", err) + } + + // Check victim folder + f, err := os.Open(victim) + if err != nil { + // codepath taken if victim folder was removed + return fmt.Errorf("archive breakout: error reading %q: %v", victim, err) + } + defer f.Close() + + // Check contents of victim folder + // + // We are only interested in getting 2 files from the victim folder, because if all is well + // we expect only one result, the `hello` file. If there is a second result, it cannot + // hold the same name `hello` and we assume that a new file got created in the victim folder. + // That is enough to detect an archive breakout. + names, err := f.Readdirnames(2) + if err != nil { + // codepath taken if victim is not a folder + return fmt.Errorf("archive breakout: error reading directory content of %q: %v", victim, err) + } + for _, name := range names { + if name != "hello" { + // codepath taken if new file was created in victim folder + return fmt.Errorf("archive breakout: new file %q", name) + } + } + + // Check victim/hello + f, err = os.Open(hello) + if err != nil { + // codepath taken if read permissions were removed + return fmt.Errorf("archive breakout: could not lstat %q: %v", hello, err) + } + defer f.Close() + b, err := ioutil.ReadAll(f) + if err != nil { + return err + } + fi, err := f.Stat() + if err != nil { + return err + } + if helloStat.IsDir() != fi.IsDir() || + // TODO: cannot check for fi.ModTime() change + helloStat.Mode() != fi.Mode() || + helloStat.Size() != fi.Size() || + !bytes.Equal(helloData, b) { + // codepath taken if hello has been modified + return fmt.Errorf("archive breakout: file %q has been modified. Contents: expected=%q, got=%q. FileInfo: expected=%#v, got=%#v.", hello, helloData, b, helloStat, fi) + } + + // Check that nothing in dest/ has the same content as victim/hello. + // Since victim/hello was generated with time.Now(), it is safe to assume + // that any file whose content matches exactly victim/hello, managed somehow + // to access victim/hello. + return filepath.Walk(dest, func(path string, info os.FileInfo, err error) error { + if info.IsDir() { + if err != nil { + // skip directory if error + return filepath.SkipDir + } + // enter directory + return nil + } + if err != nil { + // skip file if error + return nil + } + b, err := ioutil.ReadFile(path) + if err != nil { + // Houston, we have a problem. Aborting (space)walk. + return err + } + if bytes.Equal(helloData, b) { + return fmt.Errorf("archive breakout: file %q has been accessed via %q", hello, path) + } + return nil + }) +} diff --git a/pkg/chrootarchive/archive.go b/pkg/chrootarchive/archive.go new file mode 100644 index 000000000..fc2bea2c4 --- /dev/null +++ b/pkg/chrootarchive/archive.go @@ -0,0 +1,90 @@ +package chrootarchive + +import ( + "bytes" + "encoding/json" + "flag" + "fmt" + "io" + "os" + "runtime" + "strings" + "syscall" + + "github.com/docker/docker/pkg/archive" + "github.com/docker/docker/pkg/reexec" +) + +func untar() { + runtime.LockOSThread() + flag.Parse() + + if err := syscall.Chroot(flag.Arg(0)); err != nil { + fatal(err) + } + if err := syscall.Chdir("/"); err != nil { + fatal(err) + } + options := new(archive.TarOptions) + dec := json.NewDecoder(strings.NewReader(flag.Arg(1))) + if err := dec.Decode(options); err != nil { + fatal(err) + } + if err := archive.Untar(os.Stdin, "/", options); err != nil { + fatal(err) + } + os.Exit(0) +} + +var ( + chrootArchiver = &archive.Archiver{Untar} +) + +func Untar(archive io.Reader, dest string, options *archive.TarOptions) error { + var buf bytes.Buffer + enc := json.NewEncoder(&buf) + if err := enc.Encode(options); err != nil { + return fmt.Errorf("Untar json encode: %v", err) + } + if _, err := os.Stat(dest); os.IsNotExist(err) { + if err := os.MkdirAll(dest, 0777); err != nil { + return err + } + } + + cmd := reexec.Command("docker-untar", dest, buf.String()) + cmd.Stdin = archive + out, err := cmd.CombinedOutput() + if err != nil { + return fmt.Errorf("Untar %s %s", err, out) + } + return nil +} + +func TarUntar(src, dst string) error { + return chrootArchiver.TarUntar(src, dst) +} + +// CopyWithTar creates a tar archive of filesystem path `src`, and +// unpacks it at filesystem path `dst`. +// The archive is streamed directly with fixed buffering and no +// intermediary disk IO. +func CopyWithTar(src, dst string) error { + return chrootArchiver.CopyWithTar(src, dst) +} + +// CopyFileWithTar emulates the behavior of the 'cp' command-line +// for a single file. It copies a regular file from path `src` to +// path `dst`, and preserves all its metadata. +// +// If `dst` ends with a trailing slash '/', the final destination path +// will be `dst/base(src)`. +func CopyFileWithTar(src, dst string) (err error) { + return chrootArchiver.CopyFileWithTar(src, dst) +} + +// UntarPath is a convenience function which looks for an archive +// at filesystem path `src`, and unpacks it at `dst`. +func UntarPath(src, dst string) error { + return chrootArchiver.UntarPath(src, dst) +} diff --git a/pkg/chrootarchive/archive_test.go b/pkg/chrootarchive/archive_test.go new file mode 100644 index 000000000..69e18e319 --- /dev/null +++ b/pkg/chrootarchive/archive_test.go @@ -0,0 +1,44 @@ +package chrootarchive + +import ( + "io/ioutil" + "os" + "path/filepath" + "testing" + + "github.com/docker/docker/pkg/archive" + "github.com/docker/docker/pkg/reexec" +) + +func init() { + reexec.Init() +} + +func TestChrootTarUntar(t *testing.T) { + tmpdir, err := ioutil.TempDir("", "docker-TestChrootTarUntar") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tmpdir) + src := filepath.Join(tmpdir, "src") + if err := os.MkdirAll(src, 0700); err != nil { + t.Fatal(err) + } + if err := ioutil.WriteFile(filepath.Join(src, "toto"), []byte("hello toto"), 0644); err != nil { + t.Fatal(err) + } + if err := ioutil.WriteFile(filepath.Join(src, "lolo"), []byte("hello lolo"), 0644); err != nil { + t.Fatal(err) + } + stream, err := archive.Tar(src, archive.Uncompressed) + if err != nil { + t.Fatal(err) + } + dest := filepath.Join(tmpdir, "src") + if err := os.MkdirAll(dest, 0700); err != nil { + t.Fatal(err) + } + if err := Untar(stream, dest, &archive.TarOptions{Excludes: []string{"lolo"}}); err != nil { + t.Fatal(err) + } +} diff --git a/pkg/chrootarchive/diff.go b/pkg/chrootarchive/diff.go new file mode 100644 index 000000000..2653aefe9 --- /dev/null +++ b/pkg/chrootarchive/diff.go @@ -0,0 +1,46 @@ +package chrootarchive + +import ( + "flag" + "fmt" + "io/ioutil" + "os" + "runtime" + "syscall" + + "github.com/docker/docker/pkg/archive" + "github.com/docker/docker/pkg/reexec" +) + +func applyLayer() { + runtime.LockOSThread() + flag.Parse() + + if err := syscall.Chroot(flag.Arg(0)); err != nil { + fatal(err) + } + if err := syscall.Chdir("/"); err != nil { + fatal(err) + } + tmpDir, err := ioutil.TempDir("/", "temp-docker-extract") + if err != nil { + fatal(err) + } + os.Setenv("TMPDIR", tmpDir) + if err := archive.ApplyLayer("/", os.Stdin); err != nil { + os.RemoveAll(tmpDir) + fatal(err) + } + os.RemoveAll(tmpDir) + os.Exit(0) +} + +func ApplyLayer(dest string, layer archive.ArchiveReader) error { + cmd := reexec.Command("docker-applyLayer", dest) + cmd.Stdin = layer + out, err := cmd.CombinedOutput() + if err != nil { + return fmt.Errorf("ApplyLayer %s %s", err, out) + } + return nil +} diff --git a/pkg/chrootarchive/init.go b/pkg/chrootarchive/init.go new file mode 100644 index 000000000..b548e9fe7 --- /dev/null +++ b/pkg/chrootarchive/init.go @@ -0,0 +1,18 @@ +package chrootarchive + +import ( + "fmt" + "os" + + "github.com/docker/docker/pkg/reexec" +) + +func init() { + reexec.Register("docker-untar", untar) + reexec.Register("docker-applyLayer", applyLayer) +} + +func fatal(err error) { + fmt.Fprint(os.Stderr, err) + os.Exit(1) +} diff --git a/pkg/reexec/MAINTAINERS b/pkg/reexec/MAINTAINERS new file mode 100644 index 000000000..e48a0c7d4 --- /dev/null +++ b/pkg/reexec/MAINTAINERS @@ -0,0 +1 @@ +Michael Crosby (@crosbymichael) diff --git a/reexec/README.md b/pkg/reexec/README.md similarity index 100% rename from reexec/README.md rename to pkg/reexec/README.md diff --git a/pkg/reexec/command_linux.go b/pkg/reexec/command_linux.go new file mode 100644 index 000000000..8dc3f3a4a --- /dev/null +++ b/pkg/reexec/command_linux.go @@ -0,0 +1,18 @@ +// +build linux + +package reexec + +import ( + "os/exec" + "syscall" +) + +func Command(args ...string) *exec.Cmd { + return &exec.Cmd{ + Path: Self(), + Args: args, + SysProcAttr: &syscall.SysProcAttr{ + Pdeathsig: syscall.SIGTERM, + }, + } +} diff --git a/pkg/reexec/command_unsupported.go b/pkg/reexec/command_unsupported.go new file mode 100644 index 000000000..a579318e8 --- /dev/null +++ b/pkg/reexec/command_unsupported.go @@ -0,0 +1,11 @@ +// +build !linux + +package reexec + +import ( + "os/exec" +) + +func Command(args ...string) *exec.Cmd { + return nil +} diff --git a/reexec/reexec.go b/pkg/reexec/reexec.go similarity index 99% rename from reexec/reexec.go rename to pkg/reexec/reexec.go index 136b905bd..774e71c76 100644 --- a/reexec/reexec.go +++ b/pkg/reexec/reexec.go @@ -27,19 +27,16 @@ func Init() bool { return true } - return false } // Self returns the path to the current processes binary func Self() string { name := os.Args[0] - if filepath.Base(name) == name { if lp, err := exec.LookPath(name); err == nil { name = lp } } - return name } diff --git a/pkg/symlink/fs.go b/pkg/symlink/fs.go index da9c59067..6ce99c6bd 100644 --- a/pkg/symlink/fs.go +++ b/pkg/symlink/fs.go @@ -12,6 +12,12 @@ const maxLoopCounter = 100 // FollowSymlink will follow an existing link and scope it to the root // path provided. +// The role of this function is to return an absolute path in the root +// or normalize to the root if the symlink leads to a path which is +// outside of the root. +// Errors encountered while attempting to follow the symlink in path +// will be reported. +// Normalizations to the root don't constitute errors. func FollowSymlinkInScope(link, root string) (string, error) { root, err := filepath.Abs(root) if err != nil { @@ -35,7 +41,6 @@ func FollowSymlinkInScope(link, root string) (string, error) { for _, p := range strings.Split(link, "/") { prev = filepath.Join(prev, p) - prev = filepath.Clean(prev) loopCounter := 0 for { @@ -61,25 +66,36 @@ func FollowSymlinkInScope(link, root string) (string, error) { } return "", err } - if stat.Mode()&os.ModeSymlink == os.ModeSymlink { - dest, err := os.Readlink(prev) - if err != nil { - return "", err - } - if path.IsAbs(dest) { - prev = filepath.Join(root, dest) - } else { - prev, _ = filepath.Abs(prev) - - if prev = filepath.Clean(filepath.Join(filepath.Dir(prev), dest)); len(prev) < len(root) { - prev = filepath.Join(root, filepath.Base(dest)) - } - } - } else { + // let's break if we're not dealing with a symlink + if stat.Mode()&os.ModeSymlink != os.ModeSymlink { break } + + // process the symlink + dest, err := os.Readlink(prev) + if err != nil { + return "", err + } + + if path.IsAbs(dest) { + prev = filepath.Join(root, dest) + } else { + prev, _ = filepath.Abs(prev) + + dir := filepath.Dir(prev) + prev = filepath.Join(dir, dest) + if dir == root && !strings.HasPrefix(prev, root) { + prev = root + } + if len(prev) < len(root) || (len(prev) == len(root) && prev != root) { + prev = filepath.Join(root, filepath.Base(dest)) + } + } } } + if prev == "/" { + prev = root + } return prev, nil } diff --git a/pkg/symlink/fs_test.go b/pkg/symlink/fs_test.go index d85fd6da7..0e2f948b6 100644 --- a/pkg/symlink/fs_test.go +++ b/pkg/symlink/fs_test.go @@ -46,6 +46,7 @@ func TestFollowSymLinkUnderLinkedDir(t *testing.T) { if err != nil { t.Fatal(err) } + defer os.RemoveAll(dir) os.Mkdir(filepath.Join(dir, "realdir"), 0700) os.Symlink("realdir", filepath.Join(dir, "linkdir")) @@ -97,25 +98,151 @@ func TestFollowSymLinkRelativeLink(t *testing.T) { } func TestFollowSymLinkRelativeLinkScope(t *testing.T) { - link := "testdata/fs/a/f" + // avoid letting symlink f lead us out of the "testdata" scope + // we don't normalize because symlink f is in scope and there is no + // information leak + { + link := "testdata/fs/a/f" - rewrite, err := FollowSymlinkInScope(link, "testdata") - if err != nil { - t.Fatal(err) + rewrite, err := FollowSymlinkInScope(link, "testdata") + if err != nil { + t.Fatal(err) + } + + if expected := abs(t, "testdata/test"); expected != rewrite { + t.Fatalf("Expected %s got %s", expected, rewrite) + } } - if expected := abs(t, "testdata/test"); expected != rewrite { - t.Fatalf("Expected %s got %s", expected, rewrite) + // avoid letting symlink f lead us out of the "testdata/fs" scope + // we don't normalize because symlink f is in scope and there is no + // information leak + { + link := "testdata/fs/a/f" + + rewrite, err := FollowSymlinkInScope(link, "testdata/fs") + if err != nil { + t.Fatal(err) + } + + if expected := abs(t, "testdata/fs/test"); expected != rewrite { + t.Fatalf("Expected %s got %s", expected, rewrite) + } } - link = "testdata/fs/b/h" + // avoid letting symlink g (pointed at by symlink h) take out of scope + // TODO: we should probably normalize to scope here because ../[....]/root + // is out of scope and we leak information + { + link := "testdata/fs/b/h" - rewrite, err = FollowSymlinkInScope(link, "testdata") - if err != nil { - t.Fatal(err) + rewrite, err := FollowSymlinkInScope(link, "testdata") + if err != nil { + t.Fatal(err) + } + + if expected := abs(t, "testdata/root"); expected != rewrite { + t.Fatalf("Expected %s got %s", expected, rewrite) + } } - if expected := abs(t, "testdata/root"); expected != rewrite { - t.Fatalf("Expected %s got %s", expected, rewrite) + // avoid letting allowing symlink e lead us to ../b + // normalize to the "testdata/fs/a" + { + link := "testdata/fs/a/e" + + rewrite, err := FollowSymlinkInScope(link, "testdata/fs/a") + if err != nil { + t.Fatal(err) + } + + if expected := abs(t, "testdata/fs/a"); expected != rewrite { + t.Fatalf("Expected %s got %s", expected, rewrite) + } + } + + // avoid letting symlink -> ../directory/file escape from scope + // normalize to "testdata/fs/j" + { + link := "testdata/fs/j/k" + + rewrite, err := FollowSymlinkInScope(link, "testdata/fs/j") + if err != nil { + t.Fatal(err) + } + + if expected := abs(t, "testdata/fs/j"); expected != rewrite { + t.Fatalf("Expected %s got %s", expected, rewrite) + } + } + + // make sure we don't allow escaping to / + // normalize to dir + { + dir, err := ioutil.TempDir("", "docker-fs-test") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(dir) + + linkFile := filepath.Join(dir, "foo") + os.Mkdir(filepath.Join(dir, ""), 0700) + os.Symlink("/", linkFile) + + rewrite, err := FollowSymlinkInScope(linkFile, dir) + if err != nil { + t.Fatal(err) + } + + if rewrite != dir { + t.Fatalf("Expected %s got %s", dir, rewrite) + } + } + + // make sure we don't allow escaping to / + // normalize to dir + { + dir, err := ioutil.TempDir("", "docker-fs-test") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(dir) + + linkFile := filepath.Join(dir, "foo") + os.Mkdir(filepath.Join(dir, ""), 0700) + os.Symlink("/../../", linkFile) + + rewrite, err := FollowSymlinkInScope(linkFile, dir) + if err != nil { + t.Fatal(err) + } + + if rewrite != dir { + t.Fatalf("Expected %s got %s", dir, rewrite) + } + } + + // make sure we stay in scope without leaking information + // this also checks for escaping to / + // normalize to dir + { + dir, err := ioutil.TempDir("", "docker-fs-test") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(dir) + + linkFile := filepath.Join(dir, "foo") + os.Mkdir(filepath.Join(dir, ""), 0700) + os.Symlink("../../", linkFile) + + rewrite, err := FollowSymlinkInScope(linkFile, dir) + if err != nil { + t.Fatal(err) + } + + if rewrite != dir { + t.Fatalf("Expected %s got %s", dir, rewrite) + } } } diff --git a/pkg/symlink/testdata/fs/j/k b/pkg/symlink/testdata/fs/j/k new file mode 120000 index 000000000..f559e8fda --- /dev/null +++ b/pkg/symlink/testdata/fs/j/k @@ -0,0 +1 @@ +../i/a \ No newline at end of file diff --git a/registry/auth.go b/registry/auth.go index 7c0709a47..dad58c163 100644 --- a/registry/auth.go +++ b/registry/auth.go @@ -7,6 +7,7 @@ import ( "fmt" "io/ioutil" "net/http" + "net/url" "os" "path" "strings" @@ -27,8 +28,17 @@ const ( var ( ErrConfigFileMissing = errors.New("The Auth config file is missing") + IndexServerURL *url.URL ) +func init() { + url, err := url.Parse(INDEXSERVER) + if err != nil { + panic(err) + } + IndexServerURL = url +} + type AuthConfig struct { Username string `json:"username,omitempty"` Password string `json:"password,omitempty"` diff --git a/registry/endpoint.go b/registry/endpoint.go index 6dd4e1f60..d65fd7e8a 100644 --- a/registry/endpoint.go +++ b/registry/endpoint.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "io/ioutil" + "net" "net/http" "net/url" "strings" @@ -11,6 +12,9 @@ import ( "github.com/docker/docker/pkg/log" ) +// for mocking in unit tests +var lookupIP = net.LookupIP + // scans string for api version in the URL path. returns the trimmed hostname, if version found, string and API version. func scanForApiVersion(hostname string) (string, APIVersion) { var ( @@ -33,9 +37,40 @@ func scanForApiVersion(hostname string) (string, APIVersion) { return hostname, DefaultAPIVersion } -func NewEndpoint(hostname string, secure bool) (*Endpoint, error) { +func NewEndpoint(hostname string, insecureRegistries []string) (*Endpoint, error) { + endpoint, err := newEndpoint(hostname, insecureRegistries) + if err != nil { + return nil, err + } + + // Try HTTPS ping to registry + endpoint.URL.Scheme = "https" + if _, err := endpoint.Ping(); err != nil { + + //TODO: triggering highland build can be done there without "failing" + + if endpoint.secure { + // If registry is secure and HTTPS failed, show user the error and tell them about `--insecure-registry` + // in case that's what they need. DO NOT accept unknown CA certificates, and DO NOT fallback to HTTP. + return nil, fmt.Errorf("Invalid registry endpoint %s: %v. If this private registry supports only HTTP or HTTPS with an unknown CA certificate, please add `--insecure-registry %s` to the daemon's arguments. In the case of HTTPS, if you have access to the registry's CA certificate, no need for the flag; simply place the CA certificate at /etc/docker/certs.d/%s/ca.crt", endpoint, err, endpoint.URL.Host, endpoint.URL.Host) + } + + // If registry is insecure and HTTPS failed, fallback to HTTP. + log.Debugf("Error from registry %q marked as insecure: %v. Insecurely falling back to HTTP", endpoint, err) + endpoint.URL.Scheme = "http" + _, err2 := endpoint.Ping() + if err2 == nil { + return endpoint, nil + } + + return nil, fmt.Errorf("Invalid registry endpoint %q. HTTPS attempt: %v. HTTP attempt: %v", endpoint, err, err2) + } + + return endpoint, nil +} +func newEndpoint(hostname string, insecureRegistries []string) (*Endpoint, error) { var ( - endpoint = Endpoint{secure: secure} + endpoint = Endpoint{} trimmedHostname string err error ) @@ -47,30 +82,10 @@ func NewEndpoint(hostname string, secure bool) (*Endpoint, error) { if err != nil { return nil, err } - - // Try HTTPS ping to registry - endpoint.URL.Scheme = "https" - if _, err := endpoint.Ping(); err != nil { - - //TODO: triggering highland build can be done there without "failing" - - if secure { - // If registry is secure and HTTPS failed, show user the error and tell them about `--insecure-registry` - // in case that's what they need. DO NOT accept unknown CA certificates, and DO NOT fallback to HTTP. - return nil, fmt.Errorf("Invalid registry endpoint %s: %v. If this private registry supports only HTTP or HTTPS with an unknown CA certificate, please add `--insecure-registry %s` to the daemon's arguments. In the case of HTTPS, if you have access to the registry's CA certificate, no need for the flag; simply place the CA certificate at /etc/docker/certs.d/%s/ca.crt", endpoint, err, endpoint.URL.Host, endpoint.URL.Host) - } - - // If registry is insecure and HTTPS failed, fallback to HTTP. - log.Debugf("Error from registry %q marked as insecure: %v. Insecurely falling back to HTTP", endpoint, err) - endpoint.URL.Scheme = "http" - _, err2 := endpoint.Ping() - if err2 == nil { - return &endpoint, nil - } - - return nil, fmt.Errorf("Invalid registry endpoint %q. HTTPS attempt: %v. HTTP attempt: %v", endpoint, err, err2) + endpoint.secure, err = isSecure(endpoint.URL.Host, insecureRegistries) + if err != nil { + return nil, err } - return &endpoint, nil } @@ -141,18 +156,58 @@ func (e Endpoint) Ping() (RegistryInfo, error) { return info, nil } -// IsSecure returns false if the provided hostname is part of the list of insecure registries. +// isSecure returns false if the provided hostname is part of the list of insecure registries. // Insecure registries accept HTTP and/or accept HTTPS with certificates from unknown CAs. -func IsSecure(hostname string, insecureRegistries []string) bool { - if hostname == IndexServerAddress() { - return true +// +// The list of insecure registries can contain an element with CIDR notation to specify a whole subnet. +// If the subnet contains one of the IPs of the registry specified by hostname, the latter is considered +// insecure. +// +// hostname should be a URL.Host (`host:port` or `host`) +func isSecure(hostname string, insecureRegistries []string) (bool, error) { + if hostname == IndexServerURL.Host { + return true, nil } - for _, h := range insecureRegistries { - if hostname == h { - return false + host, _, err := net.SplitHostPort(hostname) + if err != nil { + // assume hostname is of the form `host` without the port and go on. + host = hostname + } + addrs, err := lookupIP(host) + if err != nil { + ip := net.ParseIP(host) + if ip == nil { + // if resolving `host` fails, error out, since host is to be net.Dial-ed anyway + return true, fmt.Errorf("issecure: could not resolve %q: %v", host, err) + } + addrs = []net.IP{ip} + } + if len(addrs) == 0 { + return true, fmt.Errorf("issecure: could not resolve %q", host) + } + + for _, addr := range addrs { + for _, r := range insecureRegistries { + // hostname matches insecure registry + if hostname == r { + return false, nil + } + + // now assume a CIDR was passed to --insecure-registry + _, ipnet, err := net.ParseCIDR(r) + if err != nil { + // if could not parse it as a CIDR, even after removing + // assume it's not a CIDR and go on with the next candidate + continue + } + + // check if the addr falls in the subnet + if ipnet.Contains(addr) { + return false, nil + } } } - return true + return true, nil } diff --git a/registry/endpoint_test.go b/registry/endpoint_test.go new file mode 100644 index 000000000..54105ec17 --- /dev/null +++ b/registry/endpoint_test.go @@ -0,0 +1,27 @@ +package registry + +import "testing" + +func TestEndpointParse(t *testing.T) { + testData := []struct { + str string + expected string + }{ + {IndexServerAddress(), IndexServerAddress()}, + {"http://0.0.0.0:5000", "http://0.0.0.0:5000/v1/"}, + {"0.0.0.0:5000", "https://0.0.0.0:5000/v1/"}, + } + for _, td := range testData { + e, err := newEndpoint(td.str, insecureRegistries) + if err != nil { + t.Errorf("%q: %s", td.str, err) + } + if e == nil { + t.Logf("something's fishy, endpoint for %q is nil", td.str) + continue + } + if e.String() != td.expected { + t.Errorf("expected %q, got %q", td.expected, e.String()) + } + } +} diff --git a/registry/registry_mock_test.go b/registry/registry_mock_test.go index 379dc78f4..50724f0f9 100644 --- a/registry/registry_mock_test.go +++ b/registry/registry_mock_test.go @@ -2,9 +2,11 @@ package registry import ( "encoding/json" + "errors" "fmt" "io" "io/ioutil" + "net" "net/http" "net/http/httptest" "net/url" @@ -19,8 +21,9 @@ import ( ) var ( - testHttpServer *httptest.Server - testLayers = map[string]map[string]string{ + testHTTPServer *httptest.Server + insecureRegistries []string + testLayers = map[string]map[string]string{ "77dbf71da1d00e3fbddc480176eac8994025630c6590d11cfc8fe1209c2a1d20": { "json": `{"id":"77dbf71da1d00e3fbddc480176eac8994025630c6590d11cfc8fe1209c2a1d20", "comment":"test base image","created":"2013-03-23T12:53:11.10432-07:00", @@ -79,6 +82,11 @@ var ( "latest": "42d718c941f5c532ac049bf0b0ab53f0062f09a03afd4aa4a02c098e46032b9d", }, } + mockHosts = map[string][]net.IP{ + "": {net.ParseIP("0.0.0.0")}, + "localhost": {net.ParseIP("127.0.0.1"), net.ParseIP("::1")}, + "example.com": {net.ParseIP("42.42.42.42")}, + } ) func init() { @@ -99,7 +107,31 @@ func init() { // /v2/ r.HandleFunc("/v2/version", handlerGetPing).Methods("GET") - testHttpServer = httptest.NewServer(handlerAccessLog(r)) + testHTTPServer = httptest.NewServer(handlerAccessLog(r)) + URL, err := url.Parse(testHTTPServer.URL) + if err != nil { + panic(err) + } + insecureRegistries = []string{URL.Host} + + // override net.LookupIP + lookupIP = func(host string) ([]net.IP, error) { + if host == "127.0.0.1" { + // I believe in future Go versions this will fail, so let's fix it later + return net.LookupIP(host) + } + for h, addrs := range mockHosts { + if host == h { + return addrs, nil + } + for _, addr := range addrs { + if addr.String() == host { + return []net.IP{addr}, nil + } + } + } + return nil, errors.New("lookup: no such host") + } } func handlerAccessLog(handler http.Handler) http.Handler { @@ -111,7 +143,7 @@ func handlerAccessLog(handler http.Handler) http.Handler { } func makeURL(req string) string { - return testHttpServer.URL + req + return testHTTPServer.URL + req } func writeHeaders(w http.ResponseWriter) { @@ -301,7 +333,7 @@ func handlerUsers(w http.ResponseWriter, r *http.Request) { } func handlerImages(w http.ResponseWriter, r *http.Request) { - u, _ := url.Parse(testHttpServer.URL) + u, _ := url.Parse(testHTTPServer.URL) w.Header().Add("X-Docker-Endpoints", fmt.Sprintf("%s , %s ", u.Host, "test.example.com")) w.Header().Add("X-Docker-Token", fmt.Sprintf("FAKE-SESSION-%d", time.Now().UnixNano())) if r.Method == "PUT" { diff --git a/registry/registry_test.go b/registry/registry_test.go index c9a9fc81b..1ffb44f31 100644 --- a/registry/registry_test.go +++ b/registry/registry_test.go @@ -18,7 +18,7 @@ var ( func spawnTestRegistrySession(t *testing.T) *Session { authConfig := &AuthConfig{} - endpoint, err := NewEndpoint(makeURL("/v1/"), false) + endpoint, err := NewEndpoint(makeURL("/v1/"), insecureRegistries) if err != nil { t.Fatal(err) } @@ -30,7 +30,7 @@ func spawnTestRegistrySession(t *testing.T) *Session { } func TestPingRegistryEndpoint(t *testing.T) { - ep, err := NewEndpoint(makeURL("/v1/"), false) + ep, err := NewEndpoint(makeURL("/v1/"), insecureRegistries) if err != nil { t.Fatal(err) } @@ -316,3 +316,40 @@ func TestAddRequiredHeadersToRedirectedRequests(t *testing.T) { } } } + +func TestIsSecure(t *testing.T) { + tests := []struct { + addr string + insecureRegistries []string + expected bool + }{ + {IndexServerURL.Host, nil, true}, + {"example.com", []string{}, true}, + {"example.com", []string{"example.com"}, false}, + {"localhost", []string{"localhost:5000"}, false}, + {"localhost:5000", []string{"localhost:5000"}, false}, + {"localhost", []string{"example.com"}, false}, + {"127.0.0.1:5000", []string{"127.0.0.1:5000"}, false}, + {"localhost", nil, false}, + {"localhost:5000", nil, false}, + {"127.0.0.1", nil, false}, + {"localhost", []string{"example.com"}, false}, + {"127.0.0.1", []string{"example.com"}, false}, + {"example.com", nil, true}, + {"example.com", []string{"example.com"}, false}, + {"127.0.0.1", []string{"example.com"}, false}, + {"127.0.0.1:5000", []string{"example.com"}, false}, + {"example.com:5000", []string{"42.42.0.0/16"}, false}, + {"example.com", []string{"42.42.0.0/16"}, false}, + {"example.com:5000", []string{"42.42.42.42/8"}, false}, + {"127.0.0.1:5000", []string{"127.0.0.0/8"}, false}, + {"42.42.42.42:5000", []string{"42.1.1.1/8"}, false}, + } + for _, tt := range tests { + // TODO: remove this once we remove localhost insecure by default + insecureRegistries := append(tt.insecureRegistries, "127.0.0.0/8") + if sec, err := isSecure(tt.addr, insecureRegistries); err != nil || sec != tt.expected { + t.Fatalf("isSecure failed for %q %v, expected %v got %v. Error: %v", tt.addr, insecureRegistries, tt.expected, sec, err) + } + } +} diff --git a/registry/service.go b/registry/service.go index 7051d9343..53e8278b0 100644 --- a/registry/service.go +++ b/registry/service.go @@ -40,7 +40,7 @@ func (s *Service) Auth(job *engine.Job) engine.Status { job.GetenvJson("authConfig", authConfig) if addr := authConfig.ServerAddress; addr != "" && addr != IndexServerAddress() { - endpoint, err := NewEndpoint(addr, IsSecure(addr, s.insecureRegistries)) + endpoint, err := NewEndpoint(addr, s.insecureRegistries) if err != nil { return job.Error(err) } @@ -92,9 +92,7 @@ func (s *Service) Search(job *engine.Job) engine.Status { return job.Error(err) } - secure := IsSecure(hostname, s.insecureRegistries) - - endpoint, err := NewEndpoint(hostname, secure) + endpoint, err := NewEndpoint(hostname, s.insecureRegistries) if err != nil { return job.Error(err) } diff --git a/runconfig/config.go b/runconfig/config.go index 28e85de64..c00110bf7 100644 --- a/runconfig/config.go +++ b/runconfig/config.go @@ -32,7 +32,6 @@ type Config struct { Entrypoint []string NetworkDisabled bool OnBuild []string - SecurityOpt []string } func ContainerConfigFromJob(job *engine.Job) *Config { @@ -56,7 +55,6 @@ func ContainerConfigFromJob(job *engine.Job) *Config { } job.GetenvJson("ExposedPorts", &config.ExposedPorts) job.GetenvJson("Volumes", &config.Volumes) - config.SecurityOpt = job.GetenvList("SecurityOpt") if PortSpecs := job.GetenvList("PortSpecs"); PortSpecs != nil { config.PortSpecs = PortSpecs } diff --git a/runconfig/hostconfig.go b/runconfig/hostconfig.go index 5c4952203..ae75434d4 100644 --- a/runconfig/hostconfig.go +++ b/runconfig/hostconfig.go @@ -56,6 +56,7 @@ type HostConfig struct { CapAdd []string CapDrop []string RestartPolicy RestartPolicy + SecurityOpt []string } // This is used by the create command when you want to set both the @@ -90,6 +91,7 @@ func ContainerHostConfigFromJob(job *engine.Job) *HostConfig { job.GetenvJson("PortBindings", &hostConfig.PortBindings) job.GetenvJson("Devices", &hostConfig.Devices) job.GetenvJson("RestartPolicy", &hostConfig.RestartPolicy) + hostConfig.SecurityOpt = job.GetenvList("SecurityOpt") if Binds := job.GetenvList("Binds"); Binds != nil { hostConfig.Binds = Binds } diff --git a/runconfig/parse.go b/runconfig/parse.go index 42ec68898..43976f604 100644 --- a/runconfig/parse.go +++ b/runconfig/parse.go @@ -256,7 +256,6 @@ func Parse(cmd *flag.FlagSet, args []string, sysInfo *sysinfo.SysInfo) (*Config, Volumes: flVolumes.GetMap(), Entrypoint: entrypoint, WorkingDir: *flWorkingDir, - SecurityOpt: flSecurityOpt.GetAll(), } hostConfig := &HostConfig{ @@ -276,6 +275,7 @@ func Parse(cmd *flag.FlagSet, args []string, sysInfo *sysinfo.SysInfo) (*Config, CapAdd: flCapAdd.GetAll(), CapDrop: flCapDrop.GetAll(), RestartPolicy: restartPolicy, + SecurityOpt: flSecurityOpt.GetAll(), } if sysInfo != nil && flMemory > 0 && !sysInfo.SwapLimit {