New container locator for docker/k8s on linux

The docker and k8s workload attestors work backwards from pid to
container by inspecting the proc filesystem. Today, this happens by
inspecting the cgroup file. Identifying the container ID (and pod UID)
from the cgroup file has been a continual arms race. The k8s and docker
workload attestors grew different mechanisms for trying to deal with the
large variety in the output.

Further, with cgroups v2 and private namespaces, the cgroup file might
not have the container ID or pod UID information within it.

This PR unifies the container ID (and pod UID) extraction for both the
docker and k8s workload attestors. The new implementation searches the
mountinfo file first for cgroups mounts. If not found, it will fall back
to the cgroup file (typically necessary only when the workload is
running in the same container as the agent).

The extraction algorithm is the same for both mountinfo and cgroup
entries, and is as follows:
1. Iterator over each entry in the file being searched, extracting
   either the cgroup mount root (mountinfo) or the cgroup group
   path (cgroup) as the source path.
2. Walk backwards through the segments in the source path looking for
   the 64-bit hex digit container ID.
3. If looking for the pod UID (K8s only), then walk backwards through
   the segments in the path looking for the pod UID pattern used by
   kubelet. Start with the segment the container ID was found in
   (truncated to remove the container ID portion).
4. If there are pod UID/container ID conflicts after searching these
   files then log and abort. Entries that have a pod UID override those
   that don't.

The container ID is very often contained in the last segment in the path
but there are situations where it isn't.

This new functionality is NOT enabled by default, but opted in using the
`use_new_container_locator` configurable in each plugin. In 1.10, we can
consider enabling it by default.

The testing for the new code is spread out a little bit. The cgroups
fallback functionality is mostly tested by the existing tests in the
k8s and docker plugin tests. The mountinfo tests are only in the new
containerinfo package.

In the long term, I'd like to see all of the container info extraction
related tests moved solely to the containerinfo package and removed from
the individual plugins.

Resolves spiffe#4004, resolves spiffe#4682, resolves spiffe#4917.

Signed-off-by: Andrew Harding <azdagron@gmail.com>

go.mod

@@ -74,6 +74,7 @@ require (
 	github.com/uber-go/tally/v4 v4.1.16
 	github.com/valyala/fastjson v1.6.4
 	github.com/zeebo/errs v1.3.0
+	go.uber.org/zap v1.27.0
 	golang.org/x/crypto v0.22.0
 	golang.org/x/exp v0.0.0-20231108232855-2478ac86f678
 	golang.org/x/net v0.24.0
@@ -88,6 +89,7 @@ require (
 	k8s.io/apimachinery v0.29.4
 	k8s.io/client-go v0.29.4
 	k8s.io/kube-aggregator v0.29.4
+	k8s.io/mount-utils v0.29.2
 	sigs.k8s.io/controller-runtime v0.17.3
 )
 
@@ -257,6 +259,7 @@ require (
 	github.com/mitchellh/mapstructure v1.5.0 // indirect
 	github.com/mitchellh/reflectwalk v1.0.2 // indirect
 	github.com/moby/docker-image-spec v1.3.1 // indirect
+	github.com/moby/sys/mountinfo v0.6.2 // indirect
 	github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
 	github.com/modern-go/reflect2 v1.0.2 // indirect
 	github.com/mozillazg/docker-credential-acr-helper v0.3.0 // indirect
@@ -326,7 +329,6 @@ require (
 	go.step.sm/crypto v0.44.2 // indirect
 	go.uber.org/atomic v1.11.0 // indirect
 	go.uber.org/multierr v1.11.0 // indirect
-	go.uber.org/zap v1.27.0 // indirect
 	golang.org/x/mod v0.16.0 // indirect
 	golang.org/x/oauth2 v0.19.0 // indirect
 	golang.org/x/term v0.19.0 // indirect

go.sum

@@ -1222,6 +1222,8 @@ github.com/mitchellh/reflectwalk v1.0.2 h1:G2LzWKi524PWgd3mLHV8Y5k7s6XUvT0Gef6zx
 github.com/mitchellh/reflectwalk v1.0.2/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw=
 github.com/moby/docker-image-spec v1.3.1 h1:jMKff3w6PgbfSa69GfNg+zN/XLhfXJGnEx3Nl2EsFP0=
 github.com/moby/docker-image-spec v1.3.1/go.mod h1:eKmb5VW8vQEh/BAr2yvVNvuiJuY6UIocYsFu/DxxRpo=
+github.com/moby/sys/mountinfo v0.6.2 h1:BzJjoreD5BMFNmD9Rus6gdd1pLuecOFPt8wC+Vygl78=
+github.com/moby/sys/mountinfo v0.6.2/go.mod h1:IJb6JQeOklcdMU9F5xQ8ZALD+CUr5VlGpwtX+VE0rpI=
 github.com/moby/term v0.5.0 h1:xt8Q1nalod/v7BqbG21f8mQPqH+xAaC9C3N3wfWbVP0=
 github.com/moby/term v0.5.0/go.mod h1:8FzsFHVUBGZdbDsJw/ot+X+d5HLUbvklYLJ9uGfcI3Y=
 github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=
@@ -2237,6 +2239,8 @@ k8s.io/kube-aggregator v0.29.4 h1:yT7vYtwIag4G8HNrktYZ3qz6p6oHKronMAXOw4eQ2WQ=
 k8s.io/kube-aggregator v0.29.4/go.mod h1:zBfe4iXXmw5HinNgN0JoAu5rpXdyCUvRfG99+FVOd68=
 k8s.io/kube-openapi v0.0.0-20231010175941-2dd684a91f00 h1:aVUu9fTY98ivBPKR9Y5w/AuzbMm96cd3YHRTU83I780=
 k8s.io/kube-openapi v0.0.0-20231010175941-2dd684a91f00/go.mod h1:AsvuZPBlUDVuCdzJ87iajxtXuR9oktsTctW/R9wwouA=
+k8s.io/mount-utils v0.29.2 h1:FrUfgvOo63nqJRPXKoqN/DW1lMnR/y0pzpFErKh6p2o=
+k8s.io/mount-utils v0.29.2/go.mod h1:9IWJTMe8tG0MYMLEp60xK9GYVeCdA3g4LowmnVi+t9Y=
 k8s.io/utils v0.0.0-20230726121419-3b25d923346b h1:sgn3ZU783SCgtaSJjpcVVlRqd6GSnlTLKgpAAttJvpI=
 k8s.io/utils v0.0.0-20230726121419-3b25d923346b/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0=
 rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8=

pkg/agent/plugin/workloadattestor/docker/docker.go

@@ -135,7 +135,7 @@ func (p *Plugin) Configure(_ context.Context, req *configv1.ConfigureRequest) (*
 		return nil, status.Errorf(codes.InvalidArgument, "unknown configurations detected: %s", strings.Join(keys, ","))
 	}
 
-	containerHelper, err := createHelper(config)
+	containerHelper, err := createHelper(config, p.log)
 	if err != nil {
 		return nil, err
 	}

pkg/agent/plugin/workloadattestor/docker/docker_posix.go

@@ -5,11 +5,15 @@ package docker
 import (
 	"errors"
 	"fmt"
+	"io"
+	"os"
+	"path/filepath"
 	"regexp"
 
 	"github.com/hashicorp/go-hclog"
 	"github.com/spiffe/spire/pkg/agent/common/cgroups"
 	"github.com/spiffe/spire/pkg/agent/plugin/workloadattestor/docker/cgroup"
+	"github.com/spiffe/spire/pkg/common/containerinfo"
 )
 
 type OSConfig struct {
@@ -19,36 +23,68 @@ type OSConfig struct {
 	// ContainerIDCGroupMatchers is a list of patterns used to discover container IDs from cgroup entries.
 	// See the documentation for cgroup.NewContainerIDFinder in the cgroup subpackage for more information. (Unix)
 	ContainerIDCGroupMatchers []string `hcl:"container_id_cgroup_matchers" json:"container_id_cgroup_matchers"`
+
+	// UseNewContainerLocator, if true, uses the new container locator
+	// mechanism instead of cgroup matchers. Currently defaults to false if
+	// unset. This will default to true in a future release. (Unix)
+	UseNewContainerLocator *bool `hcl:"use_new_container_locator"`
+
+	// Used by tests to use a fake /proc directory instead of the real one
+	rootDir string
 }
 
-func createHelper(c *dockerPluginConfig) (*containerHelper, error) {
-	var containerIDFinder cgroup.ContainerIDFinder = &defaultContainerIDFinder{}
-	var err error
-	if len(c.ContainerIDCGroupMatchers) > 0 {
+func createHelper(c *dockerPluginConfig, log hclog.Logger) (*containerHelper, error) {
+	var containerIDFinder cgroup.ContainerIDFinder
+
+	switch {
+	case c.UseNewContainerLocator != nil && *c.UseNewContainerLocator:
+		log.Info("Using the new container locator")
+	case len(c.ContainerIDCGroupMatchers) > 0:
+		log.Info("Using the legacy container locator with custom cgroup matchers. The new locator will be enabled by default in a future release. Consider using it now by setting `use_new_container_locator=true`.")
+		var err error
 		containerIDFinder, err = cgroup.NewContainerIDFinder(c.ContainerIDCGroupMatchers)
 		if err != nil {
 			return nil, err
 		}
+		// Custom matchers implies the use of the deprecated cgroup matcher.
+	default:
+		log.Info("Using the legacy container locator. The new locator will be enabled by default in a future release. Consider using it now by setting `use_new_container_locator=true`.")
+		containerIDFinder = &defaultContainerIDFinder{}
+	}
+
+	rootDir := c.rootDir
+	if rootDir == "" {
+		rootDir = "/"
 	}
 
 	return &containerHelper{
-		fs:                cgroups.OSFileSystem{},
+		rootDir:           rootDir,
 		containerIDFinder: containerIDFinder,
 	}, nil
 }
 
+type dirFS string
+
+func (d dirFS) Open(p string) (io.ReadCloser, error) {
+	return os.Open(filepath.Join(string(d), p))
+}
+
 type containerHelper struct {
+	rootDir           string
 	containerIDFinder cgroup.ContainerIDFinder
-	fs                cgroups.FileSystem
 }
 
-func (h *containerHelper) getContainerID(pID int32, _ hclog.Logger) (string, error) {
-	cgroupList, err := cgroups.GetCgroups(pID, h.fs)
-	if err != nil {
-		return "", err
+func (h *containerHelper) getContainerID(pID int32, log hclog.Logger) (string, error) {
+	if h.containerIDFinder != nil {
+		cgroupList, err := cgroups.GetCgroups(pID, dirFS(h.rootDir))
+		if err != nil {
+			return "", err
+		}
+		return getContainerIDFromCGroups(h.containerIDFinder, cgroupList)
 	}
 
-	return getContainerIDFromCGroups(h.containerIDFinder, cgroupList)
+	extractor := containerinfo.Extractor{RootDir: h.rootDir}
+	return extractor.GetContainerID(int(pID), log)
 }
 
 func getDockerHost(c *dockerPluginConfig) string {

pkg/agent/plugin/workloadattestor/docker/docker_posix_test.go

@@ -3,14 +3,13 @@
 package docker
 
 import (
-	"io"
 	"os"
-	"strings"
+	"path/filepath"
 	"testing"
 
 	dockerclient "github.com/docker/docker/client"
-	"github.com/spiffe/spire/pkg/agent/common/cgroups"
 	"github.com/spiffe/spire/pkg/agent/plugin/workloadattestor/docker/cgroup"
+	"github.com/spiffe/spire/test/spiretest"
 	"github.com/stretchr/testify/require"
 )
 
@@ -75,8 +74,7 @@ func TestContainerExtraction(t *testing.T) {
 	for _, tt := range tests {
 		tt := tt // alias loop variable as it is used in the closure
 		t.Run(tt.desc, func(t *testing.T) {
-			fs := newFakeFileSystem(tt.cgroups)
-
+			withRootDirOpt := prepareRootDirOpt(t, tt.cgroups)
 			var d Docker = dockerError{}
 			if tt.hasMatch {
 				d = fakeContainer{
@@ -88,7 +86,7 @@ func TestContainerExtraction(t *testing.T) {
 				t,
 				withConfig(t, tt.cfg), // this must be the first option
 				withDocker(d),
-				withFileSystem(fs),
+				withRootDirOpt,
 			)
 
 			selectorValues, err := doAttest(t, p)
@@ -110,12 +108,14 @@ func TestContainerExtraction(t *testing.T) {
 }
 
 func TestCgroupFileNotFound(t *testing.T) {
-	p := newTestPlugin(t, withFileSystem(FakeFileSystem{}))
+	p := newTestPlugin(t, withRootDir(spiretest.TempDir(t)))
 
+	// The new container info extraction code does not consider a missing file
+	// to be an error. It just won't return any container ID so attestation
+	// won't produce any selectors.
 	selectorValues, err := doAttest(t, p)
-	require.Error(t, err)
-	require.Contains(t, err.Error(), "file does not exist")
-	require.Nil(t, selectorValues)
+	require.NoError(t, err)
+	require.Empty(t, selectorValues)
 }
 
 func TestDockerConfigPosix(t *testing.T) {
@@ -148,17 +148,27 @@ container_id_cgroup_matchers = [
 }
 
 func verifyConfigDefault(t *testing.T, c *containerHelper) {
-	require.Equal(t, &defaultContainerIDFinder{}, c.containerIDFinder)
+	// The unit tests configure the plugin to use the new container info
+	// extraction code so the legacy finder should be set to nil.
+	require.Nil(t, c.containerIDFinder)
+}
+
+func withDefaultDataOpt(tb testing.TB) testPluginOpt {
+	return prepareRootDirOpt(tb, testCgroupEntries)
 }
 
-func withDefaultDataOpt() testPluginOpt {
-	fs := newFakeFileSystem(testCgroupEntries)
-	return withFileSystem(fs)
+func prepareRootDirOpt(tb testing.TB, cgroups string) testPluginOpt {
+	rootDir := spiretest.TempDir(tb)
+	procPidPath := filepath.Join(rootDir, "proc", "123")
+	require.NoError(tb, os.MkdirAll(procPidPath, 0755))
+	cgroupsPath := filepath.Join(procPidPath, "cgroup")
+	require.NoError(tb, os.WriteFile(cgroupsPath, []byte(cgroups), 0600))
+	return withRootDir(rootDir)
 }
 
-func withFileSystem(m cgroups.FileSystem) testPluginOpt {
+func withRootDir(dir string) testPluginOpt {
 	return func(p *Plugin) {
-		p.c.fs = m
+		p.c.rootDir = dir
 	}
 }
 
@@ -169,23 +179,3 @@ func withConfig(t *testing.T, cfg string) testPluginOpt {
 		require.NoError(t, err)
 	}
 }
-
-func newFakeFileSystem(cgroups string) FakeFileSystem {
-	return FakeFileSystem{
-		Files: map[string]string{
-			"/proc/123/cgroup": cgroups,
-		},
-	}
-}
-
-type FakeFileSystem struct {
-	Files map[string]string
-}
-
-func (fs FakeFileSystem) Open(path string) (io.ReadCloser, error) {
-	data, ok := fs.Files[path]
-	if !ok {
-		return nil, os.ErrNotExist
-	}
-	return io.NopCloser(strings.NewReader(data)), nil
-}

pkg/agent/plugin/workloadattestor/docker/docker_test.go

@@ -76,7 +76,7 @@ func TestDockerSelectors(t *testing.T) {
 				Env:    tt.mockEnv,
 			}
 
-			p := newTestPlugin(t, withDocker(d), withDefaultDataOpt())
+			p := newTestPlugin(t, withDocker(d), withDefaultDataOpt(t))
 
 			selectorValues, err := doAttest(t, p)
 			require.NoError(t, err)
@@ -89,7 +89,7 @@ func TestDockerSelectors(t *testing.T) {
 func TestDockerError(t *testing.T) {
 	p := newTestPlugin(
 		t,
-		withDefaultDataOpt(),
+		withDefaultDataOpt(t),
 		withDocker(dockerError{}),
 		withDisabledRetryer(),
 	)
@@ -107,7 +107,7 @@ func TestDockerErrorRetries(t *testing.T) {
 		t,
 		withMockClock(mockClock),
 		withDocker(dockerError{}),
-		withDefaultDataOpt(),
+		withDefaultDataOpt(t),
 	)
 
 	go func() {
@@ -131,7 +131,7 @@ func TestDockerErrorContextCancel(t *testing.T) {
 	p := newTestPlugin(
 		t,
 		withMockClock(mockClock),
-		withDefaultDataOpt(),
+		withDefaultDataOpt(t),
 	)
 
 	ctx, cancel := context.WithCancel(context.Background())
@@ -248,7 +248,9 @@ func withDisabledRetryer() testPluginOpt {
 
 func newTestPlugin(t *testing.T, opts ...testPluginOpt) *Plugin {
 	p := New()
-	err := doConfigure(t, p, "")
+	err := doConfigure(t, p, `
+		use_new_container_locator = true
+	`)
 	require.NoError(t, err)
 
 	for _, o := range opts {