From b83c273737edf46ad071b199ebf5f7e286e36930 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Mon, 22 Feb 2021 14:34:15 -0800 Subject: [PATCH] wgengine/filter: use IPSet for localNets instead of prefixes. Part of #1177, preparing for doing fancier set operations on the allowed local nets. Signed-off-by: David Anderson --- go.mod | 2 +- go.sum | 4 ++++ ipn/ipnlocal/local.go | 16 +++++++++----- wgengine/filter/filter.go | 40 +++++++++++++--------------------- wgengine/filter/filter_test.go | 7 ++++-- wgengine/tstun/tun_test.go | 5 +++-- 6 files changed, 39 insertions(+), 35 deletions(-) diff --git a/go.mod b/go.mod index 5da8e8708..38da8df50 100644 --- a/go.mod +++ b/go.mod @@ -39,7 +39,7 @@ require ( golang.zx2c4.com/wireguard/windows v0.1.2-0.20201113162609-9b85be97fdf8 gvisor.dev/gvisor v0.0.0-20210111185822-3ff3110fcdd6 honnef.co/go/tools v0.1.0 - inet.af/netaddr v0.0.0-20210105212526-648fbc18a69d + inet.af/netaddr v0.0.0-20210222205655-a1ec2b7b8c44 inet.af/peercred v0.0.0-20210216231719-993aa01eacaa rsc.io/goversion v1.2.0 ) diff --git a/go.sum b/go.sum index d3215c663..d560f1223 100644 --- a/go.sum +++ b/go.sum @@ -317,6 +317,8 @@ go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= go.uber.org/multierr v1.6.0/go.mod h1:cdWPpRnG4AhwMwsgIHip0KRBQjJy5kYEpYjJxpXp9iU= go4.org/intern v0.0.0-20210101010959-7cab76ca296a h1:28p852HIWWaOS019DYK/A3yTmpm1HJaUce63pvll4C8= go4.org/intern v0.0.0-20210101010959-7cab76ca296a/go.mod h1:vLqJ+12kCw61iCWsPto0EOHhBS+o4rO5VIucbc9g2Cc= +go4.org/intern v0.0.0-20210108033219-3eb7198706b2 h1:VFTf+jjIgsldaz/Mr00VaCSswHJrI2hIjQygE/W4IMg= +go4.org/intern v0.0.0-20210108033219-3eb7198706b2/go.mod h1:vLqJ+12kCw61iCWsPto0EOHhBS+o4rO5VIucbc9g2Cc= go4.org/mem v0.0.0-20201119185036-c04c5a6ff174 h1:vSug/WNOi2+4jrKdivxayTN/zd8EA1UrStjpWvvo1jk= go4.org/mem v0.0.0-20201119185036-c04c5a6ff174/go.mod h1:reUoABIJ9ikfM5sgtSF3Wushcza7+WeD01VB9Lirh3g= go4.org/unsafe/assume-no-moving-gc v0.0.0-20201222175341-b30ae309168e/go.mod h1:FftLjUGFEDu5k8lt0ddY+HcrH/qU/0qk+H8j9/nTl3E= @@ -559,6 +561,8 @@ honnef.co/go/tools v0.1.0 h1:AWNL1W1i7f0wNZ8VwOKNJ0sliKvOF/adn0EHenfUh+c= honnef.co/go/tools v0.1.0/go.mod h1:XtegFAyX/PfluP4921rXU5IkjkqBCDnUq4W8VCIoKvM= inet.af/netaddr v0.0.0-20210105212526-648fbc18a69d h1:6f0242aW/6x2enQBOSKgDS8KQNw6Tp7IVR8eG3x0Jc8= inet.af/netaddr v0.0.0-20210105212526-648fbc18a69d/go.mod h1:jPZo7Jy4nke2cCgISa4fKJKa5T7+EO8k5fWwWghzneg= +inet.af/netaddr v0.0.0-20210222205655-a1ec2b7b8c44 h1:p7fX77zWzZMuNdJUhniBsmN1OvFOrW9SOtvgnzqUZX4= +inet.af/netaddr v0.0.0-20210222205655-a1ec2b7b8c44/go.mod h1:I2i9ONCXRZDnG1+7O8fSuYzjcPxHQXrIfzD/IkR87x4= inet.af/peercred v0.0.0-20210216231719-993aa01eacaa h1:6qseJO2iNDHl+MLL2BkO5oURJR4A9pLmRz11Yf7KdGM= inet.af/peercred v0.0.0-20210216231719-993aa01eacaa/go.mod h1:VZeNdG7cRIUqKl9DWoFX86AHyfYwdb4RextAw1CAEO4= k8s.io/api v0.16.13/go.mod h1:QWu8UWSTiuQZMMeYjwLs6ILu5O74qKSJ0c+4vrchDxs= diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 071761434..ac6dca473 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -608,18 +608,26 @@ func (b *LocalBackend) updateFilter(netMap *netmap.NetworkMap, prefs *ipn.Prefs) haveNetmap = netMap != nil addrs []netaddr.IPPrefix packetFilter []filter.Match - advRoutes []netaddr.IPPrefix + localNetsB netaddr.IPSetBuilder shieldsUp = prefs == nil || prefs.ShieldsUp // Be conservative when not ready ) if haveNetmap { addrs = netMap.Addresses + for _, p := range addrs { + localNetsB.AddPrefix(p) + } packetFilter = netMap.PacketFilter } if prefs != nil { - advRoutes = prefs.AdvertiseRoutes + for _, r := range prefs.AdvertiseRoutes { + // TODO: when advertising default routes, trim out local + // nets. + localNetsB.AddPrefix(r) + } } + localNets := localNetsB.IPSet() - changed := deepprint.UpdateHash(&b.filterHash, haveNetmap, addrs, packetFilter, advRoutes, shieldsUp) + changed := deepprint.UpdateHash(&b.filterHash, haveNetmap, addrs, packetFilter, localNets.Ranges(), shieldsUp) if !changed { return } @@ -630,8 +638,6 @@ func (b *LocalBackend) updateFilter(netMap *netmap.NetworkMap, prefs *ipn.Prefs) return } - localNets := unmapIPPrefixes(netMap.Addresses, advRoutes) - oldFilter := b.e.GetFilter() if shieldsUp { b.logf("netmap packet filter: (shields up)") diff --git a/wgengine/filter/filter.go b/wgengine/filter/filter.go index a9cfcf47c..e23bbd4c8 100644 --- a/wgengine/filter/filter.go +++ b/wgengine/filter/filter.go @@ -20,13 +20,11 @@ import ( // Filter is a stateful packet filter. type Filter struct { logf logger.Logf - // local4 and local6 are the lists of IP prefixes that we know - // to be "local" to this node. All packets coming in over - // tailscale must have a destination within local4 or local6, - // regardless of the policy filter below. Zero values reject - // all incoming traffic. - local4 []netaddr.IPPrefix - local6 []netaddr.IPPrefix + // local is the set of IPs prefixes that we know to be "local" to + // this node. All packets coming in over tailscale must have a + // destination within local, regardless of the policy filter + // below. + local *netaddr.IPSet // matches4 and matches6 are lists of match->action rules // applied to all packets arriving over tailscale // tunnels. Matches are checked in order, and processing stops @@ -124,19 +122,22 @@ func NewAllowAllForTest(logf logger.Logf) *Filter { }, } - return New(ms, []netaddr.IPPrefix{any4, any6}, nil, logf) + var sb netaddr.IPSetBuilder + sb.AddPrefix(any4) + sb.AddPrefix(any6) + return New(ms, sb.IPSet(), nil, logf) } // NewAllowNone returns a packet filter that rejects everything. func NewAllowNone(logf logger.Logf) *Filter { - return New(nil, nil, nil, logf) + return New(nil, &netaddr.IPSet{}, nil, logf) } // NewShieldsUpFilter returns a packet filter that rejects incoming connections. // // If shareStateWith is non-nil, the returned filter shares state with the previous one, // as long as the previous one was also a shields up filter. -func NewShieldsUpFilter(localNets []netaddr.IPPrefix, shareStateWith *Filter, logf logger.Logf) *Filter { +func NewShieldsUpFilter(localNets *netaddr.IPSet, shareStateWith *Filter, logf logger.Logf) *Filter { // Don't permit sharing state with a prior filter that wasn't a shields-up filter. if shareStateWith != nil && !shareStateWith.shieldsUp { shareStateWith = nil @@ -151,7 +152,7 @@ func NewShieldsUpFilter(localNets []netaddr.IPPrefix, shareStateWith *Filter, lo // by matches. If shareStateWith is non-nil, the returned filter // shares state with the previous one, to enable changing rules at // runtime without breaking existing stateful flows. -func New(matches []Match, localNets []netaddr.IPPrefix, shareStateWith *Filter, logf logger.Logf) *Filter { +func New(matches []Match, localNets *netaddr.IPSet, shareStateWith *Filter, logf logger.Logf) *Filter { var state *filterState if shareStateWith != nil { state = shareStateWith.state @@ -164,23 +165,12 @@ func New(matches []Match, localNets []netaddr.IPPrefix, shareStateWith *Filter, logf: logf, matches4: matchesFamily(matches, netaddr.IP.Is4), matches6: matchesFamily(matches, netaddr.IP.Is6), - local4: netsFamily(localNets, netaddr.IP.Is4), - local6: netsFamily(localNets, netaddr.IP.Is6), + local: localNets, state: state, } return f } -func netsFamily(nets []netaddr.IPPrefix, keep func(netaddr.IP) bool) []netaddr.IPPrefix { - var ret []netaddr.IPPrefix - for _, net := range nets { - if keep(net.IP) { - ret = append(ret, net) - } - } - return ret -} - // matchesFamily returns the subset of ms for which keep(srcNet.IP) // and keep(dstNet.IP) are both true. func matchesFamily(ms matches, keep func(netaddr.IP) bool) matches { @@ -321,7 +311,7 @@ func (f *Filter) runIn4(q *packet.Parsed) (r Response, why string) { // A compromised peer could try to send us packets for // destinations we didn't explicitly advertise. This check is to // prevent that. - if !ipInList(q.Dst.IP, f.local4) { + if !f.local.Contains(q.Dst.IP) { return Drop, "destination not allowed" } @@ -378,7 +368,7 @@ func (f *Filter) runIn6(q *packet.Parsed) (r Response, why string) { // A compromised peer could try to send us packets for // destinations we didn't explicitly advertise. This check is to // prevent that. - if !ipInList(q.Dst.IP, f.local6) { + if !f.local.Contains(q.Dst.IP) { return Drop, "destination not allowed" } diff --git a/wgengine/filter/filter_test.go b/wgengine/filter/filter_test.go index fd9a3facf..dbdac22b5 100644 --- a/wgengine/filter/filter_test.go +++ b/wgengine/filter/filter_test.go @@ -31,9 +31,12 @@ func newFilter(logf logger.Logf) *Filter { // Expects traffic to 100.122.98.50, 1.2.3.4, 5.6.7.8, // 102.102.102.102, 119.119.119.119, 8.1.0.0/16 - localNets := nets("100.122.98.50", "1.2.3.4", "5.6.7.8", "102.102.102.102", "119.119.119.119", "8.1.0.0/16", "2001::/16") + var localNets netaddr.IPSetBuilder + for _, n := range nets("100.122.98.50", "1.2.3.4", "5.6.7.8", "102.102.102.102", "119.119.119.119", "8.1.0.0/16", "2001::/16") { + localNets.AddPrefix(n) + } - return New(matches, localNets, nil, logf) + return New(matches, localNets.IPSet(), nil, logf) } func TestFilter(t *testing.T) { diff --git a/wgengine/tstun/tun_test.go b/wgengine/tstun/tun_test.go index d6b3eb7be..a67967d23 100644 --- a/wgengine/tstun/tun_test.go +++ b/wgengine/tstun/tun_test.go @@ -110,8 +110,9 @@ func setfilter(logf logger.Logf, tun *TUN) { {Srcs: nets("5.6.7.8"), Dsts: netports("1.2.3.4:89-90")}, {Srcs: nets("1.2.3.4"), Dsts: netports("5.6.7.8:98")}, } - localNets := nets("1.2.0.0/16") - tun.SetFilter(filter.New(matches, localNets, nil, logf)) + var sb netaddr.IPSetBuilder + sb.AddPrefix(netaddr.MustParseIPPrefix("1.2.0.0/16")) + tun.SetFilter(filter.New(matches, sb.IPSet(), nil, logf)) } func newChannelTUN(logf logger.Logf, secure bool) (*tuntest.ChannelTUN, *TUN) {