From 8b3ea13af0d6830871b9ea85f7c774d10f3d600c Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Thu, 31 Aug 2023 22:54:21 -0700 Subject: [PATCH] net/tsdial: be smarter about when to close SystemDial conns It was too aggressive before, as it only had the ill-defined "Major" bool to work with. Now it can check more precisely. Updates #9040 Change-Id: I20967283b64af6a9cad3f8e90cff406de91653b8 Signed-off-by: Brad Fitzpatrick --- net/interfaces/interfaces.go | 3 +-- net/tsdial/tsdial.go | 47 ++++++++++++++++++++++++++++-------- 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/net/interfaces/interfaces.go b/net/interfaces/interfaces.go index cf467fc7b..6fe24c651 100644 --- a/net/interfaces/interfaces.go +++ b/net/interfaces/interfaces.go @@ -364,10 +364,9 @@ func (s *State) HasIP(ip netip.Addr) bool { if s == nil { return false } - want := netip.PrefixFrom(ip, ip.BitLen()) for _, pv := range s.InterfaceIPs { for _, p := range pv { - if p == want { + if p.Contains(ip) { return true } } diff --git a/net/tsdial/tsdial.go b/net/tsdial/tsdial.go index 6a35b18fc..a2dfab26f 100644 --- a/net/tsdial/tsdial.go +++ b/net/tsdial/tsdial.go @@ -139,15 +139,40 @@ func (d *Dialer) SetNetMon(netMon *netmon.Monitor) { } func (d *Dialer) linkChanged(delta *netmon.ChangeDelta) { - if !delta.Major { - return - } d.mu.Lock() defer d.mu.Unlock() for id, c := range d.activeSysConns { - go c.Close() - delete(d.activeSysConns, id) + if changeAffectsConn(delta, c) { + d.logf("tsdial: closing system connection %v->%v due to link change", c.LocalAddr(), c.RemoteAddr()) + go c.Close() + delete(d.activeSysConns, id) + } + } +} + +// changeAffectsConn reports whether the network change delta affects +// the provided connection. +func changeAffectsConn(delta *netmon.ChangeDelta, conn net.Conn) bool { + la, _ := conn.LocalAddr().(*net.TCPAddr) + ra, _ := conn.RemoteAddr().(*net.TCPAddr) + if la == nil || ra == nil { + return false // not TCP + } + lip, rip := la.AddrPort().Addr(), ra.AddrPort().Addr() + + if delta.Old == nil { + return false + } + if delta.Old.DefaultRouteInterface != delta.New.DefaultRouteInterface || + delta.Old.HTTPProxy != delta.New.HTTPProxy { + return true } + if !delta.New.HasIP(lip) && delta.Old.HasIP(lip) { + // Our interface with this source IP went away. + return true + } + _ = rip // TODO(bradfitz): use the remote IP? + return false } func (d *Dialer) closeSysConn(id int) { @@ -263,6 +288,12 @@ func ipNetOfNetwork(n string) string { return "ip" } +func (d *Dialer) logf(format string, args ...any) { + if d.Logf != nil { + d.Logf(format, args...) + } +} + // SystemDial connects to the provided network address without going over // Tailscale. It prefers going over the default interface and closes existing // connections if the default interface changes. It is used to connect to @@ -276,11 +307,7 @@ func (d *Dialer) SystemDial(ctx context.Context, network, addr string) (net.Conn } d.netnsDialerOnce.Do(func() { - logf := d.Logf - if logf == nil { - logf = logger.Discard - } - d.netnsDialer = netns.NewDialer(logf, d.netMon) + d.netnsDialer = netns.NewDialer(d.logf, d.netMon) }) c, err := d.netnsDialer.DialContext(ctx, network, addr) if err != nil {