Merge configs/secrets in unix implementation

On unix, merge secrets/configs handling. This is important because
configs can contain secrets (via templating) and potentially a config
could just simply have secret information "by accident" from the user.
This just make sure that configs are as secure as secrets and de-dups a
lot of code.
Generally this makes everything simpler and configs more secure.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>

container/container.go

@@ -68,13 +68,6 @@ type ExitStatus struct {
 	ExitedAt time.Time
 }
 
-// ConfigReference wraps swarmtypes.ConfigReference to add a Sensitive flag.
-type ConfigReference struct {
-	*swarmtypes.ConfigReference
-	// Sensitive is set if this config should not be written to disk.
-	Sensitive bool
-}
-
 // Container holds the structure defining a container object.
 type Container struct {
 	StreamConfig *stream.Config
@@ -106,7 +99,7 @@ type Container struct {
 	ExecCommands           *exec.Store                `json:"-"`
 	DependencyStore        agentexec.DependencyGetter `json:"-"`
 	SecretReferences       []*swarmtypes.SecretReference
-	ConfigReferences       []*ConfigReference
+	ConfigReferences       []*swarmtypes.ConfigReference
 	// logDriver for closing
 	LogDriver      logger.Logger  `json:"-"`
 	LogCopier      *logger.Copier `json:"-"`
@@ -1056,31 +1049,6 @@ func getSecretTargetPath(r *swarmtypes.SecretReference) string {
 	return filepath.Join(containerSecretMountPath, r.File.Name)
 }
 
-// ConfigsDirPath returns the path to the directory where configs are stored on
-// disk.
-func (container *Container) ConfigsDirPath() (string, error) {
-	return container.GetRootResourcePath("configs")
-}
-
-// ConfigFilePath returns the path to the on-disk location of a config.
-func (container *Container) ConfigFilePath(configRef swarmtypes.ConfigReference) (string, error) {
-	configs, err := container.ConfigsDirPath()
-	if err != nil {
-		return "", err
-	}
-	return filepath.Join(configs, configRef.ConfigID), nil
-}
-
-// SensitiveConfigFilePath returns the path to the location of a config mounted
-// as a secret.
-func (container *Container) SensitiveConfigFilePath(configRef swarmtypes.ConfigReference) (string, error) {
-	secretMountPath, err := container.SecretMountPath()
-	if err != nil {
-		return "", err
-	}
-	return filepath.Join(secretMountPath, configRef.ConfigID+"c"), nil
-}
-
 // CreateDaemonEnvironment creates a new environment variable slice for this container.
 func (container *Container) CreateDaemonEnvironment(tty bool, linkedEnv []string) []string {
 	// Setup environment

container/container_unix.go

@@ -5,11 +5,13 @@ package container // import "github.com/docker/docker/container"
 import (
 	"io/ioutil"
 	"os"
+	"path/filepath"
 
 	"github.com/containerd/continuity/fs"
 	"github.com/docker/docker/api/types"
 	containertypes "github.com/docker/docker/api/types/container"
 	mounttypes "github.com/docker/docker/api/types/mount"
+	swarmtypes "github.com/docker/docker/api/types/swarm"
 	"github.com/docker/docker/pkg/mount"
 	"github.com/docker/docker/pkg/stringid"
 	"github.com/docker/docker/volume"
@@ -234,10 +236,7 @@ func (container *Container) SecretMounts() ([]Mount, error) {
 		})
 	}
 	for _, r := range container.ConfigReferences {
-		if !r.Sensitive || r.File == nil {
-			continue
-		}
-		fPath, err := container.SensitiveConfigFilePath(*r.ConfigReference)
+		fPath, err := container.ConfigFilePath(*r)
 		if err != nil {
 			return nil, err
 		}
@@ -267,27 +266,6 @@ func (container *Container) UnmountSecrets() error {
 	return mount.RecursiveUnmount(p)
 }
 
-// ConfigMounts returns the mounts for configs.
-func (container *Container) ConfigMounts() ([]Mount, error) {
-	var mounts []Mount
-	for _, configRef := range container.ConfigReferences {
-		if configRef.Sensitive || configRef.File == nil {
-			continue
-		}
-		src, err := container.ConfigFilePath(*configRef.ConfigReference)
-		if err != nil {
-			return nil, err
-		}
-		mounts = append(mounts, Mount{
-			Source:      src,
-			Destination: configRef.File.Name,
-			Writable:    false,
-		})
-	}
-
-	return mounts, nil
-}
-
 type conflictingUpdateOptions string
 
 func (e conflictingUpdateOptions) Error() string {
@@ -471,3 +449,13 @@ func (container *Container) GetMountPoints() []types.MountPoint {
 	}
 	return mountPoints
 }
+
+// ConfigFilePath returns the path to the on-disk location of a config.
+// On unix, configs are always considered secret
+func (container *Container) ConfigFilePath(configRef swarmtypes.ConfigReference) (string, error) {
+	mounts, err := container.SecretMountPath()
+	if err != nil {
+		return "", err
+	}
+	return filepath.Join(mounts, configRef.ConfigID), nil
+}

container/container_windows.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/docker/docker/api/types"
 	containertypes "github.com/docker/docker/api/types/container"
+	swarmtypes "github.com/docker/docker/api/types/swarm"
 	"github.com/docker/docker/pkg/system"
 )
 
@@ -102,23 +103,20 @@ func (container *Container) CreateConfigSymlinks() error {
 }
 
 // ConfigMounts returns the mount for configs.
-// All configs are stored in a single mount on Windows. Target symlinks are
-// created for each config, pointing to the files in this mount.
-func (container *Container) ConfigMounts() ([]Mount, error) {
+// TODO: Right now Windows doesn't really have a "secure" storage for secrets,
+// however some configs may contain secrets. Once secure storage is worked out,
+// configs and secret handling should be merged.
+func (container *Container) ConfigMounts() []Mount {
 	var mounts []Mount
 	if len(container.ConfigReferences) > 0 {
-		src, err := container.ConfigsDirPath()
-		if err != nil {
-			return nil, err
-		}
 		mounts = append(mounts, Mount{
-			Source:      src,
+			Source:      container.ConfigsDirPath(),
 			Destination: containerInternalConfigsDirPath,
 			Writable:    false,
 		})
 	}
 
-	return mounts, nil
+	return mounts
 }
 
 // DetachAndUnmount unmounts all volumes.
@@ -204,3 +202,12 @@ func (container *Container) GetMountPoints() []types.MountPoint {
 	}
 	return mountPoints
 }
+
+func (container *Container) ConfigsDirPath() string {
+	return filepath.Join(container.Root, "configs")
+}
+
+// ConfigFilePath returns the path to the on-disk location of a config.
+func (container *Container) ConfigFilePath(configRef swarmtypes.ConfigReference) string {
+	return filepath.Join(container.ConfigsDirPath(), configRef.ConfigID)
+}

daemon/configs.go

@@ -2,7 +2,6 @@ package daemon // import "github.com/docker/docker/daemon"
 
 import (
 	swarmtypes "github.com/docker/docker/api/types/swarm"
-	"github.com/docker/docker/container"
 	"github.com/sirupsen/logrus"
 )
 
@@ -17,10 +16,6 @@ func (daemon *Daemon) SetContainerConfigReferences(name string, refs []*swarmtyp
 	if err != nil {
 		return err
 	}
-
-	for _, ref := range refs {
-		c.ConfigReferences = append(c.ConfigReferences, &container.ConfigReference{ConfigReference: ref})
-	}
-
+	c.ConfigReferences = append(c.ConfigReferences, refs...)
 	return nil
 }

daemon/container_operations_unix.go

@@ -161,20 +161,16 @@ func (daemon *Daemon) setupIpcDirs(c *container.Container) error {
 }
 
 func (daemon *Daemon) setupSecretDir(c *container.Container) (setupErr error) {
-	if len(c.SecretReferences) == 0 {
+	if len(c.SecretReferences) == 0 && len(c.ConfigReferences) == 0 {
 		return nil
 	}
 
-	localMountPath, err := c.SecretMountPath()
-	if err != nil {
-		return errors.Wrap(err, "error getting secrets mount path for container")
-	}
-	if err := daemon.createSecretsDir(localMountPath); err != nil {
+	if err := daemon.createSecretsDir(c); err != nil {
 		return err
 	}
 	defer func() {
 		if setupErr != nil {
-			daemon.cleanupSecretDir(localMountPath)
+			daemon.cleanupSecretDir(c)
 		}
 	}()
 
@@ -231,88 +227,16 @@ func (daemon *Daemon) setupSecretDir(c *container.Container) (setupErr error) {
 		}
 	}
 
-	return daemon.remountSecretDir(c.MountLabel, localMountPath)
-}
-
-// createSecretsDir is used to create a dir suitable for storing container secrets.
-// In practice this is using a tmpfs mount and is used for both "configs" and "secrets"
-func (daemon *Daemon) createSecretsDir(dir string) error {
-	// retrieve possible remapped range start for root UID, GID
-	rootIDs := daemon.idMappings.RootPair()
-	// create tmpfs
-	if err := idtools.MkdirAllAndChown(dir, 0700, rootIDs); err != nil {
-		return errors.Wrap(err, "error creating secret local mount path")
-	}
-
-	tmpfsOwnership := fmt.Sprintf("uid=%d,gid=%d", rootIDs.UID, rootIDs.GID)
-	if err := mount.Mount("tmpfs", dir, "tmpfs", "nodev,nosuid,noexec,"+tmpfsOwnership); err != nil {
-		return errors.Wrap(err, "unable to setup secret mount")
-	}
-
-	return nil
-}
-
-func (daemon *Daemon) remountSecretDir(mountLabel, dir string) error {
-	if err := label.Relabel(dir, mountLabel, false); err != nil {
-		logrus.WithError(err).WithField("dir", dir).Warn("Error while attempting to set selinux label")
-	}
-	rootIDs := daemon.idMappings.RootPair()
-	tmpfsOwnership := fmt.Sprintf("uid=%d,gid=%d", rootIDs.UID, rootIDs.GID)
-
-	// remount secrets ro
-	if err := mount.Mount("tmpfs", dir, "tmpfs", "remount,ro,"+tmpfsOwnership); err != nil {
-		return errors.Wrap(err, "unable to remount dir as readonly")
-	}
-
-	return nil
-}
-
-func (daemon *Daemon) cleanupSecretDir(dir string) {
-	if err := mount.RecursiveUnmount(dir); err != nil {
-		logrus.WithField("dir", dir).WithError(err).Warn("Error while attmepting to unmount dir, this may prevent removal of container.")
-	}
-	if err := os.RemoveAll(dir); err != nil && !os.IsNotExist(err) {
-		logrus.WithField("dir", dir).WithError(err).Error("Error removing dir.")
-	}
-}
-
-func (daemon *Daemon) setupConfigDir(c *container.Container) (setupErr error) {
-	if len(c.ConfigReferences) == 0 {
-		return nil
-	}
-
-	localPath, err := c.ConfigsDirPath()
-	if err != nil {
-		return err
-	}
-	logrus.Debugf("configs: setting up config dir: %s", localPath)
-	if err := daemon.createSecretsDir(localPath); err != nil {
-		return err
-	}
-	defer func() {
-		if setupErr != nil {
-			daemon.cleanupSecretDir(localPath)
-		}
-	}()
-
-	if c.DependencyStore == nil {
-		return errors.New("config store is not initialized")
-	}
-
-	// retrieve possible remapped range start for root UID, GID
-	rootIDs := daemon.idMappings.RootPair()
-
 	for _, ref := range c.ConfigReferences {
 		// TODO (ehazlett): use type switch when more are supported
 		if ref.File == nil {
 			logrus.Error("config target type is not a file target")
 			continue
 		}
-		// configs are created in the ConfigsDirPath on the host, at a
-		// single level
-		fPath, err := c.ConfigFilePath(*ref.ConfigReference)
+
+		fPath, err := c.ConfigFilePath(*ref)
 		if err != nil {
-			return err
+			return errors.Wrap(err, "error getting config file path for container")
 		}
 		if err := idtools.MkdirAllAndChown(filepath.Dir(fPath), 0700, rootIDs); err != nil {
 			return errors.Wrap(err, "error creating config mount path")
@@ -342,14 +266,67 @@ func (daemon *Daemon) setupConfigDir(c *container.Container) (setupErr error) {
 		if err := os.Chown(fPath, rootIDs.UID+uid, rootIDs.GID+gid); err != nil {
 			return errors.Wrap(err, "error setting ownership for config")
 		}
-		if err := os.Chmod(fPath, configRef.File.Mode); err != nil {
+		if err := os.Chmod(fPath, ref.File.Mode); err != nil {
 			return errors.Wrap(err, "error setting file mode for config")
 		}
+	}
+
+	return daemon.remountSecretDir(c)
+}
+
+// createSecretsDir is used to create a dir suitable for storing container secrets.
+// In practice this is using a tmpfs mount and is used for both "configs" and "secrets"
+func (daemon *Daemon) createSecretsDir(c *container.Container) error {
+	// retrieve possible remapped range start for root UID, GID
+	rootIDs := daemon.idMappings.RootPair()
+	dir, err := c.SecretMountPath()
+	if err != nil {
+		return errors.Wrap(err, "error getting container secrets dir")
+	}
+
+	// create tmpfs
+	if err := idtools.MkdirAllAndChown(dir, 0700, rootIDs); err != nil {
+		return errors.Wrap(err, "error creating secret local mount path")
+	}
+
+	tmpfsOwnership := fmt.Sprintf("uid=%d,gid=%d", rootIDs.UID, rootIDs.GID)
+	if err := mount.Mount("tmpfs", dir, "tmpfs", "nodev,nosuid,noexec,"+tmpfsOwnership); err != nil {
+		return errors.Wrap(err, "unable to setup secret mount")
+	}
 
-		label.Relabel(fPath, c.MountLabel, false)
+	return nil
+}
+
+func (daemon *Daemon) remountSecretDir(c *container.Container) error {
+	dir, err := c.SecretMountPath()
+	if err != nil {
+		return errors.Wrap(err, "error getting container secrets path")
 	}
+	if err := label.Relabel(dir, c.MountLabel, false); err != nil {
+		logrus.WithError(err).WithField("dir", dir).Warn("Error while attempting to set selinux label")
+	}
+	rootIDs := daemon.idMappings.RootPair()
+	tmpfsOwnership := fmt.Sprintf("uid=%d,gid=%d", rootIDs.UID, rootIDs.GID)
 
-	return daemon.remountSecretDir(c.MountLabel, localPath)
+	// remount secrets ro
+	if err := mount.Mount("tmpfs", dir, "tmpfs", "remount,ro,"+tmpfsOwnership); err != nil {
+		return errors.Wrap(err, "unable to remount dir as readonly")
+	}
+
+	return nil
+}
+
+func (daemon *Daemon) cleanupSecretDir(c *container.Container) {
+	dir, err := c.SecretMountPath()
+	if err != nil {
+		logrus.WithError(err).WithField("container", c.ID).Warn("error getting secrets mount path for container")
+	}
+	if err := mount.RecursiveUnmount(dir); err != nil {
+		logrus.WithField("dir", dir).WithError(err).Warn("Error while attmepting to unmount dir, this may prevent removal of container.")
+	}
+	if err := os.RemoveAll(dir); err != nil && !os.IsNotExist(err) {
+		logrus.WithField("dir", dir).WithError(err).Error("Error removing dir.")
+	}
 }
 
 func killProcessDirectly(cntr *container.Container) error {

daemon/container_operations_windows.go

@@ -21,10 +21,7 @@ func (daemon *Daemon) setupConfigDir(c *container.Container) (setupErr error) {
 		return nil
 	}
 
-	localPath, err := c.ConfigsDirPath()
-	if err != nil {
-		return err
-	}
+	localPath := c.ConfigsDirPath()
 	logrus.Debugf("configs: setting up config dir: %s", localPath)
 
 	// create local config root
@@ -51,11 +48,7 @@ func (daemon *Daemon) setupConfigDir(c *container.Container) (setupErr error) {
 			continue
 		}
 
-		fPath, err := c.ConfigFilePath(*configRef.ConfigReference)
-		if err != nil {
-			return err
-		}
-
+		fPath := c.ConfigFilePath(*configRef)
 		log := logrus.WithFields(logrus.Fields{"name": configRef.File.Name, "path": fPath})
 
 		log.Debug("injecting config")

daemon/oci_linux.go

@@ -842,18 +842,9 @@ func (daemon *Daemon) createSpec(c *container.Container) (retSpec *specs.Spec, e
 		return nil, err
 	}
 
-	secretMountPath, err := c.SecretMountPath()
-	if err != nil {
-		return nil, err
-	}
-	configsMountPath, err := c.ConfigsDirPath()
-	if err != nil {
-		return nil, err
-	}
 	defer func() {
 		if err != nil {
-			daemon.cleanupSecretDir(secretMountPath)
-			daemon.cleanupSecretDir(configsMountPath)
+			daemon.cleanupSecretDir(c)
 		}
 	}()
 
@@ -861,10 +852,6 @@ func (daemon *Daemon) createSpec(c *container.Container) (retSpec *specs.Spec, e
 		return nil, err
 	}
 
-	if err := daemon.setupConfigDir(c); err != nil {
-		return nil, err
-	}
-
 	ms, err := daemon.setupMounts(c)
 	if err != nil {
 		return nil, err
@@ -886,12 +873,6 @@ func (daemon *Daemon) createSpec(c *container.Container) (retSpec *specs.Spec, e
 	}
 	ms = append(ms, secretMounts...)
 
-	configMounts, err := c.ConfigMounts()
-	if err != nil {
-		return nil, err
-	}
-	ms = append(ms, configMounts...)
-
 	sort.Sort(mounts(ms))
 	if err := setMounts(daemon, &s, c, ms); err != nil {
 		return nil, fmt.Errorf("linux mounts: %v", err)

daemon/oci_windows.go

@@ -102,10 +102,7 @@ func (daemon *Daemon) createSpec(c *container.Container) (*specs.Spec, error) {
 		mounts = append(mounts, secretMounts...)
 	}
 
-	configMounts, err := c.ConfigMounts()
-	if err != nil {
-		return nil, err
-	}
+	configMounts := c.ConfigMounts()
 	if configMounts != nil {
 		mounts = append(mounts, configMounts...)
 	}

integration/config/config_test.go

@@ -292,15 +292,24 @@ func TestTemplatedConfig(t *testing.T) {
 		AttachStderr: true,
 	})
 
-	buf := bytes.NewBuffer(nil)
-	_, err = stdcopy.StdCopy(buf, buf, attach.Reader)
-	require.NoError(t, err)
-
 	expect := "SERVICE_NAME=svc\n" +
 		"this is a secret\n" +
 		"this is a config\n"
+	assertAttachedStream(t, attach, expect)
+
+	attach = swarm.ExecTask(t, d, task, types.ExecConfig{
+		Cmd:          []string{"mount"},
+		AttachStdout: true,
+		AttachStderr: true,
+	})
+	assertAttachedStream(t, attach, "tmpfs on /templated_config type tmpfs")
+}
 
-	assert.Equal(t, expect, buf.String())
+func assertAttachedStream(t *testing.T, attach types.HijackedResponse, expect string) {
+	buf := bytes.NewBuffer(nil)
+	_, err := stdcopy.StdCopy(buf, buf, attach.Reader)
+	require.NoError(t, err)
+	assert.Contains(t, buf.String(), expect)
 }
 
 func waitAndAssert(t *testing.T, timeout time.Duration, f func(*testing.T) bool) {

integration/secret/secret_test.go

@@ -336,15 +336,24 @@ func TestTemplatedSecret(t *testing.T) {
 		AttachStderr: true,
 	})
 
-	buf := bytes.NewBuffer(nil)
-	_, err = stdcopy.StdCopy(buf, buf, attach.Reader)
-	require.NoError(t, err)
-
 	expect := "SERVICE_NAME=svc\n" +
 		"this is a secret\n" +
 		"this is a config\n"
+	assertAttachedStream(t, attach, expect)
+
+	attach = swarm.ExecTask(t, d, task, types.ExecConfig{
+		Cmd:          []string{"mount"},
+		AttachStdout: true,
+		AttachStderr: true,
+	})
+	assertAttachedStream(t, attach, "tmpfs on /run/secrets/templated_secret type tmpfs")
+}
 
-	assert.Equal(t, expect, buf.String())
+func assertAttachedStream(t *testing.T, attach types.HijackedResponse, expect string) {
+	buf := bytes.NewBuffer(nil)
+	_, err := stdcopy.StdCopy(buf, buf, attach.Reader)
+	require.NoError(t, err)
+	assert.Contains(t, buf.String(), expect)
 }
 
 func waitAndAssert(t *testing.T, timeout time.Duration, f func(*testing.T) bool) {