Skip to content

Commit cb6fb80

Browse files
committed
fix: replace string-based tunnel route filter with proper CIDR containment check
The old TunnelNetworkPrefix string match was fragile and matched by accident. Replace it with IsTunnelRoute which derives the tunnel network prefix from the device IP and checks CIDR containment. Also update the smoke test to use the production IP range (10.255.240.x) and verify the tunnel network route is present.
1 parent 496ac0c commit cb6fb80

5 files changed

Lines changed: 57 additions & 16 deletions

File tree

cmd/smoke-test/main.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
const ifaceName = "utun69"
2929

3030
var wantRoutes = []netip.Prefix{
31+
netip.MustParsePrefix("10.255.240.0/21"),
3132
netip.MustParsePrefix("10.123.0.0/24"),
3233
netip.MustParsePrefix("10.124.0.0/16"),
3334
}
@@ -76,13 +77,13 @@ func run() error {
7677

7778
cfg := &pb.Configuration{
7879
PrivateKey: privateKey.String(),
79-
DeviceIPv4: "10.255.24.100",
80+
DeviceIPv4: "10.255.240.100",
8081
Gateways: []*pb.Gateway{
8182
{
8283
Name: "smoke-gw",
8384
PublicKey: gwPublicKey.String(),
8485
Endpoint: "127.0.0.1:51820",
85-
Ipv4: "10.255.24.1",
86+
Ipv4: "10.255.240.1",
8687
RoutesIPv4: []string{"10.123.0.0/24", "10.124.0.0/16"},
8788
},
8889
},

internal/helper/helper_darwin.go

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@ import (
55
"errors"
66
"fmt"
77
"net"
8+
"net/netip"
89
"os"
910
"os/exec"
10-
"strings"
1111
"sync"
1212
"syscall"
1313
"time"
@@ -32,6 +32,7 @@ type DarwinConfigurator struct {
3232
tunDev tun.Device
3333
uapi net.Listener
3434
ifaceName string // actual interface name assigned by macOS (may differ from requested)
35+
tunnelNet netip.Prefix
3536
}
3637

3738
var _ OSConfigurator = &DarwinConfigurator{}
@@ -78,7 +79,7 @@ func (c *DarwinConfigurator) SetupRoutes(ctx context.Context, gateways []*pb.Gat
7879
routesAdded := 0
7980
for _, gw := range gateways {
8081
for _, cidr := range gw.GetRoutesIPv4() {
81-
if strings.HasPrefix(cidr, TunnelNetworkPrefix) {
82+
if IsTunnelRoute(c.tunnelNet, cidr) {
8283
continue
8384
}
8485
if err := addRouteViaInterface(fd, cidr, iface); err != nil {
@@ -88,10 +89,7 @@ func (c *DarwinConfigurator) SetupRoutes(ctx context.Context, gateways []*pb.Gat
8889
}
8990

9091
for _, cidr := range gw.GetRoutesIPv6() {
91-
// TunnelNetworkPrefix is an IPv4 prefix ("10.255.24.") so this check
92-
// is a no-op for IPv6 CIDRs. It's kept for consistency with the IPv4
93-
// loop and as a safeguard in case the prefix changes in the future.
94-
if strings.HasPrefix(cidr, TunnelNetworkPrefix) {
92+
if IsTunnelRoute(c.tunnelNet, cidr) {
9593
continue
9694
}
9795
if err := addRouteViaInterface(fd, cidr, iface); err != nil {
@@ -108,6 +106,12 @@ func (c *DarwinConfigurator) SetupInterface(ctx context.Context, cfg *pb.Configu
108106
c.mu.Lock()
109107
defer c.mu.Unlock()
110108

109+
tunnelNet, err := TunnelNetworkFromIP(cfg.GetDeviceIPv4())
110+
if err != nil {
111+
return fmt.Errorf("derive tunnel network: %w", err)
112+
}
113+
c.tunnelNet = tunnelNet
114+
111115
if c.wgDevice != nil {
112116
return nil
113117
}

internal/helper/helper_linux.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"errors"
66
"fmt"
77
"net"
8+
"net/netip"
89
"strings"
910
"syscall"
1011

@@ -23,6 +24,7 @@ func New(helperConfig Config) *LinuxConfigurator {
2324

2425
type LinuxConfigurator struct {
2526
helperConfig Config
27+
tunnelNet netip.Prefix
2628
}
2729

2830
var _ OSConfigurator = &LinuxConfigurator{}
@@ -44,7 +46,7 @@ func (c *LinuxConfigurator) SetupRoutes(ctx context.Context, gateways []*pb.Gate
4446
routesAdded := 0
4547
for _, gw := range gateways {
4648
for _, cidr := range append(gw.GetRoutesIPv4(), gw.GetRoutesIPv6()...) {
47-
if strings.HasPrefix(cidr, TunnelNetworkPrefix) {
49+
if IsTunnelRoute(c.tunnelNet, cidr) {
4850
continue
4951
}
5052

@@ -86,6 +88,12 @@ func (c *LinuxConfigurator) SetupRoutes(ctx context.Context, gateways []*pb.Gate
8688
}
8789

8890
func (c *LinuxConfigurator) SetupInterface(ctx context.Context, cfg *pb.Configuration) error {
91+
tunnelNet, err := TunnelNetworkFromIP(cfg.DeviceIPv4)
92+
if err != nil {
93+
return fmt.Errorf("derive tunnel network: %w", err)
94+
}
95+
c.tunnelNet = tunnelNet
96+
8997
if c.interfaceExists(ctx) {
9098
return nil
9199
}

internal/helper/helper_windows.go

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,11 @@ import (
2525
type WindowsConfigurator struct {
2626
helperConfig Config
2727

28-
mu sync.Mutex
29-
wgDevice *device.Device
30-
tunDev tun.Device
31-
uapi net.Listener
28+
mu sync.Mutex
29+
wgDevice *device.Device
30+
tunDev tun.Device
31+
uapi net.Listener
32+
tunnelNet netip.Prefix
3233
}
3334

3435
var _ OSConfigurator = &WindowsConfigurator{}
@@ -64,7 +65,7 @@ func (c *WindowsConfigurator) SetupRoutes(ctx context.Context, gateways []*pb.Ga
6465
routesAdded := 0
6566
for _, gw := range gateways {
6667
for _, cidr := range append(gw.GetRoutesIPv4(), gw.GetRoutesIPv6()...) {
67-
if strings.HasPrefix(cidr, TunnelNetworkPrefix) {
68+
if IsTunnelRoute(c.tunnelNet, cidr) {
6869
continue
6970
}
7071

@@ -97,6 +98,12 @@ func (c *WindowsConfigurator) SetupInterface(ctx context.Context, cfg *pb.Config
9798
c.mu.Lock()
9899
defer c.mu.Unlock()
99100

101+
tunnelNet, err := TunnelNetworkFromIP(cfg.DeviceIPv4)
102+
if err != nil {
103+
return fmt.Errorf("derive tunnel network: %w", err)
104+
}
105+
c.tunnelNet = tunnelNet
106+
100107
if c.wgDevice != nil {
101108
return nil
102109
}

internal/helper/util.go

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,20 +5,41 @@ import (
55
"errors"
66
"fmt"
77
"io"
8+
"net/netip"
89
"os"
910
"path/filepath"
1011

1112
"github.com/nais/device/internal/ioconvenience"
13+
"github.com/nais/device/internal/iputil"
1214
"github.com/sirupsen/logrus"
1315
)
1416

1517
const (
16-
TunnelNetworkPrefix = "10.255.24."
17-
1818
// wireguardMTU is the MTU used for WireGuard tunnel interfaces across all platforms.
1919
wireguardMTU = 1360
20+
21+
// tunnelIPv4PrefixLen is the prefix length used for the IPv4 tunnel network.
22+
tunnelIPv4PrefixLen = 21
2023
)
2124

25+
// TunnelNetworkFromIP derives the tunnel network prefix from the device's IPv4 address.
26+
func TunnelNetworkFromIP(deviceIPv4 string) (netip.Prefix, error) {
27+
addr, err := netip.ParseAddr(deviceIPv4)
28+
if err != nil {
29+
return netip.Prefix{}, fmt.Errorf("parse device IPv4 %q: %w", deviceIPv4, err)
30+
}
31+
return netip.PrefixFrom(addr, tunnelIPv4PrefixLen).Masked(), nil
32+
}
33+
34+
// IsTunnelRoute reports whether cidr falls within the tunnel network.
35+
func IsTunnelRoute(tunnelNet netip.Prefix, cidr string) bool {
36+
prefix, err := iputil.ParsePrefix(cidr)
37+
if err != nil {
38+
return false
39+
}
40+
return tunnelNet.Contains(prefix.Addr()) && prefix.Bits() >= tunnelNet.Bits()
41+
}
42+
2243
func ZipLogFiles(files []string, log logrus.FieldLogger) (string, error) {
2344
if len(files) == 0 {
2445
return "nil", errors.New("can't be bothered to zip nothing")

0 commit comments

Comments
 (0)