From 6ad1abf9996bc0ef89cc490b066415ceae78a1e6 Mon Sep 17 00:00:00 2001 From: Tomas Nozicka Date: Mon, 23 Dec 2024 13:26:48 +0100 Subject: [PATCH] Validate ports --- .../set/scyllacluster/scyllacluster_listen.go | 188 ++++++++++++++ test/e2e/utils/linux/net/net.go | 234 ++++++++++++++++++ test/e2e/utils/linux/net/net_test.go | 150 +++++++++++ 3 files changed, 572 insertions(+) create mode 100644 test/e2e/set/scyllacluster/scyllacluster_listen.go create mode 100644 test/e2e/utils/linux/net/net.go create mode 100644 test/e2e/utils/linux/net/net_test.go diff --git a/test/e2e/set/scyllacluster/scyllacluster_listen.go b/test/e2e/set/scyllacluster/scyllacluster_listen.go new file mode 100644 index 00000000000..f9267529ba9 --- /dev/null +++ b/test/e2e/set/scyllacluster/scyllacluster_listen.go @@ -0,0 +1,188 @@ +// Copyright (c) 2024 ScyllaDB. + +package scyllacluster + +import ( + "context" + "net" + "strings" + + g "github.com/onsi/ginkgo/v2" + o "github.com/onsi/gomega" + scyllav1 "github.com/scylladb/scylla-operator/pkg/api/scylla/v1" + "github.com/scylladb/scylla-operator/pkg/controllerhelpers" + "github.com/scylladb/scylla-operator/pkg/naming" + "github.com/scylladb/scylla-operator/test/e2e/framework" + "github.com/scylladb/scylla-operator/test/e2e/tools" + "github.com/scylladb/scylla-operator/test/e2e/utils" + linuxnetutils "github.com/scylladb/scylla-operator/test/e2e/utils/linux/net" + scyllaclusterverification "github.com/scylladb/scylla-operator/test/e2e/utils/verification/scyllacluster" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +var _ = g.Describe("ScyllaCluster", func() { + f := framework.NewFramework("scyllacluster") + + g.It("listens only on secure ports", func() { + ctx, cancel := context.WithTimeout(context.Background(), testTimeout) + defer cancel() + + sc := f.GetDefaultScyllaCluster() + sc.Spec.Datacenter.Racks[0].Members = 1 + sc.Spec.ExposeOptions = &scyllav1.ExposeOptions{ + BroadcastOptions: &scyllav1.NodeBroadcastOptions{ + Nodes: scyllav1.BroadcastOptions{ + Type: scyllav1.BroadcastAddressTypePodIP, + }, + Clients: scyllav1.BroadcastOptions{ + Type: scyllav1.BroadcastAddressTypeServiceClusterIP, + }, + }, + } + sc.Spec.Alternator = &scyllav1.AlternatorSpec{} + + framework.By("Creating a ScyllaCluster with 1 member") + sc, err := f.ScyllaClient().ScyllaV1().ScyllaClusters(f.Namespace()).Create(ctx, sc, metav1.CreateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + framework.By("Waiting for the ScyllaCluster to rollout (RV=%s)", sc.ResourceVersion) + waitCtx1, waitCtx1Cancel := utils.ContextForRollout(ctx, sc) + defer waitCtx1Cancel() + sc, err = controllerhelpers.WaitForScyllaClusterState(waitCtx1, f.ScyllaClient().ScyllaV1().ScyllaClusters(sc.Namespace), sc.Name, controllerhelpers.WaitForStateOptions{}, utils.IsScyllaClusterRolledOut) + o.Expect(err).NotTo(o.HaveOccurred()) + + scyllaclusterverification.Verify(ctx, f.KubeClient(), f.ScyllaClient(), sc) + + serviceName := naming.MemberServiceNameForScyllaCluster(sc.Spec.Datacenter.Racks[0], sc, 0) + nodeService, err := f.KubeClient().CoreV1().Services(sc.Namespace).Get(ctx, serviceName, metav1.GetOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + nodeServiceIP := nodeService.Spec.ClusterIP + o.Expect(nodeServiceIP).NotTo(o.BeEmpty()) + + nodePod, err := f.KubeClient().CoreV1().Pods(sc.Namespace).Get(ctx, naming.PodNameFromService(nodeService), metav1.GetOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + nodePodIP := nodePod.Status.PodIP + o.Expect(nodePodIP).NotTo(o.BeEmpty()) + + framework.By("Fetching raw network information from /proc/net/tcp{,6}") + // Because the network stack is shared between containers in the same Pod, it doesn't matter which container we use. + // We could also create an ephemeral Container in this Pod with an image we need, but the dependency here + // is minimal (bash + tail) so, as long as one of the images in this Pod has those, using exec is fine and simplifies the test. + stdout, stderr, err := tools.PodExec( + f.KubeClient().CoreV1().RESTClient(), + f.ClientConfig(), + nodePod.Namespace, + nodePod.Name, + "scylla", + []string{ + "/usr/bin/bash", + "-euEo", + "pipefail", + "-O", + "inherit_errexit", + "-c", + strings.TrimPrefix(` +tail -n +2 /proc/net/tcp +tail -n +2 /proc/net/tcp6 +`, "\n"), + }, + nil, + ) + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(stderr).To(o.BeEmpty()) + o.Expect(stdout).NotTo(o.BeEmpty()) + + procNetEntries, err := linuxnetutils.ParseProcNetEntries(stdout) + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(procNetEntries).NotTo(o.BeEmpty()) + + listenProcNetEntries := procNetEntries.FilterListen() + /* + * This is a list of allowed ports to be exposed using default configuration. + * All of these ports should use encryption and force AuthN/AuthZ, unless it is not appropriate + * or required for the type of data, e.g. /readyz probes. + * We have some techdebt in this area so this contains some historical ports that don't uphold + * to this standard. All such entries should have an issue link attached. + * + * Note: Some applications bind dedicated IPv4 and IPv6 socket while some reuse the IPv6 socket for IPv4 as well. + */ + o.Expect(listenProcNetEntries).To(o.ConsistOf([]linuxnetutils.AddressPort{ + { + Address: net.ParseIP("::"), + Port: 5090, // ScyllaDB Manager agent - metrics (insecure) + }, + { + Address: net.ParseIP("127.0.0.1"), + Port: 5112, // ScyllaDB Manager agent - debug (insecure) + }, + { + Address: net.ParseIP("0.0.0.0"), + Port: 7000, // ScyllaDB inter-node (insecure) + // FIXME: Remove + // https://github.com/scylladb/scylla-operator/issues/1217 + }, + { + Address: net.ParseIP("0.0.0.0"), + Port: 8043, // Alternator TLS + }, + { + Address: net.ParseIP("::"), + Port: 8080, // Scylla Operator probes (insecure, non-sensitive data) + }, + { + Address: net.ParseIP("::"), + Port: 42081, // ScyllaDB ignition probe + }, + { + Address: net.ParseIP("127.0.0.1"), + Port: 9001, // supervisord (planned for removal with cont) + // FIXME: Remove + // https://github.com/scylladb/scylla-operator/issues/1769 + }, + { + Address: net.ParseIP("::"), + Port: 9100, // Node exporter - metrics (insecure) + }, + { + Address: net.ParseIP("0.0.0.0"), + Port: 9042, // CQL (insecure) + // FIXME: Remove + // https://github.com/scylladb/scylla-operator/issues/1764 + }, + { + Address: net.ParseIP("0.0.0.0"), + Port: 9142, // CQL TLS (not AuthZ by default) + // FIXME: Enforce AuthN+AuthZ by default + // https://github.com/scylladb/scylla-operator/issues/1770 + }, + { + Address: net.ParseIP("0.0.0.0"), + Port: 9180, // ScyllaDB - metrics (insecure) + }, + { + Address: net.ParseIP("127.0.0.1"), + Port: 10000, // ScyllaDB API (insecure and unprotected) + }, + { + Address: net.ParseIP("::"), + Port: 10001, // ScyllaDB Manager API (insecure but authorized) + // FIXME: This needs to be replaced with TLS + // https://github.com/scylladb/scylla-operator/issues/1772 + }, + { + Address: net.ParseIP("0.0.0.0"), + Port: 19042, // Shard-aware CQL (insecure) + // FIXME: Remove in favour of port 19142 + // https://github.com/scylladb/scylla-operator/issues/1764 + }, + { + Address: net.ParseIP("0.0.0.0"), + Port: 19142, // Shard-aware CQL TLS + // FIXME: Enforce AuthN+AuthZ by default + // https://github.com/scylladb/scylla-operator/issues/1770 + }, + })) + }) +}) diff --git a/test/e2e/utils/linux/net/net.go b/test/e2e/utils/linux/net/net.go new file mode 100644 index 00000000000..e60708b848a --- /dev/null +++ b/test/e2e/utils/linux/net/net.go @@ -0,0 +1,234 @@ +// Copyright (c) 2024 ScyllaDB. + +package net + +import ( + "encoding/hex" + "fmt" + "net" + "regexp" + "strconv" + "strings" + + "github.com/scylladb/scylla-operator/pkg/helpers/slices" +) + +const ( + procNetEntryRegexLocalIP = "local_ip" + procNetEntryRegexLocalPort = "local_port" + procNetEntryRegexRemoteIP = "remote_ip" + procNetEntryRegexRemotePort = "remote_port" + procNetEntryRegexState = "type" +) + +var ( + procNetEntryRegex = regexp.MustCompile(`^(\s+)?\d+:\s+(?P[0-9A-F]{8,}):(?P[0-9A-F]{4})\s+(?P[0-9A-F]{8,}):(?P[0-9A-F]{4})\s+(?P[0-9A-F]{2})\s+`) +) + +const ( + _ = iota + entryStateEstablishedIndex + entryStateSentIndex + entryStateReceivedIndex + entryStateFinWait1Index + entryStateFinWait2Index + entryStateTimeWaitIndex + entryStateCloseIndex + entryStateCloseWaitIndex + entryStateLastAckIndex + entryStateListenIndex + entryStateClosingIndex + entryStateNewSynReceivedIndex + entryStateBoundInactiveIndex + entryStateMaxStatesIndex +) + +type EntryState string + +// Comes from net/tcp_states.h +const ( + EntryStateEstablished EntryState = "Established" + EntryStateSent EntryState = "Sent" + EntryStateReceived EntryState = "Received" + EntryStateFinWait1 EntryState = "FinWait1" + EntryStateFinWait2 EntryState = "FinWait2" + EntryStateTimeWait EntryState = "TimeWait" + EntryStateClose EntryState = "Close" + EntryStateCloseWait EntryState = "CloseWait" + EntryStateLastAck EntryState = "LastAck" + EntryStateListen EntryState = "Listen" + EntryStateClosing EntryState = "Closing" + EntryStateNewSynReceived EntryState = "NewSynReceived" + EntryStateBoundInactive EntryState = "BoundInactive" + EntryStateMaxStates EntryState = "MaxStates" +) + +func parseEntryState(v int) (EntryState, error) { + switch v { + case entryStateEstablishedIndex: + return EntryStateEstablished, nil + case entryStateSentIndex: + return EntryStateSent, nil + case entryStateReceivedIndex: + return EntryStateReceived, nil + case entryStateFinWait1Index: + return EntryStateFinWait1, nil + case entryStateFinWait2Index: + return EntryStateFinWait2, nil + case entryStateTimeWaitIndex: + return EntryStateTimeWait, nil + case entryStateCloseIndex: + return EntryStateClose, nil + case entryStateCloseWaitIndex: + return EntryStateCloseWait, nil + case entryStateLastAckIndex: + return EntryStateLastAck, nil + case entryStateListenIndex: + return EntryStateListen, nil + case entryStateClosingIndex: + return EntryStateClosing, nil + case entryStateNewSynReceivedIndex: + return EntryStateNewSynReceived, nil + case entryStateBoundInactiveIndex: + return EntryStateBoundInactive, nil + case entryStateMaxStatesIndex: + return EntryStateMaxStates, nil + default: + return "", fmt.Errorf("can't parse entry state %q", v) + } +} + +type AddressPort struct { + Address net.IP + Port int +} + +type ProcNetEntry struct { + LocalAddress AddressPort + RemoteAddress AddressPort + State EntryState +} + +func reverseBytes(data []byte) []byte { + res := make([]byte, 0, len(data)) + + for i := len(data) - 1; i >= 0; i-- { + res = append(res, data[i]) + } + + return res +} + +func decodeIPAddress(hexString string) (net.IP, error) { + v, err := hex.DecodeString(hexString) + if err != nil { + return nil, fmt.Errorf("can't decode address %q: %w", hexString, err) + } + + l := len(v) + if l != net.IPv4len && l != net.IPv6len { + return nil, fmt.Errorf("unexpected address length %d for address %q", l, v) + } + + return net.IP(reverseBytes(v)).To16(), nil +} + +func decodePort(hexString string) (int, error) { + v, err := strconv.ParseUint(hexString, 16, 16) + if err != nil { + return 0, fmt.Errorf("can't parse port %q: %w", hexString, err) + } + + if v >= (1 << 16) { + return 0, fmt.Errorf("port %d is out of range", v) + } + + return int(v), nil +} + +func decodeState(hexString string) (*EntryState, error) { + v, err := strconv.ParseUint(hexString, 16, 16) + if err != nil { + return nil, fmt.Errorf("can't parse state %q: %w", hexString, err) + } + + state, err := parseEntryState(int(v)) + if err != nil { + return nil, fmt.Errorf("can't parse state %q: %w", v, err) + } + + return &state, nil +} + +func ParseProcNetEntry(line string) (*ProcNetEntry, error) { + matches := procNetEntryRegex.FindStringSubmatch(line) + if matches == nil { + return nil, fmt.Errorf("can't parse line %q", line) + } + + localAddress, err := decodeIPAddress(matches[procNetEntryRegex.SubexpIndex(procNetEntryRegexLocalIP)]) + if err != nil { + return nil, fmt.Errorf("can't decode local address: %w", err) + } + + localPort, err := decodePort(matches[procNetEntryRegex.SubexpIndex(procNetEntryRegexLocalPort)]) + if err != nil { + return nil, fmt.Errorf("can't decode local port: %w", err) + } + + remoteAddress, err := decodeIPAddress(matches[procNetEntryRegex.SubexpIndex(procNetEntryRegexRemoteIP)]) + if err != nil { + return nil, fmt.Errorf("can't decode remote address: %w", err) + } + + remotePort, err := decodePort(matches[procNetEntryRegex.SubexpIndex(procNetEntryRegexRemotePort)]) + if err != nil { + return nil, fmt.Errorf("can't decode remote port: %w", err) + } + + state, err := decodeState(matches[procNetEntryRegex.SubexpIndex(procNetEntryRegexState)]) + if err != nil { + return nil, fmt.Errorf("can't decode state: %w", err) + } + + return &ProcNetEntry{ + LocalAddress: AddressPort{ + Address: localAddress, + Port: localPort, + }, + RemoteAddress: AddressPort{ + Address: remoteAddress, + Port: remotePort, + }, + State: *state, + }, nil +} + +type ProcNetEntries []ProcNetEntry + +func ParseProcNetEntries(data string) (ProcNetEntries, error) { + var res ProcNetEntries + + lines := strings.Split(strings.Trim(data, "\n"), "\n") + var procNetEntry *ProcNetEntry + var err error + for _, line := range lines { + procNetEntry, err = ParseProcNetEntry(line) + if err != nil { + return nil, fmt.Errorf("can't parse entry state: %w", err) + } + + res = append(res, *procNetEntry) + } + + return res, nil +} + +func (pe ProcNetEntries) FilterListen() []AddressPort { + filtered := slices.Filter(pe, func(entry ProcNetEntry) bool { + return entry.State == EntryStateListen + }) + return slices.ConvertSlice(filtered, func(from ProcNetEntry) AddressPort { + return from.LocalAddress + }) +} diff --git a/test/e2e/utils/linux/net/net_test.go b/test/e2e/utils/linux/net/net_test.go new file mode 100644 index 00000000000..164937b3fa8 --- /dev/null +++ b/test/e2e/utils/linux/net/net_test.go @@ -0,0 +1,150 @@ +package net + +import ( + "net" + "reflect" + "strings" + "testing" + + "github.com/google/go-cmp/cmp" +) + +func TestParseProcNetEntries(t *testing.T) { + tt := []struct { + name string + procNetString string + expectedEntries ProcNetEntries + expectedErr error + }{ + { + name: "valid entries", + procNetString: strings.TrimLeft(` + 0: 00000000:1F6B 00000000:0000 0A 00000000:00000000 00:00000000 00000000 0 0 9979636 1 0000000000000000 100 0 0 10 0 + 1: 0100007F:2329 00000000:0000 0A 00000000:00000000 00:00000000 00000000 0 0 9977307 1 0000000000000000 100 0 0 10 0 + 11: 0100007F:A6B6 0100007F:2710 06 00000000:00000000 03:000016E6 00000000 0 0 0 3 0000000000000000 + 12: 0100007F:C648 0100007F:2710 01 00000000:00000000 02:00000855 00000000 0 0 10404562 2 0000000000000000 20 4 20 10 -1 + 68: 9700550A:81AC 0100600A:01BB 01 00000000:00000000 02:000003F6 00000000 0 0 9976360 2 0000000000000000 20 4 30 10 -1 + 0: 00000000000000000000000000000000:2711 00000000000000000000000000000000:0000 0A 00000000:00000000 00:00000000 00000000 0 0 9977447 1 0000000000000000 100 0 0 10 0 + 2: 0000000000000000FFFF00000100007F:1C1F 00000000000000000000000000000000:0000 0A 00000000:00000000 00:00000000 00000000 0 0 9976573 1 0000000000000000 100 0 0 10 0 + 7: 0000000000000000FFFF00009700550A:1F90 0000000000000000FFFF00000100550A:8F3E 06 00000000:00000000 03:0000035E 00000000 0 0 0 3 0000000000000000 + 8: 0000000000000000FFFF00009700550A:2711 0000000000000000FFFF00005100550A:929C 01 00000000:00000000 02:00000404 00000000 0 0 10355352 2 0000000000000000 21 4 15 10 -1 +`, "\n"), + expectedEntries: ProcNetEntries{ + { + LocalAddress: AddressPort{ + Address: net.ParseIP("0.0.0.0"), + Port: 8043, + }, + RemoteAddress: AddressPort{ + Address: net.ParseIP("0.0.0.0"), + Port: 0, + }, + State: EntryStateListen, + }, + { + LocalAddress: AddressPort{ + Address: net.ParseIP("127.0.0.1"), + Port: 9001, + }, + RemoteAddress: AddressPort{ + Address: net.ParseIP("0.0.0.0"), + Port: 0, + }, + State: EntryStateListen, + }, + { + LocalAddress: AddressPort{ + Address: net.ParseIP("127.0.0.1"), + Port: 42678, + }, + RemoteAddress: AddressPort{ + Address: net.ParseIP("127.0.0.1"), + Port: 10000, + }, + State: EntryStateTimeWait, + }, + { + LocalAddress: AddressPort{ + Address: net.ParseIP("127.0.0.1"), + Port: 50760, + }, + RemoteAddress: AddressPort{ + Address: net.ParseIP("127.0.0.1"), + Port: 10000, + }, + State: EntryStateEstablished, + }, + { + LocalAddress: AddressPort{ + Address: net.ParseIP("10.85.0.151"), + Port: 33196, + }, + RemoteAddress: AddressPort{ + Address: net.ParseIP("10.96.0.1"), + Port: 443, + }, + State: EntryStateEstablished, + }, + { + LocalAddress: AddressPort{ + Address: net.ParseIP("::"), + Port: 10001, + }, + RemoteAddress: AddressPort{ + Address: net.ParseIP("::"), + Port: 0, + }, + State: EntryStateListen, + }, + { + LocalAddress: AddressPort{ + Address: net.ParseIP("7f00:1:0:ffff::"), + Port: 7199, + }, + RemoteAddress: AddressPort{ + Address: net.ParseIP("::"), + Port: 0, + }, + State: EntryStateListen, + }, + { + LocalAddress: AddressPort{ + Address: net.ParseIP("a55:97:0:ffff::"), + Port: 8080, + }, + RemoteAddress: AddressPort{ + Address: net.ParseIP("a55:1:0:ffff::"), + Port: 36670, + }, + State: EntryStateTimeWait, + }, + { + LocalAddress: AddressPort{ + Address: net.ParseIP("a55:97:0:ffff::"), + Port: 10001, + }, + RemoteAddress: AddressPort{ + Address: net.ParseIP("a55:51:0:ffff::"), + Port: 37532, + }, + State: EntryStateEstablished, + }, + }, + }, + } + + for i := range tt { + tc := &tt[i] + t.Run(tc.name, func(t *testing.T) { + got, err := ParseProcNetEntries(tc.procNetString) + + if !reflect.DeepEqual(tc.expectedErr, err) { + t.Errorf("expected and got error differ: %s", cmp.Diff(tc.expectedErr, err)) + } + + if !reflect.DeepEqual(tc.expectedEntries, got) { + t.Errorf("expected and got entries differ: %s", cmp.Diff(tc.expectedEntries, got)) + } + }) + } +}