Accepting request 546676 from home:cyphar:containers:docker_bsc1057743

- Include secrets fix to handle "old" containers that have orphaned secret
  data. It's not clear why Docker caches these secrets, but fix the problem by
  trashing the references manually. bsc#1057743
  * secrets-0002-SUSE-implement-SUSE-container-secrets.patch

OBS-URL: https://build.opensuse.org/request/show/546676
OBS-URL: https://build.opensuse.org/package/show/Virtualization:containers/docker?expand=0&rev=223

docker.changes

@@ -1,3 +1,11 @@
+-------------------------------------------------------------------
+Thu Nov 30 10:15:20 UTC 2017 - asarai@suse.com
+
+- Include secrets fix to handle "old" containers that have orphaned secret
+  data. It's not clear why Docker caches these secrets, but fix the problem by
+  trashing the references manually. bsc#1057743
+  * secrets-0002-SUSE-implement-SUSE-container-secrets.patch
+
 -------------------------------------------------------------------
 Thu Nov 23 13:48:08 UTC 2017 - rbrown@suse.com
 

secrets-0002-SUSE-implement-SUSE-container-secrets.patch

@@ -1,4 +1,4 @@
-From 7c03750568cb9b67b763cd03f92ce45c567ca043 Mon Sep 17 00:00:00 2001
+From c62fb8fa766b6917839987b7e1323f0523166d32 Mon Sep 17 00:00:00 2001
 From: Aleksa Sarai <asarai@suse.de>
 Date: Wed, 8 Mar 2017 11:43:29 +1100
 Subject: [PATCH 2/2] SUSE: implement SUSE container secrets
@@ -10,11 +10,12 @@ THIS PATCH IS NOT TO BE UPSTREAMED, DUE TO THE FACT THAT IT IS
 SUSE-SPECIFIC, AND UPSTREAM DOES NOT APPROVE OF THIS CONCEPT BECAUSE IT
 MAKES BUILDS NOT ENTIRELY REPRODUCIBLE.
 
+SUSE-Bugs: bsc#1057743 bsc#1055676 bsc#1030702
 Signed-off-by: Aleksa Sarai <asarai@suse.de>
 ---
  daemon/start.go        |   5 +
- daemon/suse_secrets.go | 331 +++++++++++++++++++++++++++++++++++++++++++++++++
+ daemon/suse_secrets.go | 391 +++++++++++++++++++++++++++++++++++++++++++++++++
- 2 files changed, 336 insertions(+)
+ 2 files changed, 396 insertions(+)
  create mode 100644 daemon/suse_secrets.go
 
 diff --git a/daemon/start.go b/daemon/start.go
@@ -35,10 +36,10 @@ index 55438cf2c45f..7dfa6cd1d055 100644
  		return err
 diff --git a/daemon/suse_secrets.go b/daemon/suse_secrets.go
 new file mode 100644
-index 000000000000..b6914adf09ce
+index 000000000000..9d0788f0410d
 --- /dev/null
 +++ b/daemon/suse_secrets.go
