Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Robust cgroup path parsing of docker container ID #1605

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
159 changes: 105 additions & 54 deletions pkg/agent/plugin/workloadattestor/docker/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"errors"
"fmt"
"regexp"
"strings"
"sync"

Expand All @@ -28,10 +29,6 @@ const (
subselectorEnv = "env"
)

var defaultContainerIDMatchers = []string{
"/docker/<id>",
}

func BuiltIn() catalog.Plugin {
return builtin(New())
}
Expand All @@ -52,11 +49,6 @@ type Plugin struct {
mtx *sync.RWMutex
retryer *retryer
containerIDFinder cgroup.ContainerIDFinder
findContainerID func(string) (string, bool)

// legacy ID extraction
cgroupPrefix string
cgroupContainerIndex int
}

func New() *Plugin {
Expand Down Expand Up @@ -94,28 +86,14 @@ func (p *Plugin) Attest(ctx context.Context, req *workloadattestor.AttestRequest
return nil, err
}

var containerID string
var hasDockerEntries bool
for _, cgroup := range cgroupList {
// We are only interested in cgroup entries that match our desired pattern. Example entry:
// "10:perf_event:/docker/2235ebefd9babe0dde4df4e7c49708e24fb31fb851edea55c0ee29a18273cdf4"
id, ok := p.findContainerID(cgroup.GroupPath)
if !ok {
continue
}
hasDockerEntries = true
containerID = id
break
}

// Not a docker workload. Since it is expected that non-docker workloads will call the
// workload API, it is fine to return a response without any selectors.
if !hasDockerEntries {
containerID, err := getContainerIDFromCGroups(p.containerIDFinder, cgroupList)
switch {
case err != nil:
return nil, err
case containerID == "":
// Not a docker workload. Nothing more to do.
return &workloadattestor.AttestResponse{}, nil
}
if containerID == "" {
return nil, fmt.Errorf("workloadattestor/docker: a pattern matched, but no container id was found")
}

var container types.ContainerJSON
err = p.retryer.Retry(ctx, func() error {
Expand Down Expand Up @@ -182,52 +160,125 @@ func (p *Plugin) Configure(ctx context.Context, req *spi.ConfigureRequest) (*spi
return nil, err
}

if config.CgroupPrefix != "" || config.CgroupContainerIndex != nil {
switch {
case config.CgroupPrefix != "" || config.CgroupContainerIndex != nil:
if config.CgroupPrefix == "" || config.CgroupContainerIndex == nil {
return nil, errors.New("cgroup_prefix and cgroup_container_index must be specified together")
}
p.log.Warn("cgroup_prefix and cgroup_container_index are deprecated and will be removed in a future release")

p.cgroupPrefix = config.CgroupPrefix
// index 0 will always be "" as the prefix must start with /.
// We add 1 to the requested index to hide this from the user.
p.cgroupContainerIndex = *config.CgroupContainerIndex + 1
p.containerIDFinder = &legacyContainerIDFinder{
log: p.log,
cgroupPrefix: config.CgroupPrefix,
// index 0 will always be "" as the prefix must start with /.
// We add 1 to the requested index to hide this from the user.
cgroupContainerIndex: *config.CgroupContainerIndex + 1,
}
case len(config.ContainerIDCGroupMatchers) > 0:
p.containerIDFinder, err = cgroup.NewContainerIDFinder(config.ContainerIDCGroupMatchers)
if err != nil {
return nil, err
}
default:
p.containerIDFinder = &defaultContainerIDFinder{}
}

p.findContainerID = p.legacyExtractID
return &spi.ConfigureResponse{}, nil
}

return &spi.ConfigureResponse{}, nil
}
func (*Plugin) GetPluginInfo(context.Context, *spi.GetPluginInfoRequest) (*spi.GetPluginInfoResponse, error) {
return &spi.GetPluginInfoResponse{}, nil
}

matchers := config.ContainerIDCGroupMatchers
if len(matchers) == 0 {
matchers = defaultContainerIDMatchers
}
// getContainerIDFromCGroups returns the container ID from a set of cgroups
// using the given finder. The container ID found on each cgroup path (if any)
// must be consistent. If no container ID is found among the cgroups, i.e.,
// this isn't a docker workload, the function returns an empty string. If more
// than one container ID is found, or the "found" container ID is blank, the
// function will fail.
func getContainerIDFromCGroups(finder cgroup.ContainerIDFinder, cgroups []cgroups.Cgroup) (string, error) {
var hasDockerEntries bool
var containerID string
for _, cgroup := range cgroups {
candidate, ok := finder.FindContainerID(cgroup.GroupPath)
if !ok {
continue
}

p.containerIDFinder, err = cgroup.NewContainerIDFinder(matchers)
if err != nil {
return nil, err
}
hasDockerEntries = true

p.findContainerID = p.containerIDFinder.FindContainerID
switch {
case containerID == "":
// This is the first container ID found so far.
containerID = candidate
case containerID != candidate:
// More than one container ID found in the cgroups.
return "", fmt.Errorf("workloadattestor/docker: multiple container IDs found in cgroups (%s, %s)",
containerID, candidate)
}
}

return &spi.ConfigureResponse{}, nil
switch {
case !hasDockerEntries:
// Not a docker workload. Since it is expected that non-docker workloads will call the
// workload API, it is fine to return a response without any selectors.
return "", nil
case containerID == "":
// The "finder" found a container ID, but it was blank. This is a
// defensive measure against bad matcher patterns and shouldn't
// be possible with the default finder.
return "", errors.New("workloadattestor/docker: a pattern matched, but no container id was found")
default:
return containerID, nil
}
}

func (*Plugin) GetPluginInfo(context.Context, *spi.GetPluginInfoRequest) (*spi.GetPluginInfoResponse, error) {
return &spi.GetPluginInfoResponse{}, nil
type legacyContainerIDFinder struct {
log hclog.Logger
cgroupPrefix string
cgroupContainerIndex int
}

func (p *Plugin) legacyExtractID(cgroupPath string) (string, bool) {
if !strings.HasPrefix(cgroupPath, p.cgroupPrefix) {
// FindContainerID returns the container ID from the given cgroup path. It only
// considers cgroup paths matching the configured prefix. The path is split
// into a number of slash separated segments. The container ID is assumed to
// occupy the segment at the configured index. If the cgroup path does not
// match the prefix or does not have enough segments to accommodate the index,
// the method returns false.
func (f *legacyContainerIDFinder) FindContainerID(cgroupPath string) (string, bool) {
if !strings.HasPrefix(cgroupPath, f.cgroupPrefix) {
return "", false
}

parts := strings.Split(cgroupPath, "/")

if len(parts) <= p.cgroupContainerIndex {
p.log.Warn("Docker entry found, but is missing the container id", telemetry.CGroupPath, cgroupPath)
if len(parts) <= f.cgroupContainerIndex {
f.log.Warn("Docker entry found, but is missing the container id", telemetry.CGroupPath, cgroupPath)
return "", false
}

return parts[p.cgroupContainerIndex], true
return parts[f.cgroupContainerIndex], true
}

// dockerCGroupRE matches cgroup paths that have the following properties.
// 1) `\bdocker\b` the whole word docker
// 2) `.+` followed by one or more characters (which will start on a word boundary due to #1)
// 3) `\b([[:xdigit:]][64])\b` followed by a 64 hex-character container id on word boundary
//
// The "docker" prefix and 64-hex character container id can be anywhere in the path. The only
// requirement is that the docker prefix comes before the id.
var dockerCGroupRE = regexp.MustCompile(`\bdocker\b.+\b([[:xdigit:]]{64})\b`)

type defaultContainerIDFinder struct{}

// FindContainerID returns the container ID in the given cgroup path. The cgroup
// path must have the whole word "docker" at some point in the path followed
// at some point by a 64 hex-character container ID. If the cgroup path does
// not match the above description, the method returns false.
func (f *defaultContainerIDFinder) FindContainerID(cgroupPath string) (string, bool) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that a small comment about this function would be good. We can point that dockerCGroupRE is used here and it would be good to explain the meaning of an empty string result and the boolean returned.

m := dockerCGroupRE.FindStringSubmatch(cgroupPath)
if m != nil {
return m[1], true
}
return "", false
}
31 changes: 25 additions & 6 deletions pkg/agent/plugin/workloadattestor/docker/docker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,25 @@ cgroup_container_index = 1`,
cfg: `cgroup_prefix = "/docker"
cgroup_container_index = 2`,
},
{
desc: "RHEL docker cgroups",
cgroups: "4:devices:/system.slice/docker-6469646e742065787065637420616e796f6e6520746f20726561642074686973.scope",
hasMatch: true,
},
{
desc: "docker for desktop",
cgroups: "6:devices:/docker/6469646e742065787065637420616e796f6e6520746f20726561642074686973/docker/6469646e742065787065637420616e796f6e6520746f20726561642074686973/system.slice/containerd.service",
hasMatch: true,
},
{
desc: "more than one id",
cgroups: testCgroupEntries + "\n" + "4:devices:/system.slice/docker-41e4ab61d2860b0e1467de0da0a9c6068012761febec402dc04a5a94f32ea867.scope",
expectErr: "multiple container IDs found in cgroups",
},
{
desc: "default finder does not match cgroup missing docker prefix",
cgroups: "4:devices:/system.slice/41e4ab61d2860b0e1467de0da0a9c6068012761febec402dc04a5a94f32ea867.scope",
},
}

for _, tt := range tests {
Expand Down Expand Up @@ -350,8 +369,11 @@ cgroup_container_index = 2

_, err := doConfigure(t, p, cfg)
require.NoError(t, err)
require.Equal(t, "/docker2", p.cgroupPrefix)
require.Equal(t, 3, p.cgroupContainerIndex)
require.Equal(t, &legacyContainerIDFinder{
log: p.log,
cgroupPrefix: "/docker2",
cgroupContainerIndex: 3,
}, p.containerIDFinder)
})
t.Run("bad matcher", func(t *testing.T) {
p := New()
Expand Down Expand Up @@ -393,15 +415,12 @@ container_id_cgroup_matchers = [
}

func TestDockerConfigDefault(t *testing.T) {
defaultFinder, err := cgroup.NewContainerIDFinder(defaultContainerIDMatchers)
require.NoError(t, err)

p := newTestPlugin(t)

require.NotNil(t, p.docker)
require.Equal(t, dockerclient.DefaultDockerHost, p.docker.(*dockerclient.Client).DaemonHost())
require.Equal(t, "1.41", p.docker.(*dockerclient.Client).ClientVersion())
require.Equal(t, defaultFinder, p.containerIDFinder)
require.Equal(t, &defaultContainerIDFinder{}, p.containerIDFinder)
}

func doAttest(t *testing.T, p *Plugin, req *workloadattestor.AttestRequest) (*workloadattestor.AttestResponse, error) {
Expand Down