diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index a9d0a3550..8e9192d2d 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -3326,7 +3326,7 @@ var ( // peerRoutes returns the routerConfig.Routes to access peers. // If there are over cgnatThreshold CGNAT routes, one big CGNAT route // is used instead. -func peerRoutes(peers []wgcfg.Peer, cgnatThreshold int) (routes []netip.Prefix) { +func peerRoutes(logf logger.Logf, peers []wgcfg.Peer, cgnatThreshold int) (routes []netip.Prefix) { tsULA := tsaddr.TailscaleULARange() cgNAT := tsaddr.CGNATRange() var didULA bool @@ -3334,6 +3334,18 @@ func peerRoutes(peers []wgcfg.Peer, cgnatThreshold int) (routes []netip.Prefix) for _, peer := range peers { for _, aip := range peer.AllowedIPs { aip = unmapIPPrefix(aip) + + // Ensure that we're only accepting properly-masked + // prefixes; the control server should be masking + // these, so if we get them, skip. + if mm := aip.Masked(); aip != mm { + // To avoid a DoS where a peer could cause all + // reconfigs to fail by sending a bad prefix, we just + // skip, but don't error, on an unmasked route. + logf("advertised route %s from %s has non-address bits set; expected %s", aip, peer.PublicKey.ShortString(), mm) + continue + } + // Only add the Tailscale IPv6 ULA once, if we see anybody using part of it. if aip.Addr().Is6() && aip.IsSingleIP() && tsULA.Contains(aip.Addr()) { if !didULA { @@ -3366,12 +3378,13 @@ func (b *LocalBackend) routerConfig(cfg *wgcfg.Config, prefs ipn.PrefsView, oneC if oneCGNATRoute { singleRouteThreshold = 1 } + rs := &router.Config{ LocalAddrs: unmapIPPrefixes(cfg.Addresses), SubnetRoutes: unmapIPPrefixes(prefs.AdvertiseRoutes().AsSlice()), SNATSubnetRoutes: !prefs.NoSNAT(), NetfilterMode: prefs.NetfilterMode(), - Routes: peerRoutes(cfg.Peers, singleRouteThreshold), + Routes: peerRoutes(b.logf, cfg.Peers, singleRouteThreshold), } if distro.Get() == distro.Synology { diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index 4f1f017b3..556a7f17e 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -21,6 +21,7 @@ import ( "tailscale.com/net/tsaddr" "tailscale.com/tailcfg" "tailscale.com/tstest" + "tailscale.com/types/key" "tailscale.com/types/logger" "tailscale.com/types/netmap" "tailscale.com/wgengine" @@ -333,10 +334,25 @@ func TestPeerRoutes(t *testing.T) { pp("100.64.0.2/32"), }, }, + { + name: "skip-unmasked-prefixes", + peers: []wgcfg.Peer{ + { + PublicKey: key.NewNode().Public(), + AllowedIPs: []netip.Prefix{ + pp("100.64.0.2/32"), + pp("10.0.0.100/16"), + }, + }, + }, + want: []netip.Prefix{ + pp("100.64.0.2/32"), + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := peerRoutes(tt.peers, 2) + got := peerRoutes(t.Logf, tt.peers, 2) if !reflect.DeepEqual(got, tt.want) { t.Errorf("got = %v; want %v", got, tt.want) }