diff --git a/docs/cmd-coil-egress.md b/docs/cmd-coil-egress.md index 24370d94..78d40a9a 100644 --- a/docs/cmd-coil-egress.md +++ b/docs/cmd-coil-egress.md @@ -123,3 +123,27 @@ This value is from the result of `iptables -t filter -L -vn` or nftables equival | `pod` | The pod name | | `protocol` | The protocol name | +### `coil_egress_nftables_peer_src_dropped_packets_total` + +This is the total number of packets from clients dropped by iptables/nftables in a egress NAT pod because their source addresses are not the IP addresses of the clients. +This value is the sum of the counters of the rules commented as `coil-peer-src-*` in the result of `iptables -t filter -L FORWARD -vn` and `iptables -t filter -L INPUT -vn` or nftables equivalent. + +| Label | Description | +| ----------- | ----------------------------- | +| `namespace` | The egress resource namespace | +| `egress` | The egress resource name | +| `pod` | The pod name | +| `protocol` | The protocol name | + +### `coil_egress_nftables_peer_src_dropped_bytes_total` + +This is the total bytes of packets from clients dropped by iptables/nftables in a egress NAT pod because their source addresses are not the IP addresses of the clients. +This value is the sum of the counters of the rules commented as `coil-peer-src-*` in the result of `iptables -t filter -L FORWARD -vn` and `iptables -t filter -L INPUT -vn` or nftables equivalent. + +| Label | Description | +| ----------- | ----------------------------- | +| `namespace` | The egress resource namespace | +| `egress` | The egress resource name | +| `pod` | The pod name | +| `protocol` | The protocol name | + diff --git a/docs/usage.md b/docs/usage.md index 390a4c2a..06b57e6a 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -22,6 +22,7 @@ For installation, read [setup.md](setup.md). - [Use NetworkPolicy to prohibit NAT usage](#use-networkpolicy-to-prohibit-nat-usage) - [Session affinity](#session-affinity) - [Use egress only for connections originating on the client](#use-egress-only-for-connections-originating-on-the-client) + - [Source address verification](#source-address-verification) - [Metrics](#metrics) - [How to scrape metrics](#how-to-scrape-metrics) @@ -321,6 +322,24 @@ In case of incomming connections, the same interface will be used for egress tra e.g. if connection will be estabilished on `eth0`, the traffic will not be routed through `fou`, but will be handled by `eth0`. +### Source address verification + +NAT servers forward packets from a client only when their source address is +the IP address of the client Pod. Other packets from the client are dropped. +This prevents a client from spoofing the source address to make the NAT server +deliver reply packets to another Pod. +The number of dropped packets is exported as +`coil_egress_nftables_peer_src_dropped_packets_total` metrics. + +If a client Pod forwards traffic of nested network namespaces or containers +through Egress, the traffic must be SNATed to the Pod IP address before it +leaves the Pod. Docker's default masquerade rule does this, so containers +run by Docker in a client Pod work without extra configuration. + +NAT servers also drop FoU packets whose outer source address is not the IP +address of a client Pod. Therefore, traffic from client Pods to the ClusterIP +of the Egress Service must not be SNATed. + ## Metrics Coil exposes two types of Prometheus metrics. diff --git a/v2/cmd/coil-egress/sub/run.go b/v2/cmd/coil-egress/sub/run.go index 44f63826..8df232ff 100644 --- a/v2/cmd/coil-egress/sub/run.go +++ b/v2/cmd/coil-egress/sub/run.go @@ -45,6 +45,8 @@ func init() { metrics.Registry.MustRegister(egressMetrics.NfTableMasqueradePackets) metrics.Registry.MustRegister(egressMetrics.NfTableInvalidBytes) metrics.Registry.MustRegister(egressMetrics.NfTableInvalidPackets) + metrics.Registry.MustRegister(egressMetrics.NfTablePeerSrcDroppedBytes) + metrics.Registry.MustRegister(egressMetrics.NfTablePeerSrcDroppedPackets) } func subMain() error { @@ -102,7 +104,7 @@ func subMain() error { } setupLog.Info("initialize FoU tunnel", "port", config.port, "ipv4", ipv4.String(), "ipv6", ipv6.String()) - ft := fou.NewFoUTunnel(config.port, ipv4, ipv6, nil) + ft := fou.NewServerFoUTunnel(config.port, ipv4, ipv6, nil) if err := ft.Init(); err != nil { return err } diff --git a/v2/controllers/egress_watcher.go b/v2/controllers/egress_watcher.go index 10f7cf76..2fd8be34 100644 --- a/v2/controllers/egress_watcher.go +++ b/v2/controllers/egress_watcher.go @@ -198,7 +198,7 @@ func (r *EgressWatcher) hook(gwn gwNets, log *logr.Logger) func(ipv4, ipv6 net.I return func(ipv4, ipv6 net.IP) error { // We assume that coild already has configured NAT for the client, // so we ensure that both FoUTunnel and NATClient have been initialized. - ft := fou.NewFoUTunnel(r.EgressPort, ipv4, ipv6, func(message string) { + ft := fou.NewClientFoUTunnel(r.EgressPort, ipv4, ipv6, func(message string) { log.Info(message) }) if !ft.IsInitialized() { @@ -212,6 +212,10 @@ func (r *EgressWatcher) hook(gwn gwNets, log *logr.Logger) func(ipv4, ipv6 net.I return fmt.Errorf("natClient hasn't been initialized: %w", err) } + if err := cl.EnsureFlowBasedDevRules(); err != nil { + return err + } + link, err := ft.AddPeer(gwn.gateway, gwn.sportAuto) if errors.Is(err, fou.ErrIPFamilyMismatch) { // ignore unsupported IP family link diff --git a/v2/controllers/pod_watcher.go b/v2/controllers/pod_watcher.go index 16120995..6852da05 100644 --- a/v2/controllers/pod_watcher.go +++ b/v2/controllers/pod_watcher.go @@ -266,9 +266,14 @@ OUTER2: } } logger.Info("delete peer", "caller", "addPod", "ip", eip.String(), "podIPs", podIPs, "existing", existing) + // Delete the link before the source address verification rule + // so that the link never forwards unverified packets. if err := r.ft.DelPeer(eip); err != nil { return err } + if err := r.nat.DelClient(eip); err != nil { + return err + } if n := ClientPodInfo.DeletePartialMatch(prometheus.Labels{"namespace": pod.GetNamespace(), "pod": pod.GetName(), "pod_ip": eip.String(), "egress": r.myName, "egress_namespace": r.myNS}); n != 1 { logger.Error(errors.New("metrics deletion error"), "the number of deleted metrics is not one for", "pod", pod.GetName(), "namespace", pod.GetNamespace()) } @@ -314,9 +319,13 @@ func (r *podWatcher) delPeer(n types.NamespacedName, caller, podPhase string, lo } if !existsLivePeers { logger.Info("delete peer", "caller", caller, "ip", ip.String(), "podIPs", r.podAddrs[key], "podPhase", podPhase) + // See addPod for the order of deletion. if err := r.ft.DelPeer(ip); err != nil { return err } + if err := r.nat.DelClient(ip); err != nil { + return err + } } if keySet, ok := r.peers[ip.String()]; ok { diff --git a/v2/controllers/pod_watcher_test.go b/v2/controllers/pod_watcher_test.go index 517da08d..80482cb8 100644 --- a/v2/controllers/pod_watcher_test.go +++ b/v2/controllers/pod_watcher_test.go @@ -238,7 +238,6 @@ var _ = Describe("Pod watcher", Ordered, func() { "fd01::1": {}, "10.1.1.2": {}, "fd01::2": {}, - "fd01::3": {}, // nat.Server does not have remove API "10.1.1.7": {}, "fd01::7": {}, }) @@ -275,9 +274,7 @@ var _ = Describe("Pod watcher", Ordered, func() { Eventually(func() bool { return reflect.DeepEqual(nat.GetClients(), map[string]struct{}{ - "10.1.1.2": {}, // cannot be removed - "fd01::2": {}, // ditto - "fd01::3": {}, + "fd01::3": {}, }) }).Should(BeTrue()) @@ -333,6 +330,10 @@ var _ = Describe("Pod watcher", Ordered, func() { "10.1.1.2": true, "fd01::2": true, "fd01::3": true, + }) && reflect.DeepEqual(nat.GetClients(), map[string]struct{}{ + "10.1.1.2": {}, + "fd01::2": {}, + "fd01::3": {}, }) }).Should(BeTrue()) @@ -354,6 +355,11 @@ var _ = Describe("Pod watcher", Ordered, func() { "fd01::2": true, "fd01::3": true, "10.1.1.5": true, + }) && reflect.DeepEqual(nat.GetClients(), map[string]struct{}{ + "10.1.1.2": {}, + "fd01::2": {}, + "fd01::3": {}, + "10.1.1.5": {}, }) }, 5*time.Second, 1*time.Second).Should(BeTrue()) }) diff --git a/v2/pkg/fou/fou.go b/v2/pkg/fou/fou.go index e8575010..dc794af4 100644 --- a/v2/pkg/fou/fou.go +++ b/v2/pkg/fou/fou.go @@ -19,6 +19,13 @@ const ( FoU6LinkPrefix = "fou6_" ) +// Names of the flow based IPIP tunnel devices created for NAT clients. +// See setupFlowBasedIP4TunDevice. +const ( + FlowBasedIP4TunDevice = "coil_ipip4" + FlowBasedIP6TunDevice = "coil_ipip6" +) + const fouDummy = "fou-dummy" func fouName(addr net.IP) string { @@ -56,11 +63,41 @@ type FoUTunnel interface { DelPeer(net.IP) error } -// NewFoUTunnel creates a new FoUTunnel. +// tunnelRole represents which side of the egress NAT a FoUTunnel works for. +type tunnelRole int + +const ( + // roleClient is for NAT clients, i.e. Pods using Egress. + roleClient tunnelRole = iota + // roleServer is for NAT servers, i.e. egress Pods. + roleServer +) + +// NewClientFoUTunnel creates a new FoUTunnel for NAT clients. +// +// The tunnel sets up flow based IPIP tunnel devices to receive returning +// packets from egress Pods, and does not enable IP forwarding. +// See setupFlowBasedIP4TunDevice for details. +// // port is the UDP port to receive FoU packets. // localIPv4 is the local IPv4 address of the IPIP tunnel. This can be nil. // localIPv6 is the same as localIPv4 for IPv6. -func NewFoUTunnel(port int, localIPv4, localIPv6 net.IP, logFunc func(string)) FoUTunnel { +func NewClientFoUTunnel(port int, localIPv4, localIPv6 net.IP, logFunc func(string)) FoUTunnel { + return newFoUTunnel(roleClient, port, localIPv4, localIPv6, logFunc) +} + +// NewServerFoUTunnel creates a new FoUTunnel for NAT servers. +// +// The tunnel enables IP forwarding to route packets from clients, and does not +// set up flow based IPIP tunnel devices so that only packets from registered +// peers are decapsulated. +// +// The parameters are the same as NewClientFoUTunnel. +func NewServerFoUTunnel(port int, localIPv4, localIPv6 net.IP, logFunc func(string)) FoUTunnel { + return newFoUTunnel(roleServer, port, localIPv4, localIPv6, logFunc) +} + +func newFoUTunnel(role tunnelRole, port int, localIPv4, localIPv6 net.IP, logFunc func(string)) FoUTunnel { if localIPv4 != nil && localIPv4.To4() == nil { panic("invalid IPv4 address") } @@ -68,6 +105,7 @@ func NewFoUTunnel(port int, localIPv4, localIPv6 net.IP, logFunc func(string)) F panic("invalid IPv6 address") } return &fouTunnel{ + role: role, port: port, local4: localIPv4, local6: localIPv6, @@ -76,6 +114,7 @@ func NewFoUTunnel(port int, localIPv4, localIPv6 net.IP, logFunc func(string)) F } type fouTunnel struct { + role tunnelRole port int local4 net.IP local6 net.IP @@ -113,8 +152,11 @@ func (t *fouTunnel) Init() error { if _, err := sysctl.Sysctl("net.ipv4.conf.all.rp_filter", "0"); err != nil { return fmt.Errorf("setting net.ipv4.conf.all.rp_filter=0 failed: %w", err) } - if err := ip.EnableIP4Forward(); err != nil { - return fmt.Errorf("failed to enable IPv4 forwarding: %w", err) + // NAT clients only send and receive their own packets. + if t.role == roleServer { + if err := ip.EnableIP4Forward(); err != nil { + return fmt.Errorf("failed to enable IPv4 forwarding: %w", err) + } } } if t.local6 != nil { @@ -130,8 +172,10 @@ func (t *fouTunnel) Init() error { if err != nil { return fmt.Errorf("netlink: fou add failed: %w", err) } - if err := ip.EnableIP6Forward(); err != nil { - return fmt.Errorf("failed to enable IPv6 forwarding: %w", err) + if t.role == roleServer { + if err := ip.EnableIP6Forward(); err != nil { + return fmt.Errorf("failed to enable IPv6 forwarding: %w", err) + } } // avoid any existing DROP rule by rpfilter extension. @@ -175,8 +219,10 @@ func (t *fouTunnel) addPeer4(addr net.IP, sportAuto bool) (netlink.Link, error) return nil, err } - if err := setupFlowBasedIP4TunDevice(); err != nil { - return nil, fmt.Errorf("netlink: failed to setup ipip device: %w", err) + if t.role == roleClient { + if err := setupFlowBasedIP4TunDevice(); err != nil { + return nil, fmt.Errorf("netlink: failed to setup ipip device: %w", err) + } } return netlink.LinkByName(linkName) @@ -249,8 +295,10 @@ func (t *fouTunnel) addPeer6(addr net.IP, sportAuto bool) (netlink.Link, error) return nil, err } - if err := setupFlowBasedIP6TunDevice(); err != nil { - return nil, fmt.Errorf("netlink: failed to setup ipip device: %w", err) + if t.role == roleClient { + if err := setupFlowBasedIP6TunDevice(); err != nil { + return nil, fmt.Errorf("netlink: failed to setup ipip device: %w", err) + } } return netlink.LinkByName(linkName) @@ -312,6 +360,14 @@ func (t *fouTunnel) addOrRecreatePeer6(addr net.IP, sportAuto bool) (string, err // This flow based IPIP tunnel device is used to decapsulate packets from // the router Pods. // +// It is needed only for NAT clients. A client sends packets to the Service's +// ClusterIP, but the returning packets come from the router Pod's IP address, +// so they do not match the IPIP tunnel device configured for the ClusterIP. +// Since this device accepts packets from any source, NAT servers must not +// create it; packets from clients always match the per-peer tunnel devices. +// On NAT servers, the fallback devices are left as they are. They are down +// and never used for decapsulation. +// // Calling this function may result in tunl0 (v4) or ip6tnl0 (v6) // fallback interface being renamed to coil_tunl or coil_ip6tnl. // This is to communicate to the user that this plugin has taken @@ -326,7 +382,7 @@ func (t *fouTunnel) addOrRecreatePeer6(addr net.IP, sportAuto bool) (string, err // By default, these interfaces will be created in new network namespaces, // but this behavior can be disabled by setting net.core.fb_tunnels_only_for_init_net = 2. func setupFlowBasedIP4TunDevice() error { - ipip4Device := "coil_ipip4" + ipip4Device := FlowBasedIP4TunDevice // Set up IPv4 tunnel device if requested. if err := setupDevice(&netlink.Iptun{ LinkAttrs: netlink.LinkAttrs{Name: ipip4Device}, @@ -346,7 +402,7 @@ func setupFlowBasedIP4TunDevice() error { // See setupFlowBasedIP4TunDevice func setupFlowBasedIP6TunDevice() error { - ipip6Device := "coil_ipip6" + ipip6Device := FlowBasedIP6TunDevice // Set up IPv6 tunnel device if requested. if err := setupDevice(&netlink.Ip6tnl{ diff --git a/v2/pkg/fou/fou_test.go b/v2/pkg/fou/fou_test.go index 3c50b692..fb3a7851 100644 --- a/v2/pkg/fou/fou_test.go +++ b/v2/pkg/fou/fou_test.go @@ -8,6 +8,8 @@ import ( "testing" "github.com/containernetworking/plugins/pkg/ns" + "github.com/containernetworking/plugins/pkg/testutils" + "github.com/containernetworking/plugins/pkg/utils/sysctl" "github.com/vishvananda/netlink" ) @@ -15,6 +17,7 @@ func TestFoU(t *testing.T) { t.Run("Dual", testFoUDual) t.Run("IPv4", testFoUV4) t.Run("IPv6", testFoUV6) + t.Run("Server", testFoUServer) } func testFoUDual(t *testing.T) { @@ -50,7 +53,15 @@ func testFoUDual(t *testing.T) { return fmt.Errorf("netlink: failed to add an IPv6 address: %w", err) } - fou := NewFoUTunnel(5555, net.ParseIP("127.0.0.1"), net.ParseIP("::1"), nil) + // IPv4 forwarding of a new netns is inherited from the host. + // Reset it to check that Init does not enable forwarding. + for _, key := range []string{"net.ipv4.ip_forward", "net.ipv6.conf.all.forwarding"} { + if _, err := sysctl.Sysctl(key, "0"); err != nil { + return fmt.Errorf("failed to set %s: %w", key, err) + } + } + + fou := NewClientFoUTunnel(5555, net.ParseIP("127.0.0.1"), net.ParseIP("::1"), nil) if fou.IsInitialized() { return errors.New("expect not to be initialized, but it's already been done") } @@ -68,6 +79,14 @@ func testFoUDual(t *testing.T) { return fmt.Errorf("fou.Init again failed: %w", err) } + // NAT clients do not forward packets + if err := expectSysctl("net.ipv4.ip_forward", "0"); err != nil { + return err + } + if err := expectSysctl("net.ipv6.conf.all.forwarding", "0"); err != nil { + return err + } + fous, err := netlink.FouList(0) if err != nil { return fmt.Errorf("failed to list fou links: %w", err) @@ -261,7 +280,7 @@ func testFoUV4(t *testing.T) { return err } - fou := NewFoUTunnel(5555, net.ParseIP("127.0.0.1"), nil, nil) + fou := NewClientFoUTunnel(5555, net.ParseIP("127.0.0.1"), nil, nil) if fou.IsInitialized() { return errors.New("expect not to be initialized, but it's already been done") } @@ -358,7 +377,7 @@ func testFoUV6(t *testing.T) { return err } - fou := NewFoUTunnel(5555, nil, net.ParseIP("::1"), nil) + fou := NewClientFoUTunnel(5555, nil, net.ParseIP("::1"), nil) if fou.IsInitialized() { return errors.New("expect not to be initialized, but it's already been done") } @@ -427,3 +446,61 @@ func testFoUV6(t *testing.T) { t.Error(err) } } + +func testFoUServer(t *testing.T) { + t.Parallel() + + fNS, err := testutils.NewNS() + if err != nil { + t.Fatal(err) + } + defer fNS.Close() + + err = fNS.Do(func(ns.NetNS) error { + fou := NewServerFoUTunnel(5555, net.ParseIP("127.0.0.1"), net.ParseIP("::1"), nil) + if err := fou.Init(); err != nil { + return fmt.Errorf("fou.Init failed: %w", err) + } + + // NAT servers forward packets from clients + if err := expectSysctl("net.ipv4.ip_forward", "1"); err != nil { + return err + } + if err := expectSysctl("net.ipv6.conf.all.forwarding", "1"); err != nil { + return err + } + + if _, err := fou.AddPeer(net.ParseIP("10.1.1.1"), true); err != nil { + return fmt.Errorf("failed to call AddPeer with 10.1.1.1: %w", err) + } + if _, err := fou.AddPeer(net.ParseIP("fd02::101"), true); err != nil { + return fmt.Errorf("failed to call AddPeer with fd02::101: %w", err) + } + + // NAT servers must not accept packets from unknown peers + for _, name := range []string{"coil_ipip4", "coil_ipip6"} { + _, err := netlink.LinkByName(name) + if err == nil { + return fmt.Errorf("unexpected flow based device: %s", name) + } + if !errors.As(err, new(netlink.LinkNotFoundError)) { + return fmt.Errorf("failed to get %s: %w", name, err) + } + } + return nil + }) + if err != nil { + t.Error(err) + } +} + +func expectSysctl(key, want string) error { + got, err := sysctl.Sysctl(key) + if err != nil { + return fmt.Errorf("failed to get %s: %w", key, err) + } + if got != want { + return fmt.Errorf("unexpected %s: got %s, want %s", key, got, want) + } + return nil +} diff --git a/v2/pkg/metrics/egress.go b/v2/pkg/metrics/egress.go index 7ee32e5f..7e7e7785 100644 --- a/v2/pkg/metrics/egress.go +++ b/v2/pkg/metrics/egress.go @@ -10,9 +10,11 @@ import ( "github.com/google/nftables" "github.com/google/nftables/expr" + "github.com/google/nftables/xt" "github.com/prometheus/client_golang/prometheus" "github.com/cybozu-go/coil/v2/pkg/constants" + "github.com/cybozu-go/coil/v2/pkg/nat/netfilter" ) const ( @@ -62,6 +64,20 @@ var ( Name: "nftables_invalid_bytes_total", Help: "the number of bytes that are dropped as invalid packets by nftables", }, []string{"namespace", "pod", "egress", "protocol"}) + + NfTablePeerSrcDroppedPackets = prometheus.NewGaugeVec(prometheus.GaugeOpts{ + Namespace: constants.MetricsNS, + Subsystem: "egress", + Name: "nftables_peer_src_dropped_packets_total", + Help: "the number of packets from clients that are dropped by nftables because of unexpected source addresses", + }, []string{"namespace", "pod", "egress", "protocol"}) + + NfTablePeerSrcDroppedBytes = prometheus.NewGaugeVec(prometheus.GaugeOpts{ + Namespace: constants.MetricsNS, + Subsystem: "egress", + Name: "nftables_peer_src_dropped_bytes_total", + Help: "the number of bytes from clients that are dropped by nftables because of unexpected source addresses", + }, []string{"namespace", "pod", "egress", "protocol"}) ) type egressCollector struct { @@ -72,18 +88,22 @@ type egressCollector struct { } type nfTablesPerProtocolMetrics struct { - NATPackets prometheus.Gauge - NATBytes prometheus.Gauge - InvalidPackets prometheus.Gauge - InvalidBytes prometheus.Gauge + NATPackets prometheus.Gauge + NATBytes prometheus.Gauge + InvalidPackets prometheus.Gauge + InvalidBytes prometheus.Gauge + PeerSrcDroppedPackets prometheus.Gauge + PeerSrcDroppedBytes prometheus.Gauge } func newNfTablesPerProtocolMetrics(ns, pod, egress, protocol string) *nfTablesPerProtocolMetrics { return &nfTablesPerProtocolMetrics{ - NATPackets: NfTableMasqueradePackets.WithLabelValues(ns, pod, egress, protocol), - NATBytes: NfTableMasqueradeBytes.WithLabelValues(ns, pod, egress, protocol), - InvalidPackets: NfTableInvalidPackets.WithLabelValues(ns, pod, egress, protocol), - InvalidBytes: NfTableInvalidBytes.WithLabelValues(ns, pod, egress, protocol), + NATPackets: NfTableMasqueradePackets.WithLabelValues(ns, pod, egress, protocol), + NATBytes: NfTableMasqueradeBytes.WithLabelValues(ns, pod, egress, protocol), + InvalidPackets: NfTableInvalidPackets.WithLabelValues(ns, pod, egress, protocol), + InvalidBytes: NfTableInvalidBytes.WithLabelValues(ns, pod, egress, protocol), + PeerSrcDroppedPackets: NfTablePeerSrcDroppedPackets.WithLabelValues(ns, pod, egress, protocol), + PeerSrcDroppedBytes: NfTablePeerSrcDroppedBytes.WithLabelValues(ns, pod, egress, protocol), } } @@ -94,6 +114,8 @@ func NewEgressCollector(ns, pod, egress string, protocols []string) (Collector, NfTableMasqueradePackets.Reset() NfTableInvalidPackets.Reset() NfTableInvalidBytes.Reset() + NfTablePeerSrcDroppedPackets.Reset() + NfTablePeerSrcDroppedBytes.Reset() c, err := nftables.New() if err != nil { @@ -145,6 +167,12 @@ func (c *egressCollector) Update(ctx context.Context) error { nfTablesMetrics.InvalidPackets.Set(float64(invalidPackets)) nfTablesMetrics.InvalidBytes.Set(float64(invalidBytes)) + peerSrcDroppedPackets, peerSrcDroppedBytes, err := c.getNfTablesPeerSrcDroppedCounter(protocol) + if err != nil { + return err + } + nfTablesMetrics.PeerSrcDroppedPackets.Set(float64(peerSrcDroppedPackets)) + nfTablesMetrics.PeerSrcDroppedBytes.Set(float64(peerSrcDroppedBytes)) } return nil @@ -191,15 +219,83 @@ func (c *egressCollector) getNfTablesInvalidCounter(protocol string) (uint64, ui if err != nil { return 0, 0, err } + packets, bytes, found := sumCounters(rules, func(tag string) bool { + return tag == netfilter.RuleTagInvalidDrop + }) + if !found { + return 0, 0, errors.New("a rule for invalid packets is not found") + } + return packets, bytes, nil +} + +// getNfTablesPeerSrcDroppedCounter sums the counters of the rules that drop +// packets from clients with unexpected source addresses. The rules exist +// in FORWARD and INPUT chains for each client, and there are none if +// no client has been added. +func (c *egressCollector) getNfTablesPeerSrcDroppedCounter(protocol string) (uint64, uint64, error) { + family, err := stringToTableFamily(protocol) + if err != nil { + return 0, 0, err + } + // GetRules fails if the chain does not exist, e.g. INPUT chain before + // any client is added, so get rules only from existing chains. + chains, err := c.conn.ListChainsOfTableFamily(family) + if err != nil { + return 0, 0, err + } + + var packets, bytes uint64 + for _, chain := range chains { + if chain.Table.Name != "filter" || (chain.Name != "FORWARD" && chain.Name != "INPUT") { + continue + } + rules, err := c.conn.GetRules(chain.Table, chain) + if err != nil { + return 0, 0, err + } + p, b, _ := sumCounters(rules, func(tag string) bool { + return strings.HasPrefix(tag, netfilter.RuleTagPeerSrcPrefix) + }) + packets += p + bytes += b + } + return packets, bytes, nil +} + +// sumCounters sums the counters of the rules whose tags satisfy match. +// found reports whether any rule satisfies match. +func sumCounters(rules []*nftables.Rule, match func(tag string) bool) (packets, bytes uint64, found bool) { for _, rule := range rules { + if !match(ruleTag(rule)) { + continue + } + found = true for _, e := range rule.Exprs { if counter, ok := e.(*expr.Counter); ok { - // A rule in the egress pod must be only one, so we can return by finding a first one. - return counter.Packets, counter.Bytes, nil + packets += counter.Packets + bytes += counter.Bytes } } } - return 0, 0, errors.New("a rule for invalid packets is not found") + return packets, bytes, found +} + +// ruleTag returns the tag of a rule set by coil. The tag is stored as +// UserData for nftables, and as a comment match for iptables-nft. +func ruleTag(rule *nftables.Rule) string { + if len(rule.UserData) != 0 { + return string(rule.UserData) + } + for _, e := range rule.Exprs { + m, ok := e.(*expr.Match) + if !ok || m.Name != "comment" { + continue + } + if c, ok := m.Info.(*xt.Comment); ok { + return string(*c) + } + } + return "" } func readUintFromFile(path string) (uint64, error) { diff --git a/v2/pkg/metrics/egress_test.go b/v2/pkg/metrics/egress_test.go new file mode 100644 index 00000000..5d27bb86 --- /dev/null +++ b/v2/pkg/metrics/egress_test.go @@ -0,0 +1,129 @@ +package metrics + +import ( + "strings" + "testing" + + "github.com/google/nftables" + "github.com/google/nftables/expr" + "github.com/google/nftables/xt" + + "github.com/cybozu-go/coil/v2/pkg/nat/netfilter" +) + +// nftRule returns a rule tagged with UserData as the nftables backend does. +func nftRule(tag string, packets, bytes uint64) *nftables.Rule { + return &nftables.Rule{ + UserData: []byte(tag), + Exprs: []expr.Any{ + &expr.Counter{Packets: packets, Bytes: bytes}, + &expr.Verdict{Kind: expr.VerdictDrop}, + }, + } +} + +// iptRule returns a rule tagged with a comment match as iptables-nft does. +func iptRule(tag string, packets, bytes uint64) *nftables.Rule { + comment := xt.Comment(tag) + return &nftables.Rule{ + Exprs: []expr.Any{ + &expr.Match{Name: "comment", Info: &comment}, + &expr.Counter{Packets: packets, Bytes: bytes}, + &expr.Verdict{Kind: expr.VerdictDrop}, + }, + } +} + +func TestRuleTag(t *testing.T) { + tests := []struct { + name string + rule *nftables.Rule + want string + }{ + {"nftables", nftRule(netfilter.RuleTagInvalidDrop, 0, 0), netfilter.RuleTagInvalidDrop}, + {"iptables", iptRule(netfilter.RuleTagInvalidDrop, 0, 0), netfilter.RuleTagInvalidDrop}, + {"untagged", &nftables.Rule{Exprs: []expr.Any{&expr.Counter{}}}, ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := ruleTag(tt.rule); got != tt.want { + t.Errorf("ruleTag() = %q, want %q", got, tt.want) + } + }) + } +} + +func TestSumCounters(t *testing.T) { + isInvalid := func(tag string) bool { return tag == netfilter.RuleTagInvalidDrop } + isPeerSrc := func(tag string) bool { return strings.HasPrefix(tag, netfilter.RuleTagPeerSrcPrefix) } + + tests := []struct { + name string + rules []*nftables.Rule + match func(string) bool + wantPackets uint64 + wantBytes uint64 + wantFound bool + }{ + { + name: "invalid rule is not the first one (nftables)", + rules: []*nftables.Rule{ + nftRule(netfilter.RuleTagPeerSrcPrefix+"fou4_0a000001", 1, 10), + nftRule(netfilter.RuleTagInvalidDrop, 2, 20), + }, + match: isInvalid, + wantPackets: 2, + wantBytes: 20, + wantFound: true, + }, + { + name: "invalid rule is not the first one (iptables)", + rules: []*nftables.Rule{ + iptRule(netfilter.RuleTagPeerSrcPrefix+"fou4_0a000001", 1, 10), + {Exprs: []expr.Any{&expr.Counter{Packets: 5, Bytes: 50}}}, + iptRule(netfilter.RuleTagInvalidDrop, 2, 20), + }, + match: isInvalid, + wantPackets: 2, + wantBytes: 20, + wantFound: true, + }, + { + name: "invalid rule is missing", + rules: []*nftables.Rule{ + {Exprs: []expr.Any{&expr.Counter{Packets: 5, Bytes: 50}}}, + }, + match: isInvalid, + wantFound: false, + }, + { + name: "peer source rules are summed", + rules: []*nftables.Rule{ + nftRule(netfilter.RuleTagInvalidDrop, 2, 20), + nftRule(netfilter.RuleTagPeerSrcPrefix+"fou4_0a000001", 1, 10), + nftRule(netfilter.RuleTagPeerSrcPrefix+"fou4_0a000002", 3, 30), + }, + match: isPeerSrc, + wantPackets: 4, + wantBytes: 40, + wantFound: true, + }, + { + name: "no peer source rules", + rules: []*nftables.Rule{ + nftRule(netfilter.RuleTagInvalidDrop, 2, 20), + }, + match: isPeerSrc, + wantFound: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + packets, bytes, found := sumCounters(tt.rules, tt.match) + if packets != tt.wantPackets || bytes != tt.wantBytes || found != tt.wantFound { + t.Errorf("sumCounters() = (%d, %d, %v), want (%d, %d, %v)", + packets, bytes, found, tt.wantPackets, tt.wantBytes, tt.wantFound) + } + }) + } +} diff --git a/v2/pkg/nat/mock/server.go b/v2/pkg/nat/mock/server.go index 0c254391..d077b807 100644 --- a/v2/pkg/nat/mock/server.go +++ b/v2/pkg/nat/mock/server.go @@ -34,6 +34,14 @@ func (n *NatServer) AddClient(ip net.IP, _ netlink.Link) error { return nil } +func (n *NatServer) DelClient(ip net.IP) error { + n.mu.Lock() + defer n.mu.Unlock() + + delete(n.ips, ip.String()) + return nil +} + func (n *NatServer) GetClients() map[string]struct{} { n.mu.RLock() defer n.mu.RUnlock() diff --git a/v2/pkg/nat/netfilter/client.go b/v2/pkg/nat/netfilter/client.go index 33cf889c..a5375d80 100644 --- a/v2/pkg/nat/netfilter/client.go +++ b/v2/pkg/nat/netfilter/client.go @@ -225,9 +225,45 @@ func (n *NatClient) initRules(family int) error { return fmt.Errorf("netlink: failed to add %s to rule: %w", ipn.String(), err) } } + + return n.setFlowBasedDevRules(family) +} + +// EnsureFlowBasedDevRules sets up the netfilter rules for the flow based +// tunnel devices. This is idempotent and intended for clients that have +// been initialized by an older version which did not set up these rules. +func (n *NatClient) EnsureFlowBasedDevRules() error { + if n.ipv4 != nil { + if err := n.setFlowBasedDevRules(netlink.FAMILY_V4); err != nil { + return fmt.Errorf("failed to set up IPv4 rules: %w", err) + } + } + if n.ipv6 != nil { + if err := n.setFlowBasedDevRules(netlink.FAMILY_V6); err != nil { + return fmt.Errorf("failed to set up IPv6 rules: %w", err) + } + } return nil } +// setFlowBasedDevRules drops new flows from the flow based tunnel device. +// +// The device accepts packets from any source, and IPv4 forwarding is usually +// enabled as it is inherited from the host. Dropping new flows prevents this +// client from being used as a relay and from accepting packets with spoofed +// source addresses. Only returning packets from egress Pods are accepted or +// forwarded. +func (n *NatClient) setFlowBasedDevRules(family int) error { + switch n.backend { + case constants.EgressBackendIPTables: + return setIPTablesFlowBasedDevNewDropRule(family) + case constants.EgressBackendNFTables: + return setNFTablesFlowBasedDevNewDropRule(family) + default: + return fmt.Errorf("invalid nat backend: %s", n.backend) + } +} + func (n *NatClient) clear(family int) error { var gw *net.IPNet switch family { diff --git a/v2/pkg/nat/netfilter/client_test.go b/v2/pkg/nat/netfilter/client_test.go index b9c040af..f18e4978 100644 --- a/v2/pkg/nat/netfilter/client_test.go +++ b/v2/pkg/nat/netfilter/client_test.go @@ -997,3 +997,85 @@ func newDummyDevice(name string) (netlink.Link, error) { return link, nil } + +func TestNatClient_FlowBasedDevNewDropRule(t *testing.T) { + ipv4 := net.ParseIP("10.1.1.1") + ipv6 := net.ParseIP("fd02::1") + + // EnsureFlowBasedDevRules is for clients initialized by an older version, + // so it must work without Init. + setups := map[string]func(*NatClient) error{ + "Init": (*NatClient).Init, + "EnsureFlowBasedDevRules": (*NatClient).EnsureFlowBasedDevRules, + } + + for _, backend := range []string{constants.EgressBackendIPTables, constants.EgressBackendNFTables} { + for name, setup := range setups { + t.Run(backend+"/"+name, func(t *testing.T) { + t.Parallel() + + tns, err := testutils.NewNS() + if err != nil { + t.Fatal(err) + } + defer tns.Close() + + if err := tns.Do(func(ns ns.NetNS) error { + nc := NewNatClient(ipv4, ipv6, nil, backend, nil) + + // to check idempotency, call it twice + for i := 0; i < 2; i++ { + if err := setup(nc); err != nil { + return fmt.Errorf("%s() error = %w", name, err) + } + } + + chains := []struct { + name string + nft func(nftables.TableFamily) (*nftables.Table, *nftables.Chain) + }{ + {filterChain, nftFilterForwardChain}, + {inputChain, nftFilterInputChain}, + } + for _, family := range []int{netlink.FAMILY_V4, netlink.FAMILY_V6} { + for _, chain := range chains { + var got int + switch backend { + case constants.EgressBackendIPTables: + ipt, err := newIPTables(family) + if err != nil { + return err + } + rules, err := ipt.List(filterTable, chain.name) + if err != nil { + return err + } + for _, r := range rules { + if strings.Contains(r, "-i "+flowBasedDevice(family)+" ") && strings.Contains(r, "NEW") { + got++ + } + } + default: + rules, err := listNFTablesFilterRules(family, chain.nft) + if err != nil { + return err + } + for _, r := range rules { + if string(r.UserData) == nftRuleIDFlowBasedDev { + got++ + } + } + } + if got != 1 { + return fmt.Errorf("unexpected number of rules for %s in %s: %d", flowBasedDevice(family), chain.name, got) + } + } + } + return nil + }); err != nil { + t.Error(err) + } + }) + } + } +} diff --git a/v2/pkg/nat/netfilter/iptables.go b/v2/pkg/nat/netfilter/iptables.go index add6d391..98e65203 100644 --- a/v2/pkg/nat/netfilter/iptables.go +++ b/v2/pkg/nat/netfilter/iptables.go @@ -7,6 +7,8 @@ import ( "github.com/coreos/go-iptables/iptables" "github.com/vishvananda/netlink" + + "github.com/cybozu-go/coil/v2/pkg/fou" ) func setIPTablesMasqRules(family int, iface string, ip net.IP) error { @@ -26,13 +28,85 @@ func setIPTablesMasqRules(family int, iface string, ip net.IP) error { } // drop invalid packets - spec = []string{"-o", iface, "-m", "state", "--state", "INVALID", "-j", "DROP"} + spec = []string{"-o", iface, "-m", "state", "--state", "INVALID", "-m", "comment", "--comment", RuleTagInvalidDrop, "-j", "DROP"} if err := ipt.AppendUnique(filterTable, filterChain, spec...); err != nil { return fmt.Errorf("failed to setup drop rule for invalid packets: %w", err) } return nil } +// setIPTablesPeerSrcRule drops packets from the FoU tunnel link of a client +// unless their source address is the client's address. This prevents +// clients from spoofing the source address of the tunneled packets. +// The rule is set in both FORWARD and INPUT chains to cover packets +// forwarded by and delivered to the egress Pod. +func setIPTablesPeerSrcRule(family int, linkName string, ip net.IP) error { + ipt, err := newIPTables(family) + if err != nil { + return err + } + + spec := peerSrcRuleSpec(linkName, ip) + for _, chain := range []string{filterChain, inputChain} { + if err := ipt.AppendUnique(filterTable, chain, spec...); err != nil { + return fmt.Errorf("failed to append %q rule in chain %q - %q: %w", filterTable, chain, spec, err) + } + } + return nil +} + +func removeIPTablesPeerSrcRule(family int, linkName string, ip net.IP) error { + ipt, err := newIPTables(family) + if err != nil { + return err + } + + spec := peerSrcRuleSpec(linkName, ip) + for _, chain := range []string{filterChain, inputChain} { + if err := ipt.DeleteIfExists(filterTable, chain, spec...); err != nil { + return fmt.Errorf("failed to delete %q rule in chain %q - %q: %w", filterTable, chain, spec, err) + } + } + return nil +} + +func peerSrcRuleSpec(linkName string, ip net.IP) []string { + return []string{"-i", linkName, "!", "-s", netlink.NewIPNet(ip).String(), "-m", "comment", "--comment", RuleTagPeerSrcPrefix + linkName, "-j", "DROP"} +} + +// setIPTablesFlowBasedDevNewDropRule drops new flows from the flow based +// tunnel device of NAT clients, both forwarded ones and ones destined to +// the client itself. +func setIPTablesFlowBasedDevNewDropRule(family int) error { + ipt, err := newIPTables(family) + if err != nil { + return err + } + + spec := []string{"-i", flowBasedDevice(family), "-m", "conntrack", "--ctstate", "NEW", "-j", "DROP"} + for _, chain := range []string{filterChain, inputChain} { + if err := ipt.AppendUnique(filterTable, chain, spec...); err != nil { + return fmt.Errorf("failed to append %q rule in chain %q - %q: %w", filterTable, chain, spec, err) + } + } + return nil +} + +func flowBasedDevice(family int) string { + if family == netlink.FAMILY_V6 { + return fou.FlowBasedIP6TunDevice + } + return fou.FlowBasedIP4TunDevice +} + +func newIPTables(family int) (*iptables.IPTables, error) { + ipp, err := netlinkToIptablesFamily(family) + if err != nil { + return nil, err + } + return iptables.NewWithProtocol(ipp) +} + func setIPTablesConnmarkRules(family int, link netlink.Link) error { ipp, err := netlinkToIptablesFamily(family) if err != nil { diff --git a/v2/pkg/nat/netfilter/nftables.go b/v2/pkg/nat/netfilter/nftables.go index 2c9ea252..d93d093c 100644 --- a/v2/pkg/nat/netfilter/nftables.go +++ b/v2/pkg/nat/netfilter/nftables.go @@ -23,6 +23,15 @@ const ( // Rule identifiers for deduplication nftRuleIDInputPrefix = "coil-input-" nftRuleIDOutputPrefix = "coil-output-" + nftRuleIDFlowBasedDev = "coil-flow-based-dev-new-drop" +) + +// Rule tags to identify rules set by NatServer. They are set as UserData +// for nftables and as comments for iptables, and are used by the metrics +// collector to find the rules. +const ( + RuleTagInvalidDrop = "coil-invalid-drop" + RuleTagPeerSrcPrefix = "coil-peer-src-" ) func setNFTablesMasqRules(family int, iface string, ip net.IP) (err error) { @@ -124,8 +133,9 @@ func setNFTablesMasqRules(family int, iface string, ip net.IP) (err error) { // Drop invalid or malformed packets from passing through the network. // ex. nft add rule ip filter FORWARD oifname "eth0" ct state invalid counter drop dropRule := &nftables.Rule{ - Table: t, - Chain: c, + Table: t, + Chain: c, + UserData: []byte(RuleTagInvalidDrop), Exprs: []expr.Any{ &expr.Meta{ Key: expr.MetaKeyOIFNAME, @@ -166,6 +176,219 @@ func setNFTablesMasqRules(family int, iface string, ip net.IP) (err error) { return nil } +// setNFTablesPeerSrcRule drops packets from the FoU tunnel link of a client +// unless their source address is the client's address. This prevents +// clients from spoofing the source address of the tunneled packets. +// The rule is set in both FORWARD and INPUT chains to cover packets +// forwarded by and delivered to the egress Pod. +func setNFTablesPeerSrcRule(family int, linkName string, ip net.IP) error { + conn, err := nftables.New() + if err != nil { + return fmt.Errorf("failed to create nftables connection: %w", err) + } + + nf, err := netlinkToNFTablesFamily(family) + if err != nil { + return err + } + + var offset, length uint32 + var ipData []byte + switch nf { + case nftables.TableFamilyIPv4: + offset = ipv4SrcOffset + length = ipv4SrcLen + ipData = ip.To4() + case nftables.TableFamilyIPv6: + offset = ipv6SrcOffset + length = ipv6SrcLen + ipData = ip.To16() + } + + t, forward := nftFilterForwardChain(nf) + _, input := nftFilterInputChain(nf) + conn.AddTable(t) + + for _, c := range []*nftables.Chain{forward, input} { + conn.AddChain(c) + + // ex. nft add rule ip filter FORWARD iifname "fou4_0a000001" ip saddr != 10.0.0.1 counter drop + rule := &nftables.Rule{ + Table: t, + Chain: c, + UserData: []byte(RuleTagPeerSrcPrefix + linkName), + Exprs: []expr.Any{ + &expr.Meta{ + Key: expr.MetaKeyIIFNAME, + Register: nftRegister, + }, + &expr.Cmp{ + Op: expr.CmpOpEq, + Register: nftRegister, + Data: []byte(linkName + "\x00"), + }, + &expr.Payload{ + DestRegister: nftRegister, + Base: expr.PayloadBaseNetworkHeader, + Offset: offset, + Len: length, + }, + &expr.Cmp{ + Op: expr.CmpOpNeq, + Register: nftRegister, + Data: ipData, + }, + &expr.Counter{}, + &expr.Verdict{ + Kind: expr.VerdictDrop, + }, + }, + } + + if _, err := addNFTablesRuleIfNotExists(conn, rule); err != nil { + return fmt.Errorf("failed to check peer source rule existence in chain %s: %w", c.Name, err) + } + } + + if err := conn.Flush(); err != nil { + return fmt.Errorf("failed to flush nftables rules: %w", err) + } + return nil +} + +func removeNFTablesPeerSrcRule(family int, linkName string) error { + conn, err := nftables.New() + if err != nil { + return fmt.Errorf("failed to create nftables connection: %w", err) + } + + nf, err := netlinkToNFTablesFamily(family) + if err != nil { + return err + } + + t, forward := nftFilterForwardChain(nf) + _, input := nftFilterInputChain(nf) + + id := []byte(RuleTagPeerSrcPrefix + linkName) + for _, c := range []*nftables.Chain{forward, input} { + rs, err := conn.GetRules(t, c) + if err != nil { + return fmt.Errorf("failed to get rules in chain %s: %w", c.Name, err) + } + + for _, r := range rs { + if !bytes.Equal(r.UserData, id) { + continue + } + if err := conn.DelRule(r); err != nil { + return fmt.Errorf("failed to delete peer source rule for %s in chain %s: %w", linkName, c.Name, err) + } + } + } + + if err := conn.Flush(); err != nil { + return fmt.Errorf("failed to flush nftables rules: %w", err) + } + return nil +} + +// setNFTablesFlowBasedDevNewDropRule drops new flows from the flow based +// tunnel device of NAT clients, both forwarded ones and ones destined to +// the client itself. +func setNFTablesFlowBasedDevNewDropRule(family int) error { + conn, err := nftables.New() + if err != nil { + return fmt.Errorf("failed to create nftables connection: %w", err) + } + + nf, err := netlinkToNFTablesFamily(family) + if err != nil { + return err + } + + t, forward := nftFilterForwardChain(nf) + _, input := nftFilterInputChain(nf) + conn.AddTable(t) + + for _, c := range []*nftables.Chain{forward, input} { + conn.AddChain(c) + + // ex. nft add rule ip filter FORWARD iifname "coil_ipip4" ct state new counter drop + rule := &nftables.Rule{ + Table: t, + Chain: c, + UserData: []byte(nftRuleIDFlowBasedDev), + Exprs: []expr.Any{ + &expr.Meta{ + Key: expr.MetaKeyIIFNAME, + Register: nftRegister, + }, + &expr.Cmp{ + Op: expr.CmpOpEq, + Register: nftRegister, + Data: []byte(flowBasedDevice(family) + "\x00"), + }, + &expr.Ct{ + Register: nftRegister, + Key: expr.CtKeySTATE, + }, + &expr.Bitwise{ + SourceRegister: nftRegister, + DestRegister: nftRegister, + Len: 4, + Mask: binaryutil.NativeEndian.PutUint32(expr.CtStateBitNEW), + Xor: binaryutil.NativeEndian.PutUint32(0), + }, + &expr.Cmp{ + Op: expr.CmpOpNeq, + Register: nftRegister, + Data: binaryutil.NativeEndian.PutUint32(0), + }, + &expr.Counter{}, + &expr.Verdict{ + Kind: expr.VerdictDrop, + }, + }, + } + + if _, err := addNFTablesRuleIfNotExists(conn, rule); err != nil { + return fmt.Errorf("failed to check flow based device rule existence in chain %s: %w", c.Name, err) + } + } + + if err := conn.Flush(); err != nil { + return fmt.Errorf("failed to flush nftables rules: %w", err) + } + return nil +} + +// nftFilterForwardChain returns the FORWARD chain in the filter table. +func nftFilterForwardChain(nf nftables.TableFamily) (*nftables.Table, *nftables.Chain) { + t := &nftables.Table{Family: nf, Name: filterTable} + c := &nftables.Chain{ + Name: filterChain, + Table: t, + Type: nftables.ChainTypeFilter, + Hooknum: nftables.ChainHookForward, + Priority: nftables.ChainPriorityFilter, + } + return t, c +} + +// nftFilterInputChain returns the INPUT chain in the filter table. +func nftFilterInputChain(nf nftables.TableFamily) (*nftables.Table, *nftables.Chain) { + t := &nftables.Table{Family: nf, Name: filterTable} + c := &nftables.Chain{ + Name: inputChain, + Table: t, + Type: nftables.ChainTypeFilter, + Hooknum: nftables.ChainHookInput, + Priority: nftables.ChainPriorityFilter, + } + return t, c +} + func setNFTablesConnmarkRules(family int, link netlink.Link) error { conn, err := nftables.New() if err != nil { diff --git a/v2/pkg/nat/netfilter/server.go b/v2/pkg/nat/netfilter/server.go index ad5ba615..c94d46ef 100644 --- a/v2/pkg/nat/netfilter/server.go +++ b/v2/pkg/nat/netfilter/server.go @@ -3,7 +3,6 @@ package netfilter import ( "errors" "fmt" - "maps" "net" "sync" @@ -40,7 +39,8 @@ type NatServer struct { ipv6 net.IP backend string - clients map[string]struct{} + // clients maps the IP address of a client to the name of its FoU tunnel link. + clients map[string]string mu sync.RWMutex } @@ -53,7 +53,7 @@ func NewNatServer(iface string, ipv4, ipv6 net.IP, backend string) (*NatServer, ipv4: ipv4, ipv6: ipv6, backend: backend, - clients: make(map[string]struct{}), + clients: make(map[string]string), } if err := n.init(); err != nil { return nil, err @@ -83,6 +83,15 @@ func (n *NatServer) AddClient(ip net.IP, link netlink.Link) error { family = netlink.FAMILY_V6 } + // Set up the source address verification before the link gets up + // so that no unverified packets from the link are forwarded or delivered. + // This is idempotent and done even if the route already exists, e.g. + // when coil-egress restarts. + if err := n.setPeerSrcRule(family, link.Attrs().Name, ip); err != nil { + return err + } + n.clients[ip.String()] = link.Attrs().Name + rs, err := netlink.RouteListFiltered(family, &netlink.Route{Table: nsTableID}, netlink.RT_FILTER_TABLE) if err != nil { return fmt.Errorf("netlink: failed to list routes in table %d: %w", nsTableID, err) @@ -112,15 +121,61 @@ func (n *NatServer) AddClient(ip net.IP, link netlink.Link) error { if err != nil { return fmt.Errorf("netlink: failed to add %s to table %d: %w", ip.String(), nsTableID, err) } + return nil +} + +func (n *NatServer) DelClient(ip net.IP) error { + n.mu.Lock() + defer n.mu.Unlock() + + linkName, ok := n.clients[ip.String()] + if !ok { + return nil + } - n.clients[ip.String()] = struct{}{} + family := netlink.FAMILY_V4 + if ip.To4() == nil { + family = netlink.FAMILY_V6 + } + + if err := n.removePeerSrcRule(family, linkName, ip); err != nil { + return err + } + delete(n.clients, ip.String()) return nil } func (n *NatServer) GetClients() map[string]struct{} { n.mu.RLock() defer n.mu.RUnlock() - return maps.Clone(n.clients) + + clients := make(map[string]struct{}, len(n.clients)) + for ip := range n.clients { + clients[ip] = struct{}{} + } + return clients +} + +func (n *NatServer) setPeerSrcRule(family int, linkName string, ip net.IP) error { + switch n.backend { + case constants.EgressBackendIPTables: + return setIPTablesPeerSrcRule(family, linkName, ip) + case constants.EgressBackendNFTables: + return setNFTablesPeerSrcRule(family, linkName, ip) + default: + return fmt.Errorf("invalid nat backend: %s", n.backend) + } +} + +func (n *NatServer) removePeerSrcRule(family int, linkName string, ip net.IP) error { + switch n.backend { + case constants.EgressBackendIPTables: + return removeIPTablesPeerSrcRule(family, linkName, ip) + case constants.EgressBackendNFTables: + return removeNFTablesPeerSrcRule(family, linkName) + default: + return fmt.Errorf("invalid nat backend: %s", n.backend) + } } func (n *NatServer) init() error { diff --git a/v2/pkg/nat/netfilter/server_test.go b/v2/pkg/nat/netfilter/server_test.go index b64c546d..81d84a22 100644 --- a/v2/pkg/nat/netfilter/server_test.go +++ b/v2/pkg/nat/netfilter/server_test.go @@ -7,6 +7,7 @@ import ( "fmt" "net" "reflect" + "strings" "testing" "github.com/containernetworking/plugins/pkg/ns" @@ -53,7 +54,7 @@ func TestNewNat(t *testing.T) { ipv4: ipv4, ipv6: ipv6, backend: constants.EgressBackendIPTables, - clients: make(map[string]struct{}), + clients: make(map[string]string), }, wantErr: false, }, { @@ -69,7 +70,7 @@ func TestNewNat(t *testing.T) { ipv4: ipv4, ipv6: ipv6, backend: constants.EgressBackendNFTables, - clients: make(map[string]struct{}), + clients: make(map[string]string), }, wantErr: false, }, @@ -86,7 +87,7 @@ func TestNewNat(t *testing.T) { ipv4: ipv4, ipv6: nil, backend: constants.EgressBackendIPTables, - clients: make(map[string]struct{}), + clients: make(map[string]string), }, wantErr: false, }, @@ -103,7 +104,7 @@ func TestNewNat(t *testing.T) { ipv4: ipv4, ipv6: nil, backend: constants.EgressBackendNFTables, - clients: make(map[string]struct{}), + clients: make(map[string]string), }, wantErr: false, }, @@ -120,7 +121,7 @@ func TestNewNat(t *testing.T) { ipv4: nil, ipv6: ipv6, backend: constants.EgressBackendIPTables, - clients: make(map[string]struct{}), + clients: make(map[string]string), }, wantErr: false, }, @@ -137,7 +138,7 @@ func TestNewNat(t *testing.T) { ipv4: nil, ipv6: ipv6, backend: constants.EgressBackendNFTables, - clients: make(map[string]struct{}), + clients: make(map[string]string), }, wantErr: false, }, @@ -239,7 +240,7 @@ func checkIptablesRules(iface string, ip string, family iptables.Protocol) error } // check drop invalid packets rule - exist, err = ipt.Exists(filterTable, filterChain, "-o", iface, "-m", "state", "--state", "INVALID", "-j", "DROP") + exist, err = ipt.Exists(filterTable, filterChain, "-o", iface, "-m", "state", "--state", "INVALID", "-m", "comment", "--comment", RuleTagInvalidDrop, "-j", "DROP") if err != nil { return err } @@ -447,3 +448,217 @@ func checkIPRoutes(ipv4, ipv6 net.IP) error { } return nil } + +func TestNat_PeerSrcRule(t *testing.T) { + ipv4 := net.ParseIP("127.0.0.1") + ipv6 := net.ParseIP("::1") + + clients := []struct { + family int + ip net.IP + linkName string + }{ + {family: netlink.FAMILY_V4, ip: net.ParseIP("10.1.2.3"), linkName: "fou4_test"}, + {family: netlink.FAMILY_V6, ip: net.ParseIP("fd02::1"), linkName: "fou6_test"}, + } + + for _, backend := range []string{constants.EgressBackendIPTables, constants.EgressBackendNFTables} { + t.Run(backend, func(t *testing.T) { + t.Parallel() + + tns, err := testutils.NewNS() + if err != nil { + t.Fatal(err) + } + defer tns.Close() + + if err := tns.Do(func(ns ns.NetNS) error { + n, err := NewNatServer("lo", ipv4, ipv6, backend) + if err != nil { + return fmt.Errorf("NewNatServer() error = %w", err) + } + + links := make(map[string]netlink.Link) + for _, c := range clients { + attrs := netlink.NewLinkAttrs() + attrs.Name = c.linkName + if err := netlink.LinkAdd(&netlink.Dummy{LinkAttrs: attrs}); err != nil { + return err + } + l, err := netlink.LinkByName(c.linkName) + if err != nil { + return err + } + links[c.linkName] = l + } + + // to check idempotency, call AddClient twice + for i := 0; i < 2; i++ { + for _, c := range clients { + if err := n.AddClient(c.ip, links[c.linkName]); err != nil { + return fmt.Errorf("failed to call AddClient for %s: %w", c.ip, err) + } + } + } + for _, c := range clients { + if err := expectPeerSrcRules(backend, c.family, c.linkName, c.ip, 1); err != nil { + return err + } + // the metrics collector finds the rule for invalid packets by its tag + if err := expectInvalidRuleTagged(backend, c.family); err != nil { + return err + } + } + + // AddClient restores the rule even if the route already exists + c := clients[0] + if err := removePeerSrcRule(backend, c.family, c.linkName, c.ip); err != nil { + return err + } + if err := expectPeerSrcRules(backend, c.family, c.linkName, c.ip, 0); err != nil { + return err + } + if err := n.AddClient(c.ip, links[c.linkName]); err != nil { + return fmt.Errorf("failed to call AddClient for %s: %w", c.ip, err) + } + if err := expectPeerSrcRules(backend, c.family, c.linkName, c.ip, 1); err != nil { + return err + } + + // to check idempotency, call DelClient twice + for i := 0; i < 2; i++ { + for _, c := range clients { + if err := n.DelClient(c.ip); err != nil { + return fmt.Errorf("failed to call DelClient for %s: %w", c.ip, err) + } + } + } + for _, c := range clients { + if err := expectPeerSrcRules(backend, c.family, c.linkName, c.ip, 0); err != nil { + return err + } + } + if got := n.GetClients(); len(got) != 0 { + return fmt.Errorf("unexpected clients: %v", got) + } + return nil + }); err != nil { + t.Error(err) + } + }) + } +} + +func removePeerSrcRule(backend string, family int, linkName string, ip net.IP) error { + switch backend { + case constants.EgressBackendIPTables: + return removeIPTablesPeerSrcRule(family, linkName, ip) + default: + return removeNFTablesPeerSrcRule(family, linkName) + } +} + +func expectPeerSrcRules(backend string, family int, linkName string, ip net.IP, want int) error { + chains := []struct { + name string + nft func(nftables.TableFamily) (*nftables.Table, *nftables.Chain) + }{ + {filterChain, nftFilterForwardChain}, + {inputChain, nftFilterInputChain}, + } + + for _, chain := range chains { + var got int + switch backend { + case constants.EgressBackendIPTables: + ipt, err := newIPTables(family) + if err != nil { + return err + } + rules, err := ipt.List(filterTable, chain.name) + if err != nil { + return err + } + for _, r := range rules { + if strings.Contains(r, "-i "+linkName+" ") { + got++ + } + } + if want > 0 { + exist, err := ipt.Exists(filterTable, chain.name, peerSrcRuleSpec(linkName, ip)...) + if err != nil { + return err + } + if !exist { + return fmt.Errorf("peer source rule for %s not found in %s: %v", linkName, chain.name, rules) + } + } + default: + rules, err := listNFTablesFilterRules(family, chain.nft) + if err != nil { + return err + } + for _, r := range rules { + if string(r.UserData) == RuleTagPeerSrcPrefix+linkName { + got++ + } + } + } + + if got != want { + return fmt.Errorf("unexpected number of peer source rules for %s in %s: got %d, want %d", linkName, chain.name, got, want) + } + } + return nil +} + +func expectInvalidRuleTagged(backend string, family int) error { + var got int + switch backend { + case constants.EgressBackendIPTables: + ipt, err := newIPTables(family) + if err != nil { + return err + } + rules, err := ipt.List(filterTable, filterChain) + if err != nil { + return err + } + for _, r := range rules { + if strings.Contains(r, "INVALID") && strings.Contains(r, "--comment "+RuleTagInvalidDrop) { + got++ + } + } + default: + rules, err := listNFTablesForwardRules(family) + if err != nil { + return err + } + for _, r := range rules { + if string(r.UserData) == RuleTagInvalidDrop { + got++ + } + } + } + if got != 1 { + return fmt.Errorf("unexpected number of tagged rules for invalid packets: got %d, want 1", got) + } + return nil +} + +func listNFTablesForwardRules(family int) ([]*nftables.Rule, error) { + return listNFTablesFilterRules(family, nftFilterForwardChain) +} + +func listNFTablesFilterRules(family int, chain func(nftables.TableFamily) (*nftables.Table, *nftables.Chain)) ([]*nftables.Rule, error) { + conn, err := nftables.New() + if err != nil { + return nil, err + } + nf, err := netlinkToNFTablesFamily(family) + if err != nil { + return nil, err + } + t, c := chain(nf) + return conn.GetRules(t, c) +} diff --git a/v2/pkg/nat/server.go b/v2/pkg/nat/server.go index 7f7b9f1b..ac6a4e95 100644 --- a/v2/pkg/nat/server.go +++ b/v2/pkg/nat/server.go @@ -13,6 +13,10 @@ type Server interface { // the necessary routing rules via the specified network link. AddClient(net.IP, netlink.Link) error + // DelClient unregisters a client IP address from the NAT server and + // removes the rules set up by AddClient. + DelClient(net.IP) error + // GetClients returns a copy of the currently registered client IP addresses. GetClients() map[string]struct{} } diff --git a/v2/pkg/test/nat_test.go b/v2/pkg/test/nat_test.go index a99375e8..857391ba 100644 --- a/v2/pkg/test/nat_test.go +++ b/v2/pkg/test/nat_test.go @@ -7,6 +7,7 @@ import ( "net" "net/http" "os/exec" + "strconv" "time" "github.com/containernetworking/plugins/pkg/ns" @@ -48,6 +49,10 @@ var ( // FoU tunnel port fouPort = 5555 + + // Additional client addresses that are not registered to the egress + rogueIPv4 = net.ParseIP("10.1.1.99") + rogueIPv6 = net.ParseIP("fd01::199") ) var _ = Describe("NAT", func() { @@ -93,6 +98,16 @@ var _ = Describe("NAT", func() { It("should NAT traffic with originatingOnly=true", func() { testNAT(clientNS, egressNS, targetNS, backend, true) }) + + It("should drop traffic with a spoofed source address", func() { + testNAT(clientNS, egressNS, targetNS, backend, false) + testSpoofedSource(clientNS, backend) + }) + + It("should drop traffic from unregistered peers", func() { + testNAT(clientNS, egressNS, targetNS, backend, false) + testUnregisteredPeer(clientNS, backend) + }) }) } }) @@ -100,7 +115,7 @@ var _ = Describe("NAT", func() { func testNAT(clientNS, egressNS, targetNS ns.NetNS, backend string, originatingOnly bool) { // Setup client namespace err := clientNS.Do(func(ns.NetNS) error { - ft := fou.NewFoUTunnel(fouPort, clientIPv4, clientIPv6, nil) + ft := fou.NewClientFoUTunnel(fouPort, clientIPv4, clientIPv6, nil) if err := ft.Init(); err != nil { return fmt.Errorf("ft.Init on client failed: %w", err) } @@ -132,7 +147,7 @@ func testNAT(clientNS, egressNS, targetNS ns.NetNS, backend string, originatingO // Setup egress namespace err = egressNS.Do(func(ns.NetNS) error { - ft := fou.NewFoUTunnel(fouPort, egressEth0IPv4, egressEth0IPv6, nil) + ft := fou.NewServerFoUTunnel(fouPort, egressEth0IPv4, egressEth0IPv6, nil) if err := ft.Init(); err != nil { return fmt.Errorf("ft.Init on egress failed: %w", err) } @@ -194,6 +209,93 @@ func testNAT(clientNS, egressNS, targetNS ns.NetNS, backend string, originatingO Expect(err).NotTo(HaveOccurred()) } +// testSpoofedSource sends packets through the registered FoU tunnel with +// a source address other than the client's one. The egress must drop them. +func testSpoofedSource(clientNS ns.NetNS, backend string) { + err := clientNS.Do(func(ns.NetNS) error { + runCmd("ip", "addr", "add", rogueIPv4.String()+"/32", "dev", "eth0") + runCmd("ip", "-6", "addr", "add", rogueIPv6.String()+"/128", "dev", "eth0", "nodad") + + if err := expectCurlFailure(rogueIPv4, fmt.Sprintf("http://%s", targetIPv4), backend); err != nil { + return err + } + if err := expectCurlFailure(rogueIPv6, fmt.Sprintf("http://[%s]", targetIPv6), backend); err != nil { + return err + } + + // Packets destined to the egress itself must be verified as well. + // The egress address on the target network is routed through the tunnel. + if err := expectPing(clientIPv4, egressEth1IPv4, true, backend); err != nil { + return err + } + if err := expectPing(clientIPv6, egressEth1IPv6, true, backend); err != nil { + return err + } + if err := expectPing(rogueIPv4, egressEth1IPv4, false, backend); err != nil { + return err + } + if err := expectPing(rogueIPv6, egressEth1IPv6, false, backend); err != nil { + return err + } + return nil + }) + Expect(err).NotTo(HaveOccurred()) +} + +// testUnregisteredPeer sends FoU packets to the egress from an address +// that is not registered as a client. The egress must not decapsulate them. +func testUnregisteredPeer(clientNS ns.NetNS, backend string) { + err := clientNS.Do(func(ns.NetNS) error { + runCmd("ip", "addr", "add", rogueIPv4.String()+"/32", "dev", "eth0") + runCmd("ip", "-6", "addr", "add", rogueIPv6.String()+"/128", "dev", "eth0", "nodad") + + port := strconv.Itoa(fouPort) + runCmd("ip", "link", "add", "rogue4", "type", "ipip", + "remote", egressEth0IPv4.String(), "local", rogueIPv4.String(), "ttl", "225", + "encap", "fou", "encap-sport", port, "encap-dport", port) + runCmd("ip", "link", "add", "rogue6", "type", "ip6tnl", "mode", "ip6ip6", + "remote", egressEth0IPv6.String(), "local", rogueIPv6.String(), "ttl", "225", + "encap", "fou", "encap-sport", port, "encap-dport", port) + runCmd("ip", "link", "set", "rogue4", "up") + runCmd("ip", "link", "set", "rogue6", "up") + + // route packets from the rogue addresses through the rogue tunnels + // prior to the rules configured by NatClient + runCmd("ip", "route", "add", targetNetworkIPv4.String(), "dev", "rogue4", "table", "200") + runCmd("ip", "-6", "route", "add", targetNetworkIPv6.String(), "dev", "rogue6", "table", "200") + runCmd("ip", "rule", "add", "from", rogueIPv4.String(), "lookup", "200", "priority", "100") + runCmd("ip", "-6", "rule", "add", "from", rogueIPv6.String(), "lookup", "200", "priority", "100") + + if err := expectCurlFailure(rogueIPv4, fmt.Sprintf("http://%s", targetIPv4), backend); err != nil { + return err + } + if err := expectCurlFailure(rogueIPv6, fmt.Sprintf("http://[%s]", targetIPv6), backend); err != nil { + return err + } + return nil + }) + Expect(err).NotTo(HaveOccurred()) +} + +func expectPing(src, dst net.IP, success bool, backend string) error { + out, err := exec.Command("ping", "-c", "1", "-W", "3", "-I", src.String(), dst.String()).CombinedOutput() + if success && err != nil { + return fmt.Errorf("ping from %s to %s failed (backend=%s): %w: %s", src, dst, backend, err, string(out)) + } + if !success && err == nil { + return fmt.Errorf("ping from %s to %s unexpectedly succeeded (backend=%s): %s", src, dst, backend, string(out)) + } + return nil +} + +func expectCurlFailure(src net.IP, url, backend string) error { + out, err := exec.Command("curl", "-s", "--connect-timeout", "3", "--interface", src.String(), url).CombinedOutput() + if err == nil { + return fmt.Errorf("curl from %s to %s unexpectedly succeeded (backend=%s): %s", src, url, backend, string(out)) + } + return nil +} + func setupNetwork(clientNS, egressNS, targetNS ns.NetNS) { // Create veth pairs and move them to namespaces // client eth0 <-> eth0 egress eth1 <-> eth0 target diff --git a/v2/runners/coild_server.go b/v2/runners/coild_server.go index f669b29a..b1893714 100644 --- a/v2/runners/coild_server.go +++ b/v2/runners/coild_server.go @@ -63,7 +63,7 @@ type natSetup struct { func (n natSetup) Hook(l []GWNets, backend string, log *zap.Logger) func(ipv4, ipv6 net.IP) error { return func(ipv4, ipv6 net.IP) error { - ft := fou.NewFoUTunnel(n.port, ipv4, ipv6, func(message string) { + ft := fou.NewClientFoUTunnel(n.port, ipv4, ipv6, func(message string) { log.Sugar().Info(message) }) if err := ft.Init(); err != nil {