From 01185ba3536bcc037481578420d73cb7cee96094 Mon Sep 17 00:00:00 2001 From: Zoltan Papp Date: Mon, 12 Oct 2026 10:29:26 +0200 Subject: [PATCH] [client] Configure the userspace device through its typed API (#7937) * [client] Configure the userspace device through its typed API The userspace configurer drove wireguard-go through UAPI text: every write was rendered to the configuration protocol and parsed back, and every read, including the per-second statistics poll, dumped the whole device as text and parsed it again. With 10000 peers a statistics read cost about 16 ms, and clearing one endpoint needed a peer remove and a re-add because the protocol cannot clear an endpoint or drop a single prefix in place. The allowed IP mirror in the configurer existed only to feed those rewrites. The wireguard-go fork now exposes a typed peer API, so the configurer calls the device directly: ConfigurePeer for creates and updates, single-prefix add and remove on the routing table, ClearEndpoint, and accessors for statistics. The mirror, the UAPI rendering and the parsers are gone from the userspace path; the UAPI socket for wg show stays. Clearing an endpoint keeps the peer object, its counters and its allowed IPs, but resets the session and switches the keepalive off until the next endpoint arrives. The watcher expects a fresh handshake after an endpoint reset, and an endpoint-less peer with a keepalive would retry handshakes it cannot send; this is what the remove and re-add provided before. Handshake times now carry nanosecond precision like the kernel path. Measured with 10000 peers against the UAPI implementation: clearing an endpoint drops from 713 ms to 3 ms for all peers, a statistics read from 15 ms to 0.9 ms, a full dump from 18 ms to 4 ms, and single allowed IP changes are 7 to 20 times faster. * [client] Keep the allowed IP mirror kernel-only The userspace configurer no longer uses the allowed IP mirror, so its ensure method and the preshared key config helper had no caller left on Windows and macOS, where the kernel configurer is not compiled, and the unused linter failed there. The mirror and its tests now carry the kernel build tag, the prefix normalizers that both paths share move to common.go, and the preshared key helper moves next to its only caller. * [client] Trim historical references from configurer comments * [client] Keep the endpoint zone in the userspace configurer The UAPI path wrote the endpoint as text, so a link-local IPv6 endpoint reached the device with its zone. The typed path built the address from the IP bytes only and dropped the zone, leaving the device with an unroutable fe80:: address. The activity recorder keeps the zone-less address it always had. * [client] Run the prefix conversion tests on every platform prefixesToIPNets lives in the untagged common.go and the userspace stats path uses it on every platform, but its tests stayed in the kernel-tagged allowedips_test.go, so darwin and windows CI stopped running them. Move them next to the normalizePrefix tests. The round-trip test stays tagged because ipNetsToPrefixes is kernel-only. * [client] Pin wireguard-go to the merged typed peer API The typed peer configuration API landed on master as #23, so the replace no longer needs to point at the PR branch commit. --- client/iface/configurer/allowedips.go | 29 +- client/iface/configurer/allowedips_test.go | 75 +-- client/iface/configurer/common.go | 41 +- client/iface/configurer/common_test.go | 81 +++ client/iface/configurer/kernel_unix.go | 10 + client/iface/configurer/usp.go | 615 ++++-------------- .../iface/configurer/usp_allowedips_test.go | 76 +-- client/iface/configurer/usp_test.go | 276 ++++++-- go.mod | 2 +- go.sum | 4 +- 10 files changed, 472 insertions(+), 737 deletions(-) create mode 100644 client/iface/configurer/common_test.go diff --git a/client/iface/configurer/allowedips.go b/client/iface/configurer/allowedips.go index 193197d4a..08a766ee0 100644 --- a/client/iface/configurer/allowedips.go +++ b/client/iface/configurer/allowedips.go @@ -1,3 +1,5 @@ +//go:build (linux && !android) || freebsd + package configurer import ( @@ -165,33 +167,6 @@ func (s *allowedIPStore) releaseLocked(k wgtypes.Key) { } } -// normalizePrefix puts a prefix into the form the store recognises it by. It clears the -// host bits, which a device does on its own, so a caller passing 10.20.0.1/16 still matches -// the 10.20.0.0/16 read back from the device; and it unmaps a v4-mapped prefix so that it -// compares equal to, and marshals like, the plain v4 prefix for the same network. -// -// Masking comes first because it also decides the address family: only a prefix at least 96 -// bits long keeps the mapped marker through the mask, so a shorter prefix inside the mapped -// range is a genuine v6 prefix and unmapping it would yield an invalid v4 prefix. -func normalizePrefix(prefix netip.Prefix) netip.Prefix { - masked := prefix.Masked() - - addr := masked.Addr() - if !addr.Is4In6() { - return masked - } - return netip.PrefixFrom(addr.Unmap(), masked.Bits()-96) -} - -// normalizePrefixes returns a normalized copy without changing the caller's slice. -func normalizePrefixes(prefixes []netip.Prefix) []netip.Prefix { - normalized := make([]netip.Prefix, len(prefixes)) - for i, prefix := range prefixes { - normalized[i] = normalizePrefix(prefix) - } - return normalized -} - // ipNetsToPrefixes converts addresses read back from a device. Unmap keeps a v4-mapped v6 // address comparable to the plain v4 prefix the configurer was given. func ipNetsToPrefixes(ipNets []net.IPNet) []netip.Prefix { diff --git a/client/iface/configurer/allowedips_test.go b/client/iface/configurer/allowedips_test.go index 1d272d8c4..405609e29 100644 --- a/client/iface/configurer/allowedips_test.go +++ b/client/iface/configurer/allowedips_test.go @@ -1,3 +1,5 @@ +//go:build (linux && !android) || freebsd + package configurer import ( @@ -125,24 +127,6 @@ func TestAllowedIPStoreNormalizesMappedPrefixes(t *testing.T) { assert.Equal(t, []netip.Prefix{v4}, prefixes, "a mapped prefix must be stored unmapped and not duplicated") } -func TestNormalizePrefix(t *testing.T) { - v4 := netip.MustParsePrefix("10.20.0.0/16") - v6 := netip.MustParsePrefix("fd00::/64") - - assert.Equal(t, v4, normalizePrefix(v4), "a plain v4 prefix is unchanged") - assert.Equal(t, v6, normalizePrefix(v6), "a real v6 prefix is unchanged") - assert.Equal(t, v4, normalizePrefix(netip.PrefixFrom(netip.AddrFrom16(v4.Addr().As16()), 112)), - "a mapped prefix under a 128 bit mask becomes plain v4") - // A prefix shorter than /96 inside the mapped range is a genuine v6 prefix. Unmapping it - // would pair a v4 address with a v6 sized mask, which is invalid, and the store would then - // record a zero prefix that can never recreate the allowed IP. - for _, tc := range []string{"::ffff:0:0/64", "::ffff:1.2.3.4/80", "::ffff:1.2.3.4/95"} { - got := normalizePrefix(netip.MustParsePrefix(tc)) - assert.True(t, got.IsValid(), "%s must normalize to a valid prefix", tc) - assert.False(t, got.Addr().Is4(), "%s must stay v6", tc) - } -} - func TestAllowedIPStoreAddExistingDoesNotCreate(t *testing.T) { s := newAllowedIPStore() routed := netip.MustParsePrefix("10.20.0.0/16") @@ -194,15 +178,6 @@ func TestAllowedIPStoreForgetReleasesOwnership(t *testing.T) { assert.Equal(t, []netip.Prefix{routed}, current, "the new owner must hold the prefix") } -func TestNormalizePrefixClearsHostBits(t *testing.T) { - // A device stores a prefix masked, so a caller passing host bits must still match what a - // device fallback seeded, otherwise that prefix could never be removed by value. - assert.Equal(t, netip.MustParsePrefix("10.20.0.0/16"), - normalizePrefix(netip.MustParsePrefix("10.20.0.1/16")), "host bits must be cleared") - assert.Equal(t, netip.MustParsePrefix("fd00::/64"), - normalizePrefix(netip.MustParsePrefix("fd00::1/64")), "host bits must be cleared for v6") -} - func TestIPNetsToPrefixesKeepsV6InTheMappedRange(t *testing.T) { // ::ffff:0:0/64 reads as v4-mapped but is a genuine v6 prefix: unmapping it would leave a // v4 address under a 64 bit mask, which is invalid, and the allowed IP would be dropped. @@ -215,49 +190,3 @@ func TestIPNetsToPrefixesKeepsV6InTheMappedRange(t *testing.T) { assert.False(t, got[0].Addr().Is4(), "a v6 prefix in the mapped range must not become v4") assert.Equal(t, 64, got[0].Bits(), "the prefix length must survive the conversion") } - -func TestPrefixesToIPNetsNormalizes(t *testing.T) { - // net.IPNet prints a v4-mapped address as v4 but takes the length from its 16 byte - // mask, so an unnormalized ::ffff:10.1.2.3/64 reaches a userspace device as 10.1.2.3/0, - // an allowed IP that matches every v4 address. - tests := []struct { - name string - given string - want string - }{ - {name: "mapped below /96", given: "::ffff:10.1.2.3/64", want: "::/64"}, - {name: "mapped at /112", given: "::ffff:10.1.2.3/112", want: "10.1.0.0/16"}, - {name: "host bits are cleared", given: "10.20.0.1/16", want: "10.20.0.0/16"}, - {name: "v6 is untouched", given: "fd00::1/64", want: "fd00::/64"}, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - got := prefixesToIPNets([]netip.Prefix{netip.MustParsePrefix(tc.given)}) - require.Len(t, got, 1, "the prefix must be converted, not dropped") - assert.Equal(t, tc.want, got[0].String(), "what the device is given") - assert.NotEqual(t, 0, mustOnes(t, got[0]), "a device must never be given a zero length allowed IP") - }) - } -} - -func mustOnes(t *testing.T, ipNet net.IPNet) int { - t.Helper() - - ones, _ := ipNet.Mask.Size() - return ones -} - -// TestPrefixesToIPNetsAgreesWithTheStore pins the property the store depends on: what a -// device is given and what is recorded for it are the same prefix. -func TestPrefixesToIPNetsAgreesWithTheStore(t *testing.T) { - for _, given := range []string{"::ffff:10.1.2.3/64", "::ffff:10.1.2.3/112", "10.20.0.1/16", "fd00::1/64"} { - prefix := netip.MustParsePrefix(given) - - toDevice := prefixesToIPNets([]netip.Prefix{prefix}) - recorded := normalizePrefix(prefix) - - assert.Equal(t, recorded.String(), toDevice[0].String(), - "%s must reach the device in the form the store records", given) - } -} diff --git a/client/iface/configurer/common.go b/client/iface/configurer/common.go index 40f8209e9..02d2a7e6a 100644 --- a/client/iface/configurer/common.go +++ b/client/iface/configurer/common.go @@ -3,22 +3,8 @@ package configurer import ( "net" "net/netip" - - "golang.zx2c4.com/wireguard/wgctrl/wgtypes" ) -// buildPresharedKeyConfig creates a wgtypes.Config for setting a preshared key on a peer. -// This is a shared helper used by both kernel and userspace configurers. -func buildPresharedKeyConfig(peerKey wgtypes.Key, psk wgtypes.Key, updateOnly bool) wgtypes.Config { - return wgtypes.Config{ - Peers: []wgtypes.PeerConfig{{ - PublicKey: peerKey, - PresharedKey: &psk, - UpdateOnly: updateOnly, - }}, - } -} - // prefixesToIPNets converts prefixes on their way to a device. It is the only place that // conversion happens, so it also normalizes: the device is then given the same form the // store records, and a v4-mapped prefix cannot reach net.IPNet, which prints such an @@ -35,3 +21,30 @@ func prefixesToIPNets(prefixes []netip.Prefix) []net.IPNet { } return ipNets } + +// normalizePrefix puts a prefix into the form the store recognises it by. It clears the +// host bits, which a device does on its own, so a caller passing 10.20.0.1/16 still matches +// the 10.20.0.0/16 read back from the device; and it unmaps a v4-mapped prefix so that it +// compares equal to, and marshals like, the plain v4 prefix for the same network. +// +// Masking comes first because it also decides the address family: only a prefix at least 96 +// bits long keeps the mapped marker through the mask, so a shorter prefix inside the mapped +// range is a genuine v6 prefix and unmapping it would yield an invalid v4 prefix. +func normalizePrefix(prefix netip.Prefix) netip.Prefix { + masked := prefix.Masked() + + addr := masked.Addr() + if !addr.Is4In6() { + return masked + } + return netip.PrefixFrom(addr.Unmap(), masked.Bits()-96) +} + +// normalizePrefixes returns a normalized copy without changing the caller's slice. +func normalizePrefixes(prefixes []netip.Prefix) []netip.Prefix { + normalized := make([]netip.Prefix, len(prefixes)) + for i, prefix := range prefixes { + normalized[i] = normalizePrefix(prefix) + } + return normalized +} diff --git a/client/iface/configurer/common_test.go b/client/iface/configurer/common_test.go new file mode 100644 index 000000000..487c1a734 --- /dev/null +++ b/client/iface/configurer/common_test.go @@ -0,0 +1,81 @@ +package configurer + +import ( + "net" + "net/netip" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNormalizePrefix(t *testing.T) { + v4 := netip.MustParsePrefix("10.20.0.0/16") + v6 := netip.MustParsePrefix("fd00::/64") + + assert.Equal(t, v4, normalizePrefix(v4), "a plain v4 prefix is unchanged") + assert.Equal(t, v6, normalizePrefix(v6), "a real v6 prefix is unchanged") + assert.Equal(t, v4, normalizePrefix(netip.PrefixFrom(netip.AddrFrom16(v4.Addr().As16()), 112)), + "a mapped prefix under a 128 bit mask becomes plain v4") + // A prefix shorter than /96 inside the mapped range is a genuine v6 prefix. Unmapping it + // would pair a v4 address with a v6 sized mask, which is invalid, and the store would then + // record a zero prefix that can never recreate the allowed IP. + for _, tc := range []string{"::ffff:0:0/64", "::ffff:1.2.3.4/80", "::ffff:1.2.3.4/95"} { + got := normalizePrefix(netip.MustParsePrefix(tc)) + assert.True(t, got.IsValid(), "%s must normalize to a valid prefix", tc) + assert.False(t, got.Addr().Is4(), "%s must stay v6", tc) + } +} + +func TestNormalizePrefixClearsHostBits(t *testing.T) { + // A device stores a prefix masked, so a caller passing host bits must still match what a + // device fallback seeded, otherwise that prefix could never be removed by value. + assert.Equal(t, netip.MustParsePrefix("10.20.0.0/16"), + normalizePrefix(netip.MustParsePrefix("10.20.0.1/16")), "host bits must be cleared") + assert.Equal(t, netip.MustParsePrefix("fd00::/64"), + normalizePrefix(netip.MustParsePrefix("fd00::1/64")), "host bits must be cleared for v6") +} + +func TestPrefixesToIPNetsNormalizes(t *testing.T) { + // net.IPNet prints a v4-mapped address as v4 but takes the length from its 16 byte + // mask, so an unnormalized ::ffff:10.1.2.3/64 reaches a userspace device as 10.1.2.3/0, + // an allowed IP that matches every v4 address. + tests := []struct { + name string + given string + want string + }{ + {name: "mapped below /96", given: "::ffff:10.1.2.3/64", want: "::/64"}, + {name: "mapped at /112", given: "::ffff:10.1.2.3/112", want: "10.1.0.0/16"}, + {name: "host bits are cleared", given: "10.20.0.1/16", want: "10.20.0.0/16"}, + {name: "v6 is untouched", given: "fd00::1/64", want: "fd00::/64"}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := prefixesToIPNets([]netip.Prefix{netip.MustParsePrefix(tc.given)}) + require.Len(t, got, 1, "the prefix must be converted, not dropped") + assert.Equal(t, tc.want, got[0].String(), "what the device is given") + assert.NotEqual(t, 0, mustOnes(t, got[0]), "a device must never be given a zero length allowed IP") + }) + } +} + +func TestPrefixesToIPNetsAgreesWithTheStore(t *testing.T) { + for _, given := range []string{"::ffff:10.1.2.3/64", "::ffff:10.1.2.3/112", "10.20.0.1/16", "fd00::1/64"} { + prefix := netip.MustParsePrefix(given) + + toDevice := prefixesToIPNets([]netip.Prefix{prefix}) + recorded := normalizePrefix(prefix) + + assert.Equal(t, recorded.String(), toDevice[0].String(), + "%s must reach the device in the form the store records", given) + } +} + +func mustOnes(t *testing.T, ipNet net.IPNet) int { + t.Helper() + + ones, _ := ipNet.Mask.Size() + return ones +} diff --git a/client/iface/configurer/kernel_unix.go b/client/iface/configurer/kernel_unix.go index 3a95249c1..7b1ad9683 100644 --- a/client/iface/configurer/kernel_unix.go +++ b/client/iface/configurer/kernel_unix.go @@ -371,3 +371,13 @@ func (c *KernelConfigurer) fetchStats() (map[string]WGStats, error) { } return stats, nil } + +func buildPresharedKeyConfig(peerKey wgtypes.Key, psk wgtypes.Key, updateOnly bool) wgtypes.Config { + return wgtypes.Config{ + Peers: []wgtypes.PeerConfig{{ + PublicKey: peerKey, + PresharedKey: &psk, + UpdateOnly: updateOnly, + }}, + } +} diff --git a/client/iface/configurer/usp.go b/client/iface/configurer/usp.go index 334d99369..1b0965f0d 100644 --- a/client/iface/configurer/usp.go +++ b/client/iface/configurer/usp.go @@ -1,19 +1,16 @@ package configurer import ( - "encoding/base64" - "encoding/hex" "fmt" + "math" "net" "net/netip" "os" "runtime" - "slices" - "strconv" - "strings" "time" log "github.com/sirupsen/logrus" + wgconn "golang.zx2c4.com/wireguard/conn" "golang.zx2c4.com/wireguard/device" "golang.zx2c4.com/wireguard/wgctrl/wgtypes" @@ -22,19 +19,6 @@ import ( "github.com/netbirdio/netbird/monotime" ) -const ( - privateKey = "private_key" - ipcKeyLastHandshakeTimeSec = "last_handshake_time_sec" - ipcKeyTxBytes = "tx_bytes" - ipcKeyRxBytes = "rx_bytes" - allowedIP = "allowed_ip" - endpoint = "endpoint" - fwmark = "fwmark" - listenPort = "listen_port" - publicKey = "public_key" - presharedKey = "preshared_key" -) - var ErrAllowedIPNotFound = fmt.Errorf("allowed IP not found") type WGUSPConfigurer struct { @@ -42,20 +26,13 @@ type WGUSPConfigurer struct { deviceName string activityRecorder *bind.ActivityRecorder statsCache *statsCache - allowedIPs *allowedIPStore uapiListener net.Listener } // NewUSPConfigurer creates a userspace configurer and starts its UAPI listener. func NewUSPConfigurer(device *device.Device, deviceName string, activityRecorder *bind.ActivityRecorder) *WGUSPConfigurer { - wgCfg := &WGUSPConfigurer{ - device: device, - deviceName: deviceName, - activityRecorder: activityRecorder, - allowedIPs: newAllowedIPStore(), - } - wgCfg.statsCache = newStatsCache(statsCacheTTL, wgCfg.fetchStats) + wgCfg := NewUSPConfigurerNoUAPI(device, deviceName, activityRecorder) wgCfg.startUAPI() return wgCfg } @@ -66,33 +43,32 @@ func NewUSPConfigurerNoUAPI(device *device.Device, deviceName string, activityRe device: device, deviceName: deviceName, activityRecorder: activityRecorder, - allowedIPs: newAllowedIPStore(), } wgCfg.statsCache = newStatsCache(statsCacheTTL, wgCfg.fetchStats) return wgCfg } // ConfigureInterface sets the device key, port and firewall mark, replacing all peers. -// The allowed IP mirror is reset only after the device accepts the configuration. func (c *WGUSPConfigurer) ConfigureInterface(privateKey string, port int) error { log.Debugf("adding Wireguard private key") key, err := wgtypes.ParseKey(privateKey) if err != nil { return err } - fwmark := getFwmark() - config := wgtypes.Config{ - PrivateKey: &key, - ReplacePeers: true, - FirewallMark: &fwmark, - ListenPort: &port, + if port < 0 || port > math.MaxUint16 { + return fmt.Errorf("invalid listen port %d", port) } - if err := c.device.IpcSet(toWgUserspaceString(config)); err != nil { - return err + if err := c.device.SetPrivateKey(device.NoisePrivateKey(key)); err != nil { + return fmt.Errorf("set private key: %w", err) + } + if err := c.device.SetListenPort(uint16(port)); err != nil { + return fmt.Errorf("set listen port %d: %w", port, err) + } + c.device.RemoveAllPeers() + if err := c.device.BindSetMark(uint32(getFwmark())); err != nil { + return fmt.Errorf("set fwmark: %w", err) } - - c.allowedIPs.reset() return nil } @@ -104,131 +80,91 @@ func (c *WGUSPConfigurer) SetPresharedKey(peerKey string, psk wgtypes.Key, updat return err } - cfg := buildPresharedKeyConfig(parsedPeerKey, psk, updateOnly) - if err := c.device.IpcSet(toWgUserspaceString(cfg)); err != nil { - return err - } - - // Without updateOnly this creates the peer when it is absent, so the store has to - // know about it even though no allowed IP was configured. - if !updateOnly { - c.allowedIPs.ensure(parsedPeerKey) - } - return nil + presharedKey := device.NoisePresharedKey(psk) + return c.device.ConfigurePeer(device.NoisePublicKey(parsedPeerKey), device.PeerConfig{ + PresharedKey: &presharedKey, + UpdateOnly: updateOnly, + }) } // UpdatePeer creates or updates a peer, merging allowed IPs with its existing set. -// It validates the endpoint before writing and records changes after a successful write. func (c *WGUSPConfigurer) UpdatePeer(peerKey string, allowedIps []netip.Prefix, keepAlive time.Duration, endpoint *net.UDPAddr, preSharedKey *wgtypes.Key) error { peerKeyParsed, err := wgtypes.ParseKey(peerKey) if err != nil { return err } - // Everything that can fail is done before the device is touched, so a failure here - // cannot leave the device holding a peer that the activity recorder and the allowed - // IP store never learned about. + prefixes, err := validPrefixes(allowedIps) + if err != nil { + return err + } + + cfg := device.PeerConfig{ + PersistentKeepalive: &keepAlive, + AllowedIPs: prefixes, + } + var addrPort netip.AddrPort if endpoint != nil { - addr, err := netip.ParseAddr(endpoint.IP.String()) - if err != nil { - return fmt.Errorf("parse endpoint address: %w", err) + addr, ok := netip.AddrFromSlice(endpoint.IP) + if !ok { + return fmt.Errorf("parse endpoint address %v", endpoint.IP) } - addrPort = netip.AddrPortFrom(addr.Unmap(), uint16(endpoint.Port)) + addr = addr.Unmap() + addrPort = netip.AddrPortFrom(addr, uint16(endpoint.Port)) + cfg.Endpoint = &bind.Endpoint{AddrPort: netip.AddrPortFrom(addr.WithZone(endpoint.Zone), addrPort.Port())} } - peer := wgtypes.PeerConfig{ - PublicKey: peerKeyParsed, - ReplaceAllowedIPs: false, - // don't replace allowed ips, wg will handle duplicated peer IP - AllowedIPs: prefixesToIPNets(allowedIps), - PersistentKeepaliveInterval: &keepAlive, - PresharedKey: preSharedKey, - Endpoint: endpoint, + if preSharedKey != nil { + psk := device.NoisePresharedKey(*preSharedKey) + cfg.PresharedKey = &psk } - config := wgtypes.Config{ - Peers: []wgtypes.PeerConfig{peer}, - } - - if ipcErr := c.device.IpcSet(toWgUserspaceString(config)); ipcErr != nil { - return ipcErr + if err := c.device.ConfigurePeer(device.NoisePublicKey(peerKeyParsed), cfg); err != nil { + return fmt.Errorf("configure peer: %w", err) } if endpoint != nil { c.activityRecorder.UpsertAddress(peerKey, addrPort) } - - c.allowedIPs.add(peerKeyParsed, allowedIps) return nil } // RemoveEndpointAddress clears the endpoint of a peer while keeping it configured. -// The UAPI cannot clear an endpoint in place, so the peer is removed and re-added with the -// allowed IPs it already had. +// The session is reset and the keepalive is switched off until the next endpoint arrives. func (c *WGUSPConfigurer) RemoveEndpointAddress(peerKey string) error { peerKeyParsed, err := wgtypes.ParseKey(peerKey) if err != nil { return fmt.Errorf("parse peer key: %w", err) } - allowedIPs, err := c.peerAllowedIPs(peerKeyParsed) - if err != nil { - return err + peer := c.device.LookupPeer(device.NoisePublicKey(peerKeyParsed)) + if peer == nil { + return ErrPeerNotFound } - peer := wgtypes.PeerConfig{ - PublicKey: peerKeyParsed, - Remove: true, - } + peer.ClearEndpoint() + peer.ZeroAndFlushAll() - config := wgtypes.Config{ - Peers: []wgtypes.PeerConfig{peer}, + var noKeepAlive time.Duration + if err := c.device.ConfigurePeer(device.NoisePublicKey(peerKeyParsed), device.PeerConfig{ + PersistentKeepalive: &noKeepAlive, + UpdateOnly: true, + }); err != nil { + return fmt.Errorf("disable keepalive: %w", err) } - if ipcErr := c.device.IpcSet(toWgUserspaceString(config)); ipcErr != nil { - return fmt.Errorf("remove peer: %w", ipcErr) - } - - peer = wgtypes.PeerConfig{ - PublicKey: peerKeyParsed, - ReplaceAllowedIPs: true, - AllowedIPs: prefixesToIPNets(allowedIPs), - } - - config = wgtypes.Config{ - Peers: []wgtypes.PeerConfig{peer}, - } - - if err := c.device.IpcSet(toWgUserspaceString(config)); err != nil { - c.allowedIPs.forget(peerKeyParsed) - return fmt.Errorf("re-add peer without endpoint: %w", err) - } - return nil } -// RemovePeer removes a peer, then clears its activity and allowed IP records. -// A failed device write leaves both records intact. +// RemovePeer removes a peer and clears its activity record. func (c *WGUSPConfigurer) RemovePeer(peerKey string) error { peerKeyParsed, err := wgtypes.ParseKey(peerKey) if err != nil { return err } - peer := wgtypes.PeerConfig{ - PublicKey: peerKeyParsed, - Remove: true, - } - - config := wgtypes.Config{ - Peers: []wgtypes.PeerConfig{peer}, - } - if ipcErr := c.device.IpcSet(toWgUserspaceString(config)); ipcErr != nil { - return ipcErr - } - + c.device.RemovePeer(device.NoisePublicKey(peerKeyParsed)) c.activityRecorder.Remove(peerKey) - c.allowedIPs.forget(peerKeyParsed) return nil } @@ -238,22 +174,16 @@ func (c *WGUSPConfigurer) AddAllowedIP(peerKey string, allowedIP netip.Prefix) e if err != nil { return err } - peer := wgtypes.PeerConfig{ - PublicKey: peerKeyParsed, - UpdateOnly: true, - ReplaceAllowedIPs: false, - AllowedIPs: prefixesToIPNets([]netip.Prefix{allowedIP}), + if !allowedIP.IsValid() { + return fmt.Errorf("invalid allowed IP %v", allowedIP) } - config := wgtypes.Config{ - Peers: []wgtypes.PeerConfig{peer}, + peer := c.device.LookupPeer(device.NoisePublicKey(peerKeyParsed)) + if peer == nil { + return nil } - if err := c.device.IpcSet(toWgUserspaceString(config)); err != nil { - return err - } - - c.allowedIPs.addExisting(peerKeyParsed, []netip.Prefix{allowedIP}) + peer.AddAllowedIP(normalizePrefix(allowedIP)) return nil } @@ -264,86 +194,64 @@ func (c *WGUSPConfigurer) RemoveAllowedIP(peerKey string, allowedIP netip.Prefix if err != nil { return fmt.Errorf("parse peer key: %w", err) } - - currentAllowedIPs, err := c.peerAllowedIPs(peerKeyParsed) - if err != nil { - return err - } - - idx := slices.Index(currentAllowedIPs, normalizePrefix(allowedIP)) - if idx < 0 { + if !allowedIP.IsValid() { return ErrAllowedIPNotFound } - newAllowedIPs := slices.Delete(currentAllowedIPs, idx, idx+1) - peer := wgtypes.PeerConfig{ - PublicKey: peerKeyParsed, - UpdateOnly: true, - ReplaceAllowedIPs: true, - AllowedIPs: prefixesToIPNets(newAllowedIPs), + peer := c.device.LookupPeer(device.NoisePublicKey(peerKeyParsed)) + if peer == nil { + return ErrPeerNotFound } - config := wgtypes.Config{ - Peers: []wgtypes.PeerConfig{peer}, + if !peer.RemoveAllowedIP(normalizePrefix(allowedIP)) { + return ErrAllowedIPNotFound } - if err := c.device.IpcSet(toWgUserspaceString(config)); err != nil { - return fmt.Errorf("remove allowed IP %s: %w", allowedIP, err) - } - - c.allowedIPs.set(peerKeyParsed, newAllowedIPs) return nil } -// peerAllowedIPs returns the allowed IPs configured for a peer, reading them from the device -// only for a peer the store has not seen. Reading them back means dumping and parsing the -// whole device configuration, and this runs on every relay and ICE transition. -func (c *WGUSPConfigurer) peerAllowedIPs(peerKey wgtypes.Key) ([]netip.Prefix, error) { - if prefixes, ok := c.allowedIPs.get(peerKey); ok { - return prefixes, nil - } - - ipcStr, err := c.device.IpcGet() - if err != nil { - return nil, fmt.Errorf("get IPC config: %w", err) - } - - stats, err := parseStatus(c.deviceName, ipcStr) - if err != nil { - return nil, fmt.Errorf("parse IPC config: %w", err) - } - - // parseStatus reports keys in their textual form, so the comparison needs it once. - wanted := peerKey.String() - for _, peer := range stats.Peers { - if peer.PublicKey != wanted { - continue - } - - prefixes := ipNetsToPrefixes(peer.AllowedIPs) - c.allowedIPs.set(peerKey, prefixes) - return prefixes, nil - } - - return nil, ErrPeerNotFound -} - func (c *WGUSPConfigurer) FullStats() (*Stats, error) { - ipcStr, err := c.device.IpcGet() - if err != nil { - return nil, fmt.Errorf("IpcGet failed: %w", err) + peers := c.device.Peers() + stats := &Stats{ + DeviceName: c.deviceName, + PublicKey: wgtypes.Key(c.device.PublicKey()).String(), + ListenPort: int(c.device.ListenPort()), + FWMark: int(c.device.FirewallMark()), + Peers: make([]Peer, 0, len(peers)), } - - return parseStatus(c.deviceName, ipcStr) + for _, peer := range peers { + stats.Peers = append(stats.Peers, peerStats(peer)) + } + return stats, nil } func (c *WGUSPConfigurer) LastActivities() map[string]monotime.Time { return c.activityRecorder.GetLastActivities() } +func (c *WGUSPConfigurer) GetStats() (map[string]WGStats, error) { + return c.statsCache.get() +} + +func (c *WGUSPConfigurer) Close() { + if c.uapiListener != nil { + err := c.uapiListener.Close() + if err != nil { + log.Errorf("failed to close uapi listener: %v", err) + } + } + + if runtime.GOOS == "linux" { + sockPath := "/var/run/wireguard/" + c.deviceName + ".sock" + if _, statErr := os.Stat(sockPath); statErr == nil { + _ = os.Remove(sockPath) + } + } +} + // startUAPI starts the UAPI listener for managing the WireGuard interface via external tool -func (t *WGUSPConfigurer) startUAPI() { +func (c *WGUSPConfigurer) startUAPI() { var err error - t.uapiListener, err = openUAPI(t.deviceName) + c.uapiListener, err = openUAPI(c.deviceName) if err != nil { log.Errorf("failed to open uapi listener: %v", err) return @@ -357,175 +265,61 @@ func (t *WGUSPConfigurer) startUAPI() { return } go func() { - t.device.IpcHandle(uapiConn) + c.device.IpcHandle(uapiConn) }() } - }(t.uapiListener) + }(c.uapiListener) } -func (t *WGUSPConfigurer) Close() { - if t.uapiListener != nil { - err := t.uapiListener.Close() - if err != nil { - log.Errorf("failed to close uapi listener: %v", err) +func (c *WGUSPConfigurer) fetchStats() (map[string]WGStats, error) { + peers := c.device.Peers() + stats := make(map[string]WGStats, len(peers)) + for _, peer := range peers { + stats[wgtypes.Key(peer.PublicKey()).String()] = WGStats{ + LastHandshake: peer.LastHandshake(), + TxBytes: int64(peer.TxBytes()), + RxBytes: int64(peer.RxBytes()), } } - - if runtime.GOOS == "linux" { - sockPath := "/var/run/wireguard/" + t.deviceName + ".sock" - if _, statErr := os.Stat(sockPath); statErr == nil { - _ = os.Remove(sockPath) - } - } -} - -func (t *WGUSPConfigurer) GetStats() (map[string]WGStats, error) { - return t.statsCache.get() -} - -func (t *WGUSPConfigurer) fetchStats() (map[string]WGStats, error) { - ipc, err := t.device.IpcGet() - if err != nil { - return nil, fmt.Errorf("ipc get: %w", err) - } - - return parseTransfers(ipc) -} - -func parseTransfers(ipc string) (map[string]WGStats, error) { - stats := make(map[string]WGStats) - var ( - currentKey string - currentStats WGStats - hasPeer bool - ) - lines := strings.Split(ipc, "\n") - for _, line := range lines { - line = strings.TrimSpace(line) - - // If we're within the details of the found peer and encounter another public key, - // this means we're starting another peer's details. So, stop. - if strings.HasPrefix(line, "public_key=") { - peerID := strings.TrimPrefix(line, "public_key=") - h, err := hex.DecodeString(peerID) - if err != nil { - return nil, fmt.Errorf("decode peerID: %w", err) - } - currentKey = base64.StdEncoding.EncodeToString(h) - currentStats = WGStats{} // Reset stats for the new peer - hasPeer = true - stats[currentKey] = currentStats - continue - } - - if !hasPeer { - continue - } - - key := strings.SplitN(line, "=", 2) - if len(key) != 2 { - continue - } - switch key[0] { - case ipcKeyLastHandshakeTimeSec: - hs, err := toLastHandshake(key[1]) - if err != nil { - return nil, err - } - currentStats.LastHandshake = hs - stats[currentKey] = currentStats - case ipcKeyRxBytes: - rxBytes, err := toBytes(key[1]) - if err != nil { - return nil, fmt.Errorf("parse rx_bytes: %w", err) - } - currentStats.RxBytes = rxBytes - stats[currentKey] = currentStats - case ipcKeyTxBytes: - TxBytes, err := toBytes(key[1]) - if err != nil { - return nil, fmt.Errorf("parse tx_bytes: %w", err) - } - currentStats.TxBytes = TxBytes - stats[currentKey] = currentStats - } - } - return stats, nil } -func toWgUserspaceString(wgCfg wgtypes.Config) string { - var sb strings.Builder - if wgCfg.PrivateKey != nil { - hexKey := hex.EncodeToString(wgCfg.PrivateKey[:]) - sb.WriteString(fmt.Sprintf("private_key=%s\n", hexKey)) +func peerStats(peer *device.Peer) Peer { + stats := Peer{ + PublicKey: wgtypes.Key(peer.PublicKey()).String(), + AllowedIPs: prefixesToIPNets(peer.AllowedIPs()), + TxBytes: int64(peer.TxBytes()), + RxBytes: int64(peer.RxBytes()), + LastHandshake: peer.LastHandshake(), + PresharedKey: [32]byte(peer.PresharedKey()), } - - if wgCfg.ListenPort != nil { - sb.WriteString(fmt.Sprintf("listen_port=%d\n", *wgCfg.ListenPort)) + if addrPort, ok := endpointAddrPort(peer.Endpoint()); ok { + stats.Endpoint = net.UDPAddr{IP: addrPort.Addr().AsSlice(), Port: int(addrPort.Port())} } - - if wgCfg.ReplacePeers { - sb.WriteString("replace_peers=true\n") - } - - if wgCfg.FirewallMark != nil { - sb.WriteString(fmt.Sprintf("fwmark=%d\n", *wgCfg.FirewallMark)) - } - - for _, p := range wgCfg.Peers { - hexKey := hex.EncodeToString(p.PublicKey[:]) - sb.WriteString(fmt.Sprintf("public_key=%s\n", hexKey)) - - if p.Remove { - sb.WriteString("remove=true\n") - } - - if p.UpdateOnly { - sb.WriteString("update_only=true\n") - } - - if p.PresharedKey != nil { - preSharedHexKey := hex.EncodeToString(p.PresharedKey[:]) - sb.WriteString(fmt.Sprintf("preshared_key=%s\n", preSharedHexKey)) - } - - if p.Endpoint != nil { - sb.WriteString(fmt.Sprintf("endpoint=%s\n", p.Endpoint.String())) - } - - if p.PersistentKeepaliveInterval != nil { - sb.WriteString(fmt.Sprintf("persistent_keepalive_interval=%d\n", int(p.PersistentKeepaliveInterval.Seconds()))) - } - - if p.ReplaceAllowedIPs { - sb.WriteString("replace_allowed_ips=true\n") - } - - for _, aip := range p.AllowedIPs { - sb.WriteString(fmt.Sprintf("allowed_ip=%s\n", aip.String())) - } - } - return sb.String() + return stats } -func toLastHandshake(stringVar string) (time.Time, error) { - sec, err := strconv.ParseInt(stringVar, 10, 64) +func endpointAddrPort(endpoint wgconn.Endpoint) (netip.AddrPort, bool) { + if endpoint == nil { + return netip.AddrPort{}, false + } + if ep, ok := endpoint.(*bind.Endpoint); ok { + return ep.AddrPort, true + } + addrPort, err := netip.ParseAddrPort(endpoint.DstToString()) if err != nil { - return time.Time{}, fmt.Errorf("parse handshake sec: %w", err) + return netip.AddrPort{}, false } - - // If sec is 0 (Unix epoch), return zero time instead - // This indicates no handshake has occurred - if sec == 0 { - return time.Time{}, nil - } - - return time.Unix(sec, 0), nil + return addrPort, true } -func toBytes(s string) (int64, error) { - return strconv.ParseInt(s, 10, 64) +func validPrefixes(prefixes []netip.Prefix) ([]netip.Prefix, error) { + for _, prefix := range prefixes { + if !prefix.IsValid() { + return nil, fmt.Errorf("invalid allowed IP %v", prefix) + } + } + return normalizePrefixes(prefixes), nil } func getFwmark() int { @@ -534,138 +328,3 @@ func getFwmark() int { } return 0 } - -func hexToWireguardKey(hexKey string) (wgtypes.Key, error) { - // Decode hex string to bytes - keyBytes, err := hex.DecodeString(hexKey) - if err != nil { - return wgtypes.Key{}, fmt.Errorf("failed to decode hex key: %w", err) - } - - // Check if we have the right number of bytes (WireGuard keys are 32 bytes) - if len(keyBytes) != 32 { - return wgtypes.Key{}, fmt.Errorf("invalid key length: expected 32 bytes, got %d", len(keyBytes)) - } - - // Convert to wgtypes.Key - var key wgtypes.Key - copy(key[:], keyBytes) - - return key, nil -} - -func parseStatus(deviceName, ipcStr string) (*Stats, error) { - stats := &Stats{DeviceName: deviceName} - var currentPeer *Peer - for _, line := range strings.Split(strings.TrimSpace(ipcStr), "\n") { - if line == "" { - continue - } - parts := strings.SplitN(line, "=", 2) - if len(parts) != 2 { - continue - } - key := parts[0] - val := parts[1] - - switch key { - case privateKey: - key, err := hexToWireguardKey(val) - if err != nil { - log.Errorf("failed to parse private key: %v", err) - continue - } - stats.PublicKey = key.PublicKey().String() - case publicKey: - // Save previous peer - if currentPeer != nil { - stats.Peers = append(stats.Peers, *currentPeer) - } - key, err := hexToWireguardKey(val) - if err != nil { - log.Errorf("failed to parse public key: %v", err) - continue - } - currentPeer = &Peer{ - PublicKey: key.String(), - } - case listenPort: - if port, err := strconv.Atoi(val); err == nil { - stats.ListenPort = port - } - case fwmark: - if fwmark, err := strconv.Atoi(val); err == nil { - stats.FWMark = fwmark - } - case endpoint: - if currentPeer == nil { - continue - } - - host, portStr, err := net.SplitHostPort(val) - if err != nil { - log.Errorf("failed to parse endpoint: %v", err) - continue - } - port, err := strconv.Atoi(portStr) - if err != nil { - log.Errorf("failed to parse endpoint port: %v", err) - continue - } - currentPeer.Endpoint = net.UDPAddr{ - IP: net.ParseIP(host), - Port: port, - } - case allowedIP: - if currentPeer == nil { - continue - } - _, ipnet, err := net.ParseCIDR(val) - if err == nil { - currentPeer.AllowedIPs = append(currentPeer.AllowedIPs, *ipnet) - } - case ipcKeyTxBytes: - if currentPeer == nil { - continue - } - rxBytes, err := toBytes(val) - if err != nil { - continue - } - currentPeer.TxBytes = rxBytes - case ipcKeyRxBytes: - if currentPeer == nil { - continue - } - rxBytes, err := toBytes(val) - if err != nil { - continue - } - currentPeer.RxBytes = rxBytes - - case ipcKeyLastHandshakeTimeSec: - if currentPeer == nil { - continue - } - - ts, err := toLastHandshake(val) - if err != nil { - continue - } - currentPeer.LastHandshake = ts - case presharedKey: - if currentPeer == nil { - continue - } - if val != "" && val != "0000000000000000000000000000000000000000000000000000000000000000" { - if pskKey, err := hexToWireguardKey(val); err == nil { - currentPeer.PresharedKey = [32]byte(pskKey) - } - } - } - } - if currentPeer != nil { - stats.Peers = append(stats.Peers, *currentPeer) - } - return stats, nil -} diff --git a/client/iface/configurer/usp_allowedips_test.go b/client/iface/configurer/usp_allowedips_test.go index fba0ca546..effb360e9 100644 --- a/client/iface/configurer/usp_allowedips_test.go +++ b/client/iface/configurer/usp_allowedips_test.go @@ -10,7 +10,6 @@ import ( "github.com/stretchr/testify/require" wgconn "golang.zx2c4.com/wireguard/conn" wgdevice "golang.zx2c4.com/wireguard/device" - "golang.zx2c4.com/wireguard/tun/tuntest" "golang.zx2c4.com/wireguard/wgctrl/wgtypes" "github.com/netbirdio/netbird/client/iface/bind" @@ -21,8 +20,7 @@ import ( func newTestUSPConfigurer(t *testing.T) *WGUSPConfigurer { t.Helper() - tun := tuntest.NewChannelTUN() - dev := wgdevice.NewDevice(tun.TUN(), wgconn.NewDefaultBind(), wgdevice.NewLogger(wgdevice.LogLevelSilent, "")) + dev := wgdevice.NewDevice(newIdleTUN(), wgconn.NewDefaultBind(), wgdevice.NewLogger(wgdevice.LogLevelSilent, "")) t.Cleanup(dev.Close) c := NewUSPConfigurerNoUAPI(dev, "wgtest0", bind.NewActivityRecorder()) @@ -118,27 +116,6 @@ func TestRemoveEndpointAddressDoesNotScaleWithPeerCount(t *testing.T) { large, small) } -// TestRemoveEndpointAddressFallsBackToDevice covers a peer the store never saw, which is what -// an out-of-band reconfiguration of the device leaves behind. The device stays the source of -// truth in that case, so the allowed IPs must still be preserved. -func TestRemoveEndpointAddressFallsBackToDevice(t *testing.T) { - c := newTestUSPConfigurer(t) - peerKey := seedPeers(t, c, 3)[1] - require.NoError(t, c.AddAllowedIP(peerKey, netip.MustParsePrefix("10.20.0.0/16")), "add routed prefix") - - before := peerAllowedIPs(t, c, peerKey) - c.allowedIPs.reset() - - require.NoError(t, c.RemoveEndpointAddress(peerKey), "remove endpoint address") - - assert.ElementsMatch(t, before, peerAllowedIPs(t, c, peerKey), - "allowed IPs recovered from the device must be preserved") - - recovered, ok := c.allowedIPs.get(mustParseKey(t, peerKey)) - assert.True(t, ok, "the fallback must seed the store so the next call skips the device dump") - assert.Len(t, recovered, 2, "seeded prefixes") -} - func TestRemoveAllowedIPKeepsTheOtherPrefixes(t *testing.T) { c := newTestUSPConfigurer(t) peerKey := seedPeers(t, c, 3)[0] @@ -156,10 +133,8 @@ func TestRemoveAllowedIPKeepsTheOtherPrefixes(t *testing.T) { } // TestAddAllowedIPOnAbsentPeerDoesNotResurrectIt covers the lazy connection window documented -// in #6863: AddAllowedIP is update-only, a silent no-op when the peer is absent, so it must not -// leave the store claiming prefixes the device never took. RemoveEndpointAddress re-adds a peer -// without update-only, so a phantom entry would create a peer the device had dropped, and a -// created peer would steal those allowed IPs from whichever peer legitimately holds them. +// in #6863: AddAllowedIP is update-only, a silent no-op when the peer is absent, and clearing +// the endpoint of an absent peer must not create one either. func TestAddAllowedIPOnAbsentPeerDoesNotResurrectIt(t *testing.T) { c := newTestUSPConfigurer(t) seedPeers(t, c, 2) @@ -253,16 +228,11 @@ func TestUpdatePeerDoesNotWidenAMappedPrefixOnTheDevice(t *testing.T) { onDevice := peerAllowedIPs(t, c, peerKey) assert.NotContains(t, onDevice, "0.0.0.0/0", "the device must not be given a catch-all allowed IP") assert.Equal(t, []string{"10.1.0.0/16"}, onDevice, "the device holds the normalized prefix") - - recorded, ok := c.allowedIPs.get(mustParseKey(t, peerKey)) - require.True(t, ok, "the peer must be recorded") - require.Len(t, recorded, 1, "one prefix recorded") - assert.Equal(t, onDevice[0], recorded[0].String(), "device and store must agree") } // TestUpdatePeerWithAnUnusableEndpointTouchesNothing pins the ordering: the endpoint is -// parsed before the device is configured, so a failure cannot leave the device holding a -// peer that the store never learned about, with the prefix handover skipped along with it. +// parsed before the device is configured, so a failure cannot leave a half-configured peer +// on the device. func TestUpdatePeerWithAnUnusableEndpointTouchesNothing(t *testing.T) { c := newTestUSPConfigurer(t) seedPeers(t, c, 2) @@ -279,40 +249,4 @@ func TestUpdatePeerWithAnUnusableEndpointTouchesNothing(t *testing.T) { stats, err := c.FullStats() require.NoError(t, err, "read device stats") assert.Len(t, stats.Peers, 2, "the peer must not have reached the device") - - _, ok := c.allowedIPs.get(mustParseKey(t, peerKey)) - assert.False(t, ok, "the peer must not have been recorded either") -} - -// TestRemovePeerKeepsTheRecordWhenTheDeviceRefuses covers a removal that never reached the -// device. A single peer removal is one write, so a failure leaves the peer on the device -// exactly as it was, and the record still describes it; dropping it would only force the -// next caller to read the whole device back for an answer it already had. -func TestRemovePeerKeepsTheRecordWhenTheDeviceRefuses(t *testing.T) { - c := newTestUSPConfigurer(t) - peerKey := seedPeers(t, c, 1)[0] - require.NoError(t, c.AddAllowedIP(peerKey, netip.MustParsePrefix("10.20.0.0/16")), "add routed prefix") - - before, ok := c.allowedIPs.get(mustParseKey(t, peerKey)) - require.True(t, ok, "the peer must be recorded before the removal") - require.Len(t, before, 2, "overlay address plus routed prefix") - - // A closed device refuses every write, which is the shape of any failed removal. - c.device.Close() - - require.Error(t, c.RemovePeer(peerKey), "the removal must report the failure") - - after, ok := c.allowedIPs.get(mustParseKey(t, peerKey)) - require.True(t, ok, "a peer still on the device must stay recorded") - assert.Equal(t, before, after, "the record must describe the peer the device kept") -} - -// mustParseKey turns the textual key the configurer API takes into the form the store -// keys on. -func mustParseKey(t *testing.T, key string) wgtypes.Key { - t.Helper() - - parsed, err := wgtypes.ParseKey(key) - require.NoError(t, err, "parse peer key") - return parsed } diff --git a/client/iface/configurer/usp_test.go b/client/iface/configurer/usp_test.go index e32491c54..48b5ad1f0 100644 --- a/client/iface/configurer/usp_test.go +++ b/client/iface/configurer/usp_test.go @@ -2,88 +2,222 @@ package configurer import ( "encoding/hex" + "net" + "net/netip" + "os" + "strings" "testing" + "time" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + wgdevice "golang.zx2c4.com/wireguard/device" + "golang.zx2c4.com/wireguard/tun" "golang.zx2c4.com/wireguard/wgctrl/wgtypes" ) -var ipcFixture = ` -private_key=e84b5a6d2717c1003a13b431570353dbaca9146cf150c5f8575680feba52027a -listen_port=12912 -public_key=b85996fecc9c7f1fc6d2572a76eda11d59bcd20be8e543b15ce4bd85a8e75a33 -preshared_key=188515093e952f5f22e865cef3012e72f8b5f0b598ac0309d5dacce3b70fcf52 -allowed_ip=192.168.4.4/32 -endpoint=[abcd:23::33%2]:51820 -public_key=58402e695ba1772b1cc9309755f043251ea77fdcf10fbe63989ceb7e19321376 -tx_bytes=38333 -rx_bytes=2224 -allowed_ip=192.168.4.6/32 -persistent_keepalive_interval=111 -endpoint=182.122.22.19:3233 -public_key=662e14fd594556f522604703340351258903b64f35553763f19426ab2a515c58 -endpoint=5.152.198.39:51820 -allowed_ip=192.168.4.10/32 -allowed_ip=192.168.4.11/32 -tx_bytes=1212111 -rx_bytes=1929999999 -protocol_version=1 -errno=0 +// idleTUN never reports an up event, so the wireguard-go device stays down: no socket is +// opened, no peer goroutine starts, and the configuration state is all the tests observe. +type idleTUN struct { + events chan tun.Event + closed chan struct{} +} -` +func newIdleTUN() *idleTUN { + return &idleTUN{events: make(chan tun.Event), closed: make(chan struct{})} +} -func Test_parseTransfers(t *testing.T) { - tests := []struct { - name string - peerKey string - want WGStats - }{ - { - name: "single", - peerKey: "b85996fecc9c7f1fc6d2572a76eda11d59bcd20be8e543b15ce4bd85a8e75a33", - want: WGStats{ - TxBytes: 0, - RxBytes: 0, - }, - }, - { - name: "multiple", - peerKey: "58402e695ba1772b1cc9309755f043251ea77fdcf10fbe63989ceb7e19321376", - want: WGStats{ - TxBytes: 38333, - RxBytes: 2224, - }, - }, - { - name: "lastpeer", - peerKey: "662e14fd594556f522604703340351258903b64f35553763f19426ab2a515c58", - want: WGStats{ - TxBytes: 1212111, - RxBytes: 1929999999, - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - res, err := hex.DecodeString(tt.peerKey) - require.NoError(t, err) +func (t *idleTUN) File() *os.File { return nil } - key, err := wgtypes.NewKey(res) - require.NoError(t, err) +func (t *idleTUN) Read([][]byte, []int, int) (int, error) { + <-t.closed + return 0, os.ErrClosed +} - stats, err := parseTransfers(ipcFixture) - if err != nil { - require.NoError(t, err) - return - } +func (t *idleTUN) Write(bufs [][]byte, _ int) (int, error) { return len(bufs), nil } - stat, ok := stats[key.String()] - if !ok { - require.True(t, ok) - return - } +func (t *idleTUN) MTU() (int, error) { return 1420, nil } - require.Equal(t, tt.want, stat) - }) +func (t *idleTUN) Name() (string, error) { return "idle", nil } + +func (t *idleTUN) Events() <-chan tun.Event { return t.events } + +func (t *idleTUN) Close() error { + close(t.closed) + close(t.events) + return nil +} + +func (t *idleTUN) BatchSize() int { return 1 } + +func TestConfigureInterfaceAppliesKeyAndPortAndReplacesPeers(t *testing.T) { + c := newTestUSPConfigurer(t) + seedPeers(t, c, 2) + + key, err := wgtypes.GeneratePrivateKey() + require.NoError(t, err, "generate device private key") + require.NoError(t, c.ConfigureInterface(key.String(), 51820), "reconfigure the interface") + + stats, err := c.FullStats() + require.NoError(t, err, "read device stats") + assert.Equal(t, key.PublicKey().String(), stats.PublicKey, "device public key") + assert.Equal(t, 51820, stats.ListenPort, "listen port") + assert.Empty(t, stats.Peers, "configuring the interface replaces every peer") +} + +func TestConfigureInterfaceRejectsAnInvalidPort(t *testing.T) { + c := newTestUSPConfigurer(t) + + key, err := wgtypes.GeneratePrivateKey() + require.NoError(t, err, "generate device private key") + assert.Error(t, c.ConfigureInterface(key.String(), 70000), "a port above 65535 must be rejected") +} + +func TestGetStatsIsKeyedByPeerKey(t *testing.T) { + c := newTestUSPConfigurer(t) + keys := seedPeers(t, c, 3) + + stats, err := c.GetStats() + require.NoError(t, err, "read stats") + require.Len(t, stats, 3, "one entry per peer") + for _, key := range keys { + entry, ok := stats[key] + require.True(t, ok, "peer %s must be reported", key) + assert.Zero(t, entry.TxBytes, "no traffic was sent") + assert.Zero(t, entry.RxBytes, "no traffic was received") + assert.True(t, entry.LastHandshake.IsZero(), "no handshake happened") } } + +func TestFullStatsMatchesTheDeviceDump(t *testing.T) { + c := newTestUSPConfigurer(t) + + priv, err := wgtypes.GeneratePrivateKey() + require.NoError(t, err, "generate peer private key") + peerKey := priv.PublicKey().String() + psk, err := wgtypes.GenerateKey() + require.NoError(t, err, "generate preshared key") + endpoint := &net.UDPAddr{IP: net.IPv4(192, 0, 2, 10), Port: 51820} + prefixes := []netip.Prefix{netip.MustParsePrefix("100.64.0.1/32"), netip.MustParsePrefix("10.20.0.0/16")} + require.NoError(t, c.UpdatePeer(peerKey, prefixes, 25*time.Second, endpoint, &psk), "add peer") + + stats, err := c.FullStats() + require.NoError(t, err, "read device stats") + require.Len(t, stats.Peers, 1, "one peer on the device") + peer := stats.Peers[0] + assert.Equal(t, peerKey, peer.PublicKey, "peer key") + assert.Equal(t, "192.0.2.10:51820", peer.Endpoint.String(), "endpoint") + assert.Equal(t, [32]byte(psk), peer.PresharedKey, "preshared key") + assert.Equal(t, []string{"100.64.0.1/32", "10.20.0.0/16"}, ipNetStrings(peer.AllowedIPs), "allowed IPs") + + block := uapiPeerBlock(t, c, peerKey) + assert.Contains(t, block, "endpoint=192.0.2.10:51820", "the dump must show the endpoint") + assert.Contains(t, block, "preshared_key="+hex.EncodeToString(psk[:]), "the dump must show the preshared key") + assert.Contains(t, block, "persistent_keepalive_interval=25", "the dump must show the keepalive") + assert.Contains(t, block, "allowed_ip=100.64.0.1/32", "the dump must show the first prefix") + assert.Contains(t, block, "allowed_ip=10.20.0.0/16", "the dump must show the second prefix") +} + +func TestUpdatePeerKeepsTheEndpointZone(t *testing.T) { + c := newTestUSPConfigurer(t) + + priv, err := wgtypes.GeneratePrivateKey() + require.NoError(t, err, "generate peer private key") + peerKey := priv.PublicKey().String() + // The idle device opens no socket, so the zone name does not have to exist on the host. + endpoint := &net.UDPAddr{IP: net.ParseIP("fe80::1"), Port: 51820, Zone: "eth0"} + prefixes := []netip.Prefix{netip.MustParsePrefix("100.64.0.1/32")} + require.NoError(t, c.UpdatePeer(peerKey, prefixes, 25*time.Second, endpoint, nil), "add peer") + + assert.Contains(t, uapiPeerBlock(t, c, peerKey), "endpoint=[fe80::1%eth0]:51820", + "a link-local endpoint must keep its zone") +} + +func TestRemoveEndpointAddressKeepsThePeerAndStopsTheKeepalive(t *testing.T) { + c := newTestUSPConfigurer(t) + + priv, err := wgtypes.GeneratePrivateKey() + require.NoError(t, err, "generate peer private key") + peerKey := priv.PublicKey().String() + endpoint := &net.UDPAddr{IP: net.IPv4(192, 0, 2, 10), Port: 51820} + prefixes := []netip.Prefix{netip.MustParsePrefix("100.64.0.1/32")} + require.NoError(t, c.UpdatePeer(peerKey, prefixes, 25*time.Second, endpoint, nil), "add peer") + before := c.device.LookupPeer(wgdevice.NoisePublicKey(priv.PublicKey())) + require.NotNil(t, before, "the peer must be on the device") + + require.NoError(t, c.RemoveEndpointAddress(peerKey), "remove endpoint address") + + assert.Same(t, before, c.device.LookupPeer(wgdevice.NoisePublicKey(priv.PublicKey())), + "the peer object must survive the endpoint removal") + stats, err := c.FullStats() + require.NoError(t, err, "read device stats") + require.Len(t, stats.Peers, 1, "the peer stays configured") + assert.Nil(t, stats.Peers[0].Endpoint.IP, "the endpoint must be gone") + assert.Equal(t, []string{"100.64.0.1/32"}, ipNetStrings(stats.Peers[0].AllowedIPs), "allowed IPs survive") + assert.Contains(t, uapiPeerBlock(t, c, peerKey), "persistent_keepalive_interval=0", + "an endpoint-less peer must not keep sending keepalives") + + require.NoError(t, c.UpdatePeer(peerKey, prefixes, 25*time.Second, endpoint, nil), "restore the endpoint") + block := uapiPeerBlock(t, c, peerKey) + assert.Contains(t, block, "endpoint=192.0.2.10:51820", "the endpoint must be back") + assert.Contains(t, block, "persistent_keepalive_interval=25", "the keepalive must be back") +} + +func TestUpdatePeerRejectsAnInvalidPrefix(t *testing.T) { + c := newTestUSPConfigurer(t) + + priv, err := wgtypes.GeneratePrivateKey() + require.NoError(t, err, "generate peer private key") + peerKey := priv.PublicKey().String() + + require.Error(t, c.UpdatePeer(peerKey, []netip.Prefix{{}}, 25*time.Second, nil, nil), "an invalid prefix must be rejected") + + stats, err := c.FullStats() + require.NoError(t, err, "read device stats") + assert.Empty(t, stats.Peers, "the peer must not have reached the device") +} + +func TestRemoveAllowedIPOnAbsentPeerReportsThePeer(t *testing.T) { + c := newTestUSPConfigurer(t) + + priv, err := wgtypes.GeneratePrivateKey() + require.NoError(t, err, "generate peer private key") + + assert.ErrorIs(t, c.RemoveAllowedIP(priv.PublicKey().String(), netip.MustParsePrefix("10.20.0.0/16")), ErrPeerNotFound, + "removing a prefix from a peer the device does not have must report the peer") +} + +func ipNetStrings(ipNets []net.IPNet) []string { + out := make([]string, 0, len(ipNets)) + for _, ipNet := range ipNets { + out = append(out, ipNet.String()) + } + return out +} + +// uapiPeerBlock reads the device back through the UAPI dump, so a typed write is checked +// against an independent view of the device. +func uapiPeerBlock(t *testing.T, c *WGUSPConfigurer, peerKey string) []string { + t.Helper() + + parsed, err := wgtypes.ParseKey(peerKey) + require.NoError(t, err, "parse peer key") + wanted := "public_key=" + hex.EncodeToString(parsed[:]) + + dump, err := c.device.IpcGet() + require.NoError(t, err, "dump device") + + var block []string + inBlock := false + for _, line := range strings.Split(strings.TrimSpace(dump), "\n") { + if strings.HasPrefix(line, "public_key=") { + inBlock = line == wanted + continue + } + if inBlock { + block = append(block, line) + } + } + require.NotEmpty(t, block, "peer %s must appear in the dump", peerKey) + return block +} diff --git a/go.mod b/go.mod index a365ffc27..fa7047bd8 100644 --- a/go.mod +++ b/go.mod @@ -369,7 +369,7 @@ replace github.com/kardianos/service => github.com/netbirdio/service v0.0.0-2024 replace github.com/getlantern/systray => github.com/netbirdio/systray v0.0.0-20231030152038-ef1ed2a27949 -replace golang.zx2c4.com/wireguard => github.com/netbirdio/wireguard-go v0.0.0-20260914123147-8bf8fa968f1a +replace golang.zx2c4.com/wireguard => github.com/netbirdio/wireguard-go v0.0.0-20261008122457-d2b1ce9bfee9 replace github.com/cloudflare/circl => codeberg.org/cunicu/circl v0.0.0-20230801113412-fec58fc7b5f6 diff --git a/go.sum b/go.sum index 9e3adae98..55b3edf85 100644 --- a/go.sum +++ b/go.sum @@ -541,8 +541,8 @@ github.com/netbirdio/signal-dispatcher/dispatcher v0.0.0-20250805121659-6b4ac470 github.com/netbirdio/signal-dispatcher/dispatcher v0.0.0-20250805121659-6b4ac470ca45/go.mod h1:5/sjFmLb8O96B5737VCqhHyGRzNFIaN/Bu7ZodXc3qQ= github.com/netbirdio/wails/v3 v3.0.0-beta.3.0.20260928122122-d1db5fa35318 h1:Qv2jYeucRkuqkkM9aAVFcO9avmSfPEoB+gxKKKuI4vA= github.com/netbirdio/wails/v3 v3.0.0-beta.3.0.20260928122122-d1db5fa35318/go.mod h1:62UsqQRqanuCbFi6xElIw3/2X+yGF+TKlyiWImA5taU= -github.com/netbirdio/wireguard-go v0.0.0-20260914123147-8bf8fa968f1a h1:Nt8BgkTkI56LGBPPEBywM406MVKJDqeDIVdgsZyYs80= -github.com/netbirdio/wireguard-go v0.0.0-20260914123147-8bf8fa968f1a/go.mod h1:rpwXGsirqLqN2L0JDJQlwOboGHmptD5ZD6T2VmcqhTw= +github.com/netbirdio/wireguard-go v0.0.0-20261008122457-d2b1ce9bfee9 h1:Mjt3tIBGa1/mJh1Xn8uZP30cd2VYd0NHANqTn+HBsaU= +github.com/netbirdio/wireguard-go v0.0.0-20261008122457-d2b1ce9bfee9/go.mod h1:rpwXGsirqLqN2L0JDJQlwOboGHmptD5ZD6T2VmcqhTw= github.com/nxadm/tail v1.4.4/go.mod h1:kenIhsEOeOJmVchQTgglprH7qJGnHDVpk1VPCcaMI8A= github.com/nxadm/tail v1.4.8/go.mod h1:+ncqLTQzXmGhMZNUePPaPqPvBxHAIsmXswZKocGu+AU= github.com/nxadm/tail v1.4.11 h1:8feyoE3OzPrcshW5/MJ4sGESc5cqmGkGCWlco4l0bqY=