diff --git a/pkg/seccomp/BUILD b/pkg/seccomp/BUILD index fc19e666c..480945236 100644 --- a/pkg/seccomp/BUILD +++ b/pkg/seccomp/BUILD @@ -11,6 +11,7 @@ go_library( "seccomp.go", "seccomp_amd64.go", "seccomp_arm64.go", + "seccomp_optimizer.go", "seccomp_rules.go", "seccomp_unsafe.go", ], diff --git a/pkg/seccomp/seccomp.go b/pkg/seccomp/seccomp.go index 8239710ed..50a3574f6 100644 --- a/pkg/seccomp/seccomp.go +++ b/pkg/seccomp/seccomp.go @@ -473,7 +473,7 @@ func buildBSTProgram(n *node, rules []RuleSet, program *syscallProgram) error { // check the next rule set. We need to ensure // that at the very end, we insert a direct // jump label for the unmatched case. - rule.Render(program, ruleSetLabelSet) + optimizeSyscallRule(rule).Render(program, ruleSetLabelSet) frag.MustHaveJumpedTo(ruleSetLabelSet.Matched(), ruleSetLabelSet.Mismatched()) program.Label(ruleSetLabelSet.Matched()) program.Ret(rs.Action) diff --git a/pkg/seccomp/seccomp_optimizer.go b/pkg/seccomp/seccomp_optimizer.go new file mode 100644 index 000000000..9f7a7e6fa --- /dev/null +++ b/pkg/seccomp/seccomp_optimizer.go @@ -0,0 +1,139 @@ +// Copyright 2023 The gVisor 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 seccomp + +// ruleOptimizerFunc is a function type that can optimize a SyscallRule. +// It returns the updated SyscallRule, along with whether any modification +// was made. +type ruleOptimizerFunc func(SyscallRule) (SyscallRule, bool) + +// convertSingleCompoundRuleToThatRule replaces `Or` or `And` rules with a +// single branch to just that branch. +func convertSingleCompoundRuleToThatRule[T Or | And](rule SyscallRule) (SyscallRule, bool) { + if tRule, isT := rule.(T); isT && len(tRule) == 1 { + return tRule[0], true + } + return rule, false +} + +// flattenCompoundRules turns compound rules (Or or And) embedded inside +// compound rules of the same type into a flat rule of that type. +func flattenCompoundRules[T Or | And](rule SyscallRule) (SyscallRule, bool) { + tRule, isT := rule.(T) + if !isT { + return rule, false + } + anySubT := false + for _, subRule := range tRule { + if _, subIsT := subRule.(T); subIsT { + anySubT = true + break + } + } + if !anySubT { + return rule, false + } + var newRules []SyscallRule + for _, subRule := range tRule { + if subT, subIsT := subRule.(T); subIsT { + newRules = append(newRules, subT...) + } else { + newRules = append(newRules, subRule) + } + } + return SyscallRule(T(newRules)), true +} + +// convertMatchAllOrXToMatchAll an Or rule that contains MatchAll to MatchAll. +func convertMatchAllOrXToMatchAll(rule SyscallRule) (SyscallRule, bool) { + orRule, isOr := rule.(Or) + if !isOr { + return rule, false + } + for _, subRule := range orRule { + if _, subIsMatchAll := subRule.(MatchAll); subIsMatchAll { + return MatchAll{}, true + } + } + return orRule, false +} + +// convertMatchAllAndXToX removes MatchAll clauses from And rules. +func convertMatchAllAndXToX(rule SyscallRule) (SyscallRule, bool) { + andRule, isAnd := rule.(And) + if !isAnd { + return rule, false + } + hasMatchAll := false + for _, subRule := range andRule { + if _, subIsMatchAll := subRule.(MatchAll); subIsMatchAll { + hasMatchAll = true + break + } + } + if !hasMatchAll { + return rule, false + } + var newRules []SyscallRule + for _, subRule := range andRule { + if _, subIsAny := subRule.(MatchAll); !subIsAny { + newRules = append(newRules, subRule) + } + } + if len(newRules) == 0 { + // An `And` rule with zero rules inside is invalid. + return MatchAll{}, true + } + return And(newRules), true +} + +// optimizeSyscallRuleFuncs losslessly optimizes a SyscallRule using the given +// optimization functions. +// Optimizers should be ranked in order of importance, with the most +// important first. +// An optimizer will be exhausted before the next one is ever run. +// Earlier optimizers are re-exhausted if later optimizers cause change. +func optimizeSyscallRuleFuncs(rule SyscallRule, funcs []ruleOptimizerFunc) SyscallRule { + for changed := true; changed; { + for _, fn := range funcs { + rule.Recurse(func(subRule SyscallRule) SyscallRule { + return optimizeSyscallRuleFuncs(subRule, funcs) + }) + if rule, changed = fn(rule); changed { + break + } + } + } + return rule +} + +// optimizeSyscallRule losslessly optimizes a `SyscallRule`. +func optimizeSyscallRule(rule SyscallRule) SyscallRule { + return optimizeSyscallRuleFuncs(rule, []ruleOptimizerFunc{ + // Convert Or / And rules with a single rule into that single rule. + convertSingleCompoundRuleToThatRule[Or], + convertSingleCompoundRuleToThatRule[And], + + // Flatten Or/And rules. + flattenCompoundRules[Or], + flattenCompoundRules[And], + + // Handle MatchAll. This is best done after flattening so that we + // effectively traverse the whole tree to find a MatchAll by just + // linearly scanning through the first (and only) level of rules. + convertMatchAllOrXToMatchAll, + convertMatchAllAndXToX, + }) +} diff --git a/pkg/seccomp/seccomp_rules.go b/pkg/seccomp/seccomp_rules.go index 10c6a824b..0c9d9bec1 100644 --- a/pkg/seccomp/seccomp_rules.go +++ b/pkg/seccomp/seccomp_rules.go @@ -321,6 +321,13 @@ type SyscallRule interface { // next into the program. Render(program *syscallProgram, labelSet *labelSet) + // Recurse should call the given function on all `SyscallRule`s that are + // part of this `SyscallRule`, and should replace them with the returned + // `SyscallRule`. For example, conjunctive rules should call the given + // function on each of the `SyscallRule`s that they are ANDing, replacing + // them with the rule returned by the function. + Recurse(func(SyscallRule) SyscallRule) + // String returns a human-readable string representing what the rule does. String() string } @@ -333,6 +340,9 @@ func (MatchAll) Render(program *syscallProgram, labelSet *labelSet) { program.JumpTo(labelSet.Matched()) } +// Recurse implements `SyscallRule.Recurse`. +func (MatchAll) Recurse(func(SyscallRule) SyscallRule) {} + // String implements `SyscallRule.String`. func (MatchAll) String() string { return "true" } @@ -357,6 +367,13 @@ func (or Or) Render(program *syscallProgram, labelSet *labelSet) { program.JumpTo(labelSet.Mismatched()) } +// Recurse implements `SyscallRule.Recurse`. +func (or Or) Recurse(fn func(SyscallRule) SyscallRule) { + for i, rule := range or { + or[i] = fn(rule) + } +} + // String implements `SyscallRule.String`. func (or Or) String() string { switch len(or) { @@ -399,6 +416,13 @@ func (and And) Render(program *syscallProgram, labelSet *labelSet) { program.JumpTo(labelSet.Matched()) } +// Recurse implements `SyscallRule.Recurse`. +func (and And) Recurse(fn func(SyscallRule) SyscallRule) { + for i, rule := range and { + and[i] = fn(rule) + } +} + // String implements `SyscallRule.String`. func (and And) String() string { switch len(and) { @@ -420,27 +444,6 @@ func (and And) String() string { } } -// merge merges `rule1` and `rule2`, simplifying `MatchAll` and `Or` rules. -func merge(rule1, rule2 SyscallRule) SyscallRule { - _, rule1IsMatchAll := rule1.(MatchAll) - _, rule2IsMatchAll := rule2.(MatchAll) - if rule1IsMatchAll || rule2IsMatchAll { - return MatchAll{} - } - rule1Or, rule1IsOr := rule1.(Or) - rule2Or, rule2IsOr := rule2.(Or) - if rule1IsOr && rule2IsOr { - return append(rule1Or, rule2Or...) - } - if rule1IsOr { - return append(rule1Or, rule2) - } - if rule2IsOr { - return append(rule2Or, rule1) - } - return Or{rule1, rule2} -} - // PerArg implements SyscallRule and verifies the syscall arguments and RIP. // // For example: @@ -484,6 +487,9 @@ func (pa PerArg) Render(program *syscallProgram, labelSet *labelSet) { program.JumpTo(labelSet.Matched()) } +// Recurse implements `SyscallRule.Recurse`. +func (PerArg) Recurse(fn func(SyscallRule) SyscallRule) {} + // String implements `SyscallRule.String`. func (pa PerArg) String() (s string) { if len(pa) == 0 { @@ -574,7 +580,7 @@ func (sr SyscallRules) Has(sysno uintptr) bool { // Returns itself for chainability. func (sr SyscallRules) Add(sysno uintptr, r SyscallRule) SyscallRules { if cur, ok := sr.rules[sysno]; ok { - sr.rules[sysno] = merge(cur, r) + sr.rules[sysno] = Or{cur, r} } else { sr.rules[sysno] = r } diff --git a/pkg/seccomp/seccomp_test.go b/pkg/seccomp/seccomp_test.go index 41364b473..d10c2a55f 100644 --- a/pkg/seccomp/seccomp_test.go +++ b/pkg/seccomp/seccomp_test.go @@ -1075,28 +1075,28 @@ func TestMerge(t *testing.T) { want SyscallRule }{ { - name: "AllowAll both", + name: "MatchAll both", main: MatchAll{}, merge: MatchAll{}, - want: MatchAll{}, + want: Or{MatchAll{}, MatchAll{}}, }, { - name: "AllowAll and Or", + name: "MatchAll and Or", main: MatchAll{}, - merge: Or{}, - want: MatchAll{}, + merge: Or{PerArg{EqualTo(0)}}, + want: Or{MatchAll{}, Or{PerArg{EqualTo(0)}}}, }, { - name: "Or and AllowAll", - main: Or{}, + name: "Or and MatchAll", + main: Or{PerArg{EqualTo(0)}}, merge: MatchAll{}, - want: MatchAll{}, + want: Or{Or{PerArg{EqualTo(0)}}, MatchAll{}}, }, { name: "2 Ors", main: Or{PerArg{EqualTo(0)}}, merge: Or{PerArg{EqualTo(1)}}, - want: Or{PerArg{EqualTo(0)}, PerArg{EqualTo(1)}}, + want: Or{Or{PerArg{EqualTo(0)}}, Or{PerArg{EqualTo(1)}}}, }, } { t.Run(tst.name, func(t *testing.T) { @@ -1112,3 +1112,142 @@ func TestMerge(t *testing.T) { }) } } + +// TestOptimizeSyscallRule tests the behavior of syscall rule optimizers. +func TestOptimizeSyscallRule(t *testing.T) { + for _, test := range []struct { + name string + rule SyscallRule + optimizers []ruleOptimizerFunc + want SyscallRule + }{ + { + name: "do nothing to a simple rule", + rule: PerArg{NotEqual(0xff)}, + want: PerArg{NotEqual(0xff)}, + }, + { + name: "flatten Or rule", + rule: Or{ + Or{ + PerArg{EqualTo(0x11)}, + Or{ + PerArg{EqualTo(0x22)}, + PerArg{EqualTo(0x33)}, + }, + PerArg{EqualTo(0x44)}, + }, + Or{ + PerArg{EqualTo(0x55)}, + PerArg{EqualTo(0x66)}, + }, + }, + want: Or{ + PerArg{EqualTo(0x11)}, + PerArg{EqualTo(0x22)}, + PerArg{EqualTo(0x33)}, + PerArg{EqualTo(0x44)}, + PerArg{EqualTo(0x55)}, + PerArg{EqualTo(0x66)}, + }, + }, + { + name: "flatten And rule", + rule: And{ + And{ + PerArg{NotEqual(0x11)}, + And{ + PerArg{NotEqual(0x22)}, + PerArg{NotEqual(0x33)}, + }, + PerArg{NotEqual(0x44)}, + }, + And{ + PerArg{NotEqual(0x55)}, + PerArg{NotEqual(0x66)}, + }, + }, + want: And{ + PerArg{NotEqual(0x11)}, + PerArg{NotEqual(0x22)}, + PerArg{NotEqual(0x33)}, + PerArg{NotEqual(0x44)}, + PerArg{NotEqual(0x55)}, + PerArg{NotEqual(0x66)}, + }, + }, + { + name: "simplify Or with single rule", + rule: Or{ + PerArg{EqualTo(0x11)}, + }, + want: PerArg{EqualTo(0x11)}, + }, + { + name: "simplify And with single rule", + rule: And{ + PerArg{EqualTo(0x11)}, + }, + want: PerArg{EqualTo(0x11)}, + }, + { + name: "simplify Or with MatchAll", + rule: Or{ + PerArg{EqualTo(0x11)}, + Or{ + MatchAll{}, + }, + PerArg{EqualTo(0x22)}, + }, + want: MatchAll{}, + }, + { + name: "single MatchAll in Or is not an empty rule", + rule: Or{ + MatchAll{}, + MatchAll{}, + }, + optimizers: []ruleOptimizerFunc{ + convertMatchAllOrXToMatchAll, + }, + want: MatchAll{}, + }, + { + name: "simplify And with MatchAll", + rule: And{ + PerArg{NotEqual(0x11)}, + And{ + MatchAll{}, + }, + PerArg{NotEqual(0x22)}, + }, + want: And{ + PerArg{NotEqual(0x11)}, + PerArg{NotEqual(0x22)}, + }, + }, + { + name: "single MatchAll in And is not optimized to an empty rule", + rule: And{ + MatchAll{}, + MatchAll{}, + }, + optimizers: []ruleOptimizerFunc{ + convertMatchAllAndXToX, + }, + want: MatchAll{}, + }, + } { + t.Run(test.name, func(t *testing.T) { + var got SyscallRule + if len(test.optimizers) == 0 { + got = optimizeSyscallRule(test.rule) + } else { + got = optimizeSyscallRuleFuncs(test.rule, test.optimizers) + } + if !reflect.DeepEqual(got, test.want) { + t.Errorf("got rule:\n%v\nwant rule:\n%v\n", got, test.want) + } + }) + } +}