From 3f0979b7c1fdb606aa0784b4f630e9566f81f6f1 Mon Sep 17 00:00:00 2001 From: Maximilian Hils Date: Fri, 3 May 2024 19:42:02 +0200 Subject: [PATCH] apparmor: support path globs when merging profiles --- .../pkg/daemon/bpfrecorder/bpfrecorder.go | 2 + .../pkg/manager/recordingmerger/apparmor.go | 298 ++++++++++++++++++ .../manager/recordingmerger/apparmor_test.go | 196 ++++++++++++ .../manager/recordingmerger/merge_utils.go | 171 ---------- .../recordingmerger/merge_utils_test.go | 9 + 5 files changed, 505 insertions(+), 171 deletions(-) create mode 100644 internal/pkg/manager/recordingmerger/apparmor.go create mode 100644 internal/pkg/manager/recordingmerger/apparmor_test.go diff --git a/internal/pkg/daemon/bpfrecorder/bpfrecorder.go b/internal/pkg/daemon/bpfrecorder/bpfrecorder.go index 5a360e15e4..9a2a228385 100644 --- a/internal/pkg/daemon/bpfrecorder/bpfrecorder.go +++ b/internal/pkg/daemon/bpfrecorder/bpfrecorder.go @@ -257,6 +257,8 @@ func (b *BpfRecorder) Syscalls() *bpf.BPFMap { } func (b *BpfRecorder) GetAppArmorProcessed() BpfAppArmorProcessed { + sort.Strings(b.recordedCapabilities) + var processed BpfAppArmorProcessed processed.FileProcessed = b.processExecFsEvents() diff --git a/internal/pkg/manager/recordingmerger/apparmor.go b/internal/pkg/manager/recordingmerger/apparmor.go new file mode 100644 index 0000000000..1637f21b7c --- /dev/null +++ b/internal/pkg/manager/recordingmerger/apparmor.go @@ -0,0 +1,298 @@ +/* +Copyright 2022 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package recordingmerger + +import ( + "fmt" + "log" + "regexp" + "sort" + "strings" + + "sigs.k8s.io/controller-runtime/pkg/client" + + apparmorprofileapi "sigs.k8s.io/security-profiles-operator/api/apparmorprofile/v1alpha1" +) + +type mergeableAppArmorProfile struct { + apparmorprofileapi.AppArmorProfile +} + +func (sp *mergeableAppArmorProfile) getProfile() client.Object { + return &sp.AppArmorProfile +} + +// Merge two AppArmor profiles. The first profile may use glob patterns +// for paths, whereas the second profile is expected to contain raw paths only. +func (sp *mergeableAppArmorProfile) merge(other mergeableProfile) error { + otherSP, ok := other.(*mergeableAppArmorProfile) + if !ok { + return fmt.Errorf("cannot merge AppArmorProfile with %T", other) + } + + a1 := &sp.Spec.Abstract + a2 := &otherSP.Spec.Abstract + + if a1.Executable != nil && a2.Executable != nil { + a1.Executable.AllowedExecutables = mergePaths( + a1.Executable.AllowedExecutables, + a2.Executable.AllowedExecutables, + ) + a1.Executable.AllowedLibraries = mergePaths( + a1.Executable.AllowedLibraries, + a2.Executable.AllowedLibraries, + ) + } else if a2.Executable != nil { + a1.Executable = a2.Executable + } + + mergeFilesystem(a1, a2) + + if a1.Network != nil && a2.Network != nil { + a1.Network.AllowRaw = mergeBools(a1.Network.AllowRaw, a2.Network.AllowRaw) + + if a1.Network.Protocols != nil && a2.Network.Protocols != nil { + a1.Network.Protocols.AllowTCP = mergeBools(a1.Network.Protocols.AllowTCP, a2.Network.Protocols.AllowTCP) + a1.Network.Protocols.AllowUDP = mergeBools(a1.Network.Protocols.AllowUDP, a2.Network.Protocols.AllowUDP) + } else if a2.Network.Protocols != nil { + a1.Network.Protocols = a2.Network.Protocols + } + } else if a2.Network != nil { + a1.Network = a2.Network + } + + if a1.Capability != nil && a2.Capability != nil { + a1.Capability.AllowedCapabilities = *mergeDedupSortStrings( + &a1.Capability.AllowedCapabilities, + &a2.Capability.AllowedCapabilities, + ) + } else if a2.Capability != nil { + a1.Capability = a2.Capability + } + + return nil +} + +func mergePaths(a, b *[]string) *[]string { + if a == nil { + return b + } + if b == nil { + return a + } + + merged := newAppArmorPathSet(a) + for _, path := range *b { + if !merged.Matches(path) { + merged.Add(path) + } + } + + return merged.Patterns() +} + +func mergeFilesystem(base, additions *apparmorprofileapi.AppArmorAbstract) { + //nolint:nestif // refactoring this makes it worse + if base.Filesystem != nil && additions.Filesystem != nil { + r := newAppArmorPathSet(base.Filesystem.ReadOnlyPaths) + w := newAppArmorPathSet(base.Filesystem.WriteOnlyPaths) + rw := newAppArmorPathSet(base.Filesystem.ReadWritePaths) + + if additions.Filesystem.ReadWritePaths != nil { + for _, p := range *additions.Filesystem.ReadWritePaths { + if rw.Matches(p) { + // no changes necessary + } else if pat := r.PopMatching(p); pat != nil { + rw.Add(*pat) + } else if pat := w.PopMatching(p); pat != nil { + rw.Add(*pat) + } else { + rw.Add(p) + } + } + } + if additions.Filesystem.ReadOnlyPaths != nil { + for _, p := range *additions.Filesystem.ReadOnlyPaths { + if rw.Matches(p) { + // no changes necessary + } else if r.Matches(p) { + // no changes necessary + } else if pat := w.PopMatching(p); pat != nil { + rw.Add(*pat) + } else { + r.Add(p) + } + } + } + if additions.Filesystem.WriteOnlyPaths != nil { + for _, p := range *additions.Filesystem.WriteOnlyPaths { + if rw.Matches(p) { + // no changes necessary + } else if pat := r.PopMatching(p); pat != nil { + rw.Add(*pat) + } else if w.Matches(p) { + // no changes necessary + } else { + w.Add(p) + } + } + } + base.Filesystem = &apparmorprofileapi.AppArmorFsRules{ + ReadOnlyPaths: r.Patterns(), + WriteOnlyPaths: w.Patterns(), + ReadWritePaths: rw.Patterns(), + } + } else if additions.Filesystem != nil { + base.Filesystem = additions.Filesystem + } +} + +func newAppArmorPathSet(patterns *[]string) appArmorPathSet { + m := appArmorPathSet{} + if patterns != nil { + for _, p := range *patterns { + m.Add(p) + } + } + return m +} + +type appArmorPathSet struct { + paths []apparmorPath +} + +type apparmorPath struct { + pattern string + expr *regexp.Regexp +} + +func (m *appArmorPathSet) findMatch(path string) int { + for i, p := range m.paths { + if p.pattern == path { + return i + } + if p.expr != nil && p.expr.MatchString(path) { + return i + } + } + return -1 +} + +func (m *appArmorPathSet) Matches(path string) bool { + return m.findMatch(path) >= 0 +} + +func (m *appArmorPathSet) PopMatching(path string) *string { + i := m.findMatch(path) + if i >= 0 { + ret := m.paths[i].pattern + m.paths[i] = m.paths[len(m.paths)-1] + m.paths = m.paths[:len(m.paths)-1] + return &ret + } + return nil +} + +func (m *appArmorPathSet) Add(pattern string) { + rex, err := appArmorGlobToRegex(pattern) + if err != nil { + log.Printf("Failed to parse AppArmor glob pattern '%s': %x\n", pattern, err) + } + m.paths = append(m.paths, apparmorPath{ + pattern: pattern, + expr: rex, + }) +} + +func (m *appArmorPathSet) Patterns() *[]string { + if len(m.paths) == 0 { + return nil + } + ret := make([]string, 0, len(m.paths)) + for _, p := range m.paths { + ret = append(ret, p.pattern) + } + sort.Strings(ret) + return &ret +} + +// Convert AppArmor globs (https://gitlab.com/apparmor/apparmor/-/wikis/QuickProfileLanguage#file-globbing) +// to regular expressions for evaluation. This method may be inaccurate and should not be +// used for security-sensitive use-cases, but it is good enough for common patterns. +func appArmorGlobToRegex(pattern string) (*regexp.Regexp, error) { + expr := "^" + regexp.MustCompile(`\*\*?|\?|\{.+?\}|\.`).ReplaceAllStringFunc( + pattern, func(match string) string { + switch match { + case "**": + return `[^\000]*` + case "*": + return `[^/\000]*` + case "?": + return `[^/]` + case ".": + return `\.` + default: + inner := regexp.QuoteMeta(match[1 : len(match)-1]) + inner = strings.ReplaceAll(inner, ",", "|") + return "(" + inner + ")" + } + }, + ) + "$" + return regexp.Compile(expr) +} + +func mergeBools(a, b *bool) *bool { + if a == nil { + return b + } + if b == nil { + return a + } + merged := (*a || *b) + return &merged +} + +func mergeDedupSortStrings(a, b *[]string) *[]string { + if a == nil { + return b + } + if b == nil { + return a + } + merged := append(*a, *b...) + sort.Strings(merged) + merged = compact(merged) + return &merged +} + +// TODO: replace this with slices.Compact once all platform support Go 1.21. +func compact(s []string) []string { + //nolint:all + if len(s) < 2 { + return s + } + i := 1 + for k := 1; k < len(s); k++ { + if s[k] != s[k-1] { + if i != k { + s[i] = s[k] + } + i++ + } + } + return s[:i] +} diff --git a/internal/pkg/manager/recordingmerger/apparmor_test.go b/internal/pkg/manager/recordingmerger/apparmor_test.go new file mode 100644 index 0000000000..561f853831 --- /dev/null +++ b/internal/pkg/manager/recordingmerger/apparmor_test.go @@ -0,0 +1,196 @@ +/* +Copyright 2022 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package recordingmerger + +import ( + "testing" + + "github.com/stretchr/testify/require" + + apparmorprofileapi "sigs.k8s.io/security-profiles-operator/api/apparmorprofile/v1alpha1" +) + +func TestAppArmorGlobToRegex(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + globPattern string + wantRegex string + }{ + { + name: "Basic File Match", + globPattern: "/bin/ls", + wantRegex: `^/bin/ls$`, + }, + { + name: "Single Wildcard", + globPattern: "/usr/bin/*", + wantRegex: `^/usr/bin/[^/\000]*$`, + }, + { + name: "Double Wildcard", + globPattern: "/home/**/docs", + wantRegex: `^/home/[^\000]*/docs$`, + }, + { + name: "Character Class", + globPattern: "/var/log/{kern.log,syslog}", + wantRegex: `^/var/log/(kern\.log|syslog)$`, + }, + { + name: "Question Mark", + globPattern: "/tmp/file?.txt", + wantRegex: `^/tmp/file[^/]\.txt$`, + }, + } + + for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + result, err := appArmorGlobToRegex(tc.globPattern) + require.NoError(t, err) + require.Equal(t, tc.wantRegex, result.String()) + }) + } +} + +func Test_appArmorPathSet(t *testing.T) { + t.Parallel() + + m := newAppArmorPathSet(&[]string{ + "/foo/*", + "/bar", + }) + + require.True(t, m.Matches("/foo/baz")) + require.True(t, m.Matches("/bar")) + require.False(t, m.Matches("/baz")) + + p := m.PopMatching("/foo/baz") + require.Equal(t, "/foo/*", *p) + require.False(t, m.Matches("/foo/baz")) + + p = m.PopMatching("/foo/baz") + require.Nil(t, p) + + m.Add("/baz**") + require.True(t, m.Matches("/baz/qux")) + + require.Equal(t, []string{"/bar", "/baz**"}, *m.Patterns()) +} + +func TestMergeFilesystem(t *testing.T) { + t.Parallel() + + base := &apparmorprofileapi.AppArmorAbstract{ + Filesystem: &apparmorprofileapi.AppArmorFsRules{ + ReadOnlyPaths: &[]string{"/r/*"}, + WriteOnlyPaths: &[]string{"/w/*"}, + ReadWritePaths: &[]string{"/rw/*"}, + }, + } + + testCases := []struct { + name string + additions apparmorprofileapi.AppArmorFsRules + merged apparmorprofileapi.AppArmorFsRules + }{ + { + name: "empty", + additions: apparmorprofileapi.AppArmorFsRules{}, + merged: *base.Filesystem, + }, + { + name: "matching", + additions: apparmorprofileapi.AppArmorFsRules{ + ReadOnlyPaths: &[]string{"/r/foo"}, + WriteOnlyPaths: &[]string{"/w/bar"}, + ReadWritePaths: &[]string{"/rw/baz"}, + }, + merged: *base.Filesystem, + }, + { + name: "subset", + additions: apparmorprofileapi.AppArmorFsRules{ + ReadOnlyPaths: &[]string{"/rw/foo"}, + WriteOnlyPaths: &[]string{"/rw/bar"}, + }, + merged: *base.Filesystem, + }, + { + name: "additive", + additions: apparmorprofileapi.AppArmorFsRules{ + ReadOnlyPaths: &[]string{"/w/foo"}, + WriteOnlyPaths: &[]string{"/r/bar"}, + }, + merged: apparmorprofileapi.AppArmorFsRules{ + ReadWritePaths: &[]string{"/r/*", "/rw/*", "/w/*"}, + }, + }, + { + name: "additive2", + additions: apparmorprofileapi.AppArmorFsRules{ + ReadWritePaths: &[]string{"/r/foo", "/w/foo"}, + }, + merged: apparmorprofileapi.AppArmorFsRules{ + ReadWritePaths: &[]string{"/r/*", "/rw/*", "/w/*"}, + }, + }, + { + name: "new", + additions: apparmorprofileapi.AppArmorFsRules{ + ReadOnlyPaths: &[]string{"/r2/foo"}, + WriteOnlyPaths: &[]string{"/w2/bar"}, + ReadWritePaths: &[]string{"/rw2/baz"}, + }, + merged: apparmorprofileapi.AppArmorFsRules{ + ReadOnlyPaths: &[]string{"/r/*", "/r2/foo"}, + WriteOnlyPaths: &[]string{"/w/*", "/w2/bar"}, + ReadWritePaths: &[]string{"/rw/*", "/rw2/baz"}, + }, + }, + } + + for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + b := base.DeepCopy() + mergeFilesystem(b, &apparmorprofileapi.AppArmorAbstract{ + Filesystem: &tc.additions, + }) + require.Equal(t, *b.Filesystem, tc.merged) + }) + } +} + +func TestMergeBools(t *testing.T) { + t.Parallel() + + True := true + False := false + + require.True(t, *mergeBools(&True, &True)) + require.True(t, *mergeBools(&True, nil)) + require.True(t, *mergeBools(nil, &True)) + require.True(t, *mergeBools(&True, &False)) + require.False(t, *mergeBools(&False, &False)) + require.False(t, *mergeBools(&False, nil)) + require.Nil(t, mergeBools(nil, nil)) +} diff --git a/internal/pkg/manager/recordingmerger/merge_utils.go b/internal/pkg/manager/recordingmerger/merge_utils.go index 6d3fe5e29e..0ea611ebff 100644 --- a/internal/pkg/manager/recordingmerger/merge_utils.go +++ b/internal/pkg/manager/recordingmerger/merge_utils.go @@ -20,7 +20,6 @@ import ( "context" "errors" "fmt" - "sort" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -245,173 +244,3 @@ func addAllow(union, additional selinuxprofileapi.Allow) selinuxprofileapi.Allow return union } - -type mergeableAppArmorProfile struct { - apparmorprofileapi.AppArmorProfile -} - -func (sp *mergeableAppArmorProfile) getProfile() client.Object { - return &sp.AppArmorProfile -} - -func (sp *mergeableAppArmorProfile) merge(other mergeableProfile) error { - otherSP, ok := other.(*mergeableAppArmorProfile) - if !ok { - return fmt.Errorf("cannot merge AppArmorProfile with %T", other) - } - - a1 := &sp.Spec.Abstract - a2 := &otherSP.Spec.Abstract - - if a1.Executable != nil && a2.Executable != nil { - a1.Executable.AllowedExecutables = mergeDedupSortStrings( - a1.Executable.AllowedExecutables, - a2.Executable.AllowedExecutables, - ) - a1.Executable.AllowedLibraries = mergeDedupSortStrings( - a1.Executable.AllowedLibraries, - a2.Executable.AllowedLibraries, - ) - } else if a2.Executable != nil { - a1.Executable = a2.Executable - } - - mergeFilesystem(a1, a2) - - if a1.Network != nil && a2.Network != nil { - a1.Network.AllowRaw = mergeBools(a1.Network.AllowRaw, a2.Network.AllowRaw) - - if a1.Network.Protocols != nil && a2.Network.Protocols != nil { - a1.Network.Protocols.AllowTCP = mergeBools(a1.Network.Protocols.AllowTCP, a2.Network.Protocols.AllowTCP) - a1.Network.Protocols.AllowUDP = mergeBools(a1.Network.Protocols.AllowUDP, a2.Network.Protocols.AllowUDP) - } else if a2.Network.Protocols != nil { - a1.Network.Protocols = a2.Network.Protocols - } - } else if a2.Network != nil { - a1.Network = a2.Network - } - - if a1.Capability != nil && a2.Capability != nil { - a1.Capability.AllowedCapabilities = *mergeDedupSortStrings( - &a1.Capability.AllowedCapabilities, - &a2.Capability.AllowedCapabilities, - ) - } else if a2.Capability != nil { - a1.Capability = a2.Capability - } - - return nil -} - -func mergeFilesystem(a1, a2 *apparmorprofileapi.AppArmorAbstract) { - if a1.Filesystem != nil && a2.Filesystem != nil { - read := make(map[string]bool) - write := make(map[string]bool) - for _, fs := range []apparmorprofileapi.AppArmorFsRules{*a1.Filesystem, *a2.Filesystem} { - if fs.ReadOnlyPaths != nil { - for _, p := range *fs.ReadOnlyPaths { - read[p] = true - } - } - if fs.WriteOnlyPaths != nil { - for _, p := range *fs.WriteOnlyPaths { - write[p] = true - } - } - if fs.ReadWritePaths != nil { - for _, p := range *fs.ReadWritePaths { - read[p] = true - write[p] = true - } - } - } - - r := make([]string, 0, len(read)) - w := make([]string, 0, len(write)) - rw := make([]string, 0, min(len(read), len(write))) - - for path := range read { - if _, hasWrite := write[path]; hasWrite { - rw = append(rw, path) - delete(write, path) - } else { - r = append(r, path) - } - } - for path := range write { - w = append(w, path) - } - - sort.Strings(r) - sort.Strings(w) - sort.Strings(rw) - - a1.Filesystem = &apparmorprofileapi.AppArmorFsRules{ - ReadOnlyPaths: &r, - WriteOnlyPaths: &w, - ReadWritePaths: &rw, - } - // omitempty is not sufficient here. - if len(*a1.Filesystem.ReadOnlyPaths) == 0 { - a1.Filesystem.ReadOnlyPaths = nil - } - if len(*a1.Filesystem.WriteOnlyPaths) == 0 { - a1.Filesystem.WriteOnlyPaths = nil - } - if len(*a1.Filesystem.ReadWritePaths) == 0 { - a1.Filesystem.ReadWritePaths = nil - } - } else if a2.Filesystem != nil { - a1.Filesystem = a2.Filesystem - } -} - -func mergeBools(a, b *bool) *bool { - if a == nil { - return b - } - if b == nil { - return a - } - merged := (*a || *b) - return &merged -} - -func mergeDedupSortStrings(a, b *[]string) *[]string { - if a == nil { - return b - } - if b == nil { - return a - } - merged := append(*a, *b...) - sort.Strings(merged) - merged = compact(merged) - return &merged -} - -// TODO: replace this with slices.Compact once all platform support Go 1.21. -func compact(s []string) []string { - //nolint:all - if len(s) < 2 { - return s - } - i := 1 - for k := 1; k < len(s); k++ { - if s[k] != s[k-1] { - if i != k { - s[i] = s[k] - } - i++ - } - } - return s[:i] -} - -// TODO: remove once all platform support Go 1.21. -func min(a, b int) int { - if a < b { - return a - } - return b -} diff --git a/internal/pkg/manager/recordingmerger/merge_utils_test.go b/internal/pkg/manager/recordingmerger/merge_utils_test.go index d0de817030..389049f680 100644 --- a/internal/pkg/manager/recordingmerger/merge_utils_test.go +++ b/internal/pkg/manager/recordingmerger/merge_utils_test.go @@ -191,6 +191,9 @@ func TestMergeProfiles(t *testing.T) { Network: &apparmorprofileapi.AppArmorNetworkRules{ AllowRaw: func() *bool { b := true; return &b }(), }, + Capability: &apparmorprofileapi.AppArmorCapabilityRules{ + AllowedCapabilities: []string{"sys_admin", "net_admin"}, + }, }, }, }, @@ -213,6 +216,9 @@ func TestMergeProfiles(t *testing.T) { AllowTCP: func() *bool { b := true; return &b }(), }, }, + Capability: &apparmorprofileapi.AppArmorCapabilityRules{ + AllowedCapabilities: []string{"net_admin", "net_raw"}, + }, }, }, }, @@ -240,6 +246,9 @@ func TestMergeProfiles(t *testing.T) { AllowTCP: func() *bool { b := true; return &b }(), }, }, + Capability: &apparmorprofileapi.AppArmorCapabilityRules{ + AllowedCapabilities: []string{"net_admin", "net_raw", "sys_admin"}, + }, }) return nil },