From 4b2512812fdf7ed943515a3e816ba1c758c11980 Mon Sep 17 00:00:00 2001 From: Kornilios Kourtis Date: Fri, 18 Nov 2022 15:03:01 +0100 Subject: [PATCH 1/3] tetragon: add support for gRPC over unix socket Now we can provide unix://abosolute_path arguments to both the client (tetra) and the agent (tetragon). Signed-off-by: Kornilios Kourtis --- cmd/tetragon/addr.go | 35 ++++++++++++++++++++++++++ cmd/tetragon/addr_test.go | 52 +++++++++++++++++++++++++++++++++++++++ cmd/tetragon/main.go | 16 +++++++----- 3 files changed, 97 insertions(+), 6 deletions(-) create mode 100644 cmd/tetragon/addr.go create mode 100644 cmd/tetragon/addr_test.go diff --git a/cmd/tetragon/addr.go b/cmd/tetragon/addr.go new file mode 100644 index 00000000000..50194bf2fcb --- /dev/null +++ b/cmd/tetragon/addr.go @@ -0,0 +1,35 @@ +package main + +import ( + "fmt" + "path/filepath" + "strings" +) + +// splitListenAddr splits the user-provided address a to a proto and an address field to be used +// with net.Listen. +// +// addresses can be: +// unix://absolute_path for unix sockets +// : for TCP (more specifically, an address that can be passed to net.Listen) +// +// Note that the client (tetra) uses https://github.com/grpc/grpc-go/blob/v1.51.0/clientconn.go#L135 +// With the syntax is documented in https://github.com/grpc/grpc/blob/master/doc/naming.md. The +// server uses net.Listen. And so the two are not compatible because the client expects "ipv4" or +// "ipv6" for tcp connections. +// Hence, because we want the same string to work the same way both on the client and the server, we +// only support the two addresses above. +func splitListenAddr(arg string) (string, string, error) { + + if strings.HasPrefix(arg, "unix://") { + path := strings.TrimPrefix(arg, "unix://") + if !filepath.IsAbs(path) { + return "", "", fmt.Errorf("path %s (%s) is not absolute", path, arg) + } + return "unix", path, nil + } + + // assume everything else is TCP to support strings such as "localhost:51234" and let + // net.Listen figure things out. + return "tcp", arg, nil +} diff --git a/cmd/tetragon/addr_test.go b/cmd/tetragon/addr_test.go new file mode 100644 index 00000000000..0eb76482ba5 --- /dev/null +++ b/cmd/tetragon/addr_test.go @@ -0,0 +1,52 @@ +package main + +import "testing" + +func TestSplitListenAddr(t *testing.T) { + type testCase struct { + arg string + + expectedErr bool + proto, addr string + } + testCases := []testCase{ + { + arg: "unix:///var/run/tetragon/tetragon.sock", + proto: "unix", + addr: "/var/run/tetragon/tetragon.sock", + }, { + arg: "localhost:51234", + proto: "tcp", + addr: "localhost:51234", + }, { + // NB: expect error on relative paths + arg: "unix://var/run/tetragon/tetragon.sock", + expectedErr: true, + }, + } + + for _, c := range testCases { + proto, addr, err := splitListenAddr(c.arg) + if c.expectedErr { + if err == nil { + t.Fatalf("expected error for %s", c.arg) + } + continue + } + + if err != nil { + t.Fatalf("unexpected error for %s: %s", c.arg, err) + } + + if proto != c.proto { + t.Fatalf("Proto (%s) did not match expected value (%s) for %s", proto, c.proto, c.arg) + } + + if addr != c.addr { + t.Fatalf("Addr (%s) did not match expected value (%s) for %s", addr, c.addr, c.arg) + } + + t.Logf("case %+v is OK!", c) + } + +} diff --git a/cmd/tetragon/main.go b/cmd/tetragon/main.go index 6560f1f0736..4ebd1b78d3f 100644 --- a/cmd/tetragon/main.go +++ b/cmd/tetragon/main.go @@ -359,19 +359,23 @@ func startExporter(ctx context.Context, server *server.Server) error { return nil } -func Serve(ctx context.Context, address string, server *server.Server) error { +func Serve(ctx context.Context, listenAddr string, server *server.Server) error { grpcServer := grpc.NewServer() tetragon.RegisterFineGuidanceSensorsServer(grpcServer, server) - go func(address string) { - listener, err := net.Listen("tcp", address) + proto, addr, err := splitListenAddr(listenAddr) + if err != nil { + return fmt.Errorf("failed to parse listen address: %w", err) + } + go func(proto, addr string) { + listener, err := net.Listen(proto, addr) if err != nil { - log.WithError(err).WithField("address", address).Fatal("Failed to start gRPC server") + log.WithError(err).WithField("protocol", proto).WithField("address", addr).Fatal("Failed to start gRPC server") } - log.WithField("address", address).Info("Starting gRPC server") + log.WithField("address", addr).WithField("protocol", proto).Info("Starting gRPC server") if err = grpcServer.Serve(listener); err != nil { log.WithError(err).Error("Failed to close gRPC server") } - }(address) + }(proto, addr) go func() { <-ctx.Done() grpcServer.Stop() From 9bec5e99046779ed20318246ae919eacb1683717 Mon Sep 17 00:00:00 2001 From: Kornilios Kourtis Date: Tue, 22 Nov 2022 08:46:45 +0100 Subject: [PATCH 2/3] helm: remove grpc.port and fix livenessProbe Up until now, configuring gRPC listen address for the agent via the helm chart used two variables grpc.port and grpc.address. The previous patch added support for passing a unix socket address to the agent, which does not match well the grpc.address:grpc.port configuration. This patch removes the grpc.port helm variable, and, instead, relies only on grpc.address for configuration. Users can use "localhost:54321", ":54321", or "unix:///var/run/tetragon/tetragon.sock" to configure the gRPC address that the agent listens to. Furthermore, the livenessProbe of the agent relies on checking health status via the gRPC interface via the CLI (tetra status). This patch also fixes the livenessProbe so that: - it is only defined, if grpc is enabled - the proper gRPC address is used to contact the agent Signed-off-by: Kornilios Kourtis --- install/kubernetes/README.md | 3 +-- install/kubernetes/templates/_container_tetragon.tpl | 12 ++++++++---- install/kubernetes/templates/tetragon_configmap.yaml | 2 +- install/kubernetes/values.yaml | 6 ++---- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/install/kubernetes/README.md b/install/kubernetes/README.md index aac24fc163c..219326b37d9 100644 --- a/install/kubernetes/README.md +++ b/install/kubernetes/README.md @@ -64,9 +64,8 @@ Helm chart for Tetragon | tetragon.fieldFilters | string | `"{}"` | | | tetragon.gops.address | string | `"localhost"` | The address at which to expose gops. | | tetragon.gops.port | int | `8118` | The port at which to expose gops. | -| tetragon.grpc.address | string | `"localhost"` | The address at which to expose gRPC. Set it to "" to listen on all available interfaces. | +| tetragon.grpc.address | string | `"localhost:54321"` | The address at which to expose gRPC. Examples: localhost:54321, unix:///var/run/tetragon/tetragon.sock | | tetragon.grpc.enabled | bool | `true` | Whether to enable exposing Tetragon gRPC. | -| tetragon.grpc.port | int | `54321` | The port at which to expose gRPC. | | tetragon.image.override | string | `nil` | | | tetragon.image.repository | string | `"quay.io/cilium/tetragon"` | | | tetragon.image.tag | string | `"v0.8.3"` | | diff --git a/install/kubernetes/templates/_container_tetragon.tpl b/install/kubernetes/templates/_container_tetragon.tpl index d3a8b75c9ab..47089f93aea 100644 --- a/install/kubernetes/templates/_container_tetragon.tpl +++ b/install/kubernetes/templates/_container_tetragon.tpl @@ -68,11 +68,15 @@ resources: {{- toYaml . | nindent 4 }} {{- end }} +{{- if .Values.tetragon.grpc.enabled }} livenessProbe: - exec: - command: - - tetra - - status + exec: + command: + - tetra + - status + - --server-address + - {{ .Values.tetragon.grpc.address }} +{{- end -}} {{- end -}} {{- define "container.tetragon.init-operator" -}} diff --git a/install/kubernetes/templates/tetragon_configmap.yaml b/install/kubernetes/templates/tetragon_configmap.yaml index 5d63bbfd273..c39c9c415ef 100644 --- a/install/kubernetes/templates/tetragon_configmap.yaml +++ b/install/kubernetes/templates/tetragon_configmap.yaml @@ -39,7 +39,7 @@ data: metrics-server: "" {{- end }} {{- if .Values.tetragon.grpc.enabled }} - server-address: {{ .Values.tetragon.grpc.address }}:{{ .Values.tetragon.grpc.port }} + server-address: {{ .Values.tetragon.grpc.address }} {{- else }} {{- end }} {{- if .Values.tetragon.tcpStatsSampleSegs }} diff --git a/install/kubernetes/values.yaml b/install/kubernetes/values.yaml index 5bbb65c42ab..2402f3827a1 100644 --- a/install/kubernetes/values.yaml +++ b/install/kubernetes/values.yaml @@ -134,10 +134,8 @@ tetragon: grpc: # -- Whether to enable exposing Tetragon gRPC. enabled: true - # -- The address at which to expose gRPC. Set it to "" to listen on all available interfaces. - address: "localhost" - # -- The port at which to expose gRPC. - port: 54321 + # -- The address at which to expose gRPC. Examples: localhost:54321, unix:///var/run/tetragon/tetragon.sock + address: "localhost:54321" gops: # -- The address at which to expose gops. address: "localhost" From 0d7f811b3aa1becd3d2bf9f4eb1c8c6fb48a07ff Mon Sep 17 00:00:00 2001 From: Kornilios Kourtis Date: Fri, 25 Nov 2022 11:24:37 +0100 Subject: [PATCH 3/3] tetragon: chmod unix socket to 0660 This patch ensures that the control unix socket for the agent has 0660 file permissions. Signed-off-by: Kornilios Kourtis --- cmd/tetragon/main.go | 9 +++++- pkg/unixlisten/unixlisten.go | 57 ++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 pkg/unixlisten/unixlisten.go diff --git a/cmd/tetragon/main.go b/cmd/tetragon/main.go index 4ebd1b78d3f..7cdbd4c5921 100644 --- a/cmd/tetragon/main.go +++ b/cmd/tetragon/main.go @@ -36,6 +36,7 @@ import ( "github.com/cilium/tetragon/pkg/sensors/base" "github.com/cilium/tetragon/pkg/sensors/program" "github.com/cilium/tetragon/pkg/server" + "github.com/cilium/tetragon/pkg/unixlisten" "github.com/cilium/tetragon/pkg/version" "github.com/cilium/tetragon/pkg/watcher" "github.com/cilium/tetragon/pkg/watcher/crd" @@ -367,7 +368,13 @@ func Serve(ctx context.Context, listenAddr string, server *server.Server) error return fmt.Errorf("failed to parse listen address: %w", err) } go func(proto, addr string) { - listener, err := net.Listen(proto, addr) + var listener net.Listener + var err error + if proto == "unix" { + listener, err = unixlisten.ListenWithRename(addr, 0660) + } else { + listener, err = net.Listen(proto, addr) + } if err != nil { log.WithError(err).WithField("protocol", proto).WithField("address", addr).Fatal("Failed to start gRPC server") } diff --git a/pkg/unixlisten/unixlisten.go b/pkg/unixlisten/unixlisten.go new file mode 100644 index 00000000000..c7b10bfaa34 --- /dev/null +++ b/pkg/unixlisten/unixlisten.go @@ -0,0 +1,57 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Authors of Tetragon +package unixlisten + +import ( + "fmt" + "net" + "os" + "path/filepath" +) + +// ListenWithRename creates a "unix" listener for the given path and the given mode +// +// Go's net.Listen() performs three system calls at once: +// - socket, where the file descriptor is created +// - bind, where the unix socket file is created +// - listen, where the socket can now accept connections +// +// Hence, doing a chmod(2) after Listen is racy because a client can connect +// between the listen(2) and the chmod(2) calls. One solution would be to use +// umask(2), but this is tricky to do in a multi-threaded program because it +// affects other files being created from different threads. Also, other +// threads may change the umask. +// +// This function, instead, creates the socket file in a private directory, +// performs the appropriate chmod and only then moves the file to its original +// location. Not sure about other systems, but at least on Linux renaming a +// unix socket file after the listen seems to work without issues. +func ListenWithRename(path string, mode os.FileMode) (net.Listener, error) { + os.Remove(path) + + baseName := filepath.Base(path) + dirName := filepath.Dir(path) + + // Create a temporary directory: MkdirTemp creates the directory with 0700 + tmpDir, err := os.MkdirTemp(dirName, fmt.Sprintf("%s-dir-*", baseName)) + if err != nil { + return nil, err + } + defer os.RemoveAll(tmpDir) + + tmpPath := filepath.Join(tmpDir, baseName) + l, err := net.Listen("unix", tmpPath) + if err != nil { + return nil, err + } + + if err := os.Chmod(tmpPath, mode); err != nil { + return nil, err + } + + err = os.Rename(tmpPath, path) + if err != nil { + return nil, err + } + return l, nil +}