Browse Source

Merge pull request #9323 from unclejack/merge_release_v1.3.2

Merge release v1.3.2 to master
Tibor Vass 10 years ago
parent
commit
e6ec703df3

+ 16 - 0
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

+ 1 - 1
VERSION

@@ -1 +1 @@
-1.3.1-dev
+1.3.2-dev

+ 6 - 4
builder/internals.go

@@ -24,6 +24,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/parsers"
 	"github.com/docker/docker/pkg/symlink"
 	"github.com/docker/docker/pkg/system"
@@ -47,7 +48,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
 	}
 
@@ -628,7 +630,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)
@@ -638,7 +640,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
 	}
 
@@ -651,7 +653,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
 	}
 

+ 2 - 2
daemon/create.go

@@ -83,8 +83,8 @@ func (daemon *Daemon) Create(config *runconfig.Config, hostConfig *runconfig.Hos
 	if warnings, err = daemon.mergeAndVerifyConfig(config, img); err != nil {
 		return nil, nil, err
 	}
-	if hostConfig != nil && config.SecurityOpt == nil {
-		config.SecurityOpt, err = daemon.GenerateSecurityOpt(hostConfig.IpcMode)
+	if hostConfig != nil && hostConfig.SecurityOpt == nil {
+		hostConfig.SecurityOpt, err = daemon.GenerateSecurityOpt(hostConfig.IpcMode)
 		if err != nil {
 			return nil, nil, err
 		}

+ 5 - 6
daemon/daemon.go

@@ -531,10 +531,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 {
@@ -544,7 +544,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:
@@ -552,7 +552,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
 }
 
@@ -586,7 +586,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
 }
 

+ 1 - 1
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"}

+ 2 - 1
daemon/graphdriver/aufs/aufs.go

@@ -33,6 +33,7 @@ import (
 	log "github.com/Sirupsen/logrus"
 	"github.com/docker/docker/daemon/graphdriver"
 	"github.com/docker/docker/pkg/archive"
+	"github.com/docker/docker/pkg/chrootarchive"
 	mountpk "github.com/docker/docker/pkg/mount"
 	"github.com/docker/docker/utils"
 	"github.com/docker/libcontainer/label"
@@ -305,7 +306,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

+ 5 - 0
daemon/graphdriver/aufs/aufs_test.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/docker/docker/daemon/graphdriver"
 	"github.com/docker/docker/pkg/archive"
+	"github.com/docker/docker/pkg/reexec"
 )
 
 var (
@@ -18,6 +19,10 @@ var (
 	tmp      = path.Join(tmpOuter, "aufs")
 )
 
+func init() {
+	reexec.Init()
+}
+
 func testInit(dir string, t *testing.T) graphdriver.Driver {
 	d, err := Init(dir, nil)
 	if err != nil {

+ 2 - 1
daemon/graphdriver/fsdiff.go

@@ -8,6 +8,7 @@ import (
 
 	log "github.com/Sirupsen/logrus"
 	"github.com/docker/docker/pkg/archive"
+	"github.com/docker/docker/pkg/chrootarchive"
 	"github.com/docker/docker/pkg/ioutils"
 	"github.com/docker/docker/utils"
 )
@@ -122,7 +123,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())

+ 2 - 2
daemon/graphdriver/vfs/driver.go

@@ -8,7 +8,7 @@ import (
 	"path"
 
 	"github.com/docker/docker/daemon/graphdriver"
-	"github.com/docker/docker/pkg/archive"
+	"github.com/docker/docker/pkg/chrootarchive"
 	"github.com/docker/libcontainer/label"
 )
 
@@ -66,7 +66,7 @@ func (d *Driver) Create(id, parent string) error {
 	if err != nil {
 		return fmt.Errorf("%s: %s", parent, err)
 	}
-	if err := archive.CopyWithTar(parentDir, dir); err != nil {
+	if err := chrootarchive.CopyWithTar(parentDir, dir); err != nil {
 		return err
 	}
 	return nil

+ 8 - 1
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) {

+ 1 - 0
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 {

+ 3 - 0
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 {

+ 2 - 2
daemon/volumes.go

@@ -12,7 +12,7 @@ import (
 
 	log "github.com/Sirupsen/logrus"
 	"github.com/docker/docker/daemon/execdriver"
-	"github.com/docker/docker/pkg/archive"
+	"github.com/docker/docker/pkg/chrootarchive"
 	"github.com/docker/docker/pkg/symlink"
 	"github.com/docker/docker/volumes"
 )
@@ -320,7 +320,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
 			}
 		}

+ 4 - 1
graph/load.go

@@ -1,3 +1,5 @@
+// +build linux
+
 package graph
 
 import (
@@ -11,6 +13,7 @@ import (
 	"github.com/docker/docker/engine"
 	"github.com/docker/docker/image"
 	"github.com/docker/docker/pkg/archive"
+	"github.com/docker/docker/pkg/chrootarchive"
 )
 
 // Loads a set of images into the repository. This is the complementary of ImageExport.
@@ -53,7 +56,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)
 	}
 

+ 11 - 0
graph/load_unsupported.go

@@ -0,0 +1,11 @@
+// +build !linux
+
+package graph
+
+import (
+	"github.com/docker/docker/engine"
+)
+
+func (s *TagStore) CmdLoad(job *engine.Job) engine.Status {
+	return job.Errorf("CmdLoad is not supported on this platform")
+}

+ 9 - 1
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{

+ 27 - 1
pkg/archive/archive.go

@@ -42,6 +42,11 @@ type (
 	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 (
@@ -287,11 +292,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
 		}
@@ -451,6 +470,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{}
 	}
@@ -488,6 +509,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 {
@@ -508,7 +530,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

+ 205 - 1
pkg/archive/archive_test.go

@@ -8,6 +8,7 @@ import (
 	"os"
 	"os/exec"
 	"path"
+	"path/filepath"
 	"syscall"
 	"testing"
 	"time"
@@ -214,7 +215,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)
 	}
@@ -403,3 +409,201 @@ func BenchmarkTarUntarWithLinks(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)
+		}
+	}
+}

+ 8 - 0
pkg/archive/diff.go

@@ -18,6 +18,8 @@ import (
 // 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, err := system.Umask(0)
 	if err != nil {
@@ -91,6 +93,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)

+ 191 - 0
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)
+		}
+	}
+}

+ 166 - 0
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
+	})
+}

+ 90 - 0
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)
+}

+ 44 - 0
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)
+	}
+}

+ 46 - 0
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
+}

+ 18 - 0
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)
+}

+ 18 - 0
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,
+		},
+	}
+}

+ 11 - 0
pkg/reexec/command_unsupported.go

@@ -0,0 +1,11 @@
+// +build !linux
+
+package reexec
+
+import (
+	"os/exec"
+)
+
+func Command(args ...string) *exec.Cmd {
+	return nil
+}

+ 0 - 3
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
 }

+ 31 - 14
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 {
@@ -60,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)
+			// let's break if we're not dealing with a symlink
+			if stat.Mode()&os.ModeSymlink != os.ModeSymlink {
+				break
+			}
 
-					if prev = filepath.Join(filepath.Dir(prev), dest); len(prev) < len(root) {
-						prev = filepath.Join(root, filepath.Base(dest))
-					}
-				}
+			// process the symlink
+			dest, err := os.Readlink(prev)
+			if err != nil {
+				return "", err
+			}
+
+			if path.IsAbs(dest) {
+				prev = filepath.Join(root, dest)
 			} else {
-				break
+				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
 }

+ 146 - 20
pkg/symlink/fs_test.go

@@ -98,25 +98,151 @@ func TestFollowSymLinkRelativeLink(t *testing.T) {
 }
 
 func TestFollowSymLinkRelativeLinkScope(t *testing.T) {
-	link := "testdata/fs/a/f"
-
-	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)
-	}
-
-	link = "testdata/fs/b/h"
-
-	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)
+	// 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)
+		}
+
+		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)
+		}
+	}
+
+	// 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)
+		}
+
+		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)
+		}
 	}
 }

+ 1 - 0
pkg/symlink/testdata/fs/j/k

@@ -0,0 +1 @@
+../i/a

+ 0 - 2
runconfig/config.go

@@ -33,7 +33,6 @@ type Config struct {
 	NetworkDisabled bool
 	MacAddress      string
 	OnBuild         []string
-	SecurityOpt     []string
 }
 
 func ContainerConfigFromJob(job *engine.Job) *Config {
@@ -58,7 +57,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
 	}

+ 2 - 0
runconfig/hostconfig.go

@@ -95,6 +95,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
@@ -130,6 +131,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
 	}

+ 1 - 1
runconfig/parse.go

@@ -273,7 +273,6 @@ func Parse(cmd *flag.FlagSet, args []string) (*Config, *HostConfig, *flag.FlagSe
 		MacAddress:      *flMacAddress,
 		Entrypoint:      entrypoint,
 		WorkingDir:      *flWorkingDir,
-		SecurityOpt:     flSecurityOpt.GetAll(),
 	}
 
 	hostConfig := &HostConfig{
@@ -294,6 +293,7 @@ func Parse(cmd *flag.FlagSet, args []string) (*Config, *HostConfig, *flag.FlagSe
 		CapAdd:          flCapAdd.GetAll(),
 		CapDrop:         flCapDrop.GetAll(),
 		RestartPolicy:   restartPolicy,
+		SecurityOpt:     flSecurityOpt.GetAll(),
 	}
 
 	// When allocating stdin in attached mode, close stdin at client disconnect