wgengine/wgcfg: remove Config.ListenPort

We don't use the port that wireguard-go passes to us (via magicsock.connBind.Open).
We ignore it entirely and use the port we selected.

When we tell wireguard-go that we're changing the listen_port,
it calls connBind.Close and then connBind.Open.
And in the meantime, it stops calling the receive functions,
which means that we stop receiving and processing UDP and DERP packets.
And that is Very Bad.

That was never a problem prior to b3ceca1dd7,
because we passed the SkipBindUpdate flag to our wireguard-go fork,
which told wireguard-go not to re-bind on listen_port changes.
That commit eliminated the SkipBindUpdate flag.

We could write a bunch of code to work around the gap.
We could add background readers that process UDP and DERP packets when wireguard-go isn't.
But it's simpler to never create the conditions in which wireguard-go rebinds.

The other scenario in which wireguard-go re-binds is device.Down.
Conveniently, we never call device.Down. We go from device.Up to device.Close,
and the latter only when we're shutting down a magicsock.Conn completely.

Rubber-ducked-by: Avery Pennarun <apenwarr@tailscale.com>
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
This commit is contained in:
Josh Bleecher Snyder 2021-04-02 18:04:39 -07:00 committed by Josh Bleecher Snyder
parent 748670f1e9
commit 69cdc30c6d
6 changed files with 4 additions and 15 deletions

View File

@ -38,7 +38,6 @@ func getVal() []interface{} {
&wgcfg.Config{
Name: "foo",
Addresses: []netaddr.IPPrefix{{Bits: 5, IP: netaddr.IPFrom16([16]byte{3: 3})}},
ListenPort: 5,
Peers: []wgcfg.Peer{
{
Endpoints: "foo:5",

View File

@ -460,12 +460,11 @@ func makeConfigs(t *testing.T, addrs []netaddr.IPPort) []wgcfg.Config {
}
var cfgs []wgcfg.Config
for i, addr := range addrs {
for i := range addrs {
cfg := wgcfg.Config{
Name: fmt.Sprintf("peer%d", i+1),
PrivateKey: privKeys[i],
Addresses: addresses[i],
ListenPort: addr.Port,
}
for peerNum, addr := range addrs {
if peerNum == i {

View File

@ -20,7 +20,6 @@ type Config struct {
Name string
PrivateKey PrivateKey
Addresses []netaddr.IPPrefix
ListenPort uint16
MTU uint16
DNS []netaddr.IP
Peers []Peer

View File

@ -57,7 +57,6 @@ func WGCfg(nm *netmap.NetworkMap, logf logger.Logf, flags netmap.WGConfigFlags,
Name: "tailscale",
PrivateKey: wgcfg.PrivateKey(nm.PrivateKey),
Addresses: nm.Addresses,
ListenPort: nm.LocalPort,
Peers: make([]wgcfg.Peer, 0, len(nm.Peers)),
}

View File

@ -144,11 +144,7 @@ func (cfg *Config) handleDeviceLine(key, value string) error {
// wireguard-go guarantees not to send zero value; private keys are already clamped.
cfg.PrivateKey = PrivateKey(*k)
case "listen_port":
port, err := strconv.ParseUint(value, 10, 16)
if err != nil {
return fmt.Errorf("failed to parse listen_port: %w", err)
}
cfg.ListenPort = uint16(port)
// ignore
case "fwmark":
// ignore
default:

View File

@ -40,9 +40,6 @@ func (cfg *Config) ToUAPI(w io.Writer, prev *Config) error {
if prev.PrivateKey != cfg.PrivateKey {
set("private_key", cfg.PrivateKey.HexString())
}
if prev.ListenPort != cfg.ListenPort {
setUint16("listen_port", cfg.ListenPort)
}
old := make(map[Key]Peer)
for _, p := range prev.Peers {