-@@ -0,0 +1,331 @@
+@@ -0,0 +1,391 @@
 +/*
 + * suse-secrets: patch for Docker to implement SUSE secrets
 + * Copyright (C) 2017 SUSE LLC.
@@ -66,6 +67,7 @@ index 000000000000..b6914adf09ce
 +	"io/ioutil"
 +	"os"
 +	"path/filepath"
++	"strings"
 +	"syscall"
 +
 +	"github.com/docker/docker/container"
@@ -322,17 +324,40 @@ index 000000000000..b6914adf09ce
 +	return secrets, nil
 +}
 +
++// To fake an empty store, in the case where we are operating on a container
++// that was created pre-swarmkit. Otherwise segfaults and other fun things
++// happen. See bsc#1057743.
++type (
++	suseEmptyStore  struct{}
++	suseEmptySecret struct{}
++	suseEmptyConfig struct{}
++)
++
 +// In order to reduce the amount of code touched outside of this file, we
 +// implement the swarm API for DependencyGetter. This asserts that this
-+// requirement will always be matched.
++// requirement will always be matched. In addition, for the case of the *empty*
-+var _ swarmexec.DependencyGetter = &suseDependencyStore{}
++// getters this reduces memory usage by having a global instance.
++var (
++	_           swarmexec.DependencyGetter = &suseDependencyStore{}
++	emptyStore  swarmexec.DependencyGetter = suseEmptyStore{}
++	emptySecret swarmexec.SecretGetter     = suseEmptySecret{}
++	emptyConfig swarmexec.ConfigGetter     = suseEmptyConfig{}
++)
++
++var errSuseEmptyStore = fmt.Errorf("SUSE:secrets :: tried to get a resource from empty store [this is a bug]")
++
++func (_ suseEmptyConfig) Get(_ string) (*swarmapi.Config, error) { return nil, errSuseEmptyStore }
++func (_ suseEmptySecret) Get(_ string) (*swarmapi.Secret, error) { return nil, errSuseEmptyStore }
++func (_ suseEmptyStore) Secrets() swarmexec.SecretGetter         { return emptySecret }
++func (_ suseEmptyStore) Configs() swarmexec.ConfigGetter         { return emptyConfig }
 +
 +type suseDependencyStore struct {
 +	dfl     swarmexec.DependencyGetter
 +	secrets map[string]*swarmapi.Secret
 +}
 +
-+// The following are just dumb wrappers that return ourselves.
++// The following are effectively dumb wrappers that return ourselves, or the
++// default.
 +func (s *suseDependencyStore) Secrets() swarmexec.SecretGetter { return s }
 +func (s *suseDependencyStore) Configs() swarmexec.ConfigGetter { return s.dfl.Configs() }
 +
@@ -346,15 +371,39 @@ index 000000000000..b6914adf09ce
 +		// fallthrough
 +		return s.dfl.Secrets().Get(id)
 +	}
-+
 +	return secret, nil
 +}
 +
++// removeSuseSecrets removes any SecretReferences which were added by us
++// explicitly (this is detected by checking that the prefix has a 'suse'
++// prefix). See bsc#1057743.
++func removeSuseSecrets(c *container.Container) {
++	var without []*swarmtypes.SecretReference
++	for _, secret := range c.SecretReferences {
++		if strings.HasPrefix(secret.SecretID, "suse") {
++			logrus.Warnf("SUSE:secrets :: removing 'old' suse secret %q from container %q", secret.SecretID, c.ID)
++			continue
++		}
++		without = append(without, secret)
++	}
++	c.SecretReferences = without
++}
++
 +func (daemon *Daemon) injectSuseSecretStore(c *container.Container) error {
 +	newDependencyStore := &suseDependencyStore{
 +		dfl:     c.DependencyStore,
 +		secrets: make(map[string]*swarmapi.Secret),
 +	}
++	// Handle old containers. See bsc#1057743.
++	if newDependencyStore.dfl == nil {
++		newDependencyStore.dfl = emptyStore
++	}
++
++	// We drop any "old" SUSE secrets, as it appears that old containers (when
++	// restarted) could still have references to old secrets. The .id() of all
++	// secrets have a prefix of "suse" so this is much easier. See bsc#1057743
++	// for details on why this could cause issues.
++	removeSuseSecrets(c)
 +
 +	secrets, err := getHostSuseSecretData()
 +	if err != nil {
@@ -368,6 +417,18 @@ index 000000000000..b6914adf09ce
 +	}
 +
 +	c.DependencyStore = newDependencyStore
++
++	// bsc#1057743 -- In older versions of Docker we added volumes explicitly
++	// to the mount list. This causes clashes because of duplicate namespaces.
++	// If we see an existing mount that will clash with the in-built secrets
++	// mount we assume it's our fault.
++	for _, intendedMount := range c.SecretMounts() {
++		mountPath := intendedMount.Destination
++		if volume, ok := c.MountPoints[mountPath]; ok {
++			logrus.Debugf("SUSE:secrets :: removing pre-existing %q mount: %#v", mountPath, volume)
++			delete(c.MountPoints, mountPath)
++		}
++	}
 +	return nil
 +}
 --