From 71b80591e33b4debdb1ed35e198f66e18c10ee5f Mon Sep 17 00:00:00 2001 From: Xianglin Gao Date: Thu, 24 Nov 2016 21:27:56 +0800 Subject: [PATCH 1/6] support apparmor Signed-off-by: Xianglin Gao --- server/container_create.go | 7 +++++++ server/utils.go | 26 ++++++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/server/container_create.go b/server/container_create.go index 9e8e1624..53b1562e 100644 --- a/server/container_create.go +++ b/server/container_create.go @@ -182,6 +182,13 @@ func (s *Server) createSandboxContainer(containerID string, containerName string specgen.AddAnnotation(k, v) } } + + // set this container's apparmor profile if it is set by sandbox + appArmorProfileName := GetAppArmorProfileName(sb.annotations, metadata.GetName()) + if appArmorProfileName != "" { + specgen.SetProcessApparmorProfile(appArmorProfileName) + } + if containerConfig.GetLinux().GetSecurityContext().GetPrivileged() { specgen.SetupPrivileged(true) } diff --git a/server/utils.go b/server/utils.go index 6b5c8e15..1d841401 100644 --- a/server/utils.go +++ b/server/utils.go @@ -11,6 +11,14 @@ const ( // According to http://man7.org/linux/man-pages/man5/resolv.conf.5.html: // "The search list is currently limited to six domains with a total of 256 characters." maxDNSSearches = 6 + + // ContainerAnnotationKeyPrefix is the prefix to an annotation key specifying a container profile. + ContainerAnnotationKeyPrefix = "container.apparmor.security.beta.kubernetes.io/" + + // ProfileRuntimeDefault is he profile specifying the runtime default. + ProfileRuntimeDefault = "runtime/default" + // ProfileNamePrefix is the prefix for specifying profiles loaded on the node. + ProfileNamePrefix = "localhost/" ) func int64Ptr(i int64) *int64 { @@ -156,3 +164,21 @@ func SysctlsFromPodAnnotation(annotation string) ([]Sysctl, error) { } return sysctls, nil } + +// GetAppArmorProfileName gets the profile name for the given container. +func GetAppArmorProfileName(annotations map[string]string, ctrName string) string { + profile := GetProfileNameFromPodAnnotations(annotations, ctrName) + if profile == "" || profile == ProfileRuntimeDefault { + // If the value is runtime/default, then it is equivalent to not specifying a profile. + return "" + } + + profileName := strings.TrimPrefix(profile, ProfileNamePrefix) + return profileName +} + +// GetProfileNameFromPodAnnotations gets the name of the profile to use with container from +// pod annotations +func GetProfileNameFromPodAnnotations(annotations map[string]string, containerName string) string { + return annotations[ContainerAnnotationKeyPrefix+containerName] +} From 1f863846f58905d169d663e76a34697c801d0b4f Mon Sep 17 00:00:00 2001 From: Xianglin Gao Date: Tue, 29 Nov 2016 20:34:15 +0800 Subject: [PATCH 2/6] add default apparmor profile Signed-off-by: Xianglin Gao --- server/apparmor/aaparser.go | 87 ++++++++++ server/apparmor/apparmor.go | 164 ++++++++++++++++++ server/apparmor/template.go | 43 +++++ server/container_create.go | 9 +- server/server.go | 10 +- server/utils.go | 26 --- .../docker/utils/templates/templates.go | 42 +++++ .../runc/libcontainer/apparmor/apparmor.go | 39 +++++ .../apparmor/apparmor_disabled.go | 20 +++ 9 files changed, 410 insertions(+), 30 deletions(-) create mode 100644 server/apparmor/aaparser.go create mode 100644 server/apparmor/apparmor.go create mode 100644 server/apparmor/template.go create mode 100644 vendor/src/github.com/docker/docker/utils/templates/templates.go create mode 100644 vendor/src/github.com/opencontainers/runc/libcontainer/apparmor/apparmor.go create mode 100644 vendor/src/github.com/opencontainers/runc/libcontainer/apparmor/apparmor_disabled.go diff --git a/server/apparmor/aaparser.go b/server/apparmor/aaparser.go new file mode 100644 index 00000000..2c068697 --- /dev/null +++ b/server/apparmor/aaparser.go @@ -0,0 +1,87 @@ +package apparmor + +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 -r` on a specified apparmor profile to +// replace the profile. +func LoadProfile(profilePath string) error { + _, err := cmd("", "-r", 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, " "), string(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] + + // 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/server/apparmor/apparmor.go b/server/apparmor/apparmor.go new file mode 100644 index 00000000..46821c30 --- /dev/null +++ b/server/apparmor/apparmor.go @@ -0,0 +1,164 @@ +package apparmor + +import ( + "bufio" + "io" + "io/ioutil" + "os" + "path" + "strings" + + "github.com/Sirupsen/logrus" + "github.com/docker/docker/utils/templates" + "github.com/opencontainers/runc/libcontainer/apparmor" +) + +const ( + // defaultApparmorProfile is the name of default apparmor profile name. + defaultApparmorProfile = "crio-default" + + // profileDirectory is the file store for apparmor profiles and macros. + profileDirectory = "/etc/apparmor.d" + + // ContainerAnnotationKeyPrefix is the prefix to an annotation key specifying a container profile. + ContainerAnnotationKeyPrefix = "container.apparmor.security.beta.kubernetes.io/" + + // ProfileRuntimeDefault is he profile specifying the runtime default. + ProfileRuntimeDefault = "runtime/default" + // ProfileNamePrefix is the prefix for specifying profiles loaded on the node. + ProfileNamePrefix = "localhost/" +) + +// profileData holds information about the given profile for generation. +type profileData struct { + // Name is profile name. + Name 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 +} + +// InstallDefaultAppArmorProfile installs default apparmor profile. +func InstallDefaultAppArmorProfile() { + if err := InstallDefault(defaultApparmorProfile); err != nil { + // Allow daemon to run if loading failed, but are active + // (possibly through another run, manually, or via system startup) + if err := IsLoaded(defaultApparmorProfile); err != nil { + logrus.Errorf("AppArmor enabled on system but the %s profile could not be loaded.", defaultApparmorProfile) + } + } +} + +// IsEnabled returns true if apparmor is enabled for the host. +func IsEnabled() bool { + return apparmor.IsEnabled() +} + +// GetAppArmorProfileName gets the profile name for the given container. +func GetAppArmorProfileName(annotations map[string]string, ctrName string) string { + profile := GetProfileNameFromPodAnnotations(annotations, ctrName) + + if profile == "" { + return "" + } + + if profile == ProfileRuntimeDefault { + // If the value is runtime/default, then return default profile. + logrus.Infof("get default profile name") + return defaultApparmorProfile + } + + profileName := strings.TrimPrefix(profile, ProfileNamePrefix) + return profileName +} + +// GetProfileNameFromPodAnnotations gets the name of the profile to use with container from +// pod annotations +func GetProfileNameFromPodAnnotations(annotations map[string]string, containerName string) string { + return annotations[ContainerAnnotationKeyPrefix+containerName] +} + +// 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, + } + + // Install to a temporary directory. + f, err := ioutil.TempFile("", name) + if err != nil { + return err + } + profilePath := f.Name() + + defer f.Close() + + if err := p.generateDefault(f); err != nil { + return err + } + + if err := LoadProfile(profilePath); err != nil { + return err + } + + return nil +} + +// IsLoaded checks if a passed profile has been loaded into the kernel. +func IsLoaded(name string) error { + file, err := os.Open("/sys/kernel/security/apparmor/profiles") + if err != nil { + return err + } + defer file.Close() + + r := bufio.NewReader(file) + for { + p, err := r.ReadString('\n') + if err != nil { + return err + } + if strings.HasPrefix(p, name+" ") { + return nil + } + } +} + +// generateDefault creates an apparmor profile from ProfileData. +func (p *profileData) generateDefault(out io.Writer) error { + compiled, err := templates.NewParse("apparmor_profile", 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 := GetVersion() + if err != nil { + return err + } + p.Version = ver + + if err := compiled.Execute(out, p); err != nil { + return err + } + return nil +} + +// macrosExists checks if the passed macro exists. +func macroExists(m string) bool { + _, err := os.Stat(path.Join(profileDirectory, m)) + return err == nil +} diff --git a/server/apparmor/template.go b/server/apparmor/template.go new file mode 100644 index 00000000..e611b492 --- /dev/null +++ b/server/apparmor/template.go @@ -0,0 +1,43 @@ +package apparmor + +// 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, + + 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}/mem rwklx, + deny @{PROC}/kmem 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}} + ptrace (trace,read) peer={{.Name}}, +{{end}} +} +` diff --git a/server/container_create.go b/server/container_create.go index 53b1562e..dbd17644 100644 --- a/server/container_create.go +++ b/server/container_create.go @@ -12,6 +12,7 @@ import ( "github.com/Sirupsen/logrus" "github.com/docker/docker/pkg/stringid" "github.com/kubernetes-incubator/cri-o/oci" + "github.com/kubernetes-incubator/cri-o/server/apparmor" "github.com/kubernetes-incubator/cri-o/server/seccomp" "github.com/kubernetes-incubator/cri-o/utils" "github.com/opencontainers/runc/libcontainer/label" @@ -184,9 +185,11 @@ func (s *Server) createSandboxContainer(containerID string, containerName string } // set this container's apparmor profile if it is set by sandbox - appArmorProfileName := GetAppArmorProfileName(sb.annotations, metadata.GetName()) - if appArmorProfileName != "" { - specgen.SetProcessApparmorProfile(appArmorProfileName) + if s.appArmorEnabled { + appArmorProfileName := apparmor.GetAppArmorProfileName(sb.annotations, metadata.GetName()) + if appArmorProfileName != "" { + specgen.SetProcessApparmorProfile(appArmorProfileName) + } } if containerConfig.GetLinux().GetSecurityContext().GetPrivileged() { diff --git a/server/server.go b/server/server.go index 1d842d83..ad47b416 100644 --- a/server/server.go +++ b/server/server.go @@ -13,6 +13,7 @@ import ( "github.com/docker/docker/pkg/registrar" "github.com/docker/docker/pkg/truncindex" "github.com/kubernetes-incubator/cri-o/oci" + "github.com/kubernetes-incubator/cri-o/server/apparmor" "github.com/kubernetes-incubator/cri-o/server/seccomp" "github.com/kubernetes-incubator/cri-o/utils" "github.com/opencontainers/runc/libcontainer/label" @@ -39,6 +40,8 @@ type Server struct { seccompEnabled bool seccompProfile seccomp.Seccomp + + appArmorEnabled bool } func (s *Server) loadContainer(id string) error { @@ -281,7 +284,8 @@ func New(config *Config) (*Server, error) { sandboxes: sandboxes, containers: containers, }, - seccompEnabled: seccompEnabled(), + seccompEnabled: seccompEnabled(), + appArmorEnabled: apparmor.IsEnabled(), } seccompProfile, err := ioutil.ReadFile(config.SeccompProfile) if err != nil { @@ -293,6 +297,10 @@ func New(config *Config) (*Server, error) { } s.seccompProfile = seccompConfig + if s.appArmorEnabled { + apparmor.InstallDefaultAppArmorProfile() + } + s.podIDIndex = truncindex.NewTruncIndex([]string{}) s.podNameIndex = registrar.NewRegistrar() s.ctrIDIndex = truncindex.NewTruncIndex([]string{}) diff --git a/server/utils.go b/server/utils.go index 1d841401..6b5c8e15 100644 --- a/server/utils.go +++ b/server/utils.go @@ -11,14 +11,6 @@ const ( // According to http://man7.org/linux/man-pages/man5/resolv.conf.5.html: // "The search list is currently limited to six domains with a total of 256 characters." maxDNSSearches = 6 - - // ContainerAnnotationKeyPrefix is the prefix to an annotation key specifying a container profile. - ContainerAnnotationKeyPrefix = "container.apparmor.security.beta.kubernetes.io/" - - // ProfileRuntimeDefault is he profile specifying the runtime default. - ProfileRuntimeDefault = "runtime/default" - // ProfileNamePrefix is the prefix for specifying profiles loaded on the node. - ProfileNamePrefix = "localhost/" ) func int64Ptr(i int64) *int64 { @@ -164,21 +156,3 @@ func SysctlsFromPodAnnotation(annotation string) ([]Sysctl, error) { } return sysctls, nil } - -// GetAppArmorProfileName gets the profile name for the given container. -func GetAppArmorProfileName(annotations map[string]string, ctrName string) string { - profile := GetProfileNameFromPodAnnotations(annotations, ctrName) - if profile == "" || profile == ProfileRuntimeDefault { - // If the value is runtime/default, then it is equivalent to not specifying a profile. - return "" - } - - profileName := strings.TrimPrefix(profile, ProfileNamePrefix) - return profileName -} - -// GetProfileNameFromPodAnnotations gets the name of the profile to use with container from -// pod annotations -func GetProfileNameFromPodAnnotations(annotations map[string]string, containerName string) string { - return annotations[ContainerAnnotationKeyPrefix+containerName] -} diff --git a/vendor/src/github.com/docker/docker/utils/templates/templates.go b/vendor/src/github.com/docker/docker/utils/templates/templates.go new file mode 100644 index 00000000..91c376f3 --- /dev/null +++ b/vendor/src/github.com/docker/docker/utils/templates/templates.go @@ -0,0 +1,42 @@ +package templates + +import ( + "encoding/json" + "strings" + "text/template" +) + +// basicFunctions are the set of initial +// functions provided to every template. +var basicFunctions = template.FuncMap{ + "json": func(v interface{}) string { + a, _ := json.Marshal(v) + return string(a) + }, + "split": strings.Split, + "join": strings.Join, + "title": strings.Title, + "lower": strings.ToLower, + "upper": strings.ToUpper, + "pad": padWithSpace, +} + +// Parse creates a new annonymous template with the basic functions +// and parses the given format. +func Parse(format string) (*template.Template, error) { + return NewParse("", format) +} + +// NewParse creates a new tagged template with the basic functions +// and parses the given format. +func NewParse(tag, format string) (*template.Template, error) { + return template.New(tag).Funcs(basicFunctions).Parse(format) +} + +// padWithSpace adds whitespace to the input if the input is non-empty +func padWithSpace(source string, prefix, suffix int) string { + if source == "" { + return source + } + return strings.Repeat(" ", prefix) + source + strings.Repeat(" ", suffix) +} diff --git a/vendor/src/github.com/opencontainers/runc/libcontainer/apparmor/apparmor.go b/vendor/src/github.com/opencontainers/runc/libcontainer/apparmor/apparmor.go new file mode 100644 index 00000000..82ed1a68 --- /dev/null +++ b/vendor/src/github.com/opencontainers/runc/libcontainer/apparmor/apparmor.go @@ -0,0 +1,39 @@ +// +build apparmor,linux + +package apparmor + +// #cgo LDFLAGS: -lapparmor +// #include +// #include +import "C" +import ( + "fmt" + "io/ioutil" + "os" + "unsafe" +) + +// IsEnabled returns true if apparmor is enabled for the host. +func IsEnabled() bool { + if _, err := os.Stat("/sys/kernel/security/apparmor"); err == nil && os.Getenv("container") == "" { + if _, err = os.Stat("/sbin/apparmor_parser"); err == nil { + buf, err := ioutil.ReadFile("/sys/module/apparmor/parameters/enabled") + return err == nil && len(buf) > 1 && buf[0] == 'Y' + } + } + return false +} + +// ApplyProfile will apply the profile with the specified name to the process after +// the next exec. +func ApplyProfile(name string) error { + if name == "" { + return nil + } + cName := C.CString(name) + defer C.free(unsafe.Pointer(cName)) + if _, err := C.aa_change_onexec(cName); err != nil { + return fmt.Errorf("apparmor failed to apply profile: %s", err) + } + return nil +} diff --git a/vendor/src/github.com/opencontainers/runc/libcontainer/apparmor/apparmor_disabled.go b/vendor/src/github.com/opencontainers/runc/libcontainer/apparmor/apparmor_disabled.go new file mode 100644 index 00000000..d4110cf0 --- /dev/null +++ b/vendor/src/github.com/opencontainers/runc/libcontainer/apparmor/apparmor_disabled.go @@ -0,0 +1,20 @@ +// +build !apparmor !linux + +package apparmor + +import ( + "errors" +) + +var ErrApparmorNotEnabled = errors.New("apparmor: config provided but apparmor not supported") + +func IsEnabled() bool { + return false +} + +func ApplyProfile(name string) error { + if name != "" { + return ErrApparmorNotEnabled + } + return nil +} From 26645c90ac0392d1d0ed427414a079e29219f21c Mon Sep 17 00:00:00 2001 From: Xianglin Gao Date: Wed, 30 Nov 2016 16:19:36 +0800 Subject: [PATCH 3/6] Make the profile configurable Signed-off-by: Xianglin Gao --- cmd/server/config.go | 18 ++++++++++++------ cmd/server/main.go | 7 +++++++ server/apparmor/apparmor.go | 18 ------------------ server/config.go | 4 ++++ server/container_create.go | 19 ++++++++++++++++++- server/server.go | 2 ++ 6 files changed, 43 insertions(+), 25 deletions(-) diff --git a/cmd/server/config.go b/cmd/server/config.go index 83ce9fa6..c4c3fe5b 100644 --- a/cmd/server/config.go +++ b/cmd/server/config.go @@ -11,10 +11,11 @@ import ( ) const ( - ocidRoot = "/var/lib/ocid" - conmonPath = "/usr/libexec/ocid/conmon" - pausePath = "/usr/libexec/ocid/pause" - seccompProfilePath = "/etc/ocid/seccomp.json" + ocidRoot = "/var/lib/ocid" + conmonPath = "/usr/libexec/ocid/conmon" + pausePath = "/usr/libexec/ocid/pause" + seccompProfilePath = "/etc/ocid/seccomp.json" + apparmorProfileName = "crio-default" ) var commentedConfigTemplate = template.Must(template.New("config").Parse(` @@ -64,6 +65,10 @@ selinux = {{ .SELinux }} # default for the runtime. seccomp_profile = "{{ .SeccompProfile }}" +# apparmor_profile is the apparmor profile name which is used as the +# default for the runtime. +apparmor_profile = "{{ .ApparmorProfile }}" + # The "ocid.image" table contains settings pertaining to the # management of OCI images. [ocid.image] @@ -94,8 +99,9 @@ func DefaultConfig() *server.Config { ConmonEnv: []string{ "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", }, - SELinux: selinux.SelinuxEnabled(), - SeccompProfile: seccompProfilePath, + SELinux: selinux.SelinuxEnabled(), + SeccompProfile: seccompProfilePath, + ApparmorProfile: apparmorProfileName, }, ImageConfig: server.ImageConfig{ Pause: pausePath, diff --git a/cmd/server/main.go b/cmd/server/main.go index 518eb997..6774feaf 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -59,6 +59,9 @@ func mergeConfig(config *server.Config, ctx *cli.Context) error { if ctx.GlobalIsSet("seccomp-profile") { config.SeccompProfile = ctx.GlobalString("seccomp-profile") } + if ctx.GlobalIsSet("apparmor-profile") { + config.ApparmorProfile = ctx.GlobalString("apparmor-profile") + } return nil } @@ -135,6 +138,10 @@ func main() { Name: "seccomp-profile", Usage: "default seccomp profile path", }, + cli.StringFlag{ + Name: "apparmor-profile", + Usage: "default apparmor profile name (default: \"crio-default\")", + }, cli.BoolFlag{ Name: "selinux", Usage: "enable selinux support", diff --git a/server/apparmor/apparmor.go b/server/apparmor/apparmor.go index 46821c30..f38c1bb3 100644 --- a/server/apparmor/apparmor.go +++ b/server/apparmor/apparmor.go @@ -57,24 +57,6 @@ func IsEnabled() bool { return apparmor.IsEnabled() } -// GetAppArmorProfileName gets the profile name for the given container. -func GetAppArmorProfileName(annotations map[string]string, ctrName string) string { - profile := GetProfileNameFromPodAnnotations(annotations, ctrName) - - if profile == "" { - return "" - } - - if profile == ProfileRuntimeDefault { - // If the value is runtime/default, then return default profile. - logrus.Infof("get default profile name") - return defaultApparmorProfile - } - - profileName := strings.TrimPrefix(profile, ProfileNamePrefix) - return profileName -} - // GetProfileNameFromPodAnnotations gets the name of the profile to use with container from // pod annotations func GetProfileNameFromPodAnnotations(annotations map[string]string, containerName string) string { diff --git a/server/config.go b/server/config.go index 12e143c7..75e93aa3 100644 --- a/server/config.go +++ b/server/config.go @@ -68,6 +68,10 @@ type RuntimeConfig struct { // SeccompProfile is the seccomp json profile path which is used as the // default for the runtime. SeccompProfile string `toml:"seccomp_profile"` + + // ApparmorProfile is the apparmor profile name which is used as the + // default for the runtime. + ApparmorProfile string `toml:"apparmor_profile"` } // ImageConfig represents the "ocid.image" TOML config table. diff --git a/server/container_create.go b/server/container_create.go index dbd17644..9c0bd07c 100644 --- a/server/container_create.go +++ b/server/container_create.go @@ -186,7 +186,7 @@ func (s *Server) createSandboxContainer(containerID string, containerName string // set this container's apparmor profile if it is set by sandbox if s.appArmorEnabled { - appArmorProfileName := apparmor.GetAppArmorProfileName(sb.annotations, metadata.GetName()) + appArmorProfileName := s.getAppArmorProfileName(sb.annotations, metadata.GetName()) if appArmorProfileName != "" { specgen.SetProcessApparmorProfile(appArmorProfileName) } @@ -383,3 +383,20 @@ func (s *Server) generateContainerIDandName(podName string, name string, attempt } return id, name, err } + +// getAppArmorProfileName gets the profile name for the given container. +func (s *Server) getAppArmorProfileName(annotations map[string]string, ctrName string) string { + profile := apparmor.GetProfileNameFromPodAnnotations(annotations, ctrName) + + if profile == "" { + return "" + } + + if profile == apparmor.ProfileRuntimeDefault { + // If the value is runtime/default, then return default profile. + return s.appArmorProfile + } + + profileName := strings.TrimPrefix(profile, apparmor.ProfileNamePrefix) + return profileName +} diff --git a/server/server.go b/server/server.go index ad47b416..317b9499 100644 --- a/server/server.go +++ b/server/server.go @@ -42,6 +42,7 @@ type Server struct { seccompProfile seccomp.Seccomp appArmorEnabled bool + appArmorProfile string } func (s *Server) loadContainer(id string) error { @@ -300,6 +301,7 @@ func New(config *Config) (*Server, error) { if s.appArmorEnabled { apparmor.InstallDefaultAppArmorProfile() } + s.appArmorProfile = config.ApparmorProfile s.podIDIndex = truncindex.NewTruncIndex([]string{}) s.podNameIndex = registrar.NewRegistrar() From 06cc0ba6ba765e3ca6d304a8c3df0bb463fb462d Mon Sep 17 00:00:00 2001 From: Xianglin Gao Date: Wed, 30 Nov 2016 16:36:07 +0800 Subject: [PATCH 4/6] Add docs about apparmor profile setting Signed-off-by: Xianglin Gao --- cmd/server/config.go | 2 +- cmd/server/main.go | 2 +- docs/ocid.8.md | 8 ++++++-- docs/ocid.conf.5.md | 7 +++++-- server/apparmor/apparmor.go | 2 +- 5 files changed, 14 insertions(+), 7 deletions(-) diff --git a/cmd/server/config.go b/cmd/server/config.go index c4c3fe5b..0988976e 100644 --- a/cmd/server/config.go +++ b/cmd/server/config.go @@ -15,7 +15,7 @@ const ( conmonPath = "/usr/libexec/ocid/conmon" pausePath = "/usr/libexec/ocid/pause" seccompProfilePath = "/etc/ocid/seccomp.json" - apparmorProfileName = "crio-default" + apparmorProfileName = "ocid-default" ) var commentedConfigTemplate = template.Must(template.New("config").Parse(` diff --git a/cmd/server/main.go b/cmd/server/main.go index 6774feaf..e9dcf7be 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -140,7 +140,7 @@ func main() { }, cli.StringFlag{ Name: "apparmor-profile", - Usage: "default apparmor profile name (default: \"crio-default\")", + Usage: "default apparmor profile name (default: \"ocid-default\")", }, cli.BoolFlag{ Name: "selinux", diff --git a/docs/ocid.8.md b/docs/ocid.8.md index 81f40008..aad95477 100644 --- a/docs/ocid.8.md +++ b/docs/ocid.8.md @@ -20,6 +20,7 @@ ocid - Enable OCI Kubernetes Container Runtime daemon [**--sandboxdir**=[*value*]] [**--selinux**] [**--seccomp-profile**=[*value*]] +[**--apparmor-profile**=[*value*]] [**--version**|**-v**] # DESCRIPTION @@ -76,12 +77,15 @@ ocid is meant to provide an integration path between OCI conformant runtimes and **--sandboxdir**="" OCID pod sandbox dir (default: "/var/lib/ocid/sandboxes") -**--selinux** +**--selinux**=*true*|*false* Enable selinux support (default: false) -**seccomp_profile** +**--seccomp_profile**="" Path to the seccomp json profile to be used as the runtime's default (default: "/etc/ocid/seccomp.json") +**--apparmor_profile**="" + Name of the apparmor profile to be used as the runtime's default (default: "ocid-default") + **--version, -v** Print the version diff --git a/docs/ocid.conf.5.md b/docs/ocid.conf.5.md index 2d462728..20a95dd8 100644 --- a/docs/ocid.conf.5.md +++ b/docs/ocid.conf.5.md @@ -55,12 +55,15 @@ The `ocid` table supports the following options: **runtime**="" OCI runtime path (default: "/usr/bin/runc") -**selinux** +**selinux**=*true*|*false* Enable selinux support (default: false) -**seccomp_profile** +**seccomp_profile**="" Path to the seccomp json profile to be used as the runtime's default (default: "/etc/ocid/seccomp.json") +**apparmor_profile**="" + Name of the apparmor profile to be used as the runtime's default (default: "ocid-default") + ## OCID.IMAGE TABLE **pause**="" diff --git a/server/apparmor/apparmor.go b/server/apparmor/apparmor.go index f38c1bb3..1f1b66fe 100644 --- a/server/apparmor/apparmor.go +++ b/server/apparmor/apparmor.go @@ -15,7 +15,7 @@ import ( const ( // defaultApparmorProfile is the name of default apparmor profile name. - defaultApparmorProfile = "crio-default" + defaultApparmorProfile = "ocid-default" // profileDirectory is the file store for apparmor profiles and macros. profileDirectory = "/etc/apparmor.d" From bec3c3e2aa5d155d4d46601aec86cf843dca4f86 Mon Sep 17 00:00:00 2001 From: Xianglin Gao Date: Thu, 1 Dec 2016 21:15:47 +0800 Subject: [PATCH 5/6] add test cases Signed-off-by: Xianglin Gao --- Dockerfile | 1 + Makefile | 2 +- test/apparmor.bats | 137 +++++++++++++++++++++++++ test/helpers.bash | 29 +++++- test/testdata/apparmor_test_deny_write | 10 ++ 5 files changed, 175 insertions(+), 4 deletions(-) create mode 100644 test/apparmor.bats create mode 100644 test/testdata/apparmor_test_deny_write diff --git a/Dockerfile b/Dockerfile index 6ceb28f7..7d99e145 100644 --- a/Dockerfile +++ b/Dockerfile @@ -19,6 +19,7 @@ RUN apt-get update && apt-get install -y \ protobuf-compiler \ python-minimal \ libglib2.0-dev \ + libapparmor-dev \ --no-install-recommends # install bats diff --git a/Makefile b/Makefile index 2d719955..4df37f71 100644 --- a/Makefile +++ b/Makefile @@ -16,7 +16,7 @@ ETCDIR ?= ${DESTDIR}/etc ETCDIR_OCID ?= ${ETCDIR}/ocid GO_MD2MAN ?= $(shell which go-md2man) export GOPATH := ${CURDIR}/vendor -BUILDTAGS := selinux seccomp +BUILDTAGS := selinux seccomp apparmor all: binaries ocid.conf docs diff --git a/test/apparmor.bats b/test/apparmor.bats new file mode 100644 index 00000000..4b656133 --- /dev/null +++ b/test/apparmor.bats @@ -0,0 +1,137 @@ +#!/usr/bin/env bats + +load helpers + +function teardown() { + cleanup_test +} + +# 1. test running with loading the default apparmor profile. +# test that we can run with the default apparomr profile which will not block touching a file in `.` +@test "load default apparomr profile and run a container with it" { + # this test requires docker, thus it can't yet be run in a container + if [ "$TRAVIS" = "true" ]; then # instead of $TRAVIS, add a function is_containerized to skip here + skip "cannot yet run this test in a container, use sudo make localintegration" + fi + + start_ocid + + sed -e 's/%VALUE%/,"container\.apparmor\.security\.beta\.kubernetes\.io\/testname1": "runtime\/default"/g' "$TESTDATA"/sandbox_config_seccomp.json > "$TESTDIR"/apparmor1.json + + run ocic pod create --name apparmor1 --config "$TESTDIR"/apparmor1.json + echo "$output" + [ "$status" -eq 0 ] + pod_id="$output" + run ocic ctr create --name testname1 --config "$TESTDATA"/container_redis.json --pod "$pod_id" + echo "$output" + [ "$status" -eq 0 ] + ctr_id="$output" + run ocic ctr start --id "$ctr_id" + echo "$output" + [ "$status" -eq 0 ] + run ocic ctr execsync --id "$ctr_id" touch test.txt + echo "$output" + [ "$status" -eq 0 ] + + + cleanup_ctrs + cleanup_pods + stop_ocid +} + +# 2. test running with loading a specific apparmor profile as ocid default apparmor profile. +# test that we can run with a specific apparmor profile which will block touching a file in `.` as ocid default apparmor profile. +@test "load a specific apparomr profile as default apparmor and run a container with it" { + # this test requires docker, thus it can't yet be run in a container + if [ "$TRAVIS" = "true" ]; then # instead of $TRAVIS, add a function is_containerized to skip here + skip "cannot yet run this test in a container, use sudo make localintegration" + fi + + load_apparmor_test_profile + start_ocid_with_apparmor_profile_name "$APPARMOR_TEST_PROFILE_NAME" + + sed -e 's/%VALUE%/,"container\.apparmor\.security\.beta\.kubernetes\.io\/testname2": "apparmor_test_deny_write"/g' "$TESTDATA"/sandbox_config_seccomp.json > "$TESTDIR"/apparmor2.json + + run ocic pod create --name apparmor2 --config "$TESTDIR"/apparmor2.json + echo "$output" + [ "$status" -eq 0 ] + pod_id="$output" + run ocic ctr create --name testname2 --config "$TESTDATA"/container_redis.json --pod "$pod_id" + echo "$output" + [ "$status" -eq 0 ] + ctr_id="$output" + run ocic ctr start --id "$ctr_id" + echo "$output" + [ "$status" -eq 0 ] + run ocic ctr execsync --id "$ctr_id" touch test.txt + echo "$output" + [ "$status" -ne 0 ] + [[ "$output" =~ "Permission denied" ]] + + cleanup_ctrs + cleanup_pods + stop_ocid + remove_apparmor_test_profile +} + +# 3. test running with loading a specific apparmor profile but not as ocid default apparmor profile. +# test that we can run with a specific apparmor profile which will block touching a file in `.` +@test "load default apparomr profile and run a container with another apparmor profile" { + # this test requires docker, thus it can't yet be run in a container + if [ "$TRAVIS" = "true" ]; then # instead of $TRAVIS, add a function is_containerized to skip here + skip "cannot yet run this test in a container, use sudo make localintegration" + fi + + load_apparmor_test_profile + start_ocid + + sed -e 's/%VALUE%/,"container\.apparmor\.security\.beta\.kubernetes\.io\/testname3": "apparmor_test_deny_write"/g' "$TESTDATA"/sandbox_config_seccomp.json > "$TESTDIR"/apparmor3.json + + run ocic pod create --name apparmor3 --config "$TESTDIR"/apparmor3.json + echo "$output" + [ "$status" -eq 0 ] + pod_id="$output" + run ocic ctr create --name testname3 --config "$TESTDATA"/container_redis.json --pod "$pod_id" + echo "$output" + [ "$status" -eq 0 ] + ctr_id="$output" + run ocic ctr start --id "$ctr_id" + echo "$output" + [ "$status" -eq 0 ] + run ocic ctr execsync --id "$ctr_id" touch test.txt + echo "$output" + [ "$status" -ne 0 ] + [[ "$output" =~ "Permission denied" ]] + + cleanup_ctrs + cleanup_pods + stop_ocid + remove_apparmor_test_profile +} + +# 1. test running with wrong apparmor profile name. +# test that we can will fail when running a ctr with rong apparmor profile name. +@test "run a container with wrong apparmor profile name" { + # this test requires docker, thus it can't yet be run in a container + if [ "$TRAVIS" = "true" ]; then # instead of $TRAVIS, add a function is_containerized to skip here + skip "cannot yet run this test in a container, use sudo make localintegration" + fi + + start_ocid + + sed -e 's/%VALUE%/,"container\.apparmor\.security\.beta\.kubernetes\.io\/testname4": "not-exists"/g' "$TESTDATA"/sandbox_config_seccomp.json > "$TESTDIR"/apparmor4.json + + run ocic pod create --name apparmor4 --config "$TESTDIR"/apparmor4.json + echo "$output" + [ "$status" -eq 0 ] + pod_id="$output" + run ocic ctr create --name testname4 --config "$TESTDATA"/container_redis.json --pod "$pod_id" + echo "$output" + [ "$status" -ne 0 ] + [[ "$output" =~ "Creating container failed" ]] + + + cleanup_ctrs + cleanup_pods + stop_ocid +} diff --git a/test/helpers.bash b/test/helpers.bash index d02f5988..54895fee 100644 --- a/test/helpers.bash +++ b/test/helpers.bash @@ -17,11 +17,19 @@ OCIC_BINARY=${OCIC_BINARY:-${OCID_ROOT}/cri-o/ocic} CONMON_BINARY=${CONMON_BINARY:-${OCID_ROOT}/cri-o/conmon/conmon} # Path of the pause binary. PAUSE_BINARY=${PAUSE_BINARY:-${OCID_ROOT}/cri-o/pause/pause} -# Path of the default seccomp profile +# Path of the default seccomp profile. SECCOMP_PROFILE=${SECCOMP_PROFILE:-${OCID_ROOT}/cri-o/seccomp.json} +# Name of the default apparmor profile. +APPARMOR_DEFAULT_PROFILE=${SECCOMP_PROFILE:-ocid-default} # Path of the runc binary. RUNC_PATH=$(command -v runc || true) RUNC_BINARY=${RUNC_PATH:-/usr/local/sbin/runc} +# Path of the apparmor_parser binary. +APPARMOR_PARSER_BINARY=${APPARMOR_PARSER_BINARY:-/sbin/apparmor_parser} +# Path of the apparmor profile for test. +APPARMOR_TEST_PROFILE_PATH=${APPARMOR_TEST_PROFILE_PATH:-${TESTDATA}/apparmor_test_deny_write} +# Name of the apparmor profile for test. +APPARMOR_TEST_PROFILE_NAME=${APPARMOR_TEST_PROFILE_NAME:-apparmor_test_deny_write} TESTDIR=$(mktemp -d) if [ -e /usr/sbin/selinuxenabled ] && /usr/sbin/selinuxenabled; then @@ -80,13 +88,19 @@ function wait_until_reachable() { # Start ocid. function start_ocid() { - "$OCID_BINARY" --conmon "$CONMON_BINARY" --pause "$PAUSE_BINARY" --listen "$OCID_SOCKET" --runtime "$RUNC_BINARY" --root "$TESTDIR/ocid" --sandboxdir "$TESTDIR/sandboxes" --containerdir "$TESTDIR/ocid/containers" --seccomp-profile "$SECCOMP_PROFILE" config >$OCID_CONFIG + "$OCID_BINARY" --conmon "$CONMON_BINARY" --pause "$PAUSE_BINARY" --listen "$OCID_SOCKET" --runtime "$RUNC_BINARY" --root "$TESTDIR/ocid" --sandboxdir "$TESTDIR/sandboxes" --containerdir "$TESTDIR/ocid/containers" --seccomp-profile "$SECCOMP_PROFILE" --apparmor-profile "$APPARMOR_PROFILE" config >$OCID_CONFIG "$OCID_BINARY" --debug --config "$OCID_CONFIG" & OCID_PID=$! wait_until_reachable } function start_ocid_with_seccomp_path() { - "$OCID_BINARY" --conmon "$CONMON_BINARY" --pause "$PAUSE_BINARY" --listen "$OCID_SOCKET" --runtime "$RUNC_BINARY" --root "$TESTDIR/ocid" --sandboxdir "$TESTDIR/sandboxes" --containerdir "$TESTDIR/ocid/containers" --seccomp-profile "$1" config >$OCID_CONFIG + "$OCID_BINARY" --conmon "$CONMON_BINARY" --pause "$PAUSE_BINARY" --listen "$OCID_SOCKET" --runtime "$RUNC_BINARY" --root "$TESTDIR/ocid" --sandboxdir "$TESTDIR/sandboxes" --containerdir "$TESTDIR/ocid/containers" --seccomp-profile "$1" --apparmor-profile "$APPARMOR_PROFILE" config >$OCID_CONFIG + "$OCID_BINARY" --debug --config "$OCID_CONFIG" & OCID_PID=$! + wait_until_reachable +} + +function start_ocid_with_apparmor_profile_name() { + "$OCID_BINARY" --conmon "$CONMON_BINARY" --pause "$PAUSE_BINARY" --listen "$OCID_SOCKET" --runtime "$RUNC_BINARY" --root "$TESTDIR/ocid" --sandboxdir "$TESTDIR/sandboxes" --containerdir "$TESTDIR/ocid/containers" --seccomp-profile "$SECCOMP_PROFILE" --apparmor-profile "$1" config >$OCID_CONFIG "$OCID_BINARY" --debug --config "$OCID_CONFIG" & OCID_PID=$! wait_until_reachable } @@ -128,3 +142,12 @@ function stop_ocid() { function cleanup_test() { rm -rf "$TESTDIR" } + + +function load_apparmor_test_profile() { + "$APPARMOR_PARSER_BINARY" -r "$APPARMOR_TEST_PROFILE_PATH" +} + +function remove_apparmor_test_profile() { + "$APPARMOR_PARSER_BINARY" -R "$APPARMOR_TEST_PROFILE_PATH" +} diff --git a/test/testdata/apparmor_test_deny_write b/test/testdata/apparmor_test_deny_write new file mode 100644 index 00000000..55311aaf --- /dev/null +++ b/test/testdata/apparmor_test_deny_write @@ -0,0 +1,10 @@ +#include + +profile apparmor-test-deny-write flags=(attach_disconnected) { + #include + + file, + + # Deny all file writes. + deny /** w, +} From 4f323377eee0620576a2239fb24081fe36563ee6 Mon Sep 17 00:00:00 2001 From: Xianglin Gao Date: Fri, 2 Dec 2016 15:13:41 +0800 Subject: [PATCH 6/6] add apparmor build tag and update readme Signed-off-by: Xianglin Gao --- Makefile | 2 +- README.md | 15 +++++ server/apparmor/aaparser.go | 4 +- server/apparmor/apparmor.go | 52 ++++++++------ server/apparmor/apparmor_unsupported.go | 27 ++++++++ server/apparmor/template.go | 2 + server/container_create.go | 3 +- test/apparmor.bats | 48 ++++++++----- test/helpers.bash | 56 +++++++++++---- test/seccomp.bats | 90 ++++++++++++++++++++++--- 10 files changed, 235 insertions(+), 64 deletions(-) create mode 100644 server/apparmor/apparmor_unsupported.go diff --git a/Makefile b/Makefile index 4df37f71..2d719955 100644 --- a/Makefile +++ b/Makefile @@ -16,7 +16,7 @@ ETCDIR ?= ${DESTDIR}/etc ETCDIR_OCID ?= ${ETCDIR}/ocid GO_MD2MAN ?= $(shell which go-md2man) export GOPATH := ${CURDIR}/vendor -BUILDTAGS := selinux seccomp apparmor +BUILDTAGS := selinux seccomp all: binaries ocid.conf docs diff --git a/README.md b/README.md index ad96afcb..c232356a 100644 --- a/README.md +++ b/README.md @@ -67,6 +67,21 @@ make BUILDTAGS="" sudo make install ``` +#### Build Tags + +`cri-o` supports optional build tags for compiling support of various features. +To add build tags to the make option the `BUILDTAGS` variable must be set. + +```bash +make BUILDTAGS='seccomp apparmor' +``` + +| Build Tag | Feature | Dependency | +|-----------|------------------------------------|-------------| +| seccomp | syscall filtering | libseccomp | +| selinux | selinux process and mount labeling | | +| apparmor | apparmor profile support | libapparmor | + ### Running pods and containers #### Start the server diff --git a/server/apparmor/aaparser.go b/server/apparmor/aaparser.go index 2c068697..7f0f02ac 100644 --- a/server/apparmor/aaparser.go +++ b/server/apparmor/aaparser.go @@ -1,3 +1,5 @@ +// +build apparmor + package apparmor import ( @@ -35,7 +37,7 @@ func cmd(dir string, arg ...string) (string, error) { 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, " "), string(output), err) + return "", fmt.Errorf("running `%s %s` failed with output: %s\nerror: %v", c.Path, strings.Join(c.Args, " "), output, err) } return string(output), nil diff --git a/server/apparmor/apparmor.go b/server/apparmor/apparmor.go index 1f1b66fe..32c778c1 100644 --- a/server/apparmor/apparmor.go +++ b/server/apparmor/apparmor.go @@ -1,3 +1,5 @@ +// +build apparmor + package apparmor import ( @@ -7,6 +9,7 @@ import ( "os" "path" "strings" + "time" "github.com/Sirupsen/logrus" "github.com/docker/docker/utils/templates" @@ -27,6 +30,9 @@ const ( ProfileRuntimeDefault = "runtime/default" // ProfileNamePrefix is the prefix for specifying profiles loaded on the node. ProfileNamePrefix = "localhost/" + + // readConfigTimeout is the timeout of reading apparmor profiles. + readConfigTimeout = 10 ) // profileData holds information about the given profile for generation. @@ -46,7 +52,7 @@ func InstallDefaultAppArmorProfile() { if err := InstallDefault(defaultApparmorProfile); err != nil { // Allow daemon to run if loading failed, but are active // (possibly through another run, manually, or via system startup) - if err := IsLoaded(defaultApparmorProfile); err != nil { + if !IsLoaded(defaultApparmorProfile) { logrus.Errorf("AppArmor enabled on system but the %s profile could not be loaded.", defaultApparmorProfile) } } @@ -75,38 +81,43 @@ func InstallDefault(name string) error { if err != nil { return err } - profilePath := f.Name() - defer f.Close() if err := p.generateDefault(f); err != nil { return err } - if err := LoadProfile(profilePath); err != nil { - return err - } - - return nil + return LoadProfile(f.Name()) } // IsLoaded checks if a passed profile has been loaded into the kernel. -func IsLoaded(name string) error { +func IsLoaded(name string) bool { file, err := os.Open("/sys/kernel/security/apparmor/profiles") if err != nil { - return err + return false } defer file.Close() - r := bufio.NewReader(file) - for { - p, err := r.ReadString('\n') - if err != nil { - return err - } - if strings.HasPrefix(p, name+" ") { - return nil + ch := make(chan bool, 1) + + go func() { + r := bufio.NewReader(file) + for { + p, err := r.ReadString('\n') + if err != nil { + ch <- false + } + if strings.HasPrefix(p, name+" ") { + ch <- true + } } + }() + + select { + case <-time.After(time.Duration(readConfigTimeout) * time.Second): + return false + case enabled := <-ch: + return enabled } } @@ -133,10 +144,7 @@ func (p *profileData) generateDefault(out io.Writer) error { } p.Version = ver - if err := compiled.Execute(out, p); err != nil { - return err - } - return nil + return compiled.Execute(out, p) } // macrosExists checks if the passed macro exists. diff --git a/server/apparmor/apparmor_unsupported.go b/server/apparmor/apparmor_unsupported.go new file mode 100644 index 00000000..ea9b6d08 --- /dev/null +++ b/server/apparmor/apparmor_unsupported.go @@ -0,0 +1,27 @@ +// +build !apparmor + +package apparmor + +const ( + // ContainerAnnotationKeyPrefix is the prefix to an annotation key specifying a container profile. + ContainerAnnotationKeyPrefix = "container.apparmor.security.beta.kubernetes.io/" + + // ProfileRuntimeDefault is he profile specifying the runtime default. + ProfileRuntimeDefault = "runtime/default" + // ProfileNamePrefix is the prefix for specifying profiles loaded on the node. + ProfileNamePrefix = "localhost/" +) + +// IsEnabled returns false, when build without apparmor build tag. +func IsEnabled() bool { + return false +} + +// InstallDefaultAppArmorProfile dose nothing, when build without apparmor build tag. +func InstallDefaultAppArmorProfile() { +} + +// GetProfileNameFromPodAnnotations dose nothing, when build without apparmor build tag. +func GetProfileNameFromPodAnnotations(annotations map[string]string, containerName string) string { + return "" +} diff --git a/server/apparmor/template.go b/server/apparmor/template.go index e611b492..6656ff61 100644 --- a/server/apparmor/template.go +++ b/server/apparmor/template.go @@ -1,3 +1,5 @@ +// +build apparmor + package apparmor // baseTemplate defines the default apparmor profile for containers. diff --git a/server/container_create.go b/server/container_create.go index 9c0bd07c..f0cf96d2 100644 --- a/server/container_create.go +++ b/server/container_create.go @@ -397,6 +397,5 @@ func (s *Server) getAppArmorProfileName(annotations map[string]string, ctrName s return s.appArmorProfile } - profileName := strings.TrimPrefix(profile, apparmor.ProfileNamePrefix) - return profileName + return strings.TrimPrefix(profile, apparmor.ProfileNamePrefix) } diff --git a/test/apparmor.bats b/test/apparmor.bats index 4b656133..97e9ac9d 100644 --- a/test/apparmor.bats +++ b/test/apparmor.bats @@ -7,13 +7,19 @@ function teardown() { } # 1. test running with loading the default apparmor profile. -# test that we can run with the default apparomr profile which will not block touching a file in `.` -@test "load default apparomr profile and run a container with it" { +# test that we can run with the default apparmor profile which will not block touching a file in `.` +@test "load default apparmor profile and run a container with it" { # this test requires docker, thus it can't yet be run in a container if [ "$TRAVIS" = "true" ]; then # instead of $TRAVIS, add a function is_containerized to skip here skip "cannot yet run this test in a container, use sudo make localintegration" fi + # this test requires apparmor, so skip this test if apparmor is not enabled. + enabled=is_apparmor_enabled + if [[ "$enabled" =~ "0" ]]; then + skip "skip this test since apparmor is not enabled." + fi + start_ocid sed -e 's/%VALUE%/,"container\.apparmor\.security\.beta\.kubernetes\.io\/testname1": "runtime\/default"/g' "$TESTDATA"/sandbox_config_seccomp.json > "$TESTDIR"/apparmor1.json @@ -26,8 +32,6 @@ function teardown() { echo "$output" [ "$status" -eq 0 ] ctr_id="$output" - run ocic ctr start --id "$ctr_id" - echo "$output" [ "$status" -eq 0 ] run ocic ctr execsync --id "$ctr_id" touch test.txt echo "$output" @@ -41,16 +45,22 @@ function teardown() { # 2. test running with loading a specific apparmor profile as ocid default apparmor profile. # test that we can run with a specific apparmor profile which will block touching a file in `.` as ocid default apparmor profile. -@test "load a specific apparomr profile as default apparmor and run a container with it" { +@test "load a specific apparmor profile as default apparmor and run a container with it" { # this test requires docker, thus it can't yet be run in a container if [ "$TRAVIS" = "true" ]; then # instead of $TRAVIS, add a function is_containerized to skip here skip "cannot yet run this test in a container, use sudo make localintegration" fi - load_apparmor_test_profile - start_ocid_with_apparmor_profile_name "$APPARMOR_TEST_PROFILE_NAME" + # this test requires apparmor, so skip this test if apparmor is not enabled. + enabled=is_apparmor_enabled + if [[ "$enabled" =~ "0" ]]; then + skip "skip this test since apparmor is not enabled." + fi - sed -e 's/%VALUE%/,"container\.apparmor\.security\.beta\.kubernetes\.io\/testname2": "apparmor_test_deny_write"/g' "$TESTDATA"/sandbox_config_seccomp.json > "$TESTDIR"/apparmor2.json + load_apparmor_test_profile + start_ocid "" "$APPARMOR_TEST_PROFILE_NAME" + + sed -e 's/%VALUE%/,"container\.apparmor\.security\.beta\.kubernetes\.io\/testname2": "apparmor-test-deny-write"/g' "$TESTDATA"/sandbox_config_seccomp.json > "$TESTDIR"/apparmor2.json run ocic pod create --name apparmor2 --config "$TESTDIR"/apparmor2.json echo "$output" @@ -60,8 +70,6 @@ function teardown() { echo "$output" [ "$status" -eq 0 ] ctr_id="$output" - run ocic ctr start --id "$ctr_id" - echo "$output" [ "$status" -eq 0 ] run ocic ctr execsync --id "$ctr_id" touch test.txt echo "$output" @@ -76,16 +84,22 @@ function teardown() { # 3. test running with loading a specific apparmor profile but not as ocid default apparmor profile. # test that we can run with a specific apparmor profile which will block touching a file in `.` -@test "load default apparomr profile and run a container with another apparmor profile" { +@test "load default apparmor profile and run a container with another apparmor profile" { # this test requires docker, thus it can't yet be run in a container if [ "$TRAVIS" = "true" ]; then # instead of $TRAVIS, add a function is_containerized to skip here skip "cannot yet run this test in a container, use sudo make localintegration" fi + # this test requires apparmor, so skip this test if apparmor is not enabled. + enabled=is_apparmor_enabled + if [[ "$enabled" =~ "0" ]]; then + skip "skip this test since apparmor is not enabled." + fi + load_apparmor_test_profile start_ocid - sed -e 's/%VALUE%/,"container\.apparmor\.security\.beta\.kubernetes\.io\/testname3": "apparmor_test_deny_write"/g' "$TESTDATA"/sandbox_config_seccomp.json > "$TESTDIR"/apparmor3.json + sed -e 's/%VALUE%/,"container\.apparmor\.security\.beta\.kubernetes\.io\/testname3": "apparmor-test-deny-write"/g' "$TESTDATA"/sandbox_config_seccomp.json > "$TESTDIR"/apparmor3.json run ocic pod create --name apparmor3 --config "$TESTDIR"/apparmor3.json echo "$output" @@ -95,8 +109,6 @@ function teardown() { echo "$output" [ "$status" -eq 0 ] ctr_id="$output" - run ocic ctr start --id "$ctr_id" - echo "$output" [ "$status" -eq 0 ] run ocic ctr execsync --id "$ctr_id" touch test.txt echo "$output" @@ -109,7 +121,7 @@ function teardown() { remove_apparmor_test_profile } -# 1. test running with wrong apparmor profile name. +# 4. test running with wrong apparmor profile name. # test that we can will fail when running a ctr with rong apparmor profile name. @test "run a container with wrong apparmor profile name" { # this test requires docker, thus it can't yet be run in a container @@ -117,6 +129,12 @@ function teardown() { skip "cannot yet run this test in a container, use sudo make localintegration" fi + # this test requires apparmor, so skip this test if apparmor is not enabled. + enabled=is_apparmor_enabled + if [[ "$enabled" =~ "0" ]]; then + skip "skip this test since apparmor is not enabled." + fi + start_ocid sed -e 's/%VALUE%/,"container\.apparmor\.security\.beta\.kubernetes\.io\/testname4": "not-exists"/g' "$TESTDATA"/sandbox_config_seccomp.json > "$TESTDIR"/apparmor4.json diff --git a/test/helpers.bash b/test/helpers.bash index 54895fee..9db9e839 100644 --- a/test/helpers.bash +++ b/test/helpers.bash @@ -20,7 +20,7 @@ PAUSE_BINARY=${PAUSE_BINARY:-${OCID_ROOT}/cri-o/pause/pause} # Path of the default seccomp profile. SECCOMP_PROFILE=${SECCOMP_PROFILE:-${OCID_ROOT}/cri-o/seccomp.json} # Name of the default apparmor profile. -APPARMOR_DEFAULT_PROFILE=${SECCOMP_PROFILE:-ocid-default} +APPARMOR_PROFILE=${APPARMOR_PROFILE:-ocid-default} # Path of the runc binary. RUNC_PATH=$(command -v runc || true) RUNC_BINARY=${RUNC_PATH:-/usr/local/sbin/runc} @@ -29,7 +29,11 @@ APPARMOR_PARSER_BINARY=${APPARMOR_PARSER_BINARY:-/sbin/apparmor_parser} # Path of the apparmor profile for test. APPARMOR_TEST_PROFILE_PATH=${APPARMOR_TEST_PROFILE_PATH:-${TESTDATA}/apparmor_test_deny_write} # Name of the apparmor profile for test. -APPARMOR_TEST_PROFILE_NAME=${APPARMOR_TEST_PROFILE_NAME:-apparmor_test_deny_write} +APPARMOR_TEST_PROFILE_NAME=${APPARMOR_TEST_PROFILE_NAME:-apparmor-test-deny-write} +# Path of boot config. +BOOT_CONFIG_FILE_PATH=${BOOT_CONFIG_FILE_PATH:-/boot/config-`uname -r`} +# Path of apparmor parameters file. +APPARMOR_PARAMETERS_FILE_PATH=${APPARMOR_PARAMETERS_FILE_PATH:-/sys/module/apparmor/parameters/enabled} TESTDIR=$(mktemp -d) if [ -e /usr/sbin/selinuxenabled ] && /usr/sbin/selinuxenabled; then @@ -88,19 +92,19 @@ function wait_until_reachable() { # Start ocid. function start_ocid() { - "$OCID_BINARY" --conmon "$CONMON_BINARY" --pause "$PAUSE_BINARY" --listen "$OCID_SOCKET" --runtime "$RUNC_BINARY" --root "$TESTDIR/ocid" --sandboxdir "$TESTDIR/sandboxes" --containerdir "$TESTDIR/ocid/containers" --seccomp-profile "$SECCOMP_PROFILE" --apparmor-profile "$APPARMOR_PROFILE" config >$OCID_CONFIG - "$OCID_BINARY" --debug --config "$OCID_CONFIG" & OCID_PID=$! - wait_until_reachable -} + if [[ -n "$1" ]]; then + seccomp="$1" + else + seccomp="$SECCOMP_PROFILE" + fi -function start_ocid_with_seccomp_path() { - "$OCID_BINARY" --conmon "$CONMON_BINARY" --pause "$PAUSE_BINARY" --listen "$OCID_SOCKET" --runtime "$RUNC_BINARY" --root "$TESTDIR/ocid" --sandboxdir "$TESTDIR/sandboxes" --containerdir "$TESTDIR/ocid/containers" --seccomp-profile "$1" --apparmor-profile "$APPARMOR_PROFILE" config >$OCID_CONFIG - "$OCID_BINARY" --debug --config "$OCID_CONFIG" & OCID_PID=$! - wait_until_reachable -} + if [[ -n "$2" ]]; then + apparmor="$2" + else + apparmor="$APPARMOR_PROFILE" + fi -function start_ocid_with_apparmor_profile_name() { - "$OCID_BINARY" --conmon "$CONMON_BINARY" --pause "$PAUSE_BINARY" --listen "$OCID_SOCKET" --runtime "$RUNC_BINARY" --root "$TESTDIR/ocid" --sandboxdir "$TESTDIR/sandboxes" --containerdir "$TESTDIR/ocid/containers" --seccomp-profile "$SECCOMP_PROFILE" --apparmor-profile "$1" config >$OCID_CONFIG + "$OCID_BINARY" --conmon "$CONMON_BINARY" --pause "$PAUSE_BINARY" --listen "$OCID_SOCKET" --runtime "$RUNC_BINARY" --root "$TESTDIR/ocid" --sandboxdir "$TESTDIR/sandboxes" --containerdir "$TESTDIR/ocid/containers" --seccomp-profile "$seccomp" --apparmor-profile "$apparmor" config >$OCID_CONFIG "$OCID_BINARY" --debug --config "$OCID_CONFIG" & OCID_PID=$! wait_until_reachable } @@ -151,3 +155,29 @@ function load_apparmor_test_profile() { function remove_apparmor_test_profile() { "$APPARMOR_PARSER_BINARY" -R "$APPARMOR_TEST_PROFILE_PATH" } + +function is_seccomp_enabled() { + if [[ -f "$BOOT_CONFIG_FILE_PATH" ]]; then + out=$(cat "$BOOT_CONFIG_FILE_PATH" | grep CONFIG_SECCOMP=) + if [[ "$out" =~ "CONFIG_SECCOMP=y" ]]; then + echo 1 + else + echo 0 + fi + else + echo 0 + fi +} + +function is_apparmor_enabled() { + if [[ -f "$APPARMOR_PARAMETERS_FILE_PATH" ]]; then + out=$(cat "$APPARMOR_PARAMETERS_FILE_PATH") + if [[ "$out" =~ "Y" ]]; then + echo 1 + else + echo 0 + fi + else + echo 0 + fi +} diff --git a/test/seccomp.bats b/test/seccomp.bats index 8c25f3de..d7aed036 100644 --- a/test/seccomp.bats +++ b/test/seccomp.bats @@ -14,10 +14,17 @@ function teardown() { skip "cannot yet run this test in a container, use sudo make localintegration" fi + # this test requires seccomp, so skip this test if seccomp is not enabled. + enabled=is_seccomp_enabled + if [[ "$enabled" =~ "0" ]]; then + skip "skip this test since seccomp is not enabled." + fi + sed -e 's/"chmod",//' "$OCID_ROOT"/cri-o/seccomp.json > "$TESTDIR"/seccomp_profile1.json sed -i 's/"fchmod",//' "$TESTDIR"/seccomp_profile1.json sed -i 's/"fchmodat",//g' "$TESTDIR"/seccomp_profile1.json - start_ocid_with_seccomp_path "$TESTDIR"/seccomp_profile1.json + + start_ocid "$TESTDIR"/seccomp_profile1.json sed -e 's/%VALUE%/,"security\.alpha\.kubernetes\.io\/seccomp\/container\/redhat\.test\.ocid-seccomp1-1-testname-0": "unconfined"/g' "$TESTDATA"/sandbox_config_seccomp.json > "$TESTDIR"/seccomp1.json run ocic pod create --name seccomp1 --config "$TESTDIR"/seccomp1.json @@ -48,10 +55,17 @@ function teardown() { skip "cannot yet run this test in a container, use sudo make localintegration" fi + # this test requires seccomp, so skip this test if seccomp is not enabled. + enabled=is_seccomp_enabled + if [[ "$enabled" =~ "0" ]]; then + skip "skip this test since seccomp is not enabled." + fi + sed -e 's/"chmod",//' "$OCID_ROOT"/cri-o/seccomp.json > "$TESTDIR"/seccomp_profile1.json sed -i 's/"fchmod",//' "$TESTDIR"/seccomp_profile1.json sed -i 's/"fchmodat",//g' "$TESTDIR"/seccomp_profile1.json - start_ocid_with_seccomp_path "$TESTDIR"/seccomp_profile1.json + + start_ocid "$TESTDIR"/seccomp_profile1.json sed -e 's/%VALUE%/,"security\.alpha\.kubernetes\.io\/seccomp\/container\/redhat\.test\.ocid-seccomp2-1-testname2-0": "runtime\/default"/g' "$TESTDATA"/sandbox_config_seccomp.json > "$TESTDIR"/seccomp2.json run ocic pod create --name seccomp2 --config "$TESTDIR"/seccomp2.json @@ -82,10 +96,17 @@ function teardown() { skip "cannot yet run this test in a container, use sudo make localintegration" fi + # this test requires seccomp, so skip this test if seccomp is not enabled. + enabled=is_seccomp_enabled + if [[ "$enabled" =~ "0" ]]; then + skip "skip this test since seccomp is not enabled." + fi + sed -e 's/"chmod",//' "$OCID_ROOT"/cri-o/seccomp.json > "$TESTDIR"/seccomp_profile1.json sed -i 's/"fchmod",//' "$TESTDIR"/seccomp_profile1.json sed -i 's/"fchmodat",//g' "$TESTDIR"/seccomp_profile1.json - start_ocid_with_seccomp_path "$TESTDIR"/seccomp_profile1.json + + start_ocid "$TESTDIR"/seccomp_profile1.json sed -e 's/%VALUE%/,"security\.alpha\.kubernetes\.io\/seccomp\/container\/redhat\.test\.ocid-seccomp3-1-testname3-1": "notgood"/g' "$TESTDATA"/sandbox_config_seccomp.json > "$TESTDIR"/seccomp3.json run ocic pod create --name seccomp3 --config "$TESTDIR"/seccomp3.json @@ -111,10 +132,17 @@ function teardown() { skip "cannot yet run this test in a container, use sudo make localintegration" fi + # this test requires seccomp, so skip this test if seccomp is not enabled. + enabled=is_seccomp_enabled + if [[ "$enabled" =~ "0" ]]; then + skip "skip this test since seccomp is not enabled." + fi + #sed -e 's/"chmod",//' "$OCID_ROOT"/cri-o/seccomp.json > "$TESTDIR"/seccomp_profile1.json #sed -i 's/"fchmod",//' "$TESTDIR"/seccomp_profile1.json #sed -i 's/"fchmodat",//g' "$TESTDIR"/seccomp_profile1.json - #start_ocid_with_seccomp_path "$TESTDIR"/seccomp_profile1.json + + #start_ocid "$TESTDIR"/seccomp_profile1.json skip "need https://issues.k8s.io/36997" } @@ -129,10 +157,17 @@ function teardown() { skip "cannot yet run this test in a container, use sudo make localintegration" fi + # this test requires seccomp, so skip this test if seccomp is not enabled. + enabled=is_seccomp_enabled + if [[ "$enabled" =~ "0" ]]; then + skip "skip this test since seccomp is not enabled." + fi + sed -e 's/"chmod",//' "$OCID_ROOT"/cri-o/seccomp.json > "$TESTDIR"/seccomp_profile1.json sed -i 's/"fchmod",//' "$TESTDIR"/seccomp_profile1.json sed -i 's/"fchmodat",//g' "$TESTDIR"/seccomp_profile1.json - start_ocid_with_seccomp_path "$TESTDIR"/seccomp_profile1.json + + start_ocid "$TESTDIR"/seccomp_profile1.json sed -e 's/%VALUE%/,"security\.alpha\.kubernetes\.io\/seccomp\/container\/redhat\.test\.ocid-seccomp2-1-testname2-0-not-exists": "unconfined", "security\.alpha\.kubernetes\.io\/seccomp\/pod": "runtime\/default"/g' "$TESTDATA"/sandbox_config_seccomp.json > "$TESTDIR"/seccomp5.json run ocic pod create --name seccomp5 --config "$TESTDIR"/seccomp5.json @@ -166,10 +201,17 @@ function teardown() { skip "cannot yet run this test in a container, use sudo make localintegration" fi + # this test requires seccomp, so skip this test if seccomp is not enabled. + enabled=is_seccomp_enabled + if [[ "$enabled" =~ "0" ]]; then + skip "skip this test since seccomp is not enabled." + fi + sed -e 's/"chmod",//' "$OCID_ROOT"/cri-o/seccomp.json > "$TESTDIR"/seccomp_profile1.json sed -i 's/"fchmod",//' "$TESTDIR"/seccomp_profile1.json sed -i 's/"fchmodat",//g' "$TESTDIR"/seccomp_profile1.json - start_ocid_with_seccomp_path "$TESTDIR"/seccomp_profile1.json + + start_ocid "$TESTDIR"/seccomp_profile1.json sed -e 's/%VALUE%/,"security\.alpha\.kubernetes\.io\/seccomp\/container\/redhat\.test\.ocid-seccomp6-1-testname6-0-not-exists": "runtime-default"/g' "$TESTDATA"/sandbox_config_seccomp.json > "$TESTDIR"/seccomp6.json run ocic pod create --name seccomp6 --config "$TESTDIR"/seccomp6.json @@ -200,10 +242,17 @@ function teardown() { skip "cannot yet run this test in a container, use sudo make localintegration" fi + # this test requires seccomp, so skip this test if seccomp is not enabled. + enabled=is_seccomp_enabled + if [[ "$enabled" =~ "0" ]]; then + skip "skip this test since seccomp is not enabled." + fi + sed -e 's/"chmod",//' "$OCID_ROOT"/cri-o/seccomp.json > "$TESTDIR"/seccomp_profile1.json sed -i 's/"fchmod",//' "$TESTDIR"/seccomp_profile1.json sed -i 's/"fchmodat",//g' "$TESTDIR"/seccomp_profile1.json - start_ocid_with_seccomp_path "$TESTDIR"/seccomp_profile1.json + + start_ocid "$TESTDIR"/seccomp_profile1.json sed -e 's/%VALUE%/,"security\.alpha\.kubernetes\.io\/seccomp\/pod": "unconfined"/g' "$TESTDATA"/sandbox_config_seccomp.json > "$TESTDIR"/seccomp1.json run ocic pod create --name seccomp1 --config "$TESTDIR"/seccomp1.json @@ -234,10 +283,17 @@ function teardown() { skip "cannot yet run this test in a container, use sudo make localintegration" fi + # this test requires seccomp, so skip this test if seccomp is not enabled. + enabled=is_seccomp_enabled + if [[ "$enabled" =~ "0" ]]; then + skip "skip this test since seccomp is not enabled." + fi + sed -e 's/"chmod",//' "$OCID_ROOT"/cri-o/seccomp.json > "$TESTDIR"/seccomp_profile1.json sed -i 's/"fchmod",//' "$TESTDIR"/seccomp_profile1.json sed -i 's/"fchmodat",//g' "$TESTDIR"/seccomp_profile1.json - start_ocid_with_seccomp_path "$TESTDIR"/seccomp_profile1.json + + start_ocid "$TESTDIR"/seccomp_profile1.json sed -e 's/%VALUE%/,"security\.alpha\.kubernetes\.io\/seccomp\/pod": "runtime\/default"/g' "$TESTDATA"/sandbox_config_seccomp.json > "$TESTDIR"/seccomp2.json run ocic pod create --name seccomp2 --config "$TESTDIR"/seccomp2.json @@ -268,10 +324,17 @@ function teardown() { skip "cannot yet run this test in a container, use sudo make localintegration" fi + # this test requires seccomp, so skip this test if seccomp is not enabled. + enabled=is_seccomp_enabled + if [[ "$enabled" =~ "0" ]]; then + skip "skip this test since seccomp is not enabled." + fi + sed -e 's/"chmod",//' "$OCID_ROOT"/cri-o/seccomp.json > "$TESTDIR"/seccomp_profile1.json sed -i 's/"fchmod",//' "$TESTDIR"/seccomp_profile1.json sed -i 's/"fchmodat",//g' "$TESTDIR"/seccomp_profile1.json - start_ocid_with_seccomp_path "$TESTDIR"/seccomp_profile1.json + + start_ocid "$TESTDIR"/seccomp_profile1.json # 3. test running with pod wrong profile name sed -e 's/%VALUE%/,"security\.alpha\.kubernetes\.io\/seccomp\/pod": "notgood"/g' "$TESTDATA"/sandbox_config_seccomp.json > "$TESTDIR"/seccomp3.json @@ -298,10 +361,17 @@ function teardown() { skip "cannot yet run this test in a container, use sudo make localintegration" fi + # this test requires seccomp, so skip this test if seccomp is not enabled. + enabled=is_seccomp_enabled + if [[ "$enabled" =~ "0" ]]; then + skip "skip this test since seccomp is not enabled." + fi + #sed -e 's/"chmod",//' "$OCID_ROOT"/cri-o/seccomp.json > "$TESTDIR"/seccomp_profile1.json #sed -i 's/"fchmod",//' "$TESTDIR"/seccomp_profile1.json #sed -i 's/"fchmodat",//g' "$TESTDIR"/seccomp_profile1.json - #start_ocid_with_seccomp_path "$TESTDIR"/seccomp_profile1.json + + #start_ocid "$TESTDIR"/seccomp_profile1.json skip "need https://issues.k8s.io/36997" }