diff --git a/Makefile b/Makefile index 3f81445b5..cef1e8576 100644 --- a/Makefile +++ b/Makefile @@ -203,7 +203,7 @@ nogo-tests: # For unit tests, we take everything in the root, pkg/... and tools/..., and # pull in all directories in runsc except runsc/container. unit-tests: ## Local package unit tests in pkg/..., tools/.., etc. - @$(call test,--build_tag_filters=-nogo --test_tag_filters=-nogo --test_filter=-//runsc/container/... //:all pkg/... tools/... runsc/... vdso/...) + @$(call test,--build_tag_filters=-nogo --test_tag_filters=-nogo --test_filter=-//runsc/container/... //:all pkg/... tools/... runsc/... vdso/... test/trace/...) .PHONY: unit-tests # See unit-tests: this includes runsc/container. diff --git a/pkg/sentry/seccheck/checkers/remote/test/BUILD b/pkg/sentry/seccheck/checkers/remote/test/BUILD index 3e4cca046..2e3fb6666 100644 --- a/pkg/sentry/seccheck/checkers/remote/test/BUILD +++ b/pkg/sentry/seccheck/checkers/remote/test/BUILD @@ -13,7 +13,6 @@ go_library( "//pkg/sentry/seccheck/checkers/remote/wire", "//pkg/sentry/seccheck/points:points_go_proto", "//pkg/sync", - "//pkg/test/testutil", "//pkg/unet", "@org_golang_google_protobuf//proto:go_default_library", "@org_golang_x_sys//unix:go_default_library", diff --git a/pkg/sentry/seccheck/checkers/remote/test/server.go b/pkg/sentry/seccheck/checkers/remote/test/server.go index d25ed6cf7..bbe687c3e 100644 --- a/pkg/sentry/seccheck/checkers/remote/test/server.go +++ b/pkg/sentry/seccheck/checkers/remote/test/server.go @@ -21,7 +21,6 @@ import ( "io/ioutil" "os" "path/filepath" - "time" "golang.org/x/sys/unix" "google.golang.org/protobuf/proto" @@ -30,7 +29,6 @@ import ( "gvisor.dev/gvisor/pkg/sentry/seccheck/checkers/remote/wire" pb "gvisor.dev/gvisor/pkg/sentry/seccheck/points/points_go_proto" "gvisor.dev/gvisor/pkg/sync" - "gvisor.dev/gvisor/pkg/test/testutil" "gvisor.dev/gvisor/pkg/unet" ) @@ -40,14 +38,16 @@ type Server struct { Path string socket *unet.ServerSocket - mu sync.Mutex + cond sync.Cond - // +checklocks:mu + // +checklocks:cond.L clients []*unet.Socket - // +checklocks:mu + // +checklocks:cond.L points []Message + mu sync.Mutex + // +checklocks:mu version uint32 } @@ -104,6 +104,7 @@ func newServerPath(path string) (*Server, error) { Path: path, socket: ss, version: wire.CurrentVersion, + cond: sync.Cond{L: &sync.Mutex{}}, } go server.run() cu.Release() @@ -125,9 +126,10 @@ func (s *Server) run() { _ = client.Close() continue } - s.mu.Lock() + s.cond.L.Lock() s.clients = append(s.clients, client) - s.mu.Unlock() + s.cond.Broadcast() + s.cond.L.Unlock() go s.handleClient(client) } } @@ -164,14 +166,15 @@ func (s *Server) handshake(client *unet.Socket) error { func (s *Server) handleClient(client *unet.Socket) { defer func() { - s.mu.Lock() + s.cond.L.Lock() for i, c := range s.clients { if c == client { s.clients = append(s.clients[:i], s.clients[i+1:]...) break } } - s.mu.Unlock() + s.cond.Broadcast() + s.cond.L.Unlock() _ = client.Close() }() @@ -192,28 +195,33 @@ func (s *Server) handleClient(client *unet.Socket) { if read < int(hdr.HeaderSize) { panic(fmt.Sprintf("message truncated, header size: %d, readL %d", hdr.HeaderSize, read)) } + + msgSize := read - int(hdr.HeaderSize) msg := Message{ MsgType: pb.MessageType(hdr.MessageType), - Msg: buf[hdr.HeaderSize:read], + Msg: make([]byte, msgSize), } - s.mu.Lock() + copy(msg.Msg, buf[hdr.HeaderSize:read]) + + s.cond.L.Lock() s.points = append(s.points, msg) - s.mu.Unlock() + s.cond.Broadcast() + s.cond.L.Unlock() } } // Count return the number of points it has received. func (s *Server) Count() int { - s.mu.Lock() - defer s.mu.Unlock() + s.cond.L.Lock() + defer s.cond.L.Unlock() return len(s.points) } // Reset throws aways all points received so far and returns the number of // points discarded. func (s *Server) Reset() int { - s.mu.Lock() - defer s.mu.Unlock() + s.cond.L.Lock() + defer s.cond.L.Unlock() count := len(s.points) s.points = nil return count @@ -221,8 +229,8 @@ func (s *Server) Reset() int { // GetPoints returns all points that it has received. func (s *Server) GetPoints() []Message { - s.mu.Lock() - defer s.mu.Unlock() + s.cond.L.Lock() + defer s.cond.L.Unlock() cpy := make([]Message, len(s.points)) copy(cpy, s.points) return cpy @@ -231,23 +239,33 @@ func (s *Server) GetPoints() []Message { // Close stops listenning and closes all connections. func (s *Server) Close() { _ = s.socket.Close() - s.mu.Lock() + s.cond.L.Lock() for _, client := range s.clients { _ = client.Close() } - s.mu.Unlock() + s.clients = nil + s.cond.Broadcast() + s.cond.L.Unlock() _ = os.Remove(s.Path) } -// WaitForCount waits for the number of points to reach the desired number for -// 5 seconds. It fails if not received in time. -func (s *Server) WaitForCount(count int) error { - return testutil.Poll(func() error { - if got := s.Count(); got < count { - return fmt.Errorf("waiting for points %d to arrive, received %d", count, got) - } - return nil - }, 5*time.Second) +// WaitForCount waits for the number of points to reach the desired number. +func (s *Server) WaitForCount(count int) { + s.cond.L.Lock() + defer s.cond.L.Unlock() + for len(s.points) < count { + s.cond.Wait() + } + return +} + +// WaitForNoClients waits until the number of clients connected reaches 0. +func (s *Server) WaitForNoClients() { + s.cond.L.Lock() + defer s.cond.L.Unlock() + for len(s.clients) > 0 { + s.cond.Wait() + } } // SetVersion sets the version to be used in handshake. diff --git a/pkg/sentry/seccheck/config.go b/pkg/sentry/seccheck/config.go index 82bd128d9..5f9fe4fe3 100644 --- a/pkg/sentry/seccheck/config.go +++ b/pkg/sentry/seccheck/config.go @@ -97,13 +97,13 @@ func Create(conf *SessionConfig, force bool) error { mask, err := setFields(ptConfig.OptionalFields, desc.OptionalFields) if err != nil { - return err + return fmt.Errorf("configuring point %q: %w", ptConfig.Name, err) } req.Fields.Local = mask mask, err = setFields(ptConfig.ContextFields, desc.ContextFields) if err != nil { - return err + return fmt.Errorf("configuring point %q: %w", ptConfig.Name, err) } req.Fields.Context = mask diff --git a/runsc/container/trace_test.go b/runsc/container/trace_test.go index 3d038f6ce..3b7d61c10 100644 --- a/runsc/container/trace_test.go +++ b/runsc/container/trace_test.go @@ -81,9 +81,7 @@ func TestTraceStartup(t *testing.T) { } // Wait for the point to be received and then check that fields match. - if err := server.WaitForCount(1); err != nil { - t.Fatalf("WaitForCount(1): %v", err) - } + server.WaitForCount(1) pt := server.GetPoints()[0] if want := pb.MessageType_MESSAGE_CONTAINER_START; pt.MsgType != want { t.Errorf("wrong message type, want: %v, got: %v", want, pt.MsgType) @@ -157,9 +155,7 @@ func TestTraceLifecycle(t *testing.T) { if ws, err := execute(conf, cont, "/bin/true"); err != nil || ws != 0 { t.Fatalf("exec: true, ws: %v, err: %v", ws, err) } - if err := server.WaitForCount(1); err != nil { - t.Fatalf("WaitForCount(1): %v", err) - } + server.WaitForCount(1) pt := server.GetPoints()[0] if want := pb.MessageType_MESSAGE_SENTRY_TASK_EXIT; pt.MsgType != want { t.Errorf("wrong message type, want: %v, got: %v", want, pt.MsgType) @@ -259,9 +255,7 @@ func TestTraceForceCreate(t *testing.T) { if ws, err := execute(conf, cont, "/bin/true"); err != nil || ws != 0 { t.Fatalf("exec: true, ws: %v, err: %v", ws, err) } - if err := server.WaitForCount(1); err != nil { - t.Fatalf("WaitForCount(1): %v", err) - } + server.WaitForCount(1) pt := server.GetPoints()[0] if want := pb.MessageType_MESSAGE_SENTRY_EXIT_NOTIFY_PARENT; pt.MsgType != want { t.Errorf("wrong message type, want: %v, got: %v", want, pt.MsgType) @@ -289,9 +283,7 @@ func TestTraceForceCreate(t *testing.T) { if ws, err := execute(conf, cont, "/bin/true"); err != nil || ws != 0 { t.Fatalf("exec: true, ws: %v, err: %v", ws, err) } - if err := server.WaitForCount(1); err != nil { - t.Fatalf("WaitForCount(1): %v", err) - } + server.WaitForCount(1) pt = server.GetPoints()[0] if want := pb.MessageType_MESSAGE_SENTRY_TASK_EXIT; pt.MsgType != want { t.Errorf("wrong message type, want: %v, got: %v", want, pt.MsgType) diff --git a/test/trace/BUILD b/test/trace/BUILD new file mode 100644 index 000000000..bd3564de8 --- /dev/null +++ b/test/trace/BUILD @@ -0,0 +1,31 @@ +load("//tools:defs.bzl", "go_library", "go_test") + +package(licenses = ["notice"]) + +go_test( + name = "trace_test", + size = "small", + srcs = ["trace_test.go"], + data = [ + "//runsc", + "//test/trace/workload", + ], + library = ":trace", + tags = [ + "local", + "manual", + ], + deps = [ + "//pkg/sentry/seccheck", + "//pkg/sentry/seccheck/checkers/remote/test", + "//pkg/sentry/seccheck/points:points_go_proto", + "//pkg/test/testutil", + "//runsc/boot", + "@org_golang_google_protobuf//proto:go_default_library", + ], +) + +go_library( + name = "trace", + srcs = ["trace.go"], +) diff --git a/test/trace/trace.go b/test/trace/trace.go new file mode 100644 index 000000000..4eaab42fc --- /dev/null +++ b/test/trace/trace.go @@ -0,0 +1,16 @@ +// 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 trace is empty. See trace_test.go for description. +package trace diff --git a/test/trace/trace_test.go b/test/trace/trace_test.go new file mode 100644 index 000000000..cd5de6ef8 --- /dev/null +++ b/test/trace/trace_test.go @@ -0,0 +1,341 @@ +// 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 trace provides end-to-end integration tests for `runsc trace`. +package trace + +import ( + "bufio" + "bytes" + "encoding/json" + "fmt" + "io/ioutil" + "os" + "os/exec" + "strings" + "testing" + "time" + + "google.golang.org/protobuf/proto" + "gvisor.dev/gvisor/pkg/sentry/seccheck" + "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" +) + +// TestAll enabled all trace points in the system with all optional and context +// fields enabled. Then it runs a workload that will trigger those points and +// run some basic validation over the points generated. +func TestAll(t *testing.T) { + server, err := test.NewServer() + if err != nil { + t.Fatal(err) + } + + runsc, err := testutil.FindFile("runsc/runsc") + if err != nil { + t.Fatal(err) + } + cfg, err := buildPodConfig(runsc, server.Path) + if err != nil { + t.Fatal(err) + } + + cfgFile, err := ioutil.TempFile(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) + } + + workload, err := testutil.FindFile("test/trace/workload/workload") + if err != nil { + t.Fatal(err) + } + cmd := exec.Command( + runsc, + "--debug", "--alsologtostderr", // Debug logging for troubleshooting + "--rootless", "--network=none", // Disable features that we don't care + "--pod-init-config", cfgFile.Name(), + "do", workload) + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("runsc do: %v", err) + } + t.Log(string(out)) + + // Wait until the sandbox disconnects to ensure all points were gathered. + server.WaitForNoClients() + 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 { + checker func(test.Message) error + count int + }{ + pb.MessageType_MESSAGE_CONTAINER_START: {checker: checkContainerStart}, + pb.MessageType_MESSAGE_SENTRY_TASK_EXIT: {checker: checkSentryTaskExit}, + pb.MessageType_MESSAGE_SYSCALL_RAW: {checker: checkSyscallRaw}, + pb.MessageType_MESSAGE_SYSCALL_OPEN: {checker: checkSyscallOpen}, + pb.MessageType_MESSAGE_SYSCALL_CLOSE: {checker: checkSyscallClose}, + pb.MessageType_MESSAGE_SYSCALL_READ: {checker: checkSyscallRead}, + } + for _, msg := range msgs { + t.Logf("Processing message type %v", msg.MsgType) + if handler := matchers[msg.MsgType]; handler == nil { + // All points generated should have a corresponding matcher. + t.Errorf("No matcher for message type %v", msg.MsgType) + } else { + handler.count++ + if err := handler.checker(msg); err != nil { + t.Errorf("message type %v: %v", msg.MsgType, err) + } + } + } + for msgType, match := range matchers { + t.Logf("Processed %d messages for %v", match.count, msgType) + if match.count == 0 { + // All matchers should be triggered at least once to ensure points are + // firing with the workload. + t.Errorf("no point was generated for %v", msgType) + } + } +} + +func checkContextData(data *pb.ContextData) error { + if data == nil { + return fmt.Errorf("ContextData should not be nil") + } + if !strings.HasPrefix(data.ContainerId, "runsc-") { + return fmt.Errorf("invalid container ID %q", data.ContainerId) + } + + cutoff := time.Now().Add(-time.Minute) + if data.TimeNs <= int64(cutoff.Nanosecond()) { + return fmt.Errorf("time should not be less than %d (%v), got: %d (%v)", cutoff.Nanosecond(), cutoff, data.TimeNs, time.Unix(0, data.TimeNs)) + } + if data.ThreadStartTimeNs <= int64(cutoff.Nanosecond()) { + return fmt.Errorf("thread_start_time should not be less than %d (%v), got: %d (%v)", cutoff.Nanosecond(), cutoff, data.ThreadStartTimeNs, time.Unix(0, data.ThreadStartTimeNs)) + } + if data.ThreadStartTimeNs > data.TimeNs { + return fmt.Errorf("thread_start_time should not be greater than point time: %d (%v), got: %d (%v)", data.TimeNs, time.Unix(0, data.TimeNs), data.ThreadStartTimeNs, time.Unix(0, data.ThreadStartTimeNs)) + } + if data.ThreadGroupStartTimeNs <= int64(cutoff.Nanosecond()) { + return fmt.Errorf("thread_group_start_time should not be less than %d (%v), got: %d (%v)", cutoff.Nanosecond(), cutoff, data.ThreadGroupStartTimeNs, time.Unix(0, data.ThreadGroupStartTimeNs)) + } + if data.ThreadGroupStartTimeNs > data.TimeNs { + return fmt.Errorf("thread_group_start_time should not be greater than point time: %d (%v), got: %d (%v)", data.TimeNs, time.Unix(0, data.TimeNs), data.ThreadGroupStartTimeNs, time.Unix(0, data.ThreadGroupStartTimeNs)) + } + + if data.ThreadId <= 0 { + return fmt.Errorf("invalid thread_id: %v", data.ThreadId) + } + if data.ThreadGroupId <= 0 { + return fmt.Errorf("invalid thread_group_id: %v", data.ThreadGroupId) + } + if len(data.Cwd) == 0 { + return fmt.Errorf("invalid cwd: %v", data.Cwd) + } + if len(data.ProcessName) == 0 { + return fmt.Errorf("invalid process_name: %v", data.ProcessName) + } + return nil +} + +func checkContainerStart(msg test.Message) error { + p := pb.Start{} + if err := proto.Unmarshal(msg.Msg, &p); err != nil { + return err + } + if err := checkContextData(p.ContextData); err != nil { + return err + } + if !strings.HasPrefix(p.Id, "runsc-") { + return fmt.Errorf("invalid container ID %q", p.Id) + } + cwd, err := os.Getwd() + if err != nil { + return fmt.Errorf("Getwd(): %v", err) + } + if cwd != p.Cwd { + return fmt.Errorf("invalid cwd, want: %q, got: %q", cwd, p.Cwd) + } + if len(p.Args) == 0 { + return fmt.Errorf("empty args") + } + if len(p.Env) == 0 { + return fmt.Errorf("empty env") + } + for _, e := range p.Env { + if strings.IndexRune(e, '=') == -1 { + return fmt.Errorf("malformed env: %q", e) + } + } + if p.Terminal { + return fmt.Errorf("terminal should be off") + } + return nil +} + +func checkSentryTaskExit(msg test.Message) error { + p := pb.TaskExit{} + if err := proto.Unmarshal(msg.Msg, &p); err != nil { + return err + } + if err := checkContextData(p.ContextData); err != nil { + return err + } + return nil +} + +func checkSyscallRaw(msg test.Message) error { + p := pb.Syscall{} + if err := proto.Unmarshal(msg.Msg, &p); err != nil { + return err + } + if err := checkContextData(p.ContextData); err != nil { + return err + } + // Sanity check that Sysno is within valid range. If sysno could be larger + // than the value below, update it accordingly. + if p.Sysno > 500 { + return fmt.Errorf("invalid syscall number %d", p.Sysno) + } + return nil +} + +func checkSyscallClose(msg test.Message) error { + p := pb.Close{} + if err := proto.Unmarshal(msg.Msg, &p); err != nil { + return err + } + if err := checkContextData(p.ContextData); err != nil { + return err + } + if p.Fd < 0 { + // Although negative FD is possible, it doesn't happen in the test. + return fmt.Errorf("closing negative FD: %d", p.Fd) + } + return nil +} + +func checkSyscallOpen(msg test.Message) error { + p := pb.Open{} + if err := proto.Unmarshal(msg.Msg, &p); err != nil { + return err + } + if err := checkContextData(p.ContextData); err != nil { + return err + } + return nil +} + +func checkSyscallRead(msg test.Message) error { + p := pb.Read{} + if err := proto.Unmarshal(msg.Msg, &p); err != nil { + return err + } + if err := checkContextData(p.ContextData); err != nil { + return err + } + if p.Fd < 0 { + // Although negative FD is possible, it doesn't happen in the test. + return fmt.Errorf("reading negative FD: %d", p.Fd) + } + return nil +} diff --git a/test/trace/workload/BUILD b/test/trace/workload/BUILD new file mode 100644 index 000000000..fcca7b93b --- /dev/null +++ b/test/trace/workload/BUILD @@ -0,0 +1,14 @@ +load("//tools:defs.bzl", "cc_binary") + +package(licenses = ["notice"]) + +cc_binary( + name = "workload", + testonly = 1, + srcs = [ + "workload.cc", + ], + visibility = ["//test/trace:__pkg__"], + deps = [ + ], +) diff --git a/test/trace/workload/workload.cc b/test/trace/workload/workload.cc new file mode 100644 index 000000000..72eee9bc6 --- /dev/null +++ b/test/trace/workload/workload.cc @@ -0,0 +1,16 @@ +// 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. + +// Empty for now. Actual workload will be added as more points are covered. +int main(int argc, char** argv) { return 0; }