diff --git a/apparmor/apparmor.go b/apparmor/apparmor.go index d2d3f81..c064511 100644 --- a/apparmor/apparmor.go +++ b/apparmor/apparmor.go @@ -21,20 +21,6 @@ import ( // profileDirectory is the file store for AppArmor profiles and macros. const profileDirectory = "/etc/apparmor.d" -// profileData holds information about the given profile for generation. -type profileData struct { - // Abi is the ABI version to use. - Abi string - // 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 -} - // generate creates an AppArmor profile from ProfileData. func generate(p *profileData, out io.Writer, macroExistsFn func(string) bool) error { compiled, err := template.New("apparmor_profile").Parse(baseTemplate) @@ -42,23 +28,23 @@ func generate(p *profileData, out io.Writer, macroExistsFn func(string) bool) er return err } - if p.DaemonProfile == "" { - p.DaemonProfile = "unconfined" + if p.daemonProfile == "" { + p.daemonProfile = "unconfined" } const abi = "abi/3.0" if macroExistsFn(abi) { - p.Abi = abi + p.abi = abi } if macroExistsFn("tunables/global") { - p.Imports = append(p.Imports, "#include ") + p.imports = append(p.imports, "#include ") } else { - p.Imports = append(p.Imports, "@{PROC}=/proc/") + p.imports = append(p.imports, "@{PROC}=/proc/") } if macroExistsFn("abstractions/base") { - p.InnerImports = append(p.InnerImports, "#include ") + p.innerImports = append(p.innerImports, "#include ") } return compiled.Execute(out, p) @@ -87,8 +73,8 @@ func installDefault(ctx context.Context, name string) error { } p := profileData{ - Name: name, - DaemonProfile: daemonProfile, + name: name, + daemonProfile: daemonProfile, } var buf bytes.Buffer @@ -159,9 +145,9 @@ func cleanProfileName(profile string) string { // similar to libapparmor [splitcon]. splitCon follows libapparmor's parsing // semantics and does not validate the returned mode. // -// /proc/self/attr/current returns the current label for the process, but -// unlike /sys/kernel/security/apparmor/profiles, this value may not include -// a " ()" suffix. +// /proc/self/attr/current returns the current confinement context for the +// process. Unlike /sys/kernel/security/apparmor/profiles, this value may not +// include a " ()" suffix. // // Supported forms: // diff --git a/apparmor/apparmor_linux_test.go b/apparmor/apparmor_linux_test.go index 09b46f1..4ecac43 100644 --- a/apparmor/apparmor_linux_test.go +++ b/apparmor/apparmor_linux_test.go @@ -224,60 +224,67 @@ func TestGenerateDefault(t *testing.T) { { name: "default", data: profileData{ - Name: "default", + name: "default", }, }, { name: "with-api3", data: profileData{ - Name: "with-api3", + name: "with-api3", }, macroExists: func(name string) bool { return name == "abi/3.0" }, }, { name: "with-tunables", data: profileData{ - Name: "tunables", + name: "tunables", }, macroExists: func(name string) bool { return name == "tunables/global" }, }, { name: "with-abstractions-base", data: profileData{ - Name: "abstractions-base", + name: "abstractions-base", }, macroExists: func(name string) bool { return name == "abstractions/base" }, }, { name: "with-daemon-profile", data: profileData{ - Name: "with-daemon-profile", - DaemonProfile: "my-daemon-profile", + name: "with-daemon-profile", + daemonProfile: "my-daemon-profile", }, }, { name: "with-spaces", data: profileData{ - Name: "Profile with spaces", - DaemonProfile: "Daemon Profile", + name: "Profile with spaces", + daemonProfile: "Daemon Profile", }, }, { name: "with-custom-imports", data: profileData{ - Name: "custom-imports", - Imports: []string{"#include ", "#include "}, + name: "custom-imports", + imports: []string{"#include ", "#include "}, }, skipParse: true, // Skip parsing because we use non-existing includes. }, { name: "with-custom-inner-imports", data: profileData{ - Name: "custom-inner-imports", - InnerImports: []string{"#include ", "#include "}, + name: "custom-inner-imports", + innerImports: []string{"#include ", "#include "}, }, skipParse: true, // Skip parsing because we use non-existing includes. }, + { + name: "with-special-characters", + data: profileData{ + name: `foo"bar,*?[ab]{c,d}^\baz`, + daemonProfile: `daemon"bar,*?[ab]{c,d}^\baz`, + }, + }, } for _, tc := range tests { @@ -309,6 +316,23 @@ func TestGenerateDefault(t *testing.T) { } } +func TestGenerateProfileName(t *testing.T) { + if _, err := exec.LookPath("apparmor_parser"); err != nil { + t.Skipf("apparmor_parser not available: %v", err) + } + + const name = `foo"bar,*?[ab]{c,d}^\baz` + var profile strings.Builder + if err := generate(&profileData{name: name}, &profile, func(string) bool { return false }); err != nil { + t.Fatal(err) + } + + names := validateProfile(t, profile.String()) + if len(names) != 1 || names[0] != name { + t.Fatalf("parsed profile names = %q, want [%q]", names, name) + } +} + func createTestProfiles(b *testing.B, lines int, targetProfile string) string { b.Helper() diff --git a/apparmor/template.go b/apparmor/template.go index 71a1899..927bfdd 100644 --- a/apparmor/template.go +++ b/apparmor/template.go @@ -1,10 +1,10 @@ // SPDX-FileCopyrightText: Copyright The Moby Authors // SPDX-License-Identifier: Apache-2.0 -//go:build linux - package apparmor +import "strings" + // 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 @@ -26,7 +26,7 @@ const baseTemplate = `# profile generated by github.com/moby/profiles/apparmor. {{$value}} {{- end}} -profile "{{.Name}}" flags=(attach_disconnected,mediate_deleted) { +profile {{.Name}} flags=(attach_disconnected,mediate_deleted) { {{- range $value := .InnerImports}} {{$value}} {{- end}}{{if .InnerImports}} @@ -46,9 +46,9 @@ profile "{{.Name}}" flags=(attach_disconnected,mediate_deleted) { # crun may send signals to container processes (for "docker stop" when used with crun OCI runtime). signal (receive) peer=crun, # dockerd may send signals to container processes (for "docker kill"). - signal (receive) peer="{{.DaemonProfile}}", + signal (receive) peer={{.DaemonProfile}}, # Container processes may send signals amongst themselves. - signal (send,receive) peer="{{.Name}}", + signal (send,receive) peer={{.PeerName}}, 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/** @@ -71,6 +71,100 @@ profile "{{.Name}}" flags=(attach_disconnected,mediate_deleted) { # allow processes within the container to trace each other, # provided all other LSM and yama setting allow it. - ptrace (trace,tracedby,read,readby) peer="{{.Name}}", + ptrace (trace,tracedby,read,readby) peer={{.PeerName}}, } ` + +// profileData holds information about the given profile for generation. +type profileData struct { + // abi is the ABI version to use. + abi string + // 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 +} + +// Abi returns the AppArmor ABI version used by the profile. +func (d profileData) Abi() string { + return d.abi +} + +// Name returns the quoted AppArmor profile name. +func (d profileData) Name() string { + return quoteProfileName(d.name) +} + +// PeerName returns the quoted AppArmor peer pattern matching the profile name. +func (d profileData) PeerName() string { + return quotePeerName(d.name) +} + +// Imports returns the AppArmor functions imported before the profile definition. +func (d profileData) Imports() []string { + return d.imports +} + +// InnerImports returns the AppArmor functions imported inside the profile. +func (d profileData) InnerImports() []string { + return d.innerImports +} + +// DaemonProfile returns the daemon's quoted peer pattern or the unconfined selector. +func (d profileData) DaemonProfile() string { + if d.daemonProfile == "unconfined" { + return d.daemonProfile + } + return quotePeerName(d.daemonProfile) +} + +// quoteProfileName quotes a profile declaration name. Declaration names retain +// AARE escapes, so only embedded quotes are escaped here. The parser still +// decodes recognized backslash escape sequences. +func quoteProfileName(s string) string { + if s == "" { + return "" + } + return `"` + strings.ReplaceAll(s, `"`, `\"`) + `"` +} + +// quotePeerName returns s as a quoted AppArmor peer pattern, escaping +// characters as needed to preserve the name literally rather than interpreting +// it as an AARE pattern. Empty strings are returned unchanged. +// +// AppArmor quoted identifiers may contain any character other than NUL. When +// processing an identifier, the parser decodes escape sequences while +// preserving escapes for AARE special characters so they can be handled by +// the pattern-matching backend. +// +// Callers are expected to pass valid profile names, which excludes NUL. +// +// See: +// - https://gitlab.com/apparmor/apparmor/-/blob/v5.0.2/parser/parser_lex.l#L286-287 +// - https://gitlab.com/apparmor/apparmor/-/blob/v5.0.2/parser/parser_misc.c#L468-507 +// - https://gitlab.com/apparmor/apparmor/-/blob/v5.0.2/parser/lib.c#L144-219 +func quotePeerName(s string) string { + if s == "" { + return "" + } + + var b strings.Builder + b.Grow(len(s) + 2) + + b.WriteByte('"') + for i := range len(s) { + c := s[i] + switch c { + case '\\', '"', '*', '?', '[', ']', '{', '}', '^', ',': + b.WriteByte('\\') + } + b.WriteByte(c) + } + b.WriteByte('"') + + return b.String() +} diff --git a/apparmor/template_test.go b/apparmor/template_test.go new file mode 100644 index 0000000..6199d1e --- /dev/null +++ b/apparmor/template_test.go @@ -0,0 +1,79 @@ +// SPDX-FileCopyrightText: Copyright The Moby Authors +// SPDX-License-Identifier: Apache-2.0 + +package apparmor + +import ( + "testing" +) + +func TestQuotePeerName(t *testing.T) { + tests := []struct { + doc string + value string + want string + }{ + { + doc: "empty", + want: "", + }, + { + doc: "simple", + value: "default-profile", + want: `"default-profile"`, + }, + { + doc: "spaces", + value: "with spaces", + want: `"with spaces"`, + }, + { + doc: "double quote", + value: `foo"bar`, + want: `"foo\"bar"`, + }, + { + doc: "backslash", + value: `foo\bar`, + want: `"foo\\bar"`, + }, + { + doc: "escape sequence", + value: `foo\nbar`, + want: `"foo\\nbar"`, + }, + { + doc: "AARE wildcard", + value: `foo*bar?baz`, + want: `"foo\*bar\?baz"`, + }, + { + doc: "AARE character class", + value: `foo[bar]`, + want: `"foo\[bar\]"`, + }, + { + doc: "AARE alternation", + value: `foo{bar,baz}`, + want: `"foo\{bar\,baz\}"`, + }, + { + doc: "AARE anchor", + value: `foo^bar`, + want: `"foo\^bar"`, + }, + { + doc: "invalid UTF-8 with special character", + value: "foo\xff*bar", + want: "\"foo\xff\\*bar\"", + }, + } + + for _, tc := range tests { + t.Run(tc.doc, func(t *testing.T) { + if got := quotePeerName(tc.value); got != tc.want { + t.Errorf("quotePeerName(%q) = %q, want %q", tc.value, got, tc.want) + } + }) + } +} diff --git a/apparmor/testdata/default.golden b/apparmor/testdata/default.golden index f1d8599..78d25cd 100644 --- a/apparmor/testdata/default.golden +++ b/apparmor/testdata/default.golden @@ -19,7 +19,7 @@ profile "default" flags=(attach_disconnected,mediate_deleted) { # crun may send signals to container processes (for "docker stop" when used with crun OCI runtime). signal (receive) peer=crun, # dockerd may send signals to container processes (for "docker kill"). - signal (receive) peer="unconfined", + signal (receive) peer=unconfined, # Container processes may send signals amongst themselves. signal (send,receive) peer="default", diff --git a/apparmor/testdata/with-abstractions-base.golden b/apparmor/testdata/with-abstractions-base.golden index 6dceb34..e107927 100644 --- a/apparmor/testdata/with-abstractions-base.golden +++ b/apparmor/testdata/with-abstractions-base.golden @@ -21,7 +21,7 @@ profile "abstractions-base" flags=(attach_disconnected,mediate_deleted) { # crun may send signals to container processes (for "docker stop" when used with crun OCI runtime). signal (receive) peer=crun, # dockerd may send signals to container processes (for "docker kill"). - signal (receive) peer="unconfined", + signal (receive) peer=unconfined, # Container processes may send signals amongst themselves. signal (send,receive) peer="abstractions-base", diff --git a/apparmor/testdata/with-api3.golden b/apparmor/testdata/with-api3.golden index 2eb10fe..ff37acc 100644 --- a/apparmor/testdata/with-api3.golden +++ b/apparmor/testdata/with-api3.golden @@ -19,7 +19,7 @@ profile "with-api3" flags=(attach_disconnected,mediate_deleted) { # crun may send signals to container processes (for "docker stop" when used with crun OCI runtime). signal (receive) peer=crun, # dockerd may send signals to container processes (for "docker kill"). - signal (receive) peer="unconfined", + signal (receive) peer=unconfined, # Container processes may send signals amongst themselves. signal (send,receive) peer="with-api3", diff --git a/apparmor/testdata/with-custom-imports.golden b/apparmor/testdata/with-custom-imports.golden index 6b99b8d..6084f59 100644 --- a/apparmor/testdata/with-custom-imports.golden +++ b/apparmor/testdata/with-custom-imports.golden @@ -21,7 +21,7 @@ profile "custom-imports" flags=(attach_disconnected,mediate_deleted) { # crun may send signals to container processes (for "docker stop" when used with crun OCI runtime). signal (receive) peer=crun, # dockerd may send signals to container processes (for "docker kill"). - signal (receive) peer="unconfined", + signal (receive) peer=unconfined, # Container processes may send signals amongst themselves. signal (send,receive) peer="custom-imports", diff --git a/apparmor/testdata/with-custom-inner-imports.golden b/apparmor/testdata/with-custom-inner-imports.golden index bcf150c..b05c4f7 100644 --- a/apparmor/testdata/with-custom-inner-imports.golden +++ b/apparmor/testdata/with-custom-inner-imports.golden @@ -22,7 +22,7 @@ profile "custom-inner-imports" flags=(attach_disconnected,mediate_deleted) { # crun may send signals to container processes (for "docker stop" when used with crun OCI runtime). signal (receive) peer=crun, # dockerd may send signals to container processes (for "docker kill"). - signal (receive) peer="unconfined", + signal (receive) peer=unconfined, # Container processes may send signals amongst themselves. signal (send,receive) peer="custom-inner-imports", diff --git a/apparmor/testdata/with-special-characters.golden b/apparmor/testdata/with-special-characters.golden new file mode 100644 index 0000000..cb95549 --- /dev/null +++ b/apparmor/testdata/with-special-characters.golden @@ -0,0 +1,48 @@ +# profile generated by github.com/moby/profiles/apparmor. + + +@{PROC}=/proc/ + +profile "foo\"bar,*?[ab]{c,d}^\baz" flags=(attach_disconnected,mediate_deleted) { + network, + # Disallow AF_ALG (Linux kernel crypto API); see https://copy.fail/ + deny network alg, + # Disallow AF_VSOCK to prevent host/guest communication. + deny network vsock, + capability, + file, + umount, + # Host (privileged) processes may send signals to container processes. + signal (receive) peer=unconfined, + # runc may send signals to container processes (for "docker stop"). + signal (receive) peer=runc, + # crun may send signals to container processes (for "docker stop" when used with crun OCI runtime). + signal (receive) peer=crun, + # dockerd may send signals to container processes (for "docker kill"). + signal (receive) peer="daemon\"bar\,\*\?\[ab\]\{c\,d\}\^\\baz", + # Container processes may send signals amongst themselves. + signal (send,receive) peer="foo\"bar\,\*\?\[ab\]\{c\,d\}\^\\baz", + + 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/devices/virtual/powercap/** rwklx, + deny /sys/kernel/security/** rwklx, + + # allow processes within the container to trace each other, + # provided all other LSM and yama setting allow it. + ptrace (trace,tracedby,read,readby) peer="foo\"bar\,\*\?\[ab\]\{c\,d\}\^\\baz", +} diff --git a/apparmor/testdata/with-tunables.golden b/apparmor/testdata/with-tunables.golden index b01fd70..42bc924 100644 --- a/apparmor/testdata/with-tunables.golden +++ b/apparmor/testdata/with-tunables.golden @@ -19,7 +19,7 @@ profile "tunables" flags=(attach_disconnected,mediate_deleted) { # crun may send signals to container processes (for "docker stop" when used with crun OCI runtime). signal (receive) peer=crun, # dockerd may send signals to container processes (for "docker kill"). - signal (receive) peer="unconfined", + signal (receive) peer=unconfined, # Container processes may send signals amongst themselves. signal (send,receive) peer="tunables",