Make HostInfo.remote atomic to fix torn reads on the send path (#1773)

This commit is contained in:
John Maguire
2026-07-07 18:27:53 +00:00
committed by GitHub
parent 6aa3363d85
commit 0a953915bb
7 changed files with 39 additions and 28 deletions
+2 -2
View File
@@ -305,7 +305,7 @@ func (c *Control) CloseAllTunnels(excludeLighthouses bool) (closed int) {
c.l.Debug("Sending close tunnel message", c.l.Debug("Sending close tunnel message",
"vpnAddrs", h.vpnAddrs, "vpnAddrs", h.vpnAddrs,
"udpAddr", h.remote, "udpAddr", h.GetRemote(),
) )
closed++ closed++
} }
@@ -350,7 +350,7 @@ func copyHostInfo(h *HostInfo, preferredRanges []netip.Prefix) ControlHostInfo {
RemoteAddrs: h.remotes.CopyAddrs(preferredRanges), RemoteAddrs: h.remotes.CopyAddrs(preferredRanges),
CurrentRelaysToMe: h.relayState.CopyRelayIps(), CurrentRelaysToMe: h.relayState.CopyRelayIps(),
CurrentRelaysThroughMe: h.relayState.CopyRelayForIps(), CurrentRelaysThroughMe: h.relayState.CopyRelayForIps(),
CurrentRemote: h.remote, CurrentRemote: h.GetRemote(),
} }
for i, a := range h.vpnAddrs { for i, a := range h.vpnAddrs {
+8 -6
View File
@@ -42,8 +42,7 @@ func TestControl_GetHostInfoByVpnIp(t *testing.T) {
assert.True(t, ok) assert.True(t, ok)
crt := &dummyCert{} crt := &dummyCert{}
hm.unlockedAddHostInfo(&HostInfo{ hi := &HostInfo{
remote: remote1,
remotes: remotes, remotes: remotes,
ConnectionState: &ConnectionState{ ConnectionState: &ConnectionState{
peerCert: &cert.CachedCertificate{Certificate: crt}, peerCert: &cert.CachedCertificate{Certificate: crt},
@@ -56,13 +55,14 @@ func TestControl_GetHostInfoByVpnIp(t *testing.T) {
relayForByAddr: map[netip.Addr]*Relay{}, relayForByAddr: map[netip.Addr]*Relay{},
relayForByIdx: map[uint32]*Relay{}, relayForByIdx: map[uint32]*Relay{},
}, },
}, &Interface{}) }
hi.remote.Store(&remote1)
hm.unlockedAddHostInfo(hi, &Interface{})
vpnIp2, ok := netip.AddrFromSlice(ipNet2.IP) vpnIp2, ok := netip.AddrFromSlice(ipNet2.IP)
assert.True(t, ok) assert.True(t, ok)
hm.unlockedAddHostInfo(&HostInfo{ hi2 := &HostInfo{
remote: remote1,
remotes: remotes, remotes: remotes,
ConnectionState: &ConnectionState{ ConnectionState: &ConnectionState{
peerCert: nil, peerCert: nil,
@@ -75,7 +75,9 @@ func TestControl_GetHostInfoByVpnIp(t *testing.T) {
relayForByAddr: map[netip.Addr]*Relay{}, relayForByAddr: map[netip.Addr]*Relay{},
relayForByIdx: map[uint32]*Relay{}, relayForByIdx: map[uint32]*Relay{},
}, },
}, &Interface{}) }
hi2.remote.Store(&remote1)
hm.unlockedAddHostInfo(hi2, &Interface{})
c := Control{ c := Control{
state: StateReady, state: StateReady,
+12 -5
View File
@@ -229,7 +229,7 @@ const (
) )
type HostInfo struct { type HostInfo struct {
remote netip.AddrPort remote atomic.Pointer[netip.AddrPort]
remotes *RemoteList remotes *RemoteList
promoteCounter atomic.Uint32 promoteCounter atomic.Uint32
ConnectionState *ConnectionState ConnectionState *ConnectionState
@@ -684,7 +684,7 @@ func (hm *HostMap) ForEachIndex(f controlEach) {
func (i *HostInfo) TryPromoteBest(preferredRanges []netip.Prefix, ifce *Interface) { func (i *HostInfo) TryPromoteBest(preferredRanges []netip.Prefix, ifce *Interface) {
c := i.promoteCounter.Add(1) c := i.promoteCounter.Add(1)
if c%ifce.tryPromoteEvery.Load() == 0 { if c%ifce.tryPromoteEvery.Load() == 0 {
remote := i.remote remote := i.GetRemote()
// return early if we are already on a preferred remote // return early if we are already on a preferred remote
if remote.IsValid() { if remote.IsValid() {
@@ -726,11 +726,18 @@ func (i *HostInfo) GetCert() *cert.CachedCertificate {
return nil return nil
} }
func (i *HostInfo) GetRemote() netip.AddrPort {
if p := i.remote.Load(); p != nil {
return *p
}
return netip.AddrPort{}
}
// TODO: Maybe use ViaSender here? // TODO: Maybe use ViaSender here?
func (i *HostInfo) SetRemote(remote netip.AddrPort) { func (i *HostInfo) SetRemote(remote netip.AddrPort) {
// We copy here because we likely got this remote from a source that reuses the object // We copy here because we likely got this remote from a source that reuses the object
if i.remote != remote { if i.GetRemote() != remote {
i.remote = remote i.remote.Store(&remote)
i.remotes.LearnRemote(i.vpnAddrs[0], remote) i.remotes.LearnRemote(i.vpnAddrs[0], remote)
} }
} }
@@ -742,7 +749,7 @@ func (i *HostInfo) SetRemoteIfPreferred(hm *HostMap, via ViaSender) bool {
return false return false
} }
currentRemote := i.remote currentRemote := i.GetRemote()
if !currentRemote.IsValid() { if !currentRemote.IsValid() {
i.SetRemote(via.UdpAddr) i.SetRemote(via.UdpAddr)
return true return true
+4 -4
View File
@@ -333,7 +333,7 @@ func (f *Interface) SendVia(via *HostInfo,
via.logger(f.l).Info("Failed to EncryptDanger in sendVia", "error", err) via.logger(f.l).Info("Failed to EncryptDanger in sendVia", "error", err)
return return
} }
err = f.writers[0].WriteTo(out, via.remote) err = f.writers[0].WriteTo(out, via.GetRemote())
if err != nil { if err != nil {
via.logger(f.l).Info("Failed to WriteTo in sendVia", "error", err) via.logger(f.l).Info("Failed to WriteTo in sendVia", "error", err)
} }
@@ -344,7 +344,7 @@ func (f *Interface) sendNoMetrics(t header.MessageType, st header.MessageSubType
if ci.eKey == nil { if ci.eKey == nil {
return return
} }
useRelay := !remote.IsValid() && !hostinfo.remote.IsValid() useRelay := !remote.IsValid() && !hostinfo.GetRemote().IsValid()
fullOut := out fullOut := out
if useRelay { if useRelay {
@@ -403,8 +403,8 @@ func (f *Interface) sendNoMetrics(t header.MessageType, st header.MessageSubType
"udpAddr", remote, "udpAddr", remote,
) )
} }
} else if hostinfo.remote.IsValid() { } else if hr := hostinfo.GetRemote(); hr.IsValid() {
err = f.writers[q].WriteTo(out, hostinfo.remote) err = f.writers[q].WriteTo(out, hr)
if err != nil { if err != nil {
hostinfo.logger(f.l).Error("Failed to write outgoing packet", hostinfo.logger(f.l).Error("Failed to write outgoing packet",
"error", err, "error", err,
+8 -6
View File
@@ -277,7 +277,8 @@ func (f *Interface) sendCloseTunnel(h *HostInfo) {
} }
func (f *Interface) handleHostRoaming(hostinfo *HostInfo, via ViaSender) { func (f *Interface) handleHostRoaming(hostinfo *HostInfo, via ViaSender) {
if !via.IsRelayed && hostinfo.remote != via.UdpAddr { curRemote := hostinfo.GetRemote()
if !via.IsRelayed && curRemote != via.UdpAddr {
if !f.lightHouse.GetRemoteAllowList().AllowAll(hostinfo.vpnAddrs, via.UdpAddr.Addr()) { if !f.lightHouse.GetRemoteAllowList().AllowAll(hostinfo.vpnAddrs, via.UdpAddr.Addr()) {
if f.l.Enabled(context.Background(), slog.LevelDebug) { if f.l.Enabled(context.Background(), slog.LevelDebug) {
hostinfo.logger(f.l).Debug("lighthouse.remote_allow_list denied roaming", "newAddr", via.UdpAddr) hostinfo.logger(f.l).Debug("lighthouse.remote_allow_list denied roaming", "newAddr", via.UdpAddr)
@@ -289,7 +290,7 @@ func (f *Interface) handleHostRoaming(hostinfo *HostInfo, via ViaSender) {
if f.l.Enabled(context.Background(), slog.LevelDebug) { if f.l.Enabled(context.Background(), slog.LevelDebug) {
hostinfo.logger(f.l).Debug("Suppressing roam back to previous remote", hostinfo.logger(f.l).Debug("Suppressing roam back to previous remote",
"suppressSeconds", RoamingSuppressSeconds, "suppressSeconds", RoamingSuppressSeconds,
"udpAddr", hostinfo.remote, "udpAddr", curRemote,
"newAddr", via.UdpAddr, "newAddr", via.UdpAddr,
) )
} }
@@ -297,11 +298,11 @@ func (f *Interface) handleHostRoaming(hostinfo *HostInfo, via ViaSender) {
} }
hostinfo.logger(f.l).Info("Host roamed to new udp ip/port.", hostinfo.logger(f.l).Info("Host roamed to new udp ip/port.",
"udpAddr", hostinfo.remote, "udpAddr", curRemote,
"newAddr", via.UdpAddr, "newAddr", via.UdpAddr,
) )
hostinfo.lastRoam = time.Now() hostinfo.lastRoam = time.Now()
hostinfo.lastRoamRemote = hostinfo.remote hostinfo.lastRoamRemote = curRemote
hostinfo.SetRemote(via.UdpAddr) hostinfo.SetRemote(via.UdpAddr)
} }
@@ -590,10 +591,11 @@ func (f *Interface) handleRecvError(addr netip.AddrPort, h *header.H) {
return return
} }
if hostinfo.remote.IsValid() && hostinfo.remote != addr { hr := hostinfo.GetRemote()
if hr.IsValid() && hr != addr {
f.l.Info("Someone spoofing recv_errors?", f.l.Info("Someone spoofing recv_errors?",
"addr", addr, "addr", addr,
"hostinfoRemote", hostinfo.remote, "hostinfoRemote", hr,
) )
return return
} }
+2 -2
View File
@@ -174,9 +174,9 @@ func (p *Punchy) SendPunch(hostinfo *HostInfo) {
if p.punchEverything.Load() { if p.punchEverything.Load() {
p.sendPunchToAllRemotes(hostinfo) p.sendPunchToAllRemotes(hostinfo)
} else if hostinfo.remote.IsValid() { } else if hr := hostinfo.GetRemote(); hr.IsValid() {
p.metricPunchyTx.Inc(1) p.metricPunchyTx.Inc(1)
p.punchConn.WriteTo([]byte{1}, hostinfo.remote) p.punchConn.WriteTo([]byte{1}, hr)
} }
} }
+3 -3
View File
@@ -94,7 +94,7 @@ func (rm *relayManager) StartRelays(f *Interface, vpnIp netip.Addr, hh *Handshak
} }
relayHostInfo := rm.hostmap.QueryVpnAddr(relay) relayHostInfo := rm.hostmap.QueryVpnAddr(relay)
if relayHostInfo == nil || !relayHostInfo.remote.IsValid() { if relayHostInfo == nil || !relayHostInfo.GetRemote().IsValid() {
hl.Log(context.Background(), level, "Establish tunnel to relay target", "relay", relay.String()) hl.Log(context.Background(), level, "Establish tunnel to relay target", "relay", relay.String())
f.Handshake(relay) f.Handshake(relay)
continue continue
@@ -104,7 +104,7 @@ func (rm *relayManager) StartRelays(f *Interface, vpnIp netip.Addr, hh *Handshak
existingRelay, ok := relayHostInfo.relayState.QueryRelayForByIp(vpnIp) existingRelay, ok := relayHostInfo.relayState.QueryRelayForByIp(vpnIp)
if !ok { if !ok {
// No relays exist or requested yet. // No relays exist or requested yet.
if relayHostInfo.remote.IsValid() { if relayHostInfo.GetRemote().IsValid() {
idx, err := AddRelay(rm.l, relayHostInfo, rm.hostmap, vpnIp, nil, TerminalType, Requested) idx, err := AddRelay(rm.l, relayHostInfo, rm.hostmap, vpnIp, nil, TerminalType, Requested)
if err != nil { if err != nil {
hl.Info("Failed to add relay to hostmap", "relay", relay.String(), "error", err) hl.Info("Failed to add relay to hostmap", "relay", relay.String(), "error", err)
@@ -508,7 +508,7 @@ func (rm *relayManager) handleCreateRelayRequest(v cert.Version, h *HostInfo, f
f.Handshake(target) f.Handshake(target)
return return
} }
if !peer.remote.IsValid() { if !peer.GetRemote().IsValid() {
// Only create relays to peers for whom I have a direct connection // Only create relays to peers for whom I have a direct connection
return return
} }