From 04a126955adb688e56d2914ee96683266f38fc59 Mon Sep 17 00:00:00 2001 From: SreeeS Date: Mon, 9 Oct 2023 09:10:21 -0700 Subject: [PATCH] Add apparmor support --- ecs-init/apparmor/apparmor.go | 32 +++-- ecs-init/apparmor/apparmor_test.go | 103 +++++++++++++ ecs-init/apparmor/apparmor_utils.go | 68 --------- ecs-init/engine/engine.go | 14 +- ecs-init/engine/engine_test.go | 49 +++++++ ecs-init/go.mod | 3 +- .../docker/docker/pkg/aaparser/aaparser.go | 94 ++++++++++++ .../docker/profiles/apparmor/apparmor.go | 135 ++++++++++++++++++ .../docker/profiles/apparmor/template.go | 59 ++++++++ ecs-init/vendor/modules.txt | 2 + 10 files changed, 475 insertions(+), 84 deletions(-) create mode 100644 ecs-init/apparmor/apparmor_test.go delete mode 100644 ecs-init/apparmor/apparmor_utils.go create mode 100644 ecs-init/vendor/github.com/docker/docker/pkg/aaparser/aaparser.go create mode 100644 ecs-init/vendor/github.com/docker/docker/profiles/apparmor/apparmor.go create mode 100644 ecs-init/vendor/github.com/docker/docker/profiles/apparmor/template.go diff --git a/ecs-init/apparmor/apparmor.go b/ecs-init/apparmor/apparmor.go index 418eaeaf03f..8e870bb554e 100644 --- a/ecs-init/apparmor/apparmor.go +++ b/ecs-init/apparmor/apparmor.go @@ -4,6 +4,9 @@ import ( "fmt" "os" "path/filepath" + + "github.com/docker/docker/pkg/aaparser" + aaprofile "github.com/docker/docker/profiles/apparmor" ) const ( @@ -17,8 +20,13 @@ const ecsDefaultProfile = ` profile ecs-default flags=(attach_disconnected,mediate_deleted) { #include - network, - capability, + network inet, # Allow IPv4 traffic + network inet6, # Allow IPv6 traffic + + capability net_admin, # Allow network configuration + capability sys_admin, # Allow ECS Agent to invoke the setns system call + capability dac_override, # Allow ECS Agent to file read, write, and execute permission + file, umount, # Host (privileged) processes may send signals to container processes. @@ -52,18 +60,24 @@ profile ecs-default flags=(attach_disconnected,mediate_deleted) { } ` +var ( + isProfileLoaded = aaprofile.IsLoaded + loadPath = aaparser.LoadProfile + createFile = os.Create +) + // LoadDefaultProfile ensures the default profile to be loaded with the given name. // Returns nil error if the profile is already loaded. func LoadDefaultProfile(profileName string) error { - yes, err := isLoaded(profileName) - if err != nil { - return err - } + yes, err := isProfileLoaded(profileName) if yes { return nil } + if err != nil { + return err + } - f, err := os.Create(filepath.Join(appArmorProfileDir, profileName)) + f, err := createFile(filepath.Join(appArmorProfileDir, profileName)) if err != nil { return err } @@ -74,8 +88,8 @@ func LoadDefaultProfile(profileName string) error { } path := f.Name() - if err := load(path); err != nil { - return fmt.Errorf("load apparmor profile %s: %w", path, err) + if err := loadPath(path); err != nil { + return fmt.Errorf("error loading apparmor profile %s: %w", path, err) } return nil } diff --git a/ecs-init/apparmor/apparmor_test.go b/ecs-init/apparmor/apparmor_test.go new file mode 100644 index 00000000000..a3a82f4da80 --- /dev/null +++ b/ecs-init/apparmor/apparmor_test.go @@ -0,0 +1,103 @@ +// Copyright 2015-2018 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package apparmor + +import ( + "errors" + "os" + "path/filepath" + "testing" + + "github.com/containerd/containerd/pkg/apparmor" + "github.com/docker/docker/pkg/aaparser" + aaprofile "github.com/docker/docker/profiles/apparmor" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestLoadDefaultProfile(t *testing.T) { + testCases := []struct { + name string + profileName string + isLoadedResponse bool + isLoadedError error + loadError error + expectedError error + }{ + { + name: "ProfileIsAlreadyLoaded", + profileName: "testProfile.txt", + isLoadedResponse: true, + isLoadedError: nil, + loadError: nil, + expectedError: nil, + }, + { + name: "ProfileNotLoaded", + profileName: "testProfile.txt", + isLoadedResponse: false, + isLoadedError: nil, + loadError: nil, + expectedError: nil, + }, + { + name: "IsLoadedError", + profileName: "testProfile.txt", + isLoadedResponse: false, + isLoadedError: errors.New("mock isLoaded error"), + loadError: nil, + expectedError: errors.New("mock isLoaded error"), + }, + { + name: "LoadProfileError", + profileName: "testProfile.txt", + isLoadedResponse: false, + isLoadedError: nil, + loadError: errors.New("mock load error"), + expectedError: errors.New("mock load error"), + }, + } + defer func() { + isProfileLoaded = aaprofile.IsLoaded + loadPath = aaparser.LoadProfile + createFile = os.Create + }() + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + if !apparmor.HostSupports() { + t.Skip() + } + tmpdir := os.TempDir() + filePath, err := os.MkdirTemp(tmpdir, "test") + require.NoError(t, err) + createFile = func(profileName string) (*os.File, error) { + f, err := os.Create(filepath.Join(filePath, tc.profileName)) + return f, err + } + defer os.RemoveAll(filePath) + isProfileLoaded = func(profileName string) (bool, error) { + return tc.isLoadedResponse, tc.isLoadedError + } + loadPath = func(profile string) error { + return tc.loadError + } + err = LoadDefaultProfile(tc.profileName) + if tc.loadError == nil { + assert.Equal(t, tc.expectedError, err) + } else { + assert.Error(t, err) + } + }) + } +} diff --git a/ecs-init/apparmor/apparmor_utils.go b/ecs-init/apparmor/apparmor_utils.go deleted file mode 100644 index 0829909a2d2..00000000000 --- a/ecs-init/apparmor/apparmor_utils.go +++ /dev/null @@ -1,68 +0,0 @@ -/* - Copyright The docker Authors. - Copyright The Moby Authors. - Copyright The containerd Authors. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. -*/ - -package apparmor - -import ( - "bufio" - "fmt" - "io" - "os" - "strings" - - exec "golang.org/x/sys/execabs" -) - -// NOTE: This code is copied from . -// If you plan to make any changes, please make sure they are also sent -// upstream. - -func load(path string) error { - out, err := aaParser("-Kr", path) - if err != nil { - return fmt.Errorf("parser error(%q): %w", strings.TrimSpace(out), err) - } - return nil -} - -func aaParser(args ...string) (string, error) { - out, err := exec.Command("apparmor_parser", args...).CombinedOutput() - return string(out), err -} - -func isLoaded(name string) (bool, error) { - f, err := os.Open("/sys/kernel/security/apparmor/profiles") - if err != nil { - return false, err - } - defer f.Close() - r := bufio.NewReader(f) - for { - p, err := r.ReadString('\n') - if err == io.EOF { - break - } - if err != nil { - return false, err - } - if strings.HasPrefix(p, name+" ") { - return true, nil - } - } - return false, nil -} diff --git a/ecs-init/engine/engine.go b/ecs-init/engine/engine.go index 344bc857d29..b4bbbeb4710 100644 --- a/ecs-init/engine/engine.go +++ b/ecs-init/engine/engine.go @@ -51,9 +51,13 @@ const ( ) // Injection point for testing purposes -var getDockerClient = func() (dockerClient, error) { - return docker.Client() -} +var ( + getDockerClient = func() (dockerClient, error) { + return docker.Client() + } + hostSupports = ctrdapparmor.HostSupports + loadDefaultProfile = apparmor.LoadDefaultProfile +) func dockerError(err error) error { return engineError("could not create docker client", err) @@ -205,9 +209,9 @@ func (e *Engine) PreStartGPU() error { // PreStartAppArmor sets up the ecs-default AppArmor profile if we're running // on an AppArmor-enabled system. func (e *Engine) PreStartAppArmor() error { - if ctrdapparmor.HostSupports() { + if hostSupports() { log.Infof("pre-start: setting up %s AppArmor profile", apparmor.ECSDefaultProfileName) - return apparmor.LoadDefaultProfile(apparmor.ECSDefaultProfileName) + return loadDefaultProfile(apparmor.ECSDefaultProfileName) } return nil } diff --git a/ecs-init/engine/engine_test.go b/ecs-init/engine/engine_test.go index 029c48aa9ba..46b92295b64 100644 --- a/ecs-init/engine/engine_test.go +++ b/ecs-init/engine/engine_test.go @@ -24,9 +24,13 @@ import ( "os" "testing" + "github.com/aws/amazon-ecs-agent/ecs-init/apparmor" "github.com/aws/amazon-ecs-agent/ecs-init/cache" "github.com/aws/amazon-ecs-agent/ecs-init/gpu" "github.com/golang/mock/gomock" + "github.com/stretchr/testify/assert" + + ctrdapparmor "github.com/containerd/containerd/pkg/apparmor" ) // getDockerClientMock backs up getDockerClient package-level function and replaces it with the mock passed as @@ -584,3 +588,48 @@ func TestPostStopCredentialsProxyRouteRemoveError(t *testing.T) { t.Errorf("engine post-stop error: %v", err) } } + +func TestPreStartAppArmorSetup(t *testing.T) { + testCases := []struct { + name string + hostSupports bool + loadProfileError error + expectedError error + }{ + { + name: "HostNotSupported", + hostSupports: false, + loadProfileError: nil, + expectedError: nil, + }, + { + name: "HostSupportedNoError", + hostSupports: true, + loadProfileError: nil, + expectedError: nil, + }, + { + name: "HostSupportedWithError", + hostSupports: true, + loadProfileError: errors.New("error loading apparmor profile"), + expectedError: errors.New("error loading apparmor profile"), + }, + } + defer func() { + hostSupports = ctrdapparmor.HostSupports + loadDefaultProfile = apparmor.LoadDefaultProfile + }() + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + hostSupports = func() bool { + return tc.hostSupports + } + loadDefaultProfile = func(profile string) error { + return tc.loadProfileError + } + engine := &Engine{} + err := engine.PreStartAppArmor() + assert.Equal(t, tc.expectedError, err) + }) + } +} diff --git a/ecs-init/go.mod b/ecs-init/go.mod index c9f92b988f6..8c20cf3a79e 100644 --- a/ecs-init/go.mod +++ b/ecs-init/go.mod @@ -7,12 +7,12 @@ require ( github.com/aws/aws-sdk-go v1.36.0 github.com/cihub/seelog v0.0.0-20170130134532-f561c5e57575 github.com/containerd/containerd v1.6.18 + github.com/docker/docker v23.0.3+incompatible github.com/docker/go-plugins-helpers v0.0.0-20181025120712-1e6269c305b8 github.com/fsouza/go-dockerclient v0.0.0-20170830181106-98edf3edfae6 github.com/golang/mock v1.6.0 github.com/pkg/errors v0.9.1 github.com/stretchr/testify v1.7.0 - golang.org/x/sys v0.6.0 ) require ( @@ -20,7 +20,6 @@ require ( github.com/Microsoft/go-winio v0.5.2 // indirect github.com/coreos/go-systemd v0.0.0-20190321100706-95778dfbb74e // indirect github.com/davecgh/go-spew v1.1.1 // indirect - github.com/docker/docker v23.0.3+incompatible // indirect github.com/docker/go-connections v0.4.0 // indirect github.com/docker/go-units v0.4.0 // indirect github.com/gogo/protobuf v1.3.2 // indirect diff --git a/ecs-init/vendor/github.com/docker/docker/pkg/aaparser/aaparser.go b/ecs-init/vendor/github.com/docker/docker/pkg/aaparser/aaparser.go new file mode 100644 index 00000000000..2b5a2605f9c --- /dev/null +++ b/ecs-init/vendor/github.com/docker/docker/pkg/aaparser/aaparser.go @@ -0,0 +1,94 @@ +// Package aaparser is a convenience package interacting with `apparmor_parser`. +package aaparser // import "github.com/docker/docker/pkg/aaparser" + +import ( + "fmt" + "os/exec" + "strconv" + "strings" +) + +const ( + binary = "apparmor_parser" +) + +// GetVersion returns the major and minor version of apparmor_parser. +func GetVersion() (int, error) { + output, err := cmd("", "--version") + if err != nil { + return -1, err + } + + return parseVersion(output) +} + +// LoadProfile runs `apparmor_parser -Kr` on a specified apparmor profile to +// replace the profile. The `-K` is necessary to make sure that apparmor_parser +// doesn't try to write to a read-only filesystem. +func LoadProfile(profilePath string) error { + _, err := cmd("", "-Kr", profilePath) + return err +} + +// cmd runs `apparmor_parser` with the passed arguments. +func cmd(dir string, arg ...string) (string, error) { + c := exec.Command(binary, arg...) + c.Dir = dir + + output, err := c.CombinedOutput() + if err != nil { + return "", fmt.Errorf("running `%s %s` failed with output: %s\nerror: %v", c.Path, strings.Join(c.Args, " "), output, err) + } + + return string(output), nil +} + +// parseVersion takes the output from `apparmor_parser --version` and returns +// a representation of the {major, minor, patch} version as a single number of +// the form MMmmPPP {major, minor, patch}. +func parseVersion(output string) (int, error) { + // output is in the form of the following: + // AppArmor parser version 2.9.1 + // Copyright (C) 1999-2008 Novell Inc. + // Copyright 2009-2012 Canonical Ltd. + + lines := strings.SplitN(output, "\n", 2) + words := strings.Split(lines[0], " ") + version := words[len(words)-1] + + // trim "-beta1" suffix from version="3.0.0-beta1" if exists + version = strings.SplitN(version, "-", 2)[0] + // also trim "~..." suffix used historically (https://gitlab.com/apparmor/apparmor/-/commit/bca67d3d27d219d11ce8c9cc70612bd637f88c10) + version = strings.SplitN(version, "~", 2)[0] + + // split by major minor version + v := strings.Split(version, ".") + if len(v) == 0 || len(v) > 3 { + return -1, fmt.Errorf("parsing version failed for output: `%s`", output) + } + + // Default the versions to 0. + var majorVersion, minorVersion, patchLevel int + + majorVersion, err := strconv.Atoi(v[0]) + if err != nil { + return -1, err + } + + if len(v) > 1 { + minorVersion, err = strconv.Atoi(v[1]) + if err != nil { + return -1, err + } + } + if len(v) > 2 { + patchLevel, err = strconv.Atoi(v[2]) + if err != nil { + return -1, err + } + } + + // major*10^5 + minor*10^3 + patch*10^0 + numericVersion := majorVersion*1e5 + minorVersion*1e3 + patchLevel + return numericVersion, nil +} diff --git a/ecs-init/vendor/github.com/docker/docker/profiles/apparmor/apparmor.go b/ecs-init/vendor/github.com/docker/docker/profiles/apparmor/apparmor.go new file mode 100644 index 00000000000..b3566b2f735 --- /dev/null +++ b/ecs-init/vendor/github.com/docker/docker/profiles/apparmor/apparmor.go @@ -0,0 +1,135 @@ +//go:build linux +// +build linux + +package apparmor // import "github.com/docker/docker/profiles/apparmor" + +import ( + "bufio" + "io" + "os" + "path" + "strings" + "text/template" + + "github.com/docker/docker/pkg/aaparser" +) + +var ( + // profileDirectory is the file store for apparmor profiles and macros. + profileDirectory = "/etc/apparmor.d" +) + +// profileData holds information about the given profile for generation. +type profileData struct { + // Name is profile name. + Name string + // DaemonProfile is the profile name of our daemon. + DaemonProfile string + // Imports defines the apparmor functions to import, before defining the profile. + Imports []string + // InnerImports defines the apparmor functions to import in the profile. + InnerImports []string + // Version is the {major, minor, patch} version of apparmor_parser as a single number. + Version int +} + +// generateDefault creates an apparmor profile from ProfileData. +func (p *profileData) generateDefault(out io.Writer) error { + compiled, err := template.New("apparmor_profile").Parse(baseTemplate) + if err != nil { + return err + } + + if macroExists("tunables/global") { + p.Imports = append(p.Imports, "#include ") + } else { + p.Imports = append(p.Imports, "@{PROC}=/proc/") + } + + if macroExists("abstractions/base") { + p.InnerImports = append(p.InnerImports, "#include ") + } + + ver, err := aaparser.GetVersion() + if err != nil { + return err + } + p.Version = ver + + return compiled.Execute(out, p) +} + +// macrosExists checks if the passed macro exists. +func macroExists(m string) bool { + _, err := os.Stat(path.Join(profileDirectory, m)) + return err == nil +} + +// InstallDefault generates a default profile in a temp directory determined by +// os.TempDir(), then loads the profile into the kernel using 'apparmor_parser'. +func InstallDefault(name string) error { + p := profileData{ + Name: name, + } + + // Figure out the daemon profile. + currentProfile, err := os.ReadFile("/proc/self/attr/current") + if err != nil { + // If we couldn't get the daemon profile, assume we are running + // unconfined which is generally the default. + currentProfile = nil + } + daemonProfile := string(currentProfile) + // Normally profiles are suffixed by " (enforcing)" or similar. AppArmor + // profiles cannot contain spaces so this doesn't restrict daemon profile + // names. + if parts := strings.SplitN(daemonProfile, " ", 2); len(parts) >= 1 { + daemonProfile = parts[0] + } + if daemonProfile == "" { + daemonProfile = "unconfined" + } + p.DaemonProfile = daemonProfile + + // Install to a temporary directory. + f, err := os.CreateTemp("", name) + if err != nil { + return err + } + profilePath := f.Name() + + defer f.Close() + defer os.Remove(profilePath) + + if err := p.generateDefault(f); err != nil { + return err + } + + return aaparser.LoadProfile(profilePath) +} + +// IsLoaded checks if a profile with the given name has been loaded into the +// kernel. +func IsLoaded(name string) (bool, error) { + file, err := os.Open("/sys/kernel/security/apparmor/profiles") + if err != nil { + return false, err + } + defer file.Close() + + r := bufio.NewReader(file) + for { + p, err := r.ReadString('\n') + if err == io.EOF { + break + } + if err != nil { + return false, err + } + if strings.HasPrefix(p, name+" ") { + return true, nil + } + } + + return false, nil +} diff --git a/ecs-init/vendor/github.com/docker/docker/profiles/apparmor/template.go b/ecs-init/vendor/github.com/docker/docker/profiles/apparmor/template.go new file mode 100644 index 00000000000..ed5892a7f6b --- /dev/null +++ b/ecs-init/vendor/github.com/docker/docker/profiles/apparmor/template.go @@ -0,0 +1,59 @@ +//go:build linux +// +build linux + +package apparmor // import "github.com/docker/docker/profiles/apparmor" + +// NOTE: This profile is replicated in containerd and libpod. If you make a +// change to this profile, please make follow-up PRs to those projects so +// that these rules can be synchronised (because any issue with this +// profile will likely affect libpod and containerd). +// TODO: Move this to a common project so we can maintain it in one spot. + +// baseTemplate defines the default apparmor profile for containers. +const baseTemplate = ` +{{range $value := .Imports}} +{{$value}} +{{end}} + +profile {{.Name}} flags=(attach_disconnected,mediate_deleted) { +{{range $value := .InnerImports}} + {{$value}} +{{end}} + + network, + capability, + file, + umount, +{{if ge .Version 208096}} + # Host (privileged) processes may send signals to container processes. + signal (receive) peer=unconfined, + # dockerd may send signals to container processes (for "docker kill"). + signal (receive) peer={{.DaemonProfile}}, + # Container processes may send signals amongst themselves. + signal (send,receive) peer={{.Name}}, +{{end}} + + deny @{PROC}/* w, # deny write for all files directly in /proc (not in a subdir) + # deny write to files not in /proc//** or /proc/sys/** + deny @{PROC}/{[^1-9],[^1-9][^0-9],[^1-9s][^0-9y][^0-9s],[^1-9][^0-9][^0-9][^0-9/]*}/** w, + deny @{PROC}/sys/[^k]** w, # deny /proc/sys except /proc/sys/k* (effectively /proc/sys/kernel) + deny @{PROC}/sys/kernel/{?,??,[^s][^h][^m]**} w, # deny everything except shm* in /proc/sys/kernel/ + deny @{PROC}/sysrq-trigger rwklx, + deny @{PROC}/kcore rwklx, + + deny mount, + + deny /sys/[^f]*/** wklx, + deny /sys/f[^s]*/** wklx, + deny /sys/fs/[^c]*/** wklx, + deny /sys/fs/c[^g]*/** wklx, + deny /sys/fs/cg[^r]*/** wklx, + deny /sys/firmware/** rwklx, + deny /sys/kernel/security/** rwklx, + +{{if ge .Version 208095}} + # suppress ptrace denials when using 'docker ps' or using 'ps' inside a container + ptrace (trace,read,tracedby,readby) peer={{.Name}}, +{{end}} +} +` diff --git a/ecs-init/vendor/modules.txt b/ecs-init/vendor/modules.txt index a173ca331f1..d4f2958f2d3 100644 --- a/ecs-init/vendor/modules.txt +++ b/ecs-init/vendor/modules.txt @@ -90,6 +90,7 @@ github.com/docker/docker/api/types/versions github.com/docker/docker/api/types/volume github.com/docker/docker/libnetwork/ipamutils github.com/docker/docker/opts +github.com/docker/docker/pkg/aaparser github.com/docker/docker/pkg/archive github.com/docker/docker/pkg/fileutils github.com/docker/docker/pkg/homedir @@ -100,6 +101,7 @@ github.com/docker/docker/pkg/longpath github.com/docker/docker/pkg/pools github.com/docker/docker/pkg/stdcopy github.com/docker/docker/pkg/system +github.com/docker/docker/profiles/apparmor # github.com/docker/go-connections v0.4.0 ## explicit github.com/docker/go-connections/nat