From ced13dfe4e1813cffa8d9a6e9f920c0331ddf575 Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Mon, 8 Aug 2022 10:35:13 +0200 Subject: [PATCH 1/4] Support Device CGroup rules update through runc With CGroups v2, device CGroup rules update is not as straight forward since it requires ebpf rules. Currently, runc does support updating this ebpf rules, but when using the runc update command new device rules are not passed yet. A patched runc version (and hopefully future runc versions) support updating device resources, see: https://github.com/opencontainers/runc/pull/3402 Use this (patched) runc capability to support Device CGroup rules updates with CGroups v2. --- cgroup/cgroup.go | 78 ++++++++++++++++++++++++++++++++++-------------- cgroup/oci.go | 63 ++++++++++++++++++++++++++++++++++++++ go.mod | 1 + go.sum | 2 ++ 4 files changed, 122 insertions(+), 22 deletions(-) create mode 100644 cgroup/oci.go diff --git a/cgroup/cgroup.go b/cgroup/cgroup.go index 072eda2..721fae5 100644 --- a/cgroup/cgroup.go +++ b/cgroup/cgroup.go @@ -1,8 +1,10 @@ package cgroup import ( + "encoding/json" "fmt" "os" + "os/exec" "path/filepath" securejoin "github.com/cyphar/filepath-securejoin" @@ -24,32 +26,64 @@ type cgroup struct { } func (d cgroup) AddDevicesAllowed(containerID string, permission string) (bool, *dbus.Error) { - // Make sure path is relative to cgroupFSDockerDevices - allowedFile, err := securejoin.SecureJoin(cgroupFSDockerDevices, containerID+string(filepath.Separator)+"devices.allow") - if err != nil { - return false, dbus.MakeFailedError(fmt.Errorf("Security issues with '%s': %s", containerID, err)) - } + // Check for CGroups v2 + if _, err := os.Stat("/sys/fs/cgroup/cgroup.controllers"); err == nil { + permissions := []string{permission} + resources, err := CreateDeviceUpdateResources(permissions) + if err != nil { + error := fmt.Errorf("Error calling runc for '%s': %s", containerID, err) + logging.Error.Printf("%s", error) + return false, dbus.MakeFailedError(error) + } - // Check if file/container exists - _, err = os.Stat(allowedFile) - if os.IsNotExist(err) { - return false, dbus.MakeFailedError(fmt.Errorf("Can't find Container '%s' for adjust CGroup devices.", containerID)) - } + fmt.Printf("Container resources update %#v\n", resources) - // Write permission adjustments - file, err := os.Create(allowedFile) - if err != nil { - return false, dbus.MakeFailedError(fmt.Errorf("Can't open CGroup devices '%s': %s", allowedFile, err)) - } - defer file.Close() + cmd := exec.Command("runc", "--root", "/var/run/docker/runtime-runc/moby/", "update", "--resources", "-", containerID) + fmt.Printf("Calling command %v", cmd.Args) - _, err = file.WriteString(permission + "\n") - if err != nil { - return false, dbus.MakeFailedError(fmt.Errorf("Can't write CGroup permission '%s': %s", permission, err)) - } + // Pass resources as OCI LinuxResources JSON object + stdin, err := cmd.StdinPipe() + enc := json.NewEncoder(stdin) + enc.Encode(resources) + stdin.Close() - logging.Info.Printf("Permission '%s', granted for Container '%s'", containerID, permission) - return true, nil + stdoutStderr, err := cmd.CombinedOutput() + if err != nil { + error := fmt.Errorf("Error calling runc for '%s': %s, output %s", containerID, err, stdoutStderr) + logging.Error.Printf("%s", error) + return false, dbus.MakeFailedError(error) + } + + logging.Info.Printf("Permission '%s', granted for Container '%s'", containerID, permission) + return true, nil + } else { + // Make sure path is relative to cgroupFSDockerDevices + allowedFile, err := securejoin.SecureJoin(cgroupFSDockerDevices, containerID+string(filepath.Separator)+"devices.allow") + if err != nil { + return false, dbus.MakeFailedError(fmt.Errorf("Security issues with '%s': %s", containerID, err)) + } + + // Check if file/container exists + _, err = os.Stat(allowedFile) + if os.IsNotExist(err) { + return false, dbus.MakeFailedError(fmt.Errorf("Can't find Container '%s' for adjust CGroup devices.", containerID)) + } + + // Write permission adjustments + file, err := os.Create(allowedFile) + if err != nil { + return false, dbus.MakeFailedError(fmt.Errorf("Can't open CGroup devices '%s': %s", allowedFile, err)) + } + defer file.Close() + + _, err = file.WriteString(permission + "\n") + if err != nil { + return false, dbus.MakeFailedError(fmt.Errorf("Can't write CGroup permission '%s': %s", permission, err)) + } + + logging.Info.Printf("Permission '%s', granted for Container '%s'", containerID, permission) + return true, nil + } } func InitializeDBus(conn *dbus.Conn) { diff --git a/cgroup/oci.go b/cgroup/oci.go new file mode 100644 index 0000000..abee30e --- /dev/null +++ b/cgroup/oci.go @@ -0,0 +1,63 @@ +package cgroup + +import ( + "fmt" + "regexp" + "strconv" + + "github.com/opencontainers/runtime-spec/specs-go" +) + +var deviceCgroupRuleRegex = regexp.MustCompile("^([acb]) ([0-9]+|\\*):([0-9]+|\\*) ([rwm]{1,3})$") + +// Lifted from Moby's oci/oci.go +func AppendDevicePermissionsFromCgroupRules(devPermissions []specs.LinuxDeviceCgroup, rules []string) ([]specs.LinuxDeviceCgroup, error) { + for _, deviceCgroupRule := range rules { + ss := deviceCgroupRuleRegex.FindAllStringSubmatch(deviceCgroupRule, -1) + if len(ss) == 0 || len(ss[0]) != 5 { + return nil, fmt.Errorf("invalid device cgroup rule format: '%s'", deviceCgroupRule) + } + matches := ss[0] + + dPermissions := specs.LinuxDeviceCgroup{ + Allow: true, + Type: matches[1], + Access: matches[4], + } + if matches[2] == "*" { + major := int64(-1) + dPermissions.Major = &major + } else { + major, err := strconv.ParseInt(matches[2], 10, 64) + if err != nil { + return nil, fmt.Errorf("invalid major value in device cgroup rule format: '%s'", deviceCgroupRule) + } + dPermissions.Major = &major + } + if matches[3] == "*" { + minor := int64(-1) + dPermissions.Minor = &minor + } else { + minor, err := strconv.ParseInt(matches[3], 10, 64) + if err != nil { + return nil, fmt.Errorf("invalid minor value in device cgroup rule format: '%s'", deviceCgroupRule) + } + dPermissions.Minor = &minor + } + devPermissions = append(devPermissions, dPermissions) + } + return devPermissions, nil +} + +func CreateDeviceUpdateResources(rules []string) (*specs.LinuxResources, error) { + resources := specs.LinuxResources{} + + devices, err := AppendDevicePermissionsFromCgroupRules(nil, rules) + if err != nil { + return nil, err + } + + resources.Devices = devices + + return &resources, nil +} diff --git a/go.mod b/go.mod index 39ca2a3..2e6c6b9 100644 --- a/go.mod +++ b/go.mod @@ -9,4 +9,5 @@ require ( github.com/getsentry/sentry-go v0.13.0 github.com/godbus/dbus/v5 v5.1.0 github.com/pkg/errors v0.9.1 // indirect + github.com/opencontainers/runtime-spec v1.0.2 ) diff --git a/go.sum b/go.sum index 76703b4..cbef9a4 100644 --- a/go.sum +++ b/go.sum @@ -103,6 +103,8 @@ github.com/nats-io/jwt v0.3.0/go.mod h1:fRYCDE99xlTsqUzISS1Bi75UBJ6ljOJQOAAu5Vgl github.com/nats-io/nats.go v1.9.1/go.mod h1:ZjDU1L/7fJ09jvUSRVBR2e7+RnLiiIQyqyzEE/Zbp4w= github.com/nats-io/nkeys v0.1.0/go.mod h1:xpnFELMwJABBLVhffcfd1MZx6VsNRFpEugbxziKVo7w= github.com/nats-io/nuid v1.0.1/go.mod h1:19wcPz3Ph3q0Jbyiqsd0kePYG7A95tJPxeL+1OSON2c= +github.com/opencontainers/runtime-spec v1.0.2 h1:UfAcuLBJB9Coz72x1hgl8O5RVzTdNiaglX6v2DM6FI0= +github.com/opencontainers/runtime-spec v1.0.2/go.mod h1:jwyrGlmzljRJv/Fgzds9SsS/C5hL+LL3ko9hs6T5lQ0= github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic= github.com/pingcap/errors v0.11.4 h1:lFuQV/oaUMGcD2tqt+01ROSmJs75VG1ToEOkZIZ4nE4= github.com/pingcap/errors v0.11.4/go.mod h1:Oi8TUi2kEtXXLMJk9l1cGmz20kV3TaQ0usTwv5KuLY8= From 841b505fd713566dfe8e7fcb89245d56e198915f Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Mon, 8 Aug 2022 16:48:36 +0200 Subject: [PATCH 2/4] Determine CGroup version at initialization time --- cgroup/cgroup.go | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/cgroup/cgroup.go b/cgroup/cgroup.go index 721fae5..13a5e53 100644 --- a/cgroup/cgroup.go +++ b/cgroup/cgroup.go @@ -21,13 +21,21 @@ const ( cgroupFSDockerDevices = "/sys/fs/cgroup/devices/docker" ) +type CGroupVersion int + +const ( + CGroupUnknown CGroupVersion = 0 + CGroupV1 = 1 + CGroupV2 = 2 +) + type cgroup struct { - conn *dbus.Conn + conn *dbus.Conn + cgroupVersion CGroupVersion } func (d cgroup) AddDevicesAllowed(containerID string, permission string) (bool, *dbus.Error) { - // Check for CGroups v2 - if _, err := os.Stat("/sys/fs/cgroup/cgroup.controllers"); err == nil { + if d.cgroupVersion == CGroupV2 { permissions := []string{permission} resources, err := CreateDeviceUpdateResources(permissions) if err != nil { @@ -88,7 +96,15 @@ func (d cgroup) AddDevicesAllowed(containerID string, permission string) (bool, func InitializeDBus(conn *dbus.Conn) { d := cgroup{ - conn: conn, + conn: conn, + cgroupVersion: CGroupUnknown, + } + + // Check for CGroups v2 + if _, err := os.Stat("/sys/fs/cgroup/cgroup.controllers"); err == nil { + d.cgroupVersion = CGroupV2 + } else { + d.cgroupVersion = CGroupV1 } err := conn.Export(d, objectPath, ifaceName) From 497df3c020b44c35c32ab491772e0d4799c673c3 Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Mon, 8 Aug 2022 16:56:13 +0200 Subject: [PATCH 3/4] Address go linter issues --- cgroup/cgroup.go | 5 +++++ cgroup/oci.go | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/cgroup/cgroup.go b/cgroup/cgroup.go index 13a5e53..1510823 100644 --- a/cgroup/cgroup.go +++ b/cgroup/cgroup.go @@ -51,6 +51,11 @@ func (d cgroup) AddDevicesAllowed(containerID string, permission string) (bool, // Pass resources as OCI LinuxResources JSON object stdin, err := cmd.StdinPipe() + if err != nil { + error := fmt.Errorf("Error calling runc for '%s': %s", containerID, err) + logging.Error.Printf("%s", error) + return false, dbus.MakeFailedError(error) + } enc := json.NewEncoder(stdin) enc.Encode(resources) stdin.Close() diff --git a/cgroup/oci.go b/cgroup/oci.go index abee30e..0628cc1 100644 --- a/cgroup/oci.go +++ b/cgroup/oci.go @@ -8,7 +8,7 @@ import ( "github.com/opencontainers/runtime-spec/specs-go" ) -var deviceCgroupRuleRegex = regexp.MustCompile("^([acb]) ([0-9]+|\\*):([0-9]+|\\*) ([rwm]{1,3})$") +var deviceCgroupRuleRegex = regexp.MustCompile(`^([acb]) ([0-9]+|\*):([0-9]+|\*) ([rwm]{1,3})$`) // Lifted from Moby's oci/oci.go func AppendDevicePermissionsFromCgroupRules(devPermissions []specs.LinuxDeviceCgroup, rules []string) ([]specs.LinuxDeviceCgroup, error) { From d94189a7651ae0cd9619f553daa9483839b295c8 Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Mon, 8 Aug 2022 17:01:14 +0200 Subject: [PATCH 4/4] Fix more linter issues --- cgroup/cgroup.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cgroup/cgroup.go b/cgroup/cgroup.go index 1510823..38a5929 100644 --- a/cgroup/cgroup.go +++ b/cgroup/cgroup.go @@ -25,8 +25,8 @@ type CGroupVersion int const ( CGroupUnknown CGroupVersion = 0 - CGroupV1 = 1 - CGroupV2 = 2 + CGroupV1 + CGroupV2 ) type cgroup struct {