diff --git a/CHANGELOG.md b/CHANGELOG.md index 35ae8d31..86af908c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- A peer whose certificate carries an unsafe network overlapping the overlay could send packets as any node in + it: the firewall accepted any source inside the unsafe network. A source address inside this node's own + networks must now be the sending peer's own; through an unsafe network it is dropped, counted as a dropped + remote address. + - Signing a v2 certificate with an empty name, a name longer than 253 bytes, or an empty group now fails. These certificates were signed successfully but could not be loaded. - `create-tunnel -address ` panicked on a nil RemoteList and, over the ssh debug server, took nebula diff --git a/firewall.go b/firewall.go index 2e1df4f8..1718ea4b 100644 --- a/firewall.go +++ b/firewall.go @@ -60,6 +60,9 @@ type Firewall struct { // assignedNetworks is a list of vpn networks assigned to us in the certificate. assignedNetworks []netip.Prefix + // vpnNetworks holds assignedNetworks for lookup: a source address inside one of them + // belongs to a node of the overlay, and only that node may send as it. + vpnNetworks *bart.Lite // unsafeNetworks is the list of unsafe networks issued to us in the certificate unsafeNetworks []netip.Prefix @@ -156,10 +159,12 @@ func NewFirewall(l *slog.Logger, tcpTimeout, UDPTimeout, defaultTimeout time.Dur } routableNetworks := new(bart.Lite) + vpnNetworks := new(bart.Lite) var assignedNetworks []netip.Prefix for _, network := range c.Networks() { nprefix := netip.PrefixFrom(network.Addr(), network.Addr().BitLen()) routableNetworks.Insert(nprefix) + vpnNetworks.Insert(network) assignedNetworks = append(assignedNetworks, network) } @@ -180,6 +185,7 @@ func NewFirewall(l *slog.Logger, tcpTimeout, UDPTimeout, defaultTimeout time.Dur DefaultTimeout: defaultTimeout, routableNetworks: routableNetworks, assignedNetworks: assignedNetworks, + vpnNetworks: vpnNetworks, unsafeNetworks: unsafeNetworks, l: l, @@ -462,7 +468,17 @@ func (f *Firewall) Drop(fp firewall.Packet, incoming bool, h *HostInfo, caPool * f.metrics(incoming).droppedRemoteAddr.Inc(1) return ErrPeerRejected // reject for now, one day this may have different FW rules case NetworkTypeUnsafe: - break // nothing special, one day this may have different FW rules + // An unsafe network says what a peer routes for, never who it is. One that + // overlaps our own networks would let the peer send as any node of the overlay, + // and whatever trusts the source address, the firewall's own rules and the + // identity a program looks up for it, would be told the wrong node. A source + // inside our networks is a node's own address, which only that node sends + // from, as NetworkTypeVPN, matched first; through an unsafe network it is + // refused. Outbound the address is a destination and claims nothing. + if incoming && f.vpnNetworks.Contains(fp.RemoteAddr) { + f.metrics(incoming).droppedRemoteAddr.Inc(1) + return ErrInvalidRemoteIP + } default: f.metrics(incoming).droppedRemoteAddr.Inc(1) return ErrUnknownNetworkType //should never happen diff --git a/firewall_test.go b/firewall_test.go index 3e4bcee0..2d02e809 100644 --- a/firewall_test.go +++ b/firewall_test.go @@ -277,6 +277,58 @@ func TestFirewall_Drop(t *testing.T) { require.NoError(t, fw.Drop(p, true, &h, cp, nil)) } +// TestFirewall_DropOverlaySourceThroughUnsafeNetwork: a peer whose unsafe network +// overlaps our overlay may not send as another node of it. Its own address and sources +// outside the overlay still pass through the unsafe network, and outbound to such an +// address, where the address is a destination, is not the firewall's to refuse here. +func TestFirewall_DropOverlaySourceThroughUnsafeNetwork(t *testing.T) { + l := test.NewLogger() + myVpnNetworksTable := new(bart.Lite) + myVpnNetworksTable.Insert(netip.MustParsePrefix("1.2.3.0/24")) + + me := dummyCert{name: "me", networks: []netip.Prefix{netip.MustParsePrefix("1.2.3.4/24")}} + // A router for 1.2.0.0/16, which contains the whole overlay. + peer := dummyCert{ + name: "router", + networks: []netip.Prefix{netip.MustParsePrefix("1.2.3.9/24")}, + unsafeNetworks: []netip.Prefix{netip.MustParsePrefix("1.2.0.0/16")}, + groups: []string{"default-group"}, + } + h := HostInfo{ + ConnectionState: &ConnectionState{ + peerCert: &cert.CachedCertificate{Certificate: &peer, InvertedGroups: map[string]struct{}{"default-group": {}}}, + }, + vpnAddrs: []netip.Addr{netip.MustParseAddr("1.2.3.9")}, + } + h.buildNetworks(myVpnNetworksTable, &peer) + + fw := NewFirewall(l, time.Second, time.Minute, time.Hour, &me) + require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"any"}, "", "", "", "", "")) + cp := cert.NewCAPool() + packet := func(remote string) firewall.Packet { + return firewall.Packet{ + LocalAddr: netip.MustParseAddr("1.2.3.4"), + RemoteAddr: netip.MustParseAddr(remote), + LocalPort: 10, + RemotePort: 90, + Protocol: iputil.IPProtocolUDP, + } + } + + // Another node's overlay address, through the unsafe network: refused. + assert.Equal(t, ErrInvalidRemoteIP, fw.Drop(packet("1.2.3.10"), true, &h, cp, nil)) + assert.Equal(t, int64(1), fw.incomingMetrics.droppedRemoteAddr.Count()) + + // The peer's own address, and an address outside the overlay it routes for: served. + require.NoError(t, fw.Drop(packet("1.2.3.9"), true, &h, cp, nil)) + require.NoError(t, fw.Drop(packet("1.2.5.5"), true, &h, cp, nil)) + + // Outbound to the overlay address via the router is not refused here; it meets the + // outbound rules as before. + resetConntrack(fw) + assert.Equal(t, ErrNoMatchingRule, fw.Drop(packet("1.2.3.10"), false, &h, cp, nil)) +} + func TestFirewall_DropV6(t *testing.T) { ob := &bytes.Buffer{} l := test.NewLoggerWithOutput(ob)