ipn/ipnlocal: reject unmasked routes

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: Ic804efd24f5f536de1f2c910de3a24372d48d54d
pull/7455/head
Andrew Dunham 2 years ago
parent 2d3ae485e3
commit a9c17dbf93

@ -3326,7 +3326,7 @@ var (
// peerRoutes returns the routerConfig.Routes to access peers. // peerRoutes returns the routerConfig.Routes to access peers.
// If there are over cgnatThreshold CGNAT routes, one big CGNAT route // If there are over cgnatThreshold CGNAT routes, one big CGNAT route
// is used instead. // 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() tsULA := tsaddr.TailscaleULARange()
cgNAT := tsaddr.CGNATRange() cgNAT := tsaddr.CGNATRange()
var didULA bool var didULA bool
@ -3334,6 +3334,18 @@ func peerRoutes(peers []wgcfg.Peer, cgnatThreshold int) (routes []netip.Prefix)
for _, peer := range peers { for _, peer := range peers {
for _, aip := range peer.AllowedIPs { for _, aip := range peer.AllowedIPs {
aip = unmapIPPrefix(aip) 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. // 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 aip.Addr().Is6() && aip.IsSingleIP() && tsULA.Contains(aip.Addr()) {
if !didULA { if !didULA {
@ -3366,12 +3378,13 @@ func (b *LocalBackend) routerConfig(cfg *wgcfg.Config, prefs ipn.PrefsView, oneC
if oneCGNATRoute { if oneCGNATRoute {
singleRouteThreshold = 1 singleRouteThreshold = 1
} }
rs := &router.Config{ rs := &router.Config{
LocalAddrs: unmapIPPrefixes(cfg.Addresses), LocalAddrs: unmapIPPrefixes(cfg.Addresses),
SubnetRoutes: unmapIPPrefixes(prefs.AdvertiseRoutes().AsSlice()), SubnetRoutes: unmapIPPrefixes(prefs.AdvertiseRoutes().AsSlice()),
SNATSubnetRoutes: !prefs.NoSNAT(), SNATSubnetRoutes: !prefs.NoSNAT(),
NetfilterMode: prefs.NetfilterMode(), NetfilterMode: prefs.NetfilterMode(),
Routes: peerRoutes(cfg.Peers, singleRouteThreshold), Routes: peerRoutes(b.logf, cfg.Peers, singleRouteThreshold),
} }
if distro.Get() == distro.Synology { if distro.Get() == distro.Synology {

@ -21,6 +21,7 @@ import (
"tailscale.com/net/tsaddr" "tailscale.com/net/tsaddr"
"tailscale.com/tailcfg" "tailscale.com/tailcfg"
"tailscale.com/tstest" "tailscale.com/tstest"
"tailscale.com/types/key"
"tailscale.com/types/logger" "tailscale.com/types/logger"
"tailscale.com/types/netmap" "tailscale.com/types/netmap"
"tailscale.com/wgengine" "tailscale.com/wgengine"
@ -333,10 +334,25 @@ func TestPeerRoutes(t *testing.T) {
pp("100.64.0.2/32"), 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 { for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) { 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) { if !reflect.DeepEqual(got, tt.want) {
t.Errorf("got = %v; want %v", got, tt.want) t.Errorf("got = %v; want %v", got, tt.want)
} }

Loading…
Cancel
Save