diff --git a/pkg/sentry/kernel/task_syscall.go b/pkg/sentry/kernel/task_syscall.go index edd36e519..48be595f2 100644 --- a/pkg/sentry/kernel/task_syscall.go +++ b/pkg/sentry/kernel/task_syscall.go @@ -182,7 +182,6 @@ func (t *Task) executeSyscall(sysno uintptr, args arch.SyscallArguments) (rval u }) } if seccheck.Global.SyscallEnabled(seccheck.SyscallExit, sysno) { - cb := t.SyscallTable().LookupSyscallToProto(sysno) fields := seccheck.Global.GetFieldSet(seccheck.GetPointForSyscall(seccheck.SyscallExit, sysno)) var ctxData *pb.ContextData if !fields.Context.Empty() { @@ -196,6 +195,7 @@ func (t *Task) executeSyscall(sysno uintptr, args arch.SyscallArguments) (rval u Rval: rval, Errno: ExtractErrno(err, int(sysno)), } + cb := t.SyscallTable().LookupSyscallToProto(sysno) msg, msgType := cb(t, fields, ctxData, info) seccheck.Global.SendToCheckers(func(c seccheck.Checker) error { return c.Syscall(t, fields, ctxData, msgType, msg) diff --git a/pkg/sentry/seccheck/checkers/null/BUILD b/pkg/sentry/seccheck/checkers/null/BUILD new file mode 100644 index 000000000..1136dc5bc --- /dev/null +++ b/pkg/sentry/seccheck/checkers/null/BUILD @@ -0,0 +1,13 @@ +load("//tools:defs.bzl", "go_library") + +package(licenses = ["notice"]) + +go_library( + name = "null", + srcs = ["null.go"], + visibility = ["//:sandbox"], + deps = [ + "//pkg/fd", + "//pkg/sentry/seccheck", + ], +) diff --git a/pkg/sentry/seccheck/checkers/null/null.go b/pkg/sentry/seccheck/checkers/null/null.go new file mode 100644 index 000000000..9837952cd --- /dev/null +++ b/pkg/sentry/seccheck/checkers/null/null.go @@ -0,0 +1,40 @@ +// Copyright 2021 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 null defines a seccheck.Checker that does nothing with the trace +// points, akin to /dev/null. +package null + +import ( + "gvisor.dev/gvisor/pkg/fd" + "gvisor.dev/gvisor/pkg/sentry/seccheck" +) + +func init() { + seccheck.RegisterSink(seccheck.SinkDesc{ + Name: "null", + New: new, + }) +} + +// null is a checker that does nothing with the trace points. +type null struct { + seccheck.CheckerDefaults +} + +var _ seccheck.Checker = (*null)(nil) + +func new(_ map[string]interface{}, _ *fd.FD) (seccheck.Checker, error) { + return &null{}, nil +} diff --git a/pkg/sentry/seccheck/syscall.go b/pkg/sentry/seccheck/syscall.go index 43d0cce84..4dea53546 100644 --- a/pkg/sentry/seccheck/syscall.go +++ b/pkg/sentry/seccheck/syscall.go @@ -63,5 +63,9 @@ func GetPointForSyscall(typ SyscallType, sysno uintptr) Point { // SyscallEnabled checks if the corresponding point for the syscall is enabled. func (s *State) SyscallEnabled(typ SyscallType, sysno uintptr) bool { + // Prevent overflow. + if sysno >= syscallsMax { + return false + } return s.Enabled(GetPointForSyscall(typ, sysno)) } diff --git a/runsc/boot/BUILD b/runsc/boot/BUILD index 5a7782c97..912c2faac 100644 --- a/runsc/boot/BUILD +++ b/runsc/boot/BUILD @@ -73,6 +73,7 @@ go_library( "//pkg/sentry/pgalloc", "//pkg/sentry/platform", "//pkg/sentry/seccheck", + "//pkg/sentry/seccheck/checkers/null", "//pkg/sentry/seccheck/checkers/remote", "//pkg/sentry/seccheck/points:points_go_proto", "//pkg/sentry/socket/hostinet", diff --git a/runsc/boot/seccheck.go b/runsc/boot/seccheck.go index df4a49ff0..5d9363820 100644 --- a/runsc/boot/seccheck.go +++ b/runsc/boot/seccheck.go @@ -23,6 +23,7 @@ import ( "gvisor.dev/gvisor/pkg/sentry/seccheck" // Register supported of checkers. + _ "gvisor.dev/gvisor/pkg/sentry/seccheck/checkers/null" _ "gvisor.dev/gvisor/pkg/sentry/seccheck/checkers/remote" ) diff --git a/test/runner/BUILD b/test/runner/BUILD index 7c6bebd3b..084cdbf5a 100644 --- a/test/runner/BUILD +++ b/test/runner/BUILD @@ -13,9 +13,11 @@ go_binary( visibility = ["//:sandbox"], deps = [ "//pkg/log", + "//pkg/sentry/seccheck", "//pkg/test/testutil", "//runsc/specutils", "//test/runner/gtest", + "//test/trace/config", "//test/uds", "@com_github_opencontainers_runtime_spec//specs-go:go_default_library", "@com_github_syndtr_gocapability//capability:go_default_library", diff --git a/test/runner/defs.bzl b/test/runner/defs.bzl index 7cbae734d..a14818c15 100644 --- a/test/runner/defs.bzl +++ b/test/runner/defs.bzl @@ -145,6 +145,10 @@ def _syscall_test( "--container=" + str(container), ] + # Trace points are platform agnostic, so enable them for ptrace only. + if platform == "ptrace": + runner_args.append("--trace") + # Call the rule above. _runner_test( name = name, diff --git a/test/runner/main.go b/test/runner/main.go index 313d9939f..97f8b96a4 100644 --- a/test/runner/main.go +++ b/test/runner/main.go @@ -33,9 +33,11 @@ import ( "github.com/syndtr/gocapability/capability" "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/log" + "gvisor.dev/gvisor/pkg/sentry/seccheck" "gvisor.dev/gvisor/pkg/test/testutil" "gvisor.dev/gvisor/runsc/specutils" "gvisor.dev/gvisor/test/runner/gtest" + "gvisor.dev/gvisor/test/trace/config" "gvisor.dev/gvisor/test/uds" ) @@ -52,6 +54,7 @@ var ( lisafs = flag.Bool("lisafs", false, "enable lisafs protocol if vfs2 is also enabled") container = flag.Bool("container", false, "run tests in their own namespaces (user ns, network ns, etc), pretending to be root. Implicitly enabled if network=host, or if using network namespaces") setupContainerPath = flag.String("setup-container", "", "path to setup_container binary (for use with --container)") + trace = flag.Bool("trace", false, "enables all trace points") addUDSTree = flag.Bool("add-uds-tree", false, "expose a tree of UDS utilities for use in tests") // TODO(gvisor.dev/issue/4572): properly support leak checking for runsc, and @@ -217,6 +220,14 @@ func runRunsc(tc gtest.TestCase, spec *specs.Spec) error { if *leakCheck { args = append(args, "-ref-leak-mode=log-names") } + if *trace { + flag, err := enableAllTraces(rootDir) + if err != nil { + return fmt.Errorf("enabling all traces: %w", err) + } + log.Infof("Enabling all trace points: %s", flag) + args = append(args, flag) + } testLogDir := "" if undeclaredOutputsDir, ok := unix.Getenv("TEST_UNDECLARED_OUTPUTS_DIR"); ok { @@ -562,3 +573,24 @@ func main() { testing.Main(matchString, tests, nil, nil) } + +func enableAllTraces(dir string) (string, error) { + builder := config.Builder{} + if err := builder.LoadAllPoints(specutils.ExePath); err != nil { + return "", err + } + builder.AddSink(seccheck.SinkConfig{ + Name: "null", + }) + path := filepath.Join(dir, "pod_init.json") + cfgFile, err := os.Create(path) + if err != nil { + return "", err + } + defer cfgFile.Close() + + if err := builder.WriteInitConfig(cfgFile); err != nil { + return "", fmt.Errorf("writing config file: %w", err) + } + return "--pod-init-config=" + path, nil +} diff --git a/test/trace/BUILD b/test/trace/BUILD index bd3564de8..74597ab6d 100644 --- a/test/trace/BUILD +++ b/test/trace/BUILD @@ -20,7 +20,7 @@ go_test( "//pkg/sentry/seccheck/checkers/remote/test", "//pkg/sentry/seccheck/points:points_go_proto", "//pkg/test/testutil", - "//runsc/boot", + "//test/trace/config", "@org_golang_google_protobuf//proto:go_default_library", ], ) diff --git a/test/trace/config/BUILD b/test/trace/config/BUILD new file mode 100644 index 000000000..b87bf4b53 --- /dev/null +++ b/test/trace/config/BUILD @@ -0,0 +1,14 @@ +load("//tools:defs.bzl", "go_library") + +package(licenses = ["notice"]) + +go_library( + name = "config", + testonly = 1, + srcs = ["config.go"], + visibility = ["//:sandbox"], + deps = [ + "//pkg/sentry/seccheck", + "//runsc/boot", + ], +) diff --git a/test/trace/config/config.go b/test/trace/config/config.go new file mode 100644 index 000000000..c3f4343d7 --- /dev/null +++ b/test/trace/config/config.go @@ -0,0 +1,110 @@ +// Copyright 2022 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 config providides helper functions to configure trace sessions. +package config + +import ( + "bufio" + "bytes" + "encoding/json" + "fmt" + "io" + "os/exec" + "strings" + + "gvisor.dev/gvisor/pkg/sentry/seccheck" + "gvisor.dev/gvisor/runsc/boot" +) + +// Builder helps with building of trace session configuration. +type Builder struct { + points []seccheck.PointConfig + sinks []seccheck.SinkConfig +} + +// WriteInitConfig writes the current configuration in a format compatible with +// the flag --pod-init-config. +func (b *Builder) WriteInitConfig(w io.Writer) error { + init := &boot.InitConfig{ + TraceSession: seccheck.SessionConfig{ + Name: seccheck.DefaultSessionName, + Points: b.points, + Sinks: b.sinks, + }, + } + + encoder := json.NewEncoder(w) + return encoder.Encode(&init) +} + +// LoadAllPoints enables all points together with all optional and context +// fields. +func (b *Builder) LoadAllPoints(runscPath string) error { + cmd := exec.Command(runscPath, "trace", "metadata") + out, err := cmd.CombinedOutput() + if err != nil { + return err + } + + // The command above produces an output like the following: + // POINTS (907) + // Name: container/start, optional fields: [], context fields: [time|thread_id] + scanner := bufio.NewScanner(bytes.NewReader(out)) + if !scanner.Scan() { + return fmt.Errorf("%q returned empty", cmd) + } + if !scanner.Scan() { + return fmt.Errorf("%q returned empty", cmd) + } + for line := scanner.Text(); scanner.Scan(); line = scanner.Text() { + elems := strings.Split(line, ",") + if len(elems) != 3 { + return fmt.Errorf("invalid line: %q", line) + } + name := strings.TrimPrefix(elems[0], "Name: ") + optFields, err := parseFields(elems[1], "optional fields: ") + if err != nil { + return err + } + ctxFields, err := parseFields(elems[2], "context fields: ") + if err != nil { + return err + } + b.points = append(b.points, seccheck.PointConfig{ + Name: name, + OptionalFields: optFields, + ContextFields: ctxFields, + }) + } + return scanner.Err() +} + +func parseFields(elem, prefix string) ([]string, error) { + stripped := strings.TrimPrefix(strings.TrimSpace(elem), prefix) + switch { + case len(stripped) < 2: + return nil, fmt.Errorf("invalid %s format: %q", prefix, elem) + case len(stripped) == 2: + return nil, nil + } + // Remove [] from `stripped`. + clean := stripped[1 : len(stripped)-1] + return strings.Split(clean, "|"), nil +} + +// AddSink adds the sink to the configuration. +func (b *Builder) AddSink(sink seccheck.SinkConfig) { + b.sinks = append(b.sinks, sink) +} diff --git a/test/trace/trace_test.go b/test/trace/trace_test.go index 76f370270..617c7f0b8 100644 --- a/test/trace/trace_test.go +++ b/test/trace/trace_test.go @@ -16,11 +16,7 @@ package trace import ( - "bufio" - "bytes" - "encoding/json" "fmt" - "io/ioutil" "os" "os/exec" "strings" @@ -32,7 +28,7 @@ import ( "gvisor.dev/gvisor/pkg/sentry/seccheck/checkers/remote/test" pb "gvisor.dev/gvisor/pkg/sentry/seccheck/points/points_go_proto" "gvisor.dev/gvisor/pkg/test/testutil" - "gvisor.dev/gvisor/runsc/boot" + "gvisor.dev/gvisor/test/trace/config" ) // TestAll enabled all trace points in the system with all optional and context @@ -48,19 +44,24 @@ func TestAll(t *testing.T) { if err != nil { t.Fatal(err) } - cfg, err := buildPodConfig(runsc, server.Endpoint) - if err != nil { + builder := config.Builder{} + if err := builder.LoadAllPoints(runsc); err != nil { t.Fatal(err) } + builder.AddSink(seccheck.SinkConfig{ + Name: "remote", + Config: map[string]interface{}{ + "endpoint": server.Endpoint, + }, + }) - cfgFile, err := ioutil.TempFile(testutil.TmpDir(), "config") + cfgFile, err := os.CreateTemp(testutil.TmpDir(), "config") if err != nil { t.Fatalf("error creating tmp file: %v", err) } defer cfgFile.Close() - encoder := json.NewEncoder(cfgFile) - if err := encoder.Encode(&cfg); err != nil { - t.Fatalf("JSON encode: %v", err) + if err := builder.WriteInitConfig(cfgFile); err != nil { + t.Fatalf("writing config file: %v", err) } workload, err := testutil.FindFile("test/trace/workload/workload") @@ -84,84 +85,6 @@ func TestAll(t *testing.T) { matchPoints(t, server.GetPoints()) } -func buildPodConfig(runscPath, endpoint string) (*boot.InitConfig, error) { - pts, err := allPoints(runscPath) - if err != nil { - return nil, err - } - return &boot.InitConfig{ - TraceSession: seccheck.SessionConfig{ - Name: seccheck.DefaultSessionName, - Points: pts, - Sinks: []seccheck.SinkConfig{ - { - Name: "remote", - Config: map[string]interface{}{ - "endpoint": endpoint, - }, - }, - }, - }, - }, nil -} - -func allPoints(runscPath string) ([]seccheck.PointConfig, error) { - cmd := exec.Command(runscPath, "trace", "metadata") - out, err := cmd.CombinedOutput() - if err != nil { - return nil, err - } - - // The command above produces an output like the following: - // POINTS (907) - // Name: container/start, optional fields: [], context fields: [time|thread_id] - scanner := bufio.NewScanner(bytes.NewReader(out)) - if !scanner.Scan() { - return nil, fmt.Errorf("%q returned empty", cmd) - } - if !scanner.Scan() { - return nil, fmt.Errorf("%q returned empty", cmd) - } - var points []seccheck.PointConfig - for line := scanner.Text(); scanner.Scan(); line = scanner.Text() { - elems := strings.Split(line, ",") - if len(elems) != 3 { - return nil, fmt.Errorf("invalid line: %q", line) - } - name := strings.TrimPrefix(elems[0], "Name: ") - optFields, err := parseFields(elems[1], "optional fields: ") - if err != nil { - return nil, err - } - ctxFields, err := parseFields(elems[2], "context fields: ") - if err != nil { - return nil, err - } - points = append(points, seccheck.PointConfig{ - Name: name, - OptionalFields: optFields, - ContextFields: ctxFields, - }) - } - if scanner.Err() != nil { - return nil, scanner.Err() - } - return points, nil -} - -func parseFields(elem, prefix string) ([]string, error) { - stripped := strings.TrimPrefix(strings.TrimSpace(elem), prefix) - switch { - case len(stripped) < 2: - return nil, fmt.Errorf("invalid %s format: %q", prefix, elem) - case len(stripped) == 2: - return nil, nil - } - // Remove [] from `stripped`. - clean := stripped[1 : len(stripped)-1] - return strings.Split(clean, "|"), nil -} - func matchPoints(t *testing.T, msgs []test.Message) { // Register functions that verify each available point. matchers := map[pb.MessageType]*struct {