wgengine/magicsock: improve error message for moving Mullvad node keys

The "public key moved" panic has caused confusion on multiple occasions,
and is a known issue for Mullvad. Add a loose heuristic to detect
Mullvad nodes, and trigger distinct panics for Mullvad and non-Mullvad
instances, with a link to the associated bug.

When this occurs again with Mullvad, it'll be easier for somebody to
find the existing bug.

If it occurs again with something other than Mullvad, it'll be more
obvious that it's a distinct issue.

Updates tailscale/corp#27300

Change-Id: Ie47271f45f2ff28f767578fcca5e6b21731d08a1
Signed-off-by: Alex Chan <alexc@tailscale.com>
main
Alex Chan 2 months ago committed by Alex Chan
parent 8fd02bb626
commit 0cca3bd417
  1. 14
      wgengine/magicsock/magicsock.go

@ -3080,8 +3080,18 @@ func (c *Conn) updateNodes(self tailcfg.NodeView, peers []tailcfg.NodeView) (pee
// we don't get this far. If ok was false above, that means it's a key
// that differs from the one the NodeID had. But double check.
if ep.nodeID != n.ID() {
// Server error.
devPanicf("public key moved between nodeIDs (old=%v new=%v, key=%s)", ep.nodeID, n.ID(), n.Key().String())
// Server error. This is known to be a particular issue for Mullvad
// nodes (http://go/corp/27300), so log a distinct error for the
// Mullvad and non-Mullvad cases. The error will be logged either way,
// so an approximate heuristic is fine.
//
// When #27300 is fixed, we can delete this branch and log the same
// panic for any public key moving.
if strings.HasSuffix(n.Name(), ".mullvad.ts.net.") {
devPanicf("public key moved between Mullvad nodeIDs (old=%v new=%v, key=%s); see http://go/corp/27300", ep.nodeID, n.ID(), n.Key().String())
} else {
devPanicf("public key moved between nodeIDs (old=%v new=%v, key=%s)", ep.nodeID, n.ID(), n.Key().String())
}
} else {
// Internal data structures out of sync.
devPanicf("public key found in peerMap but not by nodeID")

Loading…
Cancel
Save