From 4afbcbd36b5a4c7845726c77d98394fbb82d661e Mon Sep 17 00:00:00 2001 From: Andrew Harding Date: Tue, 12 Mar 2024 10:45:31 -0600 Subject: [PATCH 01/10] 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 #4004, resolves #4682, resolves #4917. Signed-off-by: Andrew Harding --- go.mod | 2 + go.sum | 4 + .../plugin/workloadattestor/docker/docker.go | 2 +- .../workloadattestor/docker/docker_posix.go | 58 +++- .../docker/docker_posix_test.go | 62 ++-- .../workloadattestor/docker/docker_test.go | 12 +- pkg/agent/plugin/workloadattestor/k8s/k8s.go | 47 +-- .../plugin/workloadattestor/k8s/k8s_posix.go | 42 ++- .../workloadattestor/k8s/k8s_posix_test.go | 5 +- .../plugin/workloadattestor/k8s/k8s_test.go | 13 +- pkg/common/containerinfo/extract.go | 278 ++++++++++++++++++ pkg/common/containerinfo/extract_test.go | 129 ++++++++ .../container-id-conflict/proc/123/mountinfo | 2 + .../docker/no-cgroup-mount/proc/123/mountinfo | 1 + .../testdata/docker/v1/proc/123/mountinfo | 13 + .../testdata/docker/v2/proc/123/mountinfo | 1 + .../container-id-conflict/proc/123/mountinfo | 2 + .../k8s/no-cgroup-mount/proc/123/mountinfo | 1 + .../k8s/pod-uid-conflict/proc/123/mountinfo | 2 + .../testdata/k8s/v1/proc/123/mountinfo | 13 + .../testdata/k8s/v2/proc/123/mountinfo | 1 + .../testdata/other/fallback/proc/123/cgroup | 1 + .../other/fallback/proc/123/mountinfo | 1 + .../other/malformed/proc/123/mountinfo | 1 + 24 files changed, 601 insertions(+), 92 deletions(-) create mode 100644 pkg/common/containerinfo/extract.go create mode 100644 pkg/common/containerinfo/extract_test.go create mode 100644 pkg/common/containerinfo/testdata/docker/container-id-conflict/proc/123/mountinfo create mode 100644 pkg/common/containerinfo/testdata/docker/no-cgroup-mount/proc/123/mountinfo create mode 100644 pkg/common/containerinfo/testdata/docker/v1/proc/123/mountinfo create mode 100644 pkg/common/containerinfo/testdata/docker/v2/proc/123/mountinfo create mode 100644 pkg/common/containerinfo/testdata/k8s/container-id-conflict/proc/123/mountinfo create mode 100644 pkg/common/containerinfo/testdata/k8s/no-cgroup-mount/proc/123/mountinfo create mode 100644 pkg/common/containerinfo/testdata/k8s/pod-uid-conflict/proc/123/mountinfo create mode 100644 pkg/common/containerinfo/testdata/k8s/v1/proc/123/mountinfo create mode 100644 pkg/common/containerinfo/testdata/k8s/v2/proc/123/mountinfo create mode 100644 pkg/common/containerinfo/testdata/other/fallback/proc/123/cgroup create mode 100644 pkg/common/containerinfo/testdata/other/fallback/proc/123/mountinfo create mode 100644 pkg/common/containerinfo/testdata/other/malformed/proc/123/mountinfo diff --git a/go.mod b/go.mod index 66b86d228d..c37bb3720c 100644 --- a/go.mod +++ b/go.mod @@ -88,6 +88,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 +258,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 diff --git a/go.sum b/go.sum index 29b685cf8f..fc052f7ae3 100644 --- a/go.sum +++ b/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= diff --git a/pkg/agent/plugin/workloadattestor/docker/docker.go b/pkg/agent/plugin/workloadattestor/docker/docker.go index 2709aeed01..3af7c24996 100644 --- a/pkg/agent/plugin/workloadattestor/docker/docker.go +++ b/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 } diff --git a/pkg/agent/plugin/workloadattestor/docker/docker_posix.go b/pkg/agent/plugin/workloadattestor/docker/docker_posix.go index 74045a1e9e..bff58ffb8c 100644 --- a/pkg/agent/plugin/workloadattestor/docker/docker_posix.go +++ b/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 { diff --git a/pkg/agent/plugin/workloadattestor/docker/docker_posix_test.go b/pkg/agent/plugin/workloadattestor/docker/docker_posix_test.go index acd957cf6e..5d8835d5bf 100644 --- a/pkg/agent/plugin/workloadattestor/docker/docker_posix_test.go +++ b/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 -} diff --git a/pkg/agent/plugin/workloadattestor/docker/docker_test.go b/pkg/agent/plugin/workloadattestor/docker/docker_test.go index a3055ac45b..b6908be572 100644 --- a/pkg/agent/plugin/workloadattestor/docker/docker_test.go +++ b/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 { diff --git a/pkg/agent/plugin/workloadattestor/k8s/k8s.go b/pkg/agent/plugin/workloadattestor/k8s/k8s.go index 7cf804b255..314327c2d9 100644 --- a/pkg/agent/plugin/workloadattestor/k8s/k8s.go +++ b/pkg/agent/plugin/workloadattestor/k8s/k8s.go @@ -11,6 +11,7 @@ import ( "net/http" "net/url" "os" + "path/filepath" "strconv" "strings" "sync" @@ -21,7 +22,6 @@ import ( "github.com/hashicorp/hcl" workloadattestorv1 "github.com/spiffe/spire-plugin-sdk/proto/spire/plugin/agent/workloadattestor/v1" configv1 "github.com/spiffe/spire-plugin-sdk/proto/spire/service/common/config/v1" - "github.com/spiffe/spire/pkg/agent/common/cgroups" "github.com/spiffe/spire/pkg/common/catalog" "github.com/spiffe/spire/pkg/common/pemutil" "github.com/spiffe/spire/pkg/common/telemetry" @@ -122,6 +122,11 @@ type HCLConfig struct { // (e.g. when a postStart hook has yet to complete). DisableContainerSelectors bool `hcl:"disable_container_selectors"` + // UseNewContainerLocator, if true, uses the new container locator + // mechanism instead of the legacy cgroup matchers. Defaults to false if + // unset. This will default to true in a future release. + UseNewContainerLocator *bool `hcl:"use_new_container_locator"` + // Experimental enables experimental features. Experimental *ExperimentalK8SConfig `hcl:"experimental,omitempty"` } @@ -176,19 +181,18 @@ type Plugin struct { workloadattestorv1.UnsafeWorkloadAttestorServer configv1.UnsafeConfigServer - log hclog.Logger - clock clock.Clock - fs cgroups.FileSystem - c ContainerHelper - getenv func(string) string + log hclog.Logger + clock clock.Clock + rootDir string + getenv func(string) string - mu sync.RWMutex - config *k8sConfig + mu sync.RWMutex + config *k8sConfig + containerHelper ContainerHelper } func New() *Plugin { return &Plugin{ - fs: cgroups.OSFileSystem{}, clock: clock.New(), getenv: os.Getenv, } @@ -199,12 +203,14 @@ func (p *Plugin) SetLogger(log hclog.Logger) { } func (p *Plugin) Attest(ctx context.Context, req *workloadattestorv1.AttestRequest) (*workloadattestorv1.AttestResponse, error) { - config, err := p.getConfig() + config, containerHelper, err := p.getConfig() if err != nil { return nil, err } - podUID, containerID, err := p.c.GetPodUIDAndContainerID(req.Pid, p.log) + fmt.Printf("attest: containerHelper=%#v\n", containerHelper) + + podUID, containerID, err := containerHelper.GetPodUIDAndContainerID(req.Pid, p.log) if err != nil { return nil, err } @@ -268,7 +274,7 @@ func (p *Plugin) Attest(ctx context.Context, req *workloadattestorv1.AttestReque if !config.DisableContainerSelectors { selectorValues = append(selectorValues, getSelectorValuesFromWorkloadContainerStatus(containerStatus)...) - osSelector, err := p.c.GetOSSelectors(ctx, log, containerStatus) + osSelector, err := containerHelper.GetOSSelectors(ctx, log, containerStatus) switch { case err != nil: return nil, err @@ -400,34 +406,35 @@ func (p *Plugin) Configure(_ context.Context, req *configv1.ConfigureRequest) (r } // Set the config - p.setConfig(c) - p.setContainerHelper(containerHelper) + fmt.Printf("setContainerHelper(%p)\n", containerHelper) + p.setConfig(c, containerHelper) return &configv1.ConfigureResponse{}, nil } -func (p *Plugin) setConfig(config *k8sConfig) { +func (p *Plugin) setConfig(config *k8sConfig, containerHelper ContainerHelper) { p.mu.Lock() defer p.mu.Unlock() p.config = config + p.containerHelper = containerHelper } -func (p *Plugin) getConfig() (*k8sConfig, error) { +func (p *Plugin) getConfig() (*k8sConfig, ContainerHelper, error) { p.mu.RLock() defer p.mu.RUnlock() if p.config == nil { - return nil, status.Error(codes.FailedPrecondition, "not configured") + return nil, nil, status.Error(codes.FailedPrecondition, "not configured") } if err := p.reloadKubeletClient(p.config); err != nil { p.log.Warn("Unable to load kubelet client", "err", err) } - return p.config, nil + return p.config, p.containerHelper, nil } func (p *Plugin) setContainerHelper(c ContainerHelper) { p.mu.Lock() defer p.mu.Unlock() - p.c = c + p.containerHelper = c } func (p *Plugin) reloadKubeletClient(config *k8sConfig) (err error) { @@ -580,7 +587,7 @@ func (p *Plugin) loadToken(path string) (string, error) { // readFile reads the contents of a file through the filesystem interface func (p *Plugin) readFile(path string) ([]byte, error) { - f, err := p.fs.Open(path) + f, err := os.Open(filepath.Join(p.rootDir, path)) if err != nil { return nil, err } diff --git a/pkg/agent/plugin/workloadattestor/k8s/k8s_posix.go b/pkg/agent/plugin/workloadattestor/k8s/k8s_posix.go index 7b64f4c3b9..98956552d3 100644 --- a/pkg/agent/plugin/workloadattestor/k8s/k8s_posix.go +++ b/pkg/agent/plugin/workloadattestor/k8s/k8s_posix.go @@ -4,7 +4,10 @@ package k8s import ( "context" + "io" "log" + "os" + "path/filepath" "regexp" "strings" "unicode" @@ -12,6 +15,7 @@ import ( "github.com/hashicorp/go-hclog" "github.com/spiffe/spire/pkg/agent/common/cgroups" "github.com/spiffe/spire/pkg/agent/plugin/workloadattestor/k8s/sigstore" + "github.com/spiffe/spire/pkg/common/containerinfo" "github.com/spiffe/spire/pkg/common/telemetry" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -28,14 +32,19 @@ func (p *Plugin) defaultTokenPath() string { } func createHelper(c *Plugin) ContainerHelper { + rootDir := c.rootDir + if rootDir == "" { + rootDir = "/" + } return &containerHelper{ - fs: c.fs, + rootDir: c.rootDir, } } type containerHelper struct { - fs cgroups.FileSystem - sigstoreClient sigstore.Sigstore + rootDir string + sigstoreClient sigstore.Sigstore + useNewContainerLocator bool } func (h *containerHelper) Configure(config *HCLConfig, log hclog.Logger) error { @@ -51,6 +60,13 @@ func (h *containerHelper) Configure(config *HCLConfig, log hclog.Logger) error { } } + h.useNewContainerLocator = config.UseNewContainerLocator != nil && *config.UseNewContainerLocator + if h.useNewContainerLocator { + log.Info("Using the new container locator") + } else { + 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`.") + } + return nil } @@ -69,13 +85,17 @@ func (h *containerHelper) GetOSSelectors(ctx context.Context, log hclog.Logger, return selectors, nil } -func (h *containerHelper) GetPodUIDAndContainerID(pID int32, _ hclog.Logger) (types.UID, string, error) { - cgroups, err := cgroups.GetCgroups(pID, h.fs) - if err != nil { - return "", "", status.Errorf(codes.Internal, "unable to obtain cgroups: %v", err) +func (h *containerHelper) GetPodUIDAndContainerID(pID int32, log hclog.Logger) (types.UID, string, error) { + if !h.useNewContainerLocator { + cgroups, err := cgroups.GetCgroups(pID, dirFS(h.rootDir)) + if err != nil { + return "", "", status.Errorf(codes.Internal, "unable to obtain cgroups: %v", err) + } + return getPodUIDAndContainerIDFromCGroups(cgroups) } - return getPodUIDAndContainerIDFromCGroups(cgroups) + extractor := containerinfo.Extractor{RootDir: h.rootDir} + return extractor.GetPodUIDAndContainerID(int(pID), log) } func getPodUIDAndContainerIDFromCGroups(cgroups []cgroups.Cgroup) (types.UID, string, error) { @@ -241,3 +261,9 @@ func configureSigstoreClient(client sigstore.Sigstore, c *SigstoreHCLConfig, log } return nil } + +type dirFS string + +func (d dirFS) Open(p string) (io.ReadCloser, error) { + return os.Open(filepath.Join(string(d), p)) +} diff --git a/pkg/agent/plugin/workloadattestor/k8s/k8s_posix_test.go b/pkg/agent/plugin/workloadattestor/k8s/k8s_posix_test.go index ae44b249a6..5926037b81 100644 --- a/pkg/agent/plugin/workloadattestor/k8s/k8s_posix_test.go +++ b/pkg/agent/plugin/workloadattestor/k8s/k8s_posix_test.go @@ -675,8 +675,9 @@ type osConfig struct { func (o *osConfig) getContainerHelper(p *Plugin) ContainerHelper { return &containerHelper{ - fs: p.fs, - sigstoreClient: o.fakeClient, + rootDir: p.rootDir, + sigstoreClient: o.fakeClient, + useNewContainerLocator: true, } } diff --git a/pkg/agent/plugin/workloadattestor/k8s/k8s_test.go b/pkg/agent/plugin/workloadattestor/k8s/k8s_test.go index cdd5e03f1d..56abdfe782 100644 --- a/pkg/agent/plugin/workloadattestor/k8s/k8s_test.go +++ b/pkg/agent/plugin/workloadattestor/k8s/k8s_test.go @@ -8,7 +8,6 @@ import ( "crypto/x509" "crypto/x509/pkix" "fmt" - "io" "math/big" "net" "net/http" @@ -525,7 +524,7 @@ func (s *Suite) TestConfigure() { require.NotNil(t, testCase.config, "test case missing expected config") assert.NoError(t, err) - c, err := p.getConfig() + c, _, err := p.getConfig() require.NoError(t, err) switch { @@ -557,7 +556,7 @@ func (s *Suite) TestConfigure() { func (s *Suite) newPlugin() *Plugin { p := New() - p.fs = testFS(s.dir) + p.rootDir = s.dir p.clock = s.clock p.getenv = func(key string) string { return s.env[key] @@ -615,6 +614,7 @@ func (s *Suite) loadInsecurePlugin() workloadattestor.WorkloadAttestor { kubelet_read_only_port = %d max_poll_attempts = 5 poll_retry_interval = "1s" + use_new_container_locator = true `, s.kubeletPort())) } @@ -623,6 +623,7 @@ func (s *Suite) loadInsecurePluginWithExtra(extraConfig string) workloadattestor kubelet_read_only_port = %d max_poll_attempts = 5 poll_retry_interval = "1s" + use_new_container_locator = true %s `, s.kubeletPort(), extraConfig)) } @@ -808,9 +809,3 @@ func (s *Suite) addPodListResponse(fixturePath string) { s.podList = append(s.podList, podList) } - -type testFS string - -func (fs testFS) Open(path string) (io.ReadCloser, error) { - return os.Open(filepath.Join(string(fs), path)) -} diff --git a/pkg/common/containerinfo/extract.go b/pkg/common/containerinfo/extract.go new file mode 100644 index 0000000000..c4e70781b7 --- /dev/null +++ b/pkg/common/containerinfo/extract.go @@ -0,0 +1,278 @@ +package containerinfo + +import ( + "errors" + "fmt" + "io" + "io/fs" + "os" + "path" + "path/filepath" + "regexp" + "strings" + + "github.com/hashicorp/go-hclog" + "github.com/spiffe/spire/pkg/agent/common/cgroups" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + "k8s.io/apimachinery/pkg/types" + "k8s.io/mount-utils" +) + +var ( + // This regex covers both the cgroupfs and systemd rendering of the pod + // UID. The dashes are replaced with underscores in the systemd rendition. + rePodUID = regexp.MustCompile(`\b(?:pod([[:xdigit:]]{8}[-_][[:xdigit:]]{4}[-_][[:xdigit:]]{4}[-_][[:xdigit:]]{4}[-_][[:xdigit:]]{12}))\b`) + + // The container ID is a 64-character hex string, by convention. + reContainerID = regexp.MustCompile(`\b([[:xdigit:]]{64})\b`) + + // underToDash replaces underscores with dashes. The systemd cgroups driver + // doesn't allow dashes so the pod UID component has dashes replaced with + // underscores by the Kubelet. + underToDash = strings.NewReplacer("_", "-") +) + +type Extractor struct { + RootDir string +} + +func (e *Extractor) GetContainerID(pid int, log hclog.Logger) (string, error) { + _, containerID, err := e.extractInfo(pid, log, false) + return containerID, err +} + +func (e *Extractor) GetPodUIDAndContainerID(pid int, log hclog.Logger) (types.UID, string, error) { + return e.extractInfo(pid, log, true) +} + +func (e *Extractor) extractInfo(pid int, log hclog.Logger, extractPodUID bool) (types.UID, string, error) { + // Try to get the information from /proc/pid/mountinfo first. Otherwise, + // fall back to /proc/pid/cgroup. If it isn't in mountinfo, then the + // workload being attested likely originates in the same Pod as the agent. + // + // It may not be possible to attest a process running in the same container + // as the agent because, depending on how cgroups are being used, + // /proc//mountinfo or /proc//cgroup may not contain any + // information on the container ID or pod. + + podUID, containerID, err := e.extractPodUIDAndContainerIDFromMountInfo(pid, log, extractPodUID) + if err != nil { + return "", "", err + } + + if containerID == "" { + podUID, containerID, err = e.extractPodUIDAndContainerIDFromCGroups(pid, log, extractPodUID) + if err != nil { + return "", "", err + } + } + + return podUID, containerID, nil +} + +func (e *Extractor) extractPodUIDAndContainerIDFromMountInfo(pid int, log hclog.Logger, extractPodUID bool) (types.UID, string, error) { + mountInfoPath := filepath.Join(e.RootDir, "/proc", fmt.Sprint(pid), "mountinfo") + + mountInfos, err := mount.ParseMountInfo(mountInfoPath) + if err != nil { + if errors.Is(err, fs.ErrNotExist) { + return "", "", nil + } + return "", "", status.Errorf(codes.Internal, "failed to parse mount info at %q: %v", mountInfoPath, err) + } + + for i, mountInfo := range mountInfos { + log.Debug("PID mount enumerated", + "index", i+1, + "total", len(mountInfos), + "type", mountInfo.FsType, + "root", mountInfo.Root, + ) + } + + // Scan the cgroup mounts for the pod UID and container ID. The container + // ID is in the last segment, and the pod UID will be in the second to last + // segment, but only when we are attesting a different pod than the agent + // (otherwise, the second to last segment will be "..", since the agent + // exists in the same pod). In the case of cgroup v1 (or a unified + // hierarchy), there may exist multiple cgroup mounts. Out of an abundance + // of caution, all cgroup mounts will be scanned. If a containerID and/or + // pod UID are picked out of a mount, then those extracted from any of the + // remaining mounts will be checked to ensure they match. If not, we'll log + // and fail. + ex := &extractor{extractPodUID: extractPodUID} + for _, mountInfo := range mountInfos { + switch mountInfo.FsType { + case "cgroup", "cgroup2": + default: + continue + } + + log := log.With("mountInfoRoot", mountInfo.Root) + if err := ex.Extract(mountInfo.Root, log); err != nil { + return "", "", err + } + } + return ex.PodUID(), ex.ContainerID(), nil +} + +func (e *Extractor) extractPodUIDAndContainerIDFromCGroups(pid int, log hclog.Logger, extractPodUID bool) (types.UID, string, error) { + cgroups, err := cgroups.GetCgroups(int32(pid), dirFS(e.RootDir)) + if err != nil { + if errors.Is(err, fs.ErrNotExist) { + return "", "", nil + } + return "", "", status.Errorf(codes.Internal, "unable to obtain cgroups: %v", err) + } + + for i, cgroup := range cgroups { + log.Debug("PID cgroup enumerated", + "index", i+1, + "total", len(cgroups), + "path", cgroup.GroupPath, + ) + } + + ex := &extractor{extractPodUID: extractPodUID} + for _, cgroup := range cgroups { + log := log.With("cgroupPath", cgroup.GroupPath) + if err := ex.Extract(cgroup.GroupPath, log); err != nil { + return "", "", err + } + } + return ex.PodUID(), ex.ContainerID(), nil +} + +type dirFS string + +func (d dirFS) Open(p string) (io.ReadCloser, error) { + return os.Open(filepath.Join(string(d), p)) +} + +type extractor struct { + podUID types.UID + containerID string + extractPodUID bool +} + +func (e *extractor) PodUID() types.UID { + return e.podUID +} + +func (e *extractor) ContainerID() string { + return e.containerID +} + +func (e *extractor) Extract(cgroupPathOrMountRoot string, log hclog.Logger) error { + podUID, containerID := e.extract(cgroupPathOrMountRoot) + + // An entry with a pod UID overrides an entry without. If we currently have + // a pod UID and the new entry does not, then ignore it. If we currently + // don't have a pod UID and the new entry does, then override what we have + // so far. + // + // This helps mitigate situations where there is hybrid cgroups configured + // while running kind on macOS, which ends up with something like: + // 1:cpuset:/docker/93529524695bb00d91c1f6dba692ea8d3550c3b94fb2463af7bc9ec82f992d26/kubepods/besteffort/poda2830d0d-b0f0-4ff0-81b5-0ee4e299cf80/09bc3d7ade839efec32b6bec4ec79d099027a668ddba043083ec21d3c3b8f1e6 + // 0::/docker/93529524695bb00d91c1f6dba692ea8d3550c3b94fb2463af7bc9ec82f992d26/system.slice/containerd.service + // The second entry, with only the container ID of the docker host, should + // be ignored in favor of the first entry which contains the container ID + // and pod UID of the container running in Kind. + switch { + case e.podUID != "" && podUID == "": + // We currently have a pod UID and the new entry does not. Ignore it. + return nil + case e.podUID == "" && podUID != "": + // We currently don't have a pod UID but have found one. Override + // the current values with the new entry. + e.podUID = podUID + e.containerID = containerID + } + + // Check for conflicting answers for the pod UID or container ID. The safe + // action is to not choose anything. + + if podUID != "" && e.podUID != "" && podUID != e.podUID { + log.Debug("Workload pod UID conflict", + "previous", e.podUID, + "current", podUID, + ) + return status.Errorf(codes.FailedPrecondition, "multiple pod UIDs found (%q, %q)", e.podUID, podUID) + } + + if e.containerID != "" && containerID != e.containerID { + log.Debug("Workload container ID conflict", + "previous", e.containerID, + "current", containerID, + ) + return status.Errorf(codes.FailedPrecondition, "multiple container IDs found (%q, %q)", e.containerID, containerID) + } + + e.containerID = containerID + e.podUID = podUID + return nil +} + +func (e *extractor) extract(cgroupPathOrMountRoot string) (types.UID, string) { + // The container ID is typically in the last segment but in some cases + // there can other path segments that come after. Further, some + // combinations of kubernetes/cgroups driver/cgroups version/container + // runtime, etc, use colon separators between the pod UID and containerID, + // which means they can end up in the same segment together. + // + // The basic algorithm is to walk backwards through the path segments until + // something that looks like a container ID is located. Once located, and + // if the extractor is configured for it, we'll continue walking backwards + // (starting with what's left in the segment the container ID was located + // in) looking for the pod UID. + stripSegment := func(p string) (rest string, segment string) { + rest, segment = path.Split(p) + rest = strings.TrimSuffix(rest, "/") + return rest, segment + } + + rest, segment := stripSegment(cgroupPathOrMountRoot) + + // Walk backwards through the segments looking for the container ID. If + // found, extract the container ID and truncate the segment so that the + // remainder can (optionally) be searched for the pod UID below. + var containerID string + for segment != "" { + if indices := reContainerID.FindStringSubmatchIndex(segment); len(indices) > 0 { + containerID = segment[indices[2]:indices[3]] + segment = segment[:indices[2]] + break + } + rest, segment = stripSegment(rest) + } + + // If there is no container ID, then don't try to extract the pod UID. + if containerID == "" { + return "", "" + } + + // If the extractor isn't interested in the pod UID, then we're done. + if !e.extractPodUID { + return "", containerID + } + + // If the container ID occupied the beginning of the last segment, then + // that segment is consumed and we should grab the next one. + if segment == "" { + rest, segment = stripSegment(rest) + } + + // Walk backwards through the remaining segments looking for the pod UID. + var podUID string + for segment != "" { + if m := rePodUID.FindStringSubmatch(segment); len(m) > 0 { + // For systemd, dashes in pod UIDs are escaped to underscores. Reverse that. + podUID = underToDash.Replace(m[1]) + break + } + rest, segment = stripSegment(rest) + } + + return types.UID(podUID), containerID +} diff --git a/pkg/common/containerinfo/extract_test.go b/pkg/common/containerinfo/extract_test.go new file mode 100644 index 0000000000..4fed015096 --- /dev/null +++ b/pkg/common/containerinfo/extract_test.go @@ -0,0 +1,129 @@ +package containerinfo + +import ( + "testing" + + "github.com/hashicorp/go-hclog" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/types" +) + +const ( + testPodUID = types.UID("00000000-1111-2222-3333-444444444444") + testContainerID = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" +) + +func TestExtractPodUIDAndContainerID(t *testing.T) { + log := hclog.NewNullLogger() + + assertFound := func(t *testing.T, rootDir string, wantPodUID types.UID, wantContainerID string) { + extractor := Extractor{RootDir: rootDir} + gotPodUID, gotContainerID, err := extractor.GetPodUIDAndContainerID(123, log) + require.NoError(t, err) + assert.Equal(t, wantPodUID, gotPodUID) + assert.Equal(t, wantContainerID, gotContainerID) + } + + assertNotFound := func(t *testing.T, rootDir string) { + extractor := Extractor{RootDir: rootDir} + gotPodUID, gotContainerID, err := extractor.GetPodUIDAndContainerID(123, log) + require.NoError(t, err) + assert.Empty(t, gotPodUID) + assert.Empty(t, gotContainerID) + } + + assertErrorContains := func(t *testing.T, rootDir string, wantErr string) { + extractor := Extractor{RootDir: rootDir} + gotPodUID, gotContainerID, err := extractor.GetPodUIDAndContainerID(123, log) + assert.ErrorContains(t, err, wantErr) + assert.Empty(t, gotPodUID) + assert.Empty(t, gotContainerID) + } + + t.Run("cgroups v1", func(t *testing.T) { + assertFound(t, "testdata/k8s/v1", testPodUID, testContainerID) + }) + + t.Run("cgroups v2", func(t *testing.T) { + assertFound(t, "testdata/k8s/v2", testPodUID, testContainerID) + }) + + t.Run("no cgroup mount", func(t *testing.T) { + assertNotFound(t, "testdata/k8s/no-cgroup-mount") + }) + + t.Run("cgroup mount does not match expected format", func(t *testing.T) { + assertNotFound(t, "testdata/other/malformed") + }) + + t.Run("pod UID conflict", func(t *testing.T) { + assertErrorContains(t, "testdata/k8s/pod-uid-conflict", "multiple pod UIDs found") + }) + + t.Run("container ID conflict", func(t *testing.T) { + assertErrorContains(t, "testdata/k8s/container-id-conflict", "multiple container IDs found") + }) + + t.Run("failed to read mountinfo", func(t *testing.T) { + assertNotFound(t, "testdata/does-not-exist") + }) + + t.Run("falls back to cgroup file", func(t *testing.T) { + assertFound(t, "testdata/other/fallback", "", testContainerID) + }) +} + +func TestExtractContainerID(t *testing.T) { + log := hclog.NewNullLogger() + + assertFound := func(t *testing.T, rootDir, wantContainerID string) { + extractor := Extractor{RootDir: rootDir} + gotContainerID, err := extractor.GetContainerID(123, log) + assert.NoError(t, err) + assert.Equal(t, wantContainerID, gotContainerID) + } + + assertNotFound := func(t *testing.T, rootDir string) { + extractor := Extractor{RootDir: rootDir} + gotContainerID, err := extractor.GetContainerID(123, log) + assert.NoError(t, err) + assert.Empty(t, gotContainerID) + } + + assertErrorContains := func(t *testing.T, rootDir string, wantErr string) { + extractor := Extractor{RootDir: rootDir} + gotPodUID, gotContainerID, err := extractor.GetPodUIDAndContainerID(123, log) + assert.ErrorContains(t, err, wantErr) + assert.Empty(t, gotPodUID) + assert.Empty(t, gotContainerID) + } + + t.Run("cgroups v1", func(t *testing.T) { + assertFound(t, "testdata/docker/v1", testContainerID) + }) + + t.Run("cgroups v2", func(t *testing.T) { + assertFound(t, "testdata/docker/v2", testContainerID) + }) + + t.Run("no cgroup mount", func(t *testing.T) { + assertNotFound(t, "testdata/docker/no-cgroup-mount") + }) + + t.Run("cgroup mount does not match expected format", func(t *testing.T) { + assertNotFound(t, "testdata/other/malformed") + }) + + t.Run("container ID conflict", func(t *testing.T) { + assertErrorContains(t, "testdata/docker/container-id-conflict", "multiple container IDs found") + }) + + t.Run("failed to read mountinfo", func(t *testing.T) { + assertNotFound(t, "testdata/does-not-exist") + }) + + t.Run("falls back to cgroup file", func(t *testing.T) { + assertFound(t, "testdata/other/fallback", testContainerID) + }) +} diff --git a/pkg/common/containerinfo/testdata/docker/container-id-conflict/proc/123/mountinfo b/pkg/common/containerinfo/testdata/docker/container-id-conflict/proc/123/mountinfo new file mode 100644 index 0000000000..313fce99cf --- /dev/null +++ b/pkg/common/containerinfo/testdata/docker/container-id-conflict/proc/123/mountinfo @@ -0,0 +1,2 @@ +2356 2355 0:30 /../0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef /sys/fs/cgroup ro,nosuid,nodev,noexec,relatime - cgroup2 cgroup rw +2356 2355 0:30 /../7654321089abcdef0123456789abcdef0123456789abcdef0123456789abcdef /sys/fs/cgroup ro,nosuid,nodev,noexec,relatime - cgroup2 cgroup rw diff --git a/pkg/common/containerinfo/testdata/docker/no-cgroup-mount/proc/123/mountinfo b/pkg/common/containerinfo/testdata/docker/no-cgroup-mount/proc/123/mountinfo new file mode 100644 index 0000000000..17a2a21b2b --- /dev/null +++ b/pkg/common/containerinfo/testdata/docker/no-cgroup-mount/proc/123/mountinfo @@ -0,0 +1 @@ +2356 2355 0:30 /../0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef /sys/fs/cgroup ro,nosuid,nodev,noexec,relatime - not-a-cgroup-type cgroup rw diff --git a/pkg/common/containerinfo/testdata/docker/v1/proc/123/mountinfo b/pkg/common/containerinfo/testdata/docker/v1/proc/123/mountinfo new file mode 100644 index 0000000000..ffe73406e0 --- /dev/null +++ b/pkg/common/containerinfo/testdata/docker/v1/proc/123/mountinfo @@ -0,0 +1,13 @@ +572 571 0:63 / /sys/fs/cgroup rw,nosuid,nodev,noexec,relatime - tmpfs tmpfs rw,mode=755 +573 572 0:33 /docker/0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef /sys/fs/cgroup/systemd ro,nosuid,nodev,noexec,relatime master:11 - cgroup cgroup rw,xattr,name=systemd +574 572 0:37 /docker/0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef /sys/fs/cgroup/memory ro,nosuid,nodev,noexec,relatime master:16 - cgroup cgroup rw,memory +575 572 0:38 /docker/0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef /sys/fs/cgroup/hugetlb ro,nosuid,nodev,noexec,relatime master:17 - cgroup cgroup rw,hugetlb +576 572 0:39 /docker/0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef /sys/fs/cgroup/net_cls,net_prio ro,nosuid,nodev,noexec,relatime master:18 - cgroup cgroup rw,net_cls,net_prio +577 572 0:40 /docker/0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef /sys/fs/cgroup/cpu,cpuacct ro,nosuid,nodev,noexec,relatime master:19 - cgroup cgroup rw,cpu,cpuacct +578 572 0:41 /docker/0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef /sys/fs/cgroup/cpuset ro,nosuid,nodev,noexec,relatime master:20 - cgroup cgroup rw,cpuset +579 572 0:42 /docker/0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef /sys/fs/cgroup/blkio ro,nosuid,nodev,noexec,relatime master:21 - cgroup cgroup rw,blkio +580 572 0:43 /docker/0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef /sys/fs/cgroup/freezer ro,nosuid,nodev,noexec,relatime master:22 - cgroup cgroup rw,freezer +581 572 0:44 /docker/0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef /sys/fs/cgroup/perf_event ro,nosuid,nodev,noexec,relatime master:23 - cgroup cgroup rw,perf_event +582 572 0:45 /docker/0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef /sys/fs/cgroup/pids ro,nosuid,nodev,noexec,relatime master:24 - cgroup cgroup rw,pids +583 572 0:46 /docker/0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef /sys/fs/cgroup/rdma ro,nosuid,nodev,noexec,relatime master:25 - cgroup cgroup rw,rdma +584 572 0:47 /docker/0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef /sys/fs/cgroup/devices ro,nosuid,nodev,noexec,relatime master:26 - cgroup cgroup rw,devices diff --git a/pkg/common/containerinfo/testdata/docker/v2/proc/123/mountinfo b/pkg/common/containerinfo/testdata/docker/v2/proc/123/mountinfo new file mode 100644 index 0000000000..8c92ff707b --- /dev/null +++ b/pkg/common/containerinfo/testdata/docker/v2/proc/123/mountinfo @@ -0,0 +1 @@ +2356 2355 0:30 /../0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef /sys/fs/cgroup ro,nosuid,nodev,noexec,relatime - cgroup2 cgroup rw diff --git a/pkg/common/containerinfo/testdata/k8s/container-id-conflict/proc/123/mountinfo b/pkg/common/containerinfo/testdata/k8s/container-id-conflict/proc/123/mountinfo new file mode 100644 index 0000000000..34f5143d59 --- /dev/null +++ b/pkg/common/containerinfo/testdata/k8s/container-id-conflict/proc/123/mountinfo @@ -0,0 +1,2 @@ +1543 1542 0:32 /../../kubelet-kubepods-besteffort.slice/kubelet-kubepods-besteffort-pod00000000_1111_2222_3333_444444444444.slice/cri-containerd-0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef.scope /sys/fs/cgroup ro,nosuid,nodev,noexec,relatime - cgroup2 cgroup rw,nsdelegate,memory_recursiveprot +1543 1542 0:32 /../../kubelet-kubepods-besteffort.slice/kubelet-kubepods-besteffort-pod00000000_1111_2222_3333_444444444444.slice/cri-containerd-7654321089abcdef0123456789abcdef0123456789abcdef0123456789abcdef.scope /sys/fs/cgroup ro,nosuid,nodev,noexec,relatime - cgroup2 cgroup rw,nsdelegate,memory_recursiveprot diff --git a/pkg/common/containerinfo/testdata/k8s/no-cgroup-mount/proc/123/mountinfo b/pkg/common/containerinfo/testdata/k8s/no-cgroup-mount/proc/123/mountinfo new file mode 100644 index 0000000000..5b9383ac2b --- /dev/null +++ b/pkg/common/containerinfo/testdata/k8s/no-cgroup-mount/proc/123/mountinfo @@ -0,0 +1 @@ +1543 1542 0:32 /../../kubelet-kubepods-besteffort.slice/kubelet-kubepods-besteffort-pod00000000_1111_2222_3333_444444444444.slice/cri-containerd-0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef.scope /sys/fs/cgroup ro,nosuid,nodev,noexec,relatime - not-a-cgroup-type cgroup rw,nsdelegate,memory_recursiveprot diff --git a/pkg/common/containerinfo/testdata/k8s/pod-uid-conflict/proc/123/mountinfo b/pkg/common/containerinfo/testdata/k8s/pod-uid-conflict/proc/123/mountinfo new file mode 100644 index 0000000000..1251ee0f84 --- /dev/null +++ b/pkg/common/containerinfo/testdata/k8s/pod-uid-conflict/proc/123/mountinfo @@ -0,0 +1,2 @@ +1543 1542 0:32 /../../kubelet-kubepods-besteffort.slice/kubelet-kubepods-besteffort-pod00000000_1111_2222_3333_444444444444.slice/cri-containerd-0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef.scope /sys/fs/cgroup ro,nosuid,nodev,noexec,relatime - cgroup2 cgroup rw,nsdelegate,memory_recursiveprot +1543 1542 0:32 /../../kubelet-kubepods-besteffort.slice/kubelet-kubepods-besteffort-pod76543210_89ab_cdef_0123_456789abcdef.slice/cri-containerd-0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef.scope /sys/fs/cgroup ro,nosuid,nodev,noexec,relatime - cgroup2 cgroup rw,nsdelegate,memory_recursiveprot diff --git a/pkg/common/containerinfo/testdata/k8s/v1/proc/123/mountinfo b/pkg/common/containerinfo/testdata/k8s/v1/proc/123/mountinfo new file mode 100644 index 0000000000..2f4cfe8008 --- /dev/null +++ b/pkg/common/containerinfo/testdata/k8s/v1/proc/123/mountinfo @@ -0,0 +1,13 @@ +2032 2031 0:309 / /sys/fs/cgroup rw,nosuid,nodev,noexec,relatime - tmpfs tmpfs rw,mode=755 +2033 2032 0:33 /kubelet.slice/kubelet-kubepods.slice/kubelet-kubepods-pod00000000_1111_2222_3333_444444444444.slice/cri-containerd-0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef.scope /sys/fs/cgroup/systemd ro,nosuid,nodev,noexec,relatime - cgroup systemd rw,xattr,name=systemd +2034 2032 0:37 /kubelet.slice/kubelet-kubepods.slice/kubelet-kubepods-pod00000000_1111_2222_3333_444444444444.slice/cri-containerd-0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef.scope /sys/fs/cgroup/cpu,cpuacct ro,nosuid,nodev,noexec,relatime - cgroup cgroup rw,cpu,cpuacct +2035 2032 0:38 /kubelet.slice/kubelet-kubepods.slice/kubelet-kubepods-pod00000000_1111_2222_3333_444444444444.slice/cri-containerd-0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef.scope /sys/fs/cgroup/freezer ro,nosuid,nodev,noexec,relatime - cgroup cgroup rw,freezer +2036 2032 0:39 /kubelet.slice/kubelet-kubepods.slice/kubelet-kubepods-pod00000000_1111_2222_3333_444444444444.slice/cri-containerd-0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef.scope /sys/fs/cgroup/cpuset ro,nosuid,nodev,noexec,relatime - cgroup cgroup rw,cpuset +2037 2032 0:40 /kubelet.slice/kubelet-kubepods.slice/kubelet-kubepods-pod00000000_1111_2222_3333_444444444444.slice/cri-containerd-0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef.scope /sys/fs/cgroup/net_cls,net_prio ro,nosuid,nodev,noexec,relatime - cgroup cgroup rw,net_cls,net_prio +2038 2032 0:41 /kubelet.slice/kubelet-kubepods.slice/kubelet-kubepods-pod00000000_1111_2222_3333_444444444444.slice/cri-containerd-0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef.scope /sys/fs/cgroup/perf_event ro,nosuid,nodev,noexec,relatime - cgroup cgroup rw,perf_event +2039 2032 0:42 /kubelet.slice/kubelet-kubepods.slice/kubelet-kubepods-pod00000000_1111_2222_3333_444444444444.slice/cri-containerd-0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef.scope /sys/fs/cgroup/rdma ro,nosuid,nodev,noexec,relatime - cgroup cgroup rw,rdma +2040 2032 0:43 /kubelet.slice/kubelet-kubepods.slice/kubelet-kubepods-pod00000000_1111_2222_3333_444444444444.slice/cri-containerd-0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef.scope /sys/fs/cgroup/memory ro,nosuid,nodev,noexec,relatime - cgroup cgroup rw,memory +2041 2032 0:44 /kubelet.slice/kubelet-kubepods.slice/kubelet-kubepods-pod00000000_1111_2222_3333_444444444444.slice/cri-containerd-0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef.scope /sys/fs/cgroup/blkio ro,nosuid,nodev,noexec,relatime - cgroup cgroup rw,blkio +2042 2032 0:45 /kubelet.slice/kubelet-kubepods.slice/kubelet-kubepods-pod00000000_1111_2222_3333_444444444444.slice/cri-containerd-0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef.scope /sys/fs/cgroup/hugetlb ro,nosuid,nodev,noexec,relatime - cgroup cgroup rw,hugetlb +2043 2032 0:46 /kubelet.slice/kubelet-kubepods.slice/kubelet-kubepods-pod00000000_1111_2222_3333_444444444444.slice/cri-containerd-0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef.scope /sys/fs/cgroup/devices ro,nosuid,nodev,noexec,relatime - cgroup cgroup rw,devices +2044 2032 0:47 /kubelet.slice/kubelet-kubepods.slice/kubelet-kubepods-pod00000000_1111_2222_3333_444444444444.slice/cri-containerd-0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef.scope /sys/fs/cgroup/pids ro,nosuid,nodev,noexec,relatime - cgroup cgroup rw,pids diff --git a/pkg/common/containerinfo/testdata/k8s/v2/proc/123/mountinfo b/pkg/common/containerinfo/testdata/k8s/v2/proc/123/mountinfo new file mode 100644 index 0000000000..a51c18f768 --- /dev/null +++ b/pkg/common/containerinfo/testdata/k8s/v2/proc/123/mountinfo @@ -0,0 +1 @@ +1543 1542 0:32 /../../kubelet-kubepods-besteffort.slice/kubelet-kubepods-besteffort-pod00000000_1111_2222_3333_444444444444.slice/cri-containerd-0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef.scope /sys/fs/cgroup ro,nosuid,nodev,noexec,relatime - cgroup2 cgroup rw,nsdelegate,memory_recursiveprot diff --git a/pkg/common/containerinfo/testdata/other/fallback/proc/123/cgroup b/pkg/common/containerinfo/testdata/other/fallback/proc/123/cgroup new file mode 100644 index 0000000000..7dcb86f175 --- /dev/null +++ b/pkg/common/containerinfo/testdata/other/fallback/proc/123/cgroup @@ -0,0 +1 @@ +0::/../0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef diff --git a/pkg/common/containerinfo/testdata/other/fallback/proc/123/mountinfo b/pkg/common/containerinfo/testdata/other/fallback/proc/123/mountinfo new file mode 100644 index 0000000000..cc7fb67abb --- /dev/null +++ b/pkg/common/containerinfo/testdata/other/fallback/proc/123/mountinfo @@ -0,0 +1 @@ +1543 1542 0:32 / /sys/fs/cgroup ro,nosuid,nodev,noexec,relatime - cgroup2 cgroup rw,nsdelegate,memory_recursiveprot diff --git a/pkg/common/containerinfo/testdata/other/malformed/proc/123/mountinfo b/pkg/common/containerinfo/testdata/other/malformed/proc/123/mountinfo new file mode 100644 index 0000000000..3b17c08807 --- /dev/null +++ b/pkg/common/containerinfo/testdata/other/malformed/proc/123/mountinfo @@ -0,0 +1 @@ +1543 1542 0:32 /not-a-match /sys/fs/cgroup ro,nosuid,nodev,noexec,relatime - cgroup2 cgroup rw,nsdelegate,memory_recursiveprot From 5e3d4015aa48dc1f9dc84fc17279158a4c69d051 Mon Sep 17 00:00:00 2001 From: Andrew Harding Date: Thu, 18 Apr 2024 08:44:38 -0600 Subject: [PATCH 02/10] missing new arg Signed-off-by: Andrew Harding --- pkg/agent/plugin/workloadattestor/docker/docker_posix.go | 1 - pkg/agent/plugin/workloadattestor/docker/docker_windows.go | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/agent/plugin/workloadattestor/docker/docker_posix.go b/pkg/agent/plugin/workloadattestor/docker/docker_posix.go index bff58ffb8c..31b8772475 100644 --- a/pkg/agent/plugin/workloadattestor/docker/docker_posix.go +++ b/pkg/agent/plugin/workloadattestor/docker/docker_posix.go @@ -46,7 +46,6 @@ func createHelper(c *dockerPluginConfig, log hclog.Logger) (*containerHelper, er 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{} diff --git a/pkg/agent/plugin/workloadattestor/docker/docker_windows.go b/pkg/agent/plugin/workloadattestor/docker/docker_windows.go index a4f7326e93..1101c8ee55 100644 --- a/pkg/agent/plugin/workloadattestor/docker/docker_windows.go +++ b/pkg/agent/plugin/workloadattestor/docker/docker_windows.go @@ -14,7 +14,7 @@ type OSConfig struct { DockerHost string `hcl:"docker_host" json:"docker_host"` } -func createHelper(*dockerPluginConfig) (*containerHelper, error) { +func createHelper(*dockerPluginConfig, hclog.Logger) (*containerHelper, error) { return &containerHelper{ ph: process.CreateHelper(), }, nil From 5ad232a830e3fa84f36330e5e2b31fec12bd8cf0 Mon Sep 17 00:00:00 2001 From: Andrew Harding Date: Thu, 18 Apr 2024 09:10:09 -0600 Subject: [PATCH 03/10] fix windows tests Signed-off-by: Andrew Harding --- pkg/agent/plugin/workloadattestor/docker/docker_windows_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/agent/plugin/workloadattestor/docker/docker_windows_test.go b/pkg/agent/plugin/workloadattestor/docker/docker_windows_test.go index 68da00986e..3f71a27829 100644 --- a/pkg/agent/plugin/workloadattestor/docker/docker_windows_test.go +++ b/pkg/agent/plugin/workloadattestor/docker/docker_windows_test.go @@ -50,7 +50,7 @@ func verifyConfigDefault(t *testing.T, c *containerHelper) { require.NotNil(t, c.ph) } -func withDefaultDataOpt() testPluginOpt { +func withDefaultDataOpt(testing.TB) testPluginOpt { h := &fakeProcessHelper{ containerID: testContainerID, } From 37afbec26ade465bd0f598f657855d9800a72e01 Mon Sep 17 00:00:00 2001 From: Andrew Harding Date: Thu, 18 Apr 2024 09:52:10 -0600 Subject: [PATCH 04/10] fix windows tests and lint Signed-off-by: Andrew Harding --- pkg/agent/plugin/workloadattestor/docker/docker_posix_test.go | 3 ++- pkg/agent/plugin/workloadattestor/docker/docker_test.go | 4 +--- .../plugin/workloadattestor/docker/docker_windows_test.go | 4 ++++ pkg/common/containerinfo/extract.go | 2 ++ pkg/common/containerinfo/extract_test.go | 2 ++ 5 files changed, 11 insertions(+), 4 deletions(-) diff --git a/pkg/agent/plugin/workloadattestor/docker/docker_posix_test.go b/pkg/agent/plugin/workloadattestor/docker/docker_posix_test.go index 5d8835d5bf..8bdf01ff78 100644 --- a/pkg/agent/plugin/workloadattestor/docker/docker_posix_test.go +++ b/pkg/agent/plugin/workloadattestor/docker/docker_posix_test.go @@ -14,7 +14,8 @@ import ( ) const ( - testCgroupEntries = "10:devices:/docker/6469646e742065787065637420616e796f6e6520746f20726561642074686973" + testCgroupEntries = "10:devices:/docker/6469646e742065787065637420616e796f6e6520746f20726561642074686973" + defaultPluginConfig = "use_new_container_locator = true" ) func TestContainerExtraction(t *testing.T) { diff --git a/pkg/agent/plugin/workloadattestor/docker/docker_test.go b/pkg/agent/plugin/workloadattestor/docker/docker_test.go index b6908be572..8cc83bbcd5 100644 --- a/pkg/agent/plugin/workloadattestor/docker/docker_test.go +++ b/pkg/agent/plugin/workloadattestor/docker/docker_test.go @@ -248,9 +248,7 @@ func withDisabledRetryer() testPluginOpt { func newTestPlugin(t *testing.T, opts ...testPluginOpt) *Plugin { p := New() - err := doConfigure(t, p, ` - use_new_container_locator = true - `) + err := doConfigure(t, p, defaultPluginConfig) require.NoError(t, err) for _, o := range opts { diff --git a/pkg/agent/plugin/workloadattestor/docker/docker_windows_test.go b/pkg/agent/plugin/workloadattestor/docker/docker_windows_test.go index 3f71a27829..b9d0643824 100644 --- a/pkg/agent/plugin/workloadattestor/docker/docker_windows_test.go +++ b/pkg/agent/plugin/workloadattestor/docker/docker_windows_test.go @@ -12,6 +12,10 @@ import ( "google.golang.org/grpc/codes" ) +const ( + defaultPluginConfig = "" +) + func TestFailToGetContainerID(t *testing.T) { h := &fakeProcessHelper{ err: errors.New("oh no"), diff --git a/pkg/common/containerinfo/extract.go b/pkg/common/containerinfo/extract.go index c4e70781b7..bce36a245f 100644 --- a/pkg/common/containerinfo/extract.go +++ b/pkg/common/containerinfo/extract.go @@ -1,3 +1,5 @@ +//go:build !windows + package containerinfo import ( diff --git a/pkg/common/containerinfo/extract_test.go b/pkg/common/containerinfo/extract_test.go index 4fed015096..7c0a27554c 100644 --- a/pkg/common/containerinfo/extract_test.go +++ b/pkg/common/containerinfo/extract_test.go @@ -1,3 +1,5 @@ +//go:build !windows + package containerinfo import ( From 1497de73d05ac922b26d6b3f5bbfd360087090a9 Mon Sep 17 00:00:00 2001 From: Andrew Harding Date: Tue, 23 Apr 2024 12:32:02 -0600 Subject: [PATCH 05/10] address pr comments Signed-off-by: Andrew Harding --- doc/plugin_agent_workloadattestor_docker.md | 14 ++++--- doc/plugin_agent_workloadattestor_k8s.md | 31 ++++++++------- .../workloadattestor/docker/docker_posix.go | 21 +++++++--- pkg/agent/plugin/workloadattestor/k8s/k8s.go | 7 ++-- .../plugin/workloadattestor/k8s/k8s_posix.go | 10 +++-- pkg/common/containerinfo/extract.go | 39 +++++++++++-------- pkg/common/containerinfo/extract_test.go | 4 ++ .../k8s/pod-uid-override/proc/123/mountinfo | 3 ++ 8 files changed, 80 insertions(+), 49 deletions(-) create mode 100644 pkg/common/containerinfo/testdata/k8s/pod-uid-override/proc/123/mountinfo diff --git a/doc/plugin_agent_workloadattestor_docker.md b/doc/plugin_agent_workloadattestor_docker.md index 5086453799..298a607067 100644 --- a/doc/plugin_agent_workloadattestor_docker.md +++ b/doc/plugin_agent_workloadattestor_docker.md @@ -4,12 +4,14 @@ The `docker` plugin generates selectors based on docker labels for workloads cal It does so by retrieving the workload's container ID from its cgroup membership on Unix systems or Job Object names on Windows, then querying the docker daemon for the container's labels. -| Configuration | Description | Default | -|------------------------------|------------------------------------------------------------------------------|----------------------------------| -| docker_socket_path | The location of the docker daemon socket (Unix) | "unix:///var/run/docker.sock" | -| docker_version | The API version of the docker daemon. If not specified | | -| container_id_cgroup_matchers | A list of patterns used to discover container IDs from cgroup entries (Unix) | -| docker_host | The location of the Docker Engine API endpoint (Windows only) | "npipe:////./pipe/docker_engine" | +| Configuration | Description | Default | +|--------------------------------|------------------------------------------------------------------------------------------------|----------------------------------| +| docker_socket_path | The location of the docker daemon socket (Unix) | "unix:///var/run/docker.sock" | +| docker_version | The API version of the docker daemon. If not specified | | +| container_id_cgroup_matchers | A list of patterns used to discover container IDs from cgroup entries (Unix) | | +| docker_host | The location of the Docker Engine API endpoint (Windows only) | "npipe:////./pipe/docker_engine" | +| use_new_container_locator | If true, enables the new container locator algorithm that has support for cgroups v2 | false | +| verbose_container_locator_logs | If true, enables verbose logging of mountinfo and cgroup information used to locate containers | false | A sample configuration: diff --git a/doc/plugin_agent_workloadattestor_k8s.md b/doc/plugin_agent_workloadattestor_k8s.md index b0421d5a82..df9b877fe5 100644 --- a/doc/plugin_agent_workloadattestor_k8s.md +++ b/doc/plugin_agent_workloadattestor_k8s.md @@ -46,20 +46,23 @@ server name validation against the kubelet certificate. **Note** To run on Windows containers, Kubernetes v1.24+ and containerd v1.6+ are required, since [hostprocess](https://kubernetes.io/docs/tasks/configure-pod-container/create-hostprocess-pod/) container is required on the agent container. -| Configuration | Description | -|--------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| `disable_container_selectors` | If true, container selectors are not produced. This can be used to produce pod selectors when the workload pod is known but the workload container is not ready at the time of attestation. | -| `kubelet_read_only_port` | The kubelet read-only port. This is mutually exclusive with `kubelet_secure_port`. | -| `kubelet_secure_port` | The kubelet secure port. It defaults to `10250` unless `kubelet_read_only_port` is set. | -| `kubelet_ca_path` | The path on disk to a file containing CA certificates used to verify the kubelet certificate. Required unless `skip_kubelet_verification` is set. Defaults to the cluster CA bundle `/run/secrets/kubernetes.io/serviceaccount/ca.crt`. | -| `skip_kubelet_verification` | If true, kubelet certificate verification is skipped | -| `token_path` | The path on disk to the bearer token used for kubelet authentication. Defaults to the service account token `/run/secrets/kubernetes.io/serviceaccount/token` | -| `certificate_path` | The path on disk to client certificate used for kubelet authentication | -| `private_key_path` | The path on disk to client key used for kubelet authentication | -| `use_anonymous_authentication` | If true, use anonymous authentication for kubelet communication | -| `node_name_env` | The environment variable used to obtain the node name. Defaults to `MY_NODE_NAME`. | -| `node_name` | The name of the node. Overrides the value obtained by the environment variable specified by `node_name_env`. | -| `experimental` | The experimental options that are subject to change or removal. | +| Configuration | Description | +|-------------------------------- |-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `disable_container_selectors` | If true, container selectors are not produced. This can be used to produce pod selectors when the workload pod is known but the workload container is not ready at the time of attestation. | +| `kubelet_read_only_port` | The kubelet read-only port. This is mutually exclusive with `kubelet_secure_port`. | +| `kubelet_secure_port` | The kubelet secure port. It defaults to `10250` unless `kubelet_read_only_port` is set. | +| `kubelet_ca_path` | The path on disk to a file containing CA certificates used to verify the kubelet certificate. Required unless `skip_kubelet_verification` is set. Defaults to the cluster CA bundle `/run/secrets/kubernetes.io/serviceaccount/ca.crt`. | +| `skip_kubelet_verification` | If true, kubelet certificate verification is skipped | +| `token_path` | The path on disk to the bearer token used for kubelet authentication. Defaults to the service account token `/run/secrets/kubernetes.io/serviceaccount/token` | +| `certificate_path` | The path on disk to client certificate used for kubelet authentication | +| `private_key_path` | The path on disk to client key used for kubelet authentication | +| `use_anonymous_authentication` | If true, use anonymous authentication for kubelet communication | +| `node_name_env` | The environment variable used to obtain the node name. Defaults to `MY_NODE_NAME`. | +| `node_name` | The name of the node. Overrides the value obtained by the environment variable specified by `node_name_env`. | +| `experimental` | The experimental options that are subject to change or removal. | +| `use_new_container_locator` | If true, enables the new container locator algorithm that has support for cgroups v2. Defaults to false. | +| `verbose_container_locator_logs` | If true, enables verbose logging of mountinfo and cgroup information used to locate containers. Defaults to false. | + | Experimental options | Description | |----------------------|----------------------------------------------------------------------------------------------------------------------------- | diff --git a/pkg/agent/plugin/workloadattestor/docker/docker_posix.go b/pkg/agent/plugin/workloadattestor/docker/docker_posix.go index 31b8772475..9b1b7f0693 100644 --- a/pkg/agent/plugin/workloadattestor/docker/docker_posix.go +++ b/pkg/agent/plugin/workloadattestor/docker/docker_posix.go @@ -10,10 +10,12 @@ import ( "path/filepath" "regexp" + "github.com/gogo/status" "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" + "google.golang.org/grpc/codes" ) type OSConfig struct { @@ -29,6 +31,10 @@ type OSConfig struct { // unset. This will default to true in a future release. (Unix) UseNewContainerLocator *bool `hcl:"use_new_container_locator"` + // VerboseContainerLocatorLogs, if true, dumps extra information to the log + // about mountinfo and cgroup information used to locate the container. + VerboseContainerLocatorLogs bool `hcl:"verbose_container_locator_logs"` + // Used by tests to use a fake /proc directory instead of the real one rootDir string } @@ -38,6 +44,9 @@ func createHelper(c *dockerPluginConfig, log hclog.Logger) (*containerHelper, er switch { case c.UseNewContainerLocator != nil && *c.UseNewContainerLocator: + if len(c.ContainerIDCGroupMatchers) > 0 { + return nil, status.Error(codes.InvalidArgument, "the new container locator and custom cgroup matchers cannot both be used") + } 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`.") @@ -57,8 +66,9 @@ func createHelper(c *dockerPluginConfig, log hclog.Logger) (*containerHelper, er } return &containerHelper{ - rootDir: rootDir, - containerIDFinder: containerIDFinder, + rootDir: rootDir, + containerIDFinder: containerIDFinder, + verboseContainerLocatorLogs: c.VerboseContainerLocatorLogs, }, nil } @@ -69,8 +79,9 @@ func (d dirFS) Open(p string) (io.ReadCloser, error) { } type containerHelper struct { - rootDir string - containerIDFinder cgroup.ContainerIDFinder + rootDir string + containerIDFinder cgroup.ContainerIDFinder + verboseContainerLocatorLogs bool } func (h *containerHelper) getContainerID(pID int32, log hclog.Logger) (string, error) { @@ -82,7 +93,7 @@ func (h *containerHelper) getContainerID(pID int32, log hclog.Logger) (string, e return getContainerIDFromCGroups(h.containerIDFinder, cgroupList) } - extractor := containerinfo.Extractor{RootDir: h.rootDir} + extractor := containerinfo.Extractor{RootDir: h.rootDir, VerboseLogging: h.verboseContainerLocatorLogs} return extractor.GetContainerID(int(pID), log) } diff --git a/pkg/agent/plugin/workloadattestor/k8s/k8s.go b/pkg/agent/plugin/workloadattestor/k8s/k8s.go index 314327c2d9..f9ef212a12 100644 --- a/pkg/agent/plugin/workloadattestor/k8s/k8s.go +++ b/pkg/agent/plugin/workloadattestor/k8s/k8s.go @@ -127,6 +127,10 @@ type HCLConfig struct { // unset. This will default to true in a future release. UseNewContainerLocator *bool `hcl:"use_new_container_locator"` + // VerboseContainerLocatorLogs, if true, dumps extra information to the log + // about mountinfo and cgroup information used to locate the container. + VerboseContainerLocatorLogs bool `hcl:"verbose_container_locator_logs"` + // Experimental enables experimental features. Experimental *ExperimentalK8SConfig `hcl:"experimental,omitempty"` } @@ -208,8 +212,6 @@ func (p *Plugin) Attest(ctx context.Context, req *workloadattestorv1.AttestReque return nil, err } - fmt.Printf("attest: containerHelper=%#v\n", containerHelper) - podUID, containerID, err := containerHelper.GetPodUIDAndContainerID(req.Pid, p.log) if err != nil { return nil, err @@ -406,7 +408,6 @@ func (p *Plugin) Configure(_ context.Context, req *configv1.ConfigureRequest) (r } // Set the config - fmt.Printf("setContainerHelper(%p)\n", containerHelper) p.setConfig(c, containerHelper) return &configv1.ConfigureResponse{}, nil } diff --git a/pkg/agent/plugin/workloadattestor/k8s/k8s_posix.go b/pkg/agent/plugin/workloadattestor/k8s/k8s_posix.go index 98956552d3..e52e18d21d 100644 --- a/pkg/agent/plugin/workloadattestor/k8s/k8s_posix.go +++ b/pkg/agent/plugin/workloadattestor/k8s/k8s_posix.go @@ -42,9 +42,10 @@ func createHelper(c *Plugin) ContainerHelper { } type containerHelper struct { - rootDir string - sigstoreClient sigstore.Sigstore - useNewContainerLocator bool + rootDir string + sigstoreClient sigstore.Sigstore + useNewContainerLocator bool + verboseContainerLocatorLogs bool } func (h *containerHelper) Configure(config *HCLConfig, log hclog.Logger) error { @@ -60,6 +61,7 @@ func (h *containerHelper) Configure(config *HCLConfig, log hclog.Logger) error { } } + h.verboseContainerLocatorLogs = config.VerboseContainerLocatorLogs h.useNewContainerLocator = config.UseNewContainerLocator != nil && *config.UseNewContainerLocator if h.useNewContainerLocator { log.Info("Using the new container locator") @@ -94,7 +96,7 @@ func (h *containerHelper) GetPodUIDAndContainerID(pID int32, log hclog.Logger) ( return getPodUIDAndContainerIDFromCGroups(cgroups) } - extractor := containerinfo.Extractor{RootDir: h.rootDir} + extractor := containerinfo.Extractor{RootDir: h.rootDir, VerboseLogging: h.verboseContainerLocatorLogs} return extractor.GetPodUIDAndContainerID(int(pID), log) } diff --git a/pkg/common/containerinfo/extract.go b/pkg/common/containerinfo/extract.go index bce36a245f..992453cb35 100644 --- a/pkg/common/containerinfo/extract.go +++ b/pkg/common/containerinfo/extract.go @@ -36,7 +36,8 @@ var ( ) type Extractor struct { - RootDir string + RootDir string + VerboseLogging bool } func (e *Extractor) GetContainerID(pid int, log hclog.Logger) (string, error) { @@ -84,13 +85,15 @@ func (e *Extractor) extractPodUIDAndContainerIDFromMountInfo(pid int, log hclog. return "", "", status.Errorf(codes.Internal, "failed to parse mount info at %q: %v", mountInfoPath, err) } - for i, mountInfo := range mountInfos { - log.Debug("PID mount enumerated", - "index", i+1, - "total", len(mountInfos), - "type", mountInfo.FsType, - "root", mountInfo.Root, - ) + if e.VerboseLogging { + for i, mountInfo := range mountInfos { + log.Debug("PID mount enumerated", + "index", i+1, + "total", len(mountInfos), + "type", mountInfo.FsType, + "root", mountInfo.Root, + ) + } } // Scan the cgroup mounts for the pod UID and container ID. The container @@ -111,7 +114,7 @@ func (e *Extractor) extractPodUIDAndContainerIDFromMountInfo(pid int, log hclog. continue } - log := log.With("mountInfoRoot", mountInfo.Root) + log := log.With("mount-info-root", mountInfo.Root) if err := ex.Extract(mountInfo.Root, log); err != nil { return "", "", err } @@ -128,17 +131,19 @@ func (e *Extractor) extractPodUIDAndContainerIDFromCGroups(pid int, log hclog.Lo return "", "", status.Errorf(codes.Internal, "unable to obtain cgroups: %v", err) } - for i, cgroup := range cgroups { - log.Debug("PID cgroup enumerated", - "index", i+1, - "total", len(cgroups), - "path", cgroup.GroupPath, - ) + if e.VerboseLogging { + for i, cgroup := range cgroups { + log.Debug("PID cgroup enumerated", + "index", i+1, + "total", len(cgroups), + "path", cgroup.GroupPath, + ) + } } ex := &extractor{extractPodUID: extractPodUID} for _, cgroup := range cgroups { - log := log.With("cgroupPath", cgroup.GroupPath) + log := log.With("cgroup-path", cgroup.GroupPath) if err := ex.Extract(cgroup.GroupPath, log); err != nil { return "", "", err } @@ -174,7 +179,7 @@ func (e *extractor) Extract(cgroupPathOrMountRoot string, log hclog.Logger) erro // don't have a pod UID and the new entry does, then override what we have // so far. // - // This helps mitigate situations where there is hybrid cgroups configured + // This helps mitigate situations where there is unified cgroups configured // while running kind on macOS, which ends up with something like: // 1:cpuset:/docker/93529524695bb00d91c1f6dba692ea8d3550c3b94fb2463af7bc9ec82f992d26/kubepods/besteffort/poda2830d0d-b0f0-4ff0-81b5-0ee4e299cf80/09bc3d7ade839efec32b6bec4ec79d099027a668ddba043083ec21d3c3b8f1e6 // 0::/docker/93529524695bb00d91c1f6dba692ea8d3550c3b94fb2463af7bc9ec82f992d26/system.slice/containerd.service diff --git a/pkg/common/containerinfo/extract_test.go b/pkg/common/containerinfo/extract_test.go index 7c0a27554c..b2a97ece49 100644 --- a/pkg/common/containerinfo/extract_test.go +++ b/pkg/common/containerinfo/extract_test.go @@ -63,6 +63,10 @@ func TestExtractPodUIDAndContainerID(t *testing.T) { assertErrorContains(t, "testdata/k8s/pod-uid-conflict", "multiple pod UIDs found") }) + t.Run("ignore non-pod UID entry after pod UID found", func(t *testing.T) { + assertFound(t, "testdata/k8s/pod-uid-override", testPodUID, testContainerID) + }) + t.Run("container ID conflict", func(t *testing.T) { assertErrorContains(t, "testdata/k8s/container-id-conflict", "multiple container IDs found") }) diff --git a/pkg/common/containerinfo/testdata/k8s/pod-uid-override/proc/123/mountinfo b/pkg/common/containerinfo/testdata/k8s/pod-uid-override/proc/123/mountinfo new file mode 100644 index 0000000000..5a8a01ebb1 --- /dev/null +++ b/pkg/common/containerinfo/testdata/k8s/pod-uid-override/proc/123/mountinfo @@ -0,0 +1,3 @@ +2032 2031 0:309 / /sys/fs/cgroup rw,nosuid,nodev,noexec,relatime - tmpfs tmpfs rw,mode=755 +2033 2032 0:33 /kubelet.slice/kubelet-kubepods.slice/kubelet-kubepods-pod00000000_1111_2222_3333_444444444444.slice/cri-containerd-0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef.scope /sys/fs/cgroup/systemd ro,nosuid,nodev,noexec,relatime - cgroup systemd rw,xattr,name=systemd +2034 2032 0:37 /fake/just/to/test/condition/cri-containerd-fedcba98765432100123456789abcdef0123456789abcdef0123456789abcdef.scope /sys/fs/cgroup/cpu,cpuacct ro,nosuid,nodev,noexec,relatime - cgroup cgroup rw,cpu,cpuacct From 3722c4d679c594448bae644882560eca968b85f0 Mon Sep 17 00:00:00 2001 From: Andrew Harding Date: Tue, 23 Apr 2024 12:44:22 -0600 Subject: [PATCH 06/10] markdown lint Signed-off-by: Andrew Harding --- doc/plugin_agent_workloadattestor_k8s.md | 1 - 1 file changed, 1 deletion(-) diff --git a/doc/plugin_agent_workloadattestor_k8s.md b/doc/plugin_agent_workloadattestor_k8s.md index df9b877fe5..fc467f5b74 100644 --- a/doc/plugin_agent_workloadattestor_k8s.md +++ b/doc/plugin_agent_workloadattestor_k8s.md @@ -63,7 +63,6 @@ since [hostprocess](https://kubernetes.io/docs/tasks/configure-pod-container/cre | `use_new_container_locator` | If true, enables the new container locator algorithm that has support for cgroups v2. Defaults to false. | | `verbose_container_locator_logs` | If true, enables verbose logging of mountinfo and cgroup information used to locate containers. Defaults to false. | - | Experimental options | Description | |----------------------|----------------------------------------------------------------------------------------------------------------------------- | | `sigstore` | Sigstore options. Options described below. See [Sigstore workload attestor for SPIRE](#sigstore-workload-attestor-for-spire) | From b2457607d1defef11e4ffc1ae4e1302387f1960f Mon Sep 17 00:00:00 2001 From: Andrew Harding Date: Tue, 23 Apr 2024 13:05:05 -0600 Subject: [PATCH 07/10] add agent full conf Signed-off-by: Andrew Harding --- conf/agent/agent_full.conf | 36 ++++++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/conf/agent/agent_full.conf b/conf/agent/agent_full.conf index 6823796fdb..ccbb2f947a 100644 --- a/conf/agent/agent_full.conf +++ b/conf/agent/agent_full.conf @@ -90,15 +90,15 @@ agent { # # default X.509 bundle with Envoy SDS. Default: ROOTCA. # # default_bundle_name = "ROOTCA" # - # # default_all_bundles_name: The Validation Context resource name to use to fetch + # # default_all_bundles_name: The Validation Context resource name to use to fetch # # all bundles (including federated bundles) with Envoy SDS. Cannot be used with # # Envoy releases prior to 1.18. # # default_all_bundles_name = "ALL" - + # # disable_spiffe_cert_validation: disable Envoy SDS custom SPIFFE validation. Default: false # # disable_spiffe_cert_validation = false # } - + # allowed_foreign_jwt_claims: set a list of trusted claims to be returned when validating foreign JWTSVIDs # allowed_foreign_jwt_claims = [] @@ -259,7 +259,7 @@ plugins { NodeAttestor "tpm_devid" { plugin_data { # tpm_device_path: Optional. The path to a TPM 2.0 device. If unset - # the plugin will try to autodetect the TPM path. It is not used when running + # the plugin will try to autodetect the TPM path. It is not used when running # on windows. # tpm_device_path = "/dev/tpmrm0" @@ -286,7 +286,7 @@ plugins { # devid_password = "password" } } - + # SVIDStore "gcp_secretmanager": An SVID store that stores the SVIDs in # Google Cloud Secret Manager. SVIDStore "gcp_secretmanager" { @@ -310,7 +310,7 @@ plugins { # secret_access_key = "" # region: AWS region to store the secrets. - # region = "" + # region = "" } } @@ -324,6 +324,16 @@ plugins { # docker_version: The API version of the docker daemon. If not # specified, the version is negotiated by the client. # docker_version = "" + + # use_new_container_locator: If true, enables the new container + # locator algorithm that has support for cgroups v2. Default: + # false. (Linux only) + # use_new_container_locator = false + + # verbose_container_locator_logs: If true, enables verbose logging + # of mountinfo and cgroup information used to locate containers. + # Defaults to false. (Linux only) + # verbose_container_locator_logs = false } } @@ -360,7 +370,7 @@ plugins { # private_key_path: The path on disk to client key used for kubelet # authentication. # private_key_path = "" - + # use_anonymous_authentication: If true, use anonymous authentication # for kubelet communication. # use_anonymous_authentication = false @@ -373,13 +383,23 @@ plugins { # the environment variable specified by node_name_env. # node_name = "" + # use_new_container_locator: If true, enables the new container + # locator algorithm that has support for cgroups v2. Default: + # false. (Linux only) + # use_new_container_locator = false + + # verbose_container_locator_logs: If true, enables verbose logging + # of mountinfo and cgroup information used to locate containers. + # Defaults to false. (Linux only) + # verbose_container_locator_logs = false + # experimental: Experimental features. experimental { # sigstore: sigstore options. Enables signature checking. # sigstore { # rekor_url: The URL for the rekor STL Server to use with cosign. Required. # rekor_url = "https://rekor.sigstore.dev" - + # skip_signature_verification_image_list: List of images that should # not be verified by cosign. They will receive a default # sigstore-validation:passed selector, but no other sigstore related selectors. From c00870c7016acaa775b8ba718f2138fe41ed3fa9 Mon Sep 17 00:00:00 2001 From: Andrew Harding Date: Tue, 23 Apr 2024 13:28:06 -0600 Subject: [PATCH 08/10] fix labels Signed-off-by: Andrew Harding --- pkg/common/containerinfo/extract.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/common/containerinfo/extract.go b/pkg/common/containerinfo/extract.go index 992453cb35..e18e4cccf5 100644 --- a/pkg/common/containerinfo/extract.go +++ b/pkg/common/containerinfo/extract.go @@ -114,7 +114,7 @@ func (e *Extractor) extractPodUIDAndContainerIDFromMountInfo(pid int, log hclog. continue } - log := log.With("mount-info-root", mountInfo.Root) + log := log.With("mount_info_root", mountInfo.Root) if err := ex.Extract(mountInfo.Root, log); err != nil { return "", "", err } @@ -143,7 +143,7 @@ func (e *Extractor) extractPodUIDAndContainerIDFromCGroups(pid int, log hclog.Lo ex := &extractor{extractPodUID: extractPodUID} for _, cgroup := range cgroups { - log := log.With("cgroup-path", cgroup.GroupPath) + log := log.With("cgroup_path", cgroup.GroupPath) if err := ex.Extract(cgroup.GroupPath, log); err != nil { return "", "", err } From bf723d54d48968f11f5ec52391656e1e54c136ef Mon Sep 17 00:00:00 2001 From: Andrew Harding Date: Tue, 23 Apr 2024 15:55:12 -0600 Subject: [PATCH 09/10] change log to warn Signed-off-by: Andrew Harding --- pkg/agent/plugin/workloadattestor/docker/docker_posix.go | 4 ++-- pkg/agent/plugin/workloadattestor/k8s/k8s_posix.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/agent/plugin/workloadattestor/docker/docker_posix.go b/pkg/agent/plugin/workloadattestor/docker/docker_posix.go index 9b1b7f0693..f1dc803c82 100644 --- a/pkg/agent/plugin/workloadattestor/docker/docker_posix.go +++ b/pkg/agent/plugin/workloadattestor/docker/docker_posix.go @@ -49,14 +49,14 @@ func createHelper(c *dockerPluginConfig, log hclog.Logger) (*containerHelper, er } 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`.") + log.Warn("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 } 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`.") + log.Warn("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{} } diff --git a/pkg/agent/plugin/workloadattestor/k8s/k8s_posix.go b/pkg/agent/plugin/workloadattestor/k8s/k8s_posix.go index e52e18d21d..82fab2a23d 100644 --- a/pkg/agent/plugin/workloadattestor/k8s/k8s_posix.go +++ b/pkg/agent/plugin/workloadattestor/k8s/k8s_posix.go @@ -66,7 +66,7 @@ func (h *containerHelper) Configure(config *HCLConfig, log hclog.Logger) error { if h.useNewContainerLocator { log.Info("Using the new container locator") } else { - 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`.") + log.Warn("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`.") } return nil From 5ed030716564e9292f3f210ffea437a5a029cab9 Mon Sep 17 00:00:00 2001 From: Andrew Harding Date: Tue, 23 Apr 2024 15:58:50 -0600 Subject: [PATCH 10/10] use new locator in it Signed-off-by: Andrew Harding --- test/integration/suites/k8s/conf/agent/spire-agent.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/integration/suites/k8s/conf/agent/spire-agent.yaml b/test/integration/suites/k8s/conf/agent/spire-agent.yaml index fdd901c78d..6c6b897f1e 100644 --- a/test/integration/suites/k8s/conf/agent/spire-agent.yaml +++ b/test/integration/suites/k8s/conf/agent/spire-agent.yaml @@ -73,6 +73,10 @@ data: # Minikube does not have a cert in the cluster CA bundle that # can authenticate the kubelet cert, so skip validation. skip_kubelet_verification = true + + # Use the new container locator method. This can be removed + # once it is on by default. + use_new_container_locator = true } } }