lzc256
/
tailscale
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

ipn: fix incorrect change tracking for packet filter. 

ORder of operations to trigger a problem:
 - Start an already authed tailscaled, verify you can ping stuff.
 - Run `tailscale up`. Notice you can no longer ping stuff.

The problem is that `tailscale up` stops the IPN state machine before
restarting it, which zeros out the packet filter but _not_ the packet
filter hash. Then, upon restarting IPN, the uncleared hash incorrectly
makes the code conclude that the filter doesn't need updating, and so
we stay with a zero filter (reject everything) for ever.

The fix is simply to update the filterHash correctly in all cases,
so that running -> stopped -> running correctly changes the filter
at every transition.

Signed-off-by: David Anderson <danderson@tailscale.com>
reviewable/pr633/r1
David Anderson 2020-07-31 20:03:00 +00:00 committed by Dave Anderson
parent 28e52a0492
commit 358cd3fd92
1 changed files with 24 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
ipn/local.go
View File

@ -439,38 +439,45 @@ func (b *LocalBackend) Start(opts Options) error {
// updateFilter updates the packet filter in wgengine based on the // updateFilter updates the packet filter in wgengine based on the
// given netMap and user preferences. // given netMap and user preferences.
func (b *LocalBackend) updateFilter(netMap *controlclient.NetworkMap, prefs *Prefs) { func (b *LocalBackend) updateFilter(netMap *controlclient.NetworkMap, prefs *Prefs) {
if netMap == nil { // NOTE(danderson): keep change detection as the first thing in
// Not configured yet, block everything // this function. Don't try to optimize by returning early, more
b.logf("netmap packet filter: (not ready yet)") // likely than not you'll just end up breaking the change
b.e.SetFilter(filter.NewAllowNone(b.logf)) // detection and end up with the wrong filter installed. This is
return // quite hard to debug, so save yourself the trouble.
var (
haveNetmap = netMap != nil
addrs []wgcfg.CIDR
packetFilter filter.Matches
advRoutes []wgcfg.CIDR
shieldsUp = prefs == nil || prefs.ShieldsUp // Be conservative when not ready
)
if haveNetmap {
addrs = netMap.Addresses
packetFilter = netMap.PacketFilter
} }
packetFilter := netMap.PacketFilter
var advRoutes []wgcfg.CIDR
if prefs != nil { if prefs != nil {
advRoutes = prefs.AdvertiseRoutes advRoutes = prefs.AdvertiseRoutes
} }
// Be conservative while not ready.
shieldsUp := prefs == nil || prefs.ShieldsUp
changed := deepprint.UpdateHash(&b.filterHash, packetFilter, advRoutes, shieldsUp) changed := deepprint.UpdateHash(&b.filterHash, haveNetmap, addrs, packetFilter, advRoutes, shieldsUp)
if !changed { if !changed {
return return
} }
localNets := wgCIDRsToFilter(netMap.Addresses, advRoutes) localNets := wgCIDRsToFilter(netMap.Addresses, advRoutes)
if shieldsUp { switch {
// Shields up, block everything case !haveNetmap:
b.logf("netmap packet filter: (not ready yet)")
b.e.SetFilter(filter.NewAllowNone(b.logf))
case shieldsUp:
b.logf("netmap packet filter: (shields up)") b.logf("netmap packet filter: (shields up)")
var prevFilter *filter.Filter // don't reuse old filter state var prevFilter *filter.Filter // don't reuse old filter state
b.e.SetFilter(filter.New(filter.Matches{}, localNets, prevFilter, b.logf)) b.e.SetFilter(filter.New(filter.Matches{}, localNets, prevFilter, b.logf))
return default:
b.logf("netmap packet filter: %v", packetFilter)
b.e.SetFilter(filter.New(packetFilter, localNets, b.e.GetFilter(), b.logf))
} }
b.logf("netmap packet filter: %v", packetFilter)
b.e.SetFilter(filter.New(packetFilter, localNets, b.e.GetFilter(), b.logf))
} }
// dnsCIDRsEqual determines whether two CIDR lists are equal // dnsCIDRsEqual determines whether two CIDR lists are equal