From 6eed17ce4b931c0e63f5bab60f64438c5d86cb48 Mon Sep 17 00:00:00 2001 From: Etienne Perot Date: Wed, 15 Nov 2023 20:29:22 -0800 Subject: [PATCH] `seccomp`: Add fuzz test for Sentry syscall filters. This ensures that the optimized and unoptimized seccomp programs are equivalent in behavior. Full coverage is enforced on the optimized program. It cannot be enforced on the unoptimized program, because it naturally ends up generating code that can never be satisfied. PiperOrigin-RevId: 582892981 --- pkg/seccomp/seccomp.go | 44 ++++++++++----- pkg/seccomp/seccomp_test.go | 5 +- runsc/boot/filter/BUILD | 12 ++++ runsc/boot/filter/filter_fuzz_test.go | 79 +++++++++++++++++++++++++++ 4 files changed, 123 insertions(+), 17 deletions(-) create mode 100644 runsc/boot/filter/filter_fuzz_test.go diff --git a/pkg/seccomp/seccomp.go b/pkg/seccomp/seccomp.go index 60c1047ad..8b337f8ee 100644 --- a/pkg/seccomp/seccomp.go +++ b/pkg/seccomp/seccomp.go @@ -309,6 +309,10 @@ type ProgramOptions struct { // syscall structure input doesn't match the one the program expects. BadArchAction linux.BPFAction + // Optimize specifies whether optimizations should be applied to the + // syscall rules and generated BPF bytecode. + Optimize bool + // HotSyscalls is the set of syscall numbers that are the hottest, // where "hotness" refers to frequency (regardless of the amount of // computation that the kernel will do handling them, and regardless of @@ -328,6 +332,7 @@ func DefaultProgramOptions() ProgramOptions { return ProgramOptions{ DefaultAction: action, BadArchAction: action, + Optimize: true, } } @@ -354,13 +359,11 @@ type BuildStats struct { // SyscallRules. The single generated program covers all provided RuleSets. func BuildProgram(rules []RuleSet, options ProgramOptions) ([]bpf.Instruction, BuildStats, error) { start := time.Now() - - // Make a copy of the syscall rules and optimize them. - ors, err := orderRuleSets(rules, options) + // Make a copy of the syscall rules and maybe optimize them. + ors, ruleOptimizeDuration, err := orderRuleSets(rules, options) if err != nil { return nil, BuildStats{}, err } - ruleOptimizeDuration := time.Since(start) possibleActions := make(map[linux.BPFAction]struct{}) for _, ruleSet := range rules { @@ -398,10 +401,14 @@ func BuildProgram(rules []RuleSet, options ProgramOptions) ([]bpf.Instruction, B } beforeOpt := len(insns) buildDuration := time.Since(start) - ruleOptimizeDuration - insns = bpf.Optimize(insns) - bpfOptimizeDuration := time.Since(start) - ruleOptimizeDuration - buildDuration - afterOpt := len(insns) - log.Debugf("Seccomp program optimized from %d to %d instructions; took %v to build and %v to optimize", beforeOpt, afterOpt, buildDuration, bpfOptimizeDuration) + var bpfOptimizeDuration time.Duration + afterOpt := beforeOpt + if options.Optimize { + insns = bpf.Optimize(insns) + bpfOptimizeDuration = time.Since(start) - buildDuration - ruleOptimizeDuration + afterOpt = len(insns) + log.Debugf("Seccomp program optimized from %d to %d instructions; took %v to build and %v to optimize", beforeOpt, afterOpt, buildDuration, bpfOptimizeDuration) + } return insns, BuildStats{ SizeBeforeOptimizations: beforeOpt, SizeAfterOptimizations: afterOpt, @@ -530,14 +537,16 @@ type orderedRuleSets struct { } // orderRuleSets converts a set of `RuleSet`s into an `orderedRuleSets`. -func orderRuleSets(rules []RuleSet, options ProgramOptions) (orderedRuleSets, error) { +// It orders the rulesets, along with the time to optimize the +// rules (if any). +func orderRuleSets(rules []RuleSet, options ProgramOptions) (orderedRuleSets, time.Duration, error) { // Do a pass to determine if vsyscall is consistent across syscall numbers. vsyscallBySysno := make(map[uintptr]bool) for _, rs := range rules { for sysno := range rs.Rules.rules { if prevVsyscall, ok := vsyscallBySysno[sysno]; ok { if prevVsyscall != rs.Vsyscall { - return orderedRuleSets{}, fmt.Errorf("syscall %d has conflicting vsyscall checking rules", sysno) + return orderedRuleSets{}, 0, fmt.Errorf("syscall %d has conflicting vsyscall checking rules", sysno) } } else { vsyscallBySysno[sysno] = rs.Vsyscall @@ -573,11 +582,16 @@ func orderRuleSets(rules []RuleSet, options ProgramOptions) (orderedRuleSets, er } // Optimize all rules. - for _, ruleActions := range allSyscallRuleActions { - for i, ra := range ruleActions { - ra.rule = optimizeSyscallRule(ra.rule) - ruleActions[i] = ra + var optimizeDuration time.Duration + if options.Optimize { + optimizeStart := time.Now() + for _, ruleActions := range allSyscallRuleActions { + for i, ra := range ruleActions { + ra.rule = optimizeSyscallRule(ra.rule) + ruleActions[i] = ra + } } + optimizeDuration = time.Since(optimizeStart) } // Do a pass that checks which syscall numbers are trivial. @@ -648,7 +662,7 @@ func orderRuleSets(rules []RuleSet, options ProgramOptions) (orderedRuleSets, er ors.log(log.Debugf) } - return ors, nil + return ors, optimizeDuration, nil } // log logs the set of seccomp rules to the given logger. diff --git a/pkg/seccomp/seccomp_test.go b/pkg/seccomp/seccomp_test.go index 79f7e8324..c51c66579 100644 --- a/pkg/seccomp/seccomp_test.go +++ b/pkg/seccomp/seccomp_test.go @@ -1383,7 +1383,7 @@ func TestOrderRuleSets(t *testing.T) { for _, test := range []struct { name string ruleSets []RuleSet - options ProgramOptions + options ProgramOptions // Optimizations are always enabled regardless of this want orderedRuleSets wantErr bool }{ @@ -1896,7 +1896,8 @@ func TestOrderRuleSets(t *testing.T) { }, } { t.Run(test.name, func(t *testing.T) { - got, gotErr := orderRuleSets(test.ruleSets, test.options) + test.options.Optimize = true + got, _, gotErr := orderRuleSets(test.ruleSets, test.options) if (gotErr != nil) != test.wantErr { t.Errorf("got error: %v, want error: %v", gotErr, test.wantErr) } diff --git a/runsc/boot/filter/BUILD b/runsc/boot/filter/BUILD index 8df9f76e3..194e20493 100644 --- a/runsc/boot/filter/BUILD +++ b/runsc/boot/filter/BUILD @@ -64,3 +64,15 @@ secbench_test( "@org_golang_x_sys//unix:go_default_library", ], ) + +go_test( + name = "filter_fuzz_test", + srcs = ["filter_fuzz_test.go"], + deps = [ + ":filter", + "//pkg/abi/linux", + "//pkg/seccomp", + "//pkg/sentry/platform/systrap", + "//test/secfuzz", + ], +) diff --git a/runsc/boot/filter/filter_fuzz_test.go b/runsc/boot/filter/filter_fuzz_test.go new file mode 100644 index 000000000..b436c99cb --- /dev/null +++ b/runsc/boot/filter/filter_fuzz_test.go @@ -0,0 +1,79 @@ +// 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 filter_fuzz_test + +import ( + "testing" + + "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/seccomp" + "gvisor.dev/gvisor/pkg/sentry/platform/systrap" + "gvisor.dev/gvisor/runsc/boot/filter" + "gvisor.dev/gvisor/test/secfuzz" +) + +// FuzzFilterOptimizationsResultInConsistentProgram tests that optimizations +// do not affect the behavior of the generated seccomp-bpf program. +func FuzzFilterOptimizationsResultInConsistentProgram(f *testing.F) { + filterOpts := filter.Options{ + Platform: &systrap.Systrap{}, + } + rules, denyRules := filter.Rules(filterOpts) + ruleSets := []seccomp.RuleSet{ + { + Rules: denyRules, + Action: linux.SECCOMP_RET_ERRNO, + }, + { + Rules: rules, + Action: linux.SECCOMP_RET_ALLOW, + }, + } + unoptimizedOpts := filter.SeccompOptions(filterOpts) + unoptimizedOpts.Optimize = false + unoptimized, _, err := seccomp.BuildProgram(ruleSets, unoptimizedOpts) + if err != nil { + f.Fatalf("failed to build unoptimized program: %v", err) + } + fuzzeeUnoptimized := secfuzz.Fuzzee{ + Name: "unoptimized", + Instructions: unoptimized, + + // We cannot enforce full coverage on the unoptimized program, + // because some of its checks are impossible to meet. + // For example, it ends up checking things like + // "if (A & 0) == 0" when checking both 32-bit halves of a + // "masked equal" check, and the "false" branch of that can + // never be covered. + EnforceFullCoverage: false, + } + optimizedOpts := filter.SeccompOptions(filterOpts) + optimizedOpts.Optimize = true + optimized, _, err := seccomp.BuildProgram(ruleSets, optimizedOpts) + if err != nil { + f.Fatalf("failed to build optimized program: %v", err) + } + fuzzeeOptimized := secfuzz.Fuzzee{ + Name: "optimized", + Instructions: optimized, + EnforceFullCoverage: true, + } + df, err := secfuzz.NewDiffFuzzer(f, &fuzzeeUnoptimized, &fuzzeeOptimized) + if err != nil { + f.Fatalf("failed to create diff fuzzer: %v", err) + } + df.DeriveCorpusFromRuleSets(ruleSets) + df.Fuzz() +}