tailscale/tailscale

Linux updateMagicsockPort adds firewall rule for old port instead of new

Summary

  • Context: The updateMagicsockPort function in wgengine/router/osrouter/router_linux.go manages firewall rules when the magicsock UDP port changes.

  • Bug: When updating the magicsock port, the function adds a firewall rule for the old port value instead of the new port value.

  • Actual vs. expected: The firewall rule is created for the old port while the router’s internal state is updated to the new port, creating a state inconsistency.

  • Impact: Traffic on the new port will be blocked by the firewall, causing connection failures when Tailscale changes UDP ports during NAT traversal or network changes.

Code with bug

In wgengine/router/osrouter/router_linux.go, function updateMagicsockPort:

if *magicsockPort != 0 {
    if err := r.nfr.DelMagicsockPortRule(*magicsockPort, network); err != nil {
        return fmt.Errorf("del magicsock port rule: %w", err)
    }
}

if port != 0 {
    if err := r.nfr.AddMagicsockPortRule(*magicsockPort, network); err != nil { // <-- BUG 🔴 uses old port instead of new port
        return fmt.Errorf("add magicsock port rule: %w", err)
    }
}

*magicsockPort = port

The variable *magicsockPort contains the old port value when passed to AddMagicsockPortRule, and is only updated to the new value afterward.

Test demonstration

A test script demonstrates the bug by changing the port from 12345 to 54321:

Buggy implementation output:

DelMagicsockPortRule called with port=12345, network=udp4
AddMagicsockPortRule called with port=12345, network=udp4

Results:
  Deleted ports:       [{12345 udp4}]
  Added ports:         [{12345 udp4}]

Fixed implementation output:

DelMagicsockPortRule called with port=12345, network=udp4
AddMagicsockPortRule called with port=54321, network=udp4

Results:
  Deleted ports:     [{12345 udp4}]
  Added ports:       [{54321 udp4}]

The test clearly shows the mismatch: the buggy implementation deletes the rule for port 12345 and then re-adds a rule for 12345, while the internal state is updated to 54321. This leaves no firewall rule for the actual port in use.

Recommended fix

Change the AddMagicsockPortRule call to use the port parameter instead of *magicsockPort:

if port != 0 {
    if err := r.nfr.AddMagicsockPortRule(port, network); err != nil { // <-- FIX 🟢 use new port parameter
        return fmt.Errorf("add magicsock port rule: %w", err)
    }
}

*magicsockPort = port

This ensures the firewall rule is created for the new port value before updating the internal state.