mirror of
https://github.com/tailscale/tailscale.git
synced 2024-11-29 04:55:31 +00:00
wgengine/magicsock: fix handling of derp.PeerGoneMessage (#10589)
The switch in Conn.runDerpReader() on the derp.ReceivedMessage type contained cases other than derp.ReceivedPacket that fell through to writing to c.derpRecvCh, which should only be reached for derp.ReceivedPacket. This can result in the last/previous derp.ReceivedPacket to be re-handled, effectively creating a duplicate packet. If the last derp.ReceivedPacket happens to be a disco.CallMeMaybe it may result in a disco ping scan towards the originating peer on the endpoints contained. The change in this commit moves the channel write on c.derpRecvCh and subsequent select awaiting the result into the derp.ReceivedMessage case, preventing it from being reached from any other case. Explicit continue statements are also added to non-derp.ReceivedPacket cases where they were missing, in order to signal intent to the reader. Fixes #10586 Signed-off-by: Jordan Whited <jordan@tailscale.com>
This commit is contained in:
parent
3ae562366b
commit
685b853763
@ -424,8 +424,8 @@ func (c *Conn) setPeerLastDerpLocked(peer key.NodePublic, regionID, homeID int)
|
||||
}
|
||||
}
|
||||
|
||||
// derpReadResult is the type sent by runDerpClient to ReceiveIPv4
|
||||
// when a DERP packet is available.
|
||||
// derpReadResult is the type sent by Conn.runDerpReader to connBind.receiveDERP
|
||||
// when a derp.ReceivedPacket is available.
|
||||
//
|
||||
// Notably, it doesn't include the derp.ReceivedPacket because we
|
||||
// don't want to give the receiver access to the aliased []byte. To
|
||||
@ -542,6 +542,17 @@ func (c *Conn) runDerpReader(ctx context.Context, derpFakeAddr netip.AddrPort, d
|
||||
c.addDerpPeerRoute(res.src, regionID, dc)
|
||||
}
|
||||
}
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
return
|
||||
case c.derpRecvCh <- res:
|
||||
}
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
return
|
||||
case <-didCopy:
|
||||
continue
|
||||
}
|
||||
case derp.PingMessage:
|
||||
// Best effort reply to the ping.
|
||||
pingData := [8]byte(m)
|
||||
@ -553,6 +564,7 @@ func (c *Conn) runDerpReader(ctx context.Context, derpFakeAddr netip.AddrPort, d
|
||||
continue
|
||||
case derp.HealthMessage:
|
||||
health.SetDERPRegionHealth(regionID, m.Problem)
|
||||
continue
|
||||
case derp.PeerGoneMessage:
|
||||
switch m.Reason {
|
||||
case derp.PeerGoneReasonDisconnected:
|
||||
@ -567,23 +579,11 @@ func (c *Conn) runDerpReader(ctx context.Context, derpFakeAddr netip.AddrPort, d
|
||||
regionID, key.NodePublic(m.Peer).ShortString(), m.Reason)
|
||||
}
|
||||
c.removeDerpPeerRoute(key.NodePublic(m.Peer), regionID, dc)
|
||||
continue
|
||||
default:
|
||||
// Ignore.
|
||||
continue
|
||||
}
|
||||
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
return
|
||||
case c.derpRecvCh <- res:
|
||||
}
|
||||
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
return
|
||||
case <-didCopy:
|
||||
continue
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user