apparmor: align quoting with libapparmor

apparmor/apparmor.go

@@ -21,44 +21,30 @@ 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)
 	if err != nil {
 		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 <tunables/global>")
+		p.imports = append(p.imports, "#include <tunables/global>")
 	} 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 <abstractions/base>")
+		p.innerImports = append(p.innerImports, "#include <abstractions/base>")
 	}
 
 	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 " (<mode>)" 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 " (<mode>)" suffix.
 //
 // Supported forms:
 //

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 <something/foo>", "#include <something/bar>"},
+				name:    "custom-imports",
+				imports: []string{"#include <something/foo>", "#include <something/bar>"},
 			},
 			skipParse: true, // Skip parsing because we use non-existing includes.
 		},
 		{
 			name: "with-custom-inner-imports",
 			data: profileData{
-				Name:         "custom-inner-imports",
-				InnerImports: []string{"#include <something/foo>", "#include <something/bar>"},
+				name:         "custom-inner-imports",
+				innerImports: []string{"#include <something/foo>", "#include <something/bar>"},
 			},
 			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()
 

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/<number>/** 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()
+}

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)
+			}
+		})
+	}
+}