ipn, wgengine: disable subnet routes if network has PAC configuration

Not configurable yet.

Updates tailscale/corp#653

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

ipn/local.go

@@ -126,31 +126,20 @@ func (b *LocalBackend) linkChange(major bool, ifst *interfaces.State) {
 	b.mu.Lock()
 	defer b.mu.Unlock()
 
-	// On transition from no PAC to PAC, assume we're
+	hadPAC := b.prevIfState.HasPAC()
-	// roaming into some corp network where the corp HTTP
+	b.prevIfState = ifst
-	// proxy & Windows Domain Controller & DNS etc all
-	// might be behind subnet routes that we've otherwise
-	// shadowed.
-	//
-	// So remove all our routes and reset the control connections
-	gotPAC := ifst.PAC != "" && b.prevIfState != nil && b.prevIfState.PAC == ""
-	if gotPAC {
-		b.logf("linkChange: entering PAC network, resetting; state=%v", b.state)
-		b.e.Reconfig(&wgcfg.Config{}, &router.Config{})
-		b.logf("linkChange: did wg+router reset")
 
-		if b.c != nil && b.state != Stopped {
+	// If the PAC-ness of the network changed, reconfig wireguard+route to
-			// Pause and unpause the client to reset its
+	// add/remove subnets.
-			// HTTP connections.
+	if hadPAC != ifst.HasPAC() {
-			// TODO(bradfitz): this is somewhat gross. Add
+		b.logf("linkChange: in state %v; PAC changed from %v->%v", b.state, hadPAC, ifst.HasPAC())
-			// a more explicit method to the client.
+		switch b.state {
-			b.c.SetPaused(true)
+		case NoState, Stopped:
-			b.c.SetPaused(false)
+			// Do nothing.
-			b.logf("linkChange: did control client reset")
+		default:
+			go b.authReconfig()
 		}
 	}
-
-	b.prevIfState = ifst
 }
 
 // Shutdown halts the backend and all its sub-components. The backend
@@ -1022,6 +1011,7 @@ func (b *LocalBackend) authReconfig() {
 	blocked := b.blocked
 	uc := b.prefs
 	nm := b.netMap
+	hasPAC := b.prevIfState.HasPAC()
 	b.mu.Unlock()
 
 	if blocked {
@@ -1046,6 +1036,18 @@ func (b *LocalBackend) authReconfig() {
 	if uc.AllowSingleHosts {
 		flags |= controlclient.AllowSingleHosts
 	}
+	if hasPAC {
+		// TODO(bradfitz): make this policy configurable per
+		// domain, flesh out all the edge cases where subnet
+		// routes might shadow corp HTTP proxies, DNS servers,
+		// domain controllers, etc. For now we just want
+		// Tailscale to stay enabled while laptops roam
+		// between corp & non-corp networks.
+		if flags&controlclient.AllowSubnetRoutes != 0 {
+			b.logf("authReconfig: have PAC; disabling subnet routes")
+			flags &^= controlclient.AllowSubnetRoutes
+		}
+	}
 
 	cfg, err := nm.WGCfg(b.logf, flags)
 	if err != nil {

net/interfaces/interfaces.go

@@ -242,6 +242,8 @@ func (s *State) Equal(s2 *State) bool {
 	return reflect.DeepEqual(s, s2)
 }
 
+func (s *State) HasPAC() bool { return s != nil && s.PAC != "" }
+
 // RemoveTailscaleInterfaces modifes s to remove any interfaces that
 // are owned by this process. (TODO: make this true; currently it
 // makes the Linux-only assumption that the interface is named

wgengine/userspace.go

@@ -1160,6 +1160,9 @@ func (e *userspaceEngine) SetLinkChangeCallback(cb func(major bool, newState *in
 	e.mu.Lock()
 	defer e.mu.Unlock()
 	e.linkChangeCallback = cb
+	if e.linkState != nil {
+		go cb(false, e.linkState)
+	}
 }
 
 func getLinkState() (*interfaces.State, error) {

wgengine/wgengine.go

@@ -120,6 +120,9 @@ type Engine interface {
 
 	// SetLinkChangeCallback sets the function to call when the
 	// link state changes.
+	// The provided function is run in a new goroutine once upon
+	// initial call (if the engine has a known link state) and
+	// upon any change.
 	SetLinkChangeCallback(func(major bool, newState *interfaces.State))
 
 	// DiscoPublicKey gets the public key used for path discovery