From d46a4eced5554b5947dfcd2ae8047ec44ba137fc Mon Sep 17 00:00:00 2001 From: Naman Sood Date: Tue, 5 Dec 2023 18:12:02 -0500 Subject: [PATCH] util/linuxfw, wgengine: allow ingress to magicsock UDP port on Linux (#10370) * util/linuxfw, wgengine: allow ingress to magicsock UDP port on Linux Updates #9084. Currently, we have to tell users to manually open UDP ports on Linux when certain firewalls (like ufw) are enabled. This change automates the process of adding and updating those firewall rules as magicsock changes what port it listens on. Signed-off-by: Naman Sood --- util/linuxfw/iptables_runner.go | 62 ++++++++++- util/linuxfw/nftables_runner.go | 139 ++++++++++++++++++++++++ wgengine/magicsock/magicsock.go | 22 ++++ wgengine/router/router.go | 8 ++ wgengine/router/router_fake.go | 5 + wgengine/router/router_linux.go | 70 ++++++++++++ wgengine/router/router_linux_test.go | 52 +++++++++ wgengine/router/router_openbsd.go | 7 ++ wgengine/router/router_userspace_bsd.go | 7 ++ wgengine/router/router_windows.go | 7 ++ wgengine/userspace.go | 8 ++ 11 files changed, 385 insertions(+), 2 deletions(-) diff --git a/util/linuxfw/iptables_runner.go b/util/linuxfw/iptables_runner.go index f7fe2f0f4..9211bbd16 100644 --- a/util/linuxfw/iptables_runner.go +++ b/util/linuxfw/iptables_runner.go @@ -9,6 +9,7 @@ import ( "fmt" "net/netip" "os/exec" + "strconv" "strings" "github.com/coreos/go-iptables/iptables" @@ -236,7 +237,7 @@ func (i *iptablesRunner) AddBase(tunname string) error { return nil } -// addBase4 adds some basic IPv6 processing rules to be +// addBase4 adds some basic IPv4 processing rules to be // supplemented by later calls to other helpers. func (i *iptablesRunner) addBase4(tunname string) error { // Only allow CGNAT range traffic to come from tailscale0. There @@ -311,7 +312,7 @@ func (i *iptablesRunner) ClampMSSToPMTU(tun string, addr netip.Addr) error { return table.Append("mangle", "FORWARD", "-o", tun, "-p", "tcp", "--tcp-flags", "SYN,RST", "SYN", "-j", "TCPMSS", "--clamp-mss-to-pmtu") } -// addBase6 adds some basic IPv4 processing rules to be +// addBase6 adds some basic IPv6 processing rules to be // supplemented by later calls to other helpers. func (i *iptablesRunner) addBase6(tunname string) error { // TODO: only allow traffic from Tailscale's ULA range to come @@ -437,6 +438,63 @@ func (i *iptablesRunner) DelSNATRule() error { return nil } +// buildMagicsockPortRule generates the string slice containing the arguments +// to describe a rule accepting traffic on a particular port to iptables. It is +// separated out here to avoid repetition in AddMagicsockPortRule and +// RemoveMagicsockPortRule, since it is important that the same rule is passed +// to Append() and Delete(). +func buildMagicsockPortRule(port uint16) []string { + return []string{"-p", "udp", "--dport", strconv.FormatUint(uint64(port), 10), "-j", "ACCEPT"} +} + +// AddMagicsockPortRule adds a rule to iptables to allow incoming traffic on +// the specified UDP port, so magicsock can accept incoming connections. +// network must be either "udp4" or "udp6" - this determines whether the rule +// is added for IPv4 or IPv6. +func (i *iptablesRunner) AddMagicsockPortRule(port uint16, network string) error { + var ipt iptablesInterface + switch network { + case "udp4": + ipt = i.ipt4 + case "udp6": + ipt = i.ipt6 + default: + return fmt.Errorf("unsupported network %s", network) + } + + args := buildMagicsockPortRule(port) + + if err := ipt.Append("filter", "ts-input", args...); err != nil { + return fmt.Errorf("adding %v in filter/ts-input: %w", args, err) + } + + return nil +} + +// DelMagicsockPortRule removes a rule added by AddMagicsockPortRule to accept +// incoming traffic on a particular UDP port. +// network must be either "udp4" or "udp6" - this determines whether the rule +// is removed for IPv4 or IPv6. +func (i *iptablesRunner) DelMagicsockPortRule(port uint16, network string) error { + var ipt iptablesInterface + switch network { + case "udp4": + ipt = i.ipt4 + case "udp6": + ipt = i.ipt6 + default: + return fmt.Errorf("unsupported network %s", network) + } + + args := buildMagicsockPortRule(port) + + if err := ipt.Delete("filter", "ts-input", args...); err != nil { + return fmt.Errorf("removing %v in filter/ts-input: %w", args, err) + } + + return nil +} + // IPTablesCleanup removes all Tailscale added iptables rules. // Any errors that occur are logged to the provided logf. func IPTablesCleanup(logf logger.Logf) { diff --git a/util/linuxfw/nftables_runner.go b/util/linuxfw/nftables_runner.go index 5715c0412..7e7d80ca6 100644 --- a/util/linuxfw/nftables_runner.go +++ b/util/linuxfw/nftables_runner.go @@ -509,6 +509,15 @@ type NetfilterRunner interface { // ClampMSSToPMTU adds a rule to the mangle/FORWARD chain to clamp MSS for // traffic destined for the provided tun interface. ClampMSSToPMTU(tun string, addr netip.Addr) error + + // AddMagicsockPortRule adds a rule to the ts-input chain to accept + // incoming traffic on the specified port, to allow magicsock to + // communicate. + AddMagicsockPortRule(port uint16, network string) error + + // DelMagicsockPortRule removes the rule created by AddMagicsockPortRule, + // if it exists. + DelMagicsockPortRule(port uint16, network string) error } // New creates a NetfilterRunner, auto-detecting whether to use @@ -584,6 +593,17 @@ func newLoadSaddrExpr(proto nftables.TableFamily, destReg uint32) (expr.Any, err } } +// newLoadDportExpr creates a new nftables express that loads the desination port +// of a TCP/UDP packet into the given register. +func newLoadDportExpr(destReg uint32) expr.Any { + return &expr.Payload{ + DestRegister: destReg, + Base: expr.PayloadBaseTransportHeader, + Offset: 2, + Len: 2, + } +} + // HasIPV6 reports true if the system supports IPv6. func (n *nftablesRunner) HasIPV6() bool { return n.v6Available @@ -1267,6 +1287,125 @@ func addAcceptOutgoingPacketRule(conn *nftables.Conn, table *nftables.Table, cha return nil } +// createAcceptOnPortRule creates a rule to accept incoming packets to +// a given destination UDP port. +func createAcceptOnPortRule(table *nftables.Table, chain *nftables.Chain, port uint16) *nftables.Rule { + portBytes := make([]byte, 2) + binary.BigEndian.PutUint16(portBytes, port) + return &nftables.Rule{ + Table: table, + Chain: chain, + Exprs: []expr.Any{ + &expr.Meta{ + Key: expr.MetaKeyL4PROTO, + Register: 1, + }, + &expr.Cmp{ + Op: expr.CmpOpEq, + Register: 1, + Data: []byte{unix.IPPROTO_UDP}, + }, + newLoadDportExpr(1), + &expr.Cmp{ + Op: expr.CmpOpEq, + Register: 1, + Data: portBytes, + }, + &expr.Counter{}, + &expr.Verdict{ + Kind: expr.VerdictAccept, + }, + }, + } +} + +// addAcceptOnPortRule adds a rule to accept incoming packets to +// a given destination UDP port. +func addAcceptOnPortRule(conn *nftables.Conn, table *nftables.Table, chain *nftables.Chain, port uint16) error { + rule := createAcceptOnPortRule(table, chain, port) + _ = conn.AddRule(rule) + + if err := conn.Flush(); err != nil { + return fmt.Errorf("flush add rule: %w", err) + } + + return nil +} + +// addAcceptOnPortRule removes a rule to accept incoming packets to +// a given destination UDP port. +func removeAcceptOnPortRule(conn *nftables.Conn, table *nftables.Table, chain *nftables.Chain, port uint16) error { + rule := createAcceptOnPortRule(table, chain, port) + rule, err := findRule(conn, rule) + if err != nil { + return fmt.Errorf("find rule: %v", err) + } + + _ = conn.DelRule(rule) + + if err := conn.Flush(); err != nil { + return fmt.Errorf("flush del rule: %w", err) + } + + return nil +} + +// AddMagicsockPortRule adds a rule to nftables to allow incoming traffic on +// the specified UDP port, so magicsock can accept incoming connections. +// network must be either "udp4" or "udp6" - this determines whether the rule +// is added for IPv4 or IPv6. +func (n *nftablesRunner) AddMagicsockPortRule(port uint16, network string) error { + var filterTable *nftables.Table + switch network { + case "udp4": + filterTable = n.nft4.Filter + case "udp6": + filterTable = n.nft6.Filter + default: + return fmt.Errorf("unsupported network %s", network) + } + + inputChain, err := getChainFromTable(n.conn, filterTable, chainNameInput) + if err != nil { + return fmt.Errorf("get input chain: %v", err) + } + + err = addAcceptOnPortRule(n.conn, filterTable, inputChain, port) + if err != nil { + return fmt.Errorf("add accept on port rule: %v", err) + } + + return nil +} + +// DelMagicsockPortRule removes a rule added by AddMagicsockPortRule to accept +// incoming traffic on a particular UDP port. +// network must be either "udp4" or "udp6" - this determines whether the rule +// is removed for IPv4 or IPv6. +func (n *nftablesRunner) DelMagicsockPortRule(port uint16, network string) error { + var filterTable *nftables.Table + switch network { + case "udp4": + filterTable = n.nft4.Filter + case "udp6": + filterTable = n.nft6.Filter + default: + return fmt.Errorf("unsupported network %s", network) + } + + inputChain, err := getChainFromTable(n.conn, filterTable, chainNameInput) + if err != nil { + return fmt.Errorf("get input chain: %v", err) + } + + err = removeAcceptOnPortRule(n.conn, filterTable, inputChain, port) + if err != nil { + return fmt.Errorf("add accept on port rule: %v", err) + } + + return nil +} + // createAcceptIncomingPacketRule creates a rule to accept incoming packets to // the given interface. func createAcceptIncomingPacketRule(table *nftables.Table, chain *nftables.Chain, tunname string) *nftables.Rule { diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index 35755e03d..537901cc0 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -290,6 +290,10 @@ type Conn struct { // wgPinger is the WireGuard only pinger used for latency measurements. wgPinger lazy.SyncValue[*ping.Pinger] + + // onPortUpdate is called with the new port when magicsock rebinds to + // a new port. + onPortUpdate func(port uint16, network string) } // SetDebugLoggingEnabled controls whether spammy debug logging is enabled. @@ -355,6 +359,10 @@ type Options struct { // ControlKnobs are the set of control knobs to use. // If nil, they're ignored and not updated. ControlKnobs *controlknobs.Knobs + + // OnPortUpdate is called with the new port when magicsock rebinds to + // a new port. + OnPortUpdate func(port uint16, network string) } func (o *Options) logf() logger.Logf { @@ -427,6 +435,7 @@ func NewConn(opts Options) (*Conn, error) { c.portMapper.SetGatewayLookupFunc(opts.NetMon.GatewayAndSelfIP) } c.netMon = opts.NetMon + c.onPortUpdate = opts.OnPortUpdate if err := c.rebind(keepCurrentPort); err != nil { return nil, err @@ -2342,6 +2351,19 @@ func (c *Conn) bindSocket(ruc *RebindingUDPConn, network string, curPortFate cur c.logf("magicsock: unable to bind %v port %d: %v", network, port, err) continue } + if c.onPortUpdate != nil { + _, gotPortStr, err := net.SplitHostPort(pconn.LocalAddr().String()) + if err != nil { + c.logf("could not parse port from %s: %w", pconn.LocalAddr().String(), err) + } else { + gotPort, err := strconv.ParseUint(gotPortStr, 10, 16) + if err != nil { + c.logf("could not parse port from %s: %w", gotPort, err) + } else { + c.onPortUpdate(uint16(gotPort), network) + } + } + } trySetSocketBuffer(pconn, c.logf) // Success. diff --git a/wgengine/router/router.go b/wgengine/router/router.go index 5fde0e6c7..3c5497f42 100644 --- a/wgengine/router/router.go +++ b/wgengine/router/router.go @@ -27,6 +27,14 @@ type Router interface { // implementation should handle gracefully. Set(*Config) error + // UpdateMagicsockPort tells the OS network stack what port magicsock + // is currently listening on, so it can be threaded through firewalls + // and such. This is distinct from Set() since magicsock may rebind + // ports independently from the Config changing. + // + // network should be either "udp4" or "udp6". + UpdateMagicsockPort(port uint16, network string) error + // Close closes the router. Close() error } diff --git a/wgengine/router/router_fake.go b/wgengine/router/router_fake.go index db35fc9ee..549867eca 100644 --- a/wgengine/router/router_fake.go +++ b/wgengine/router/router_fake.go @@ -27,6 +27,11 @@ func (r fakeRouter) Set(cfg *Config) error { return nil } +func (r fakeRouter) UpdateMagicsockPort(_ uint16, _ string) error { + r.logf("[v1] warning: fakeRouter.UpdateMagicsockPort: not implemented.") + return nil +} + func (r fakeRouter) Close() error { r.logf("[v1] warning: fakeRouter.Close: not implemented.") return nil diff --git a/wgengine/router/router_linux.go b/wgengine/router/router_linux.go index ddc0cee6a..f6647c72b 100644 --- a/wgengine/router/router_linux.go +++ b/wgengine/router/router_linux.go @@ -63,6 +63,9 @@ type linuxRouter struct { cmd commandRunner nfr linuxfw.NetfilterRunner + + magicsockPortV4 uint16 + magicsockPortV6 uint16 } func newUserspaceRouter(logf logger.Logf, tunDev tun.Device, netMon *netmon.Monitor) (Router, error) { @@ -400,6 +403,53 @@ func (r *linuxRouter) Set(cfg *Config) error { return multierr.New(errs...) } +// UpdateMagicsockPort implements the Router interface. +func (r *linuxRouter) UpdateMagicsockPort(port uint16, network string) error { + if r.nfr == nil { + if err := r.setupNetfilter(r.netfilterKind); err != nil { + return fmt.Errorf("could not setup netfilter: %w", err) + } + } + + var magicsockPort *uint16 + switch network { + case "udp4": + magicsockPort = &r.magicsockPortV4 + case "udp6": + if !r.nfr.HasIPV6() { + return nil + } + magicsockPort = &r.magicsockPortV6 + default: + return fmt.Errorf("unsupported network %s", network) + } + + // set the port, we'll make the firewall rule when netfilter turns back on + if r.netfilterMode == netfilterOff { + *magicsockPort = port + return nil + } + + if *magicsockPort == port { + return nil + } + + 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 { + return fmt.Errorf("add magicsock port rule: %w", err) + } + } + + *magicsockPort = port + return nil +} + // setNetfilterMode switches the router to the given netfilter // mode. Netfilter state is created or deleted appropriately to // reflect the new mode, and r.snatSubnetRoutes is updated to reflect @@ -468,6 +518,16 @@ func (r *linuxRouter) setNetfilterMode(mode preftype.NetfilterMode) error { if err := r.nfr.AddBase(r.tunname); err != nil { return err } + if r.magicsockPortV4 != 0 { + if err := r.nfr.AddMagicsockPortRule(r.magicsockPortV4, "udp4"); err != nil { + return fmt.Errorf("could not add magicsock port rule v4: %w", err) + } + } + if r.magicsockPortV6 != 0 && r.nfr.HasIPV6() { + if err := r.nfr.AddMagicsockPortRule(r.magicsockPortV6, "udp6"); err != nil { + return fmt.Errorf("could not add magicsock port rule v6: %w", err) + } + } r.snatSubnetRoutes = false case netfilterOn: if err := r.nfr.DelHooks(r.logf); err != nil { @@ -498,6 +558,16 @@ func (r *linuxRouter) setNetfilterMode(mode preftype.NetfilterMode) error { if err := r.nfr.AddBase(r.tunname); err != nil { return err } + if r.magicsockPortV4 != 0 { + if err := r.nfr.AddMagicsockPortRule(r.magicsockPortV4, "udp4"); err != nil { + return fmt.Errorf("could not add magicsock port rule v4: %w", err) + } + } + if r.magicsockPortV6 != 0 && r.nfr.HasIPV6() { + if err := r.nfr.AddMagicsockPortRule(r.magicsockPortV6, "udp6"); err != nil { + return fmt.Errorf("could not add magicsock port rule v6: %w", err) + } + } r.snatSubnetRoutes = false case netfilterNoDivert: reprocess = true diff --git a/wgengine/router/router_linux_test.go b/wgengine/router/router_linux_test.go index 1aa522ac9..b86469f6d 100644 --- a/wgengine/router/router_linux_test.go +++ b/wgengine/router/router_linux_test.go @@ -607,6 +607,58 @@ func (n *fakeIPTablesRunner) DelSNATRule() error { return nil } +// buildMagicsockPortRule builds a fake rule to use in AddMagicsockPortRule and +// DelMagicsockPortRule below. +func buildMagicsockPortRule(port uint16) string { + return fmt.Sprintf("-p udp --dport %v -j ACCEPT", port) +} + +// AddMagicsockPortRule implements the NetfilterRunner interface, but stores +// rules in fakeIPTablesRunner's internal maps rather than actually calling out +// to iptables. This is mainly to test the linux router implementation. +func (n *fakeIPTablesRunner) AddMagicsockPortRule(port uint16, network string) error { + var ipt map[string][]string + switch network { + case "udp4": + ipt = n.ipt4 + case "udp6": + ipt = n.ipt6 + default: + return fmt.Errorf("unsupported network %s", network) + } + + rule := buildMagicsockPortRule(port) + + if err := appendRule(n, ipt, "filter/ts-input", rule); err != nil { + return err + } + + return nil +} + +// DelMagicsockPortRule implements the NetfilterRunner interface, but removes +// rules from fakeIPTablesRunner's internal maps rather than actually calling +// out to iptables. This is mainly to test the linux router implementation. +func (n *fakeIPTablesRunner) DelMagicsockPortRule(port uint16, network string) error { + var ipt map[string][]string + switch network { + case "udp4": + ipt = n.ipt4 + case "udp6": + ipt = n.ipt6 + default: + return fmt.Errorf("unsupported network %s", network) + } + + rule := buildMagicsockPortRule(port) + + if err := deleteRule(n, ipt, "filter/ts-input", rule); err != nil { + return err + } + + return nil +} + func (n *fakeIPTablesRunner) HasIPV6() bool { return true } func (n *fakeIPTablesRunner) HasIPV6NAT() bool { return true } diff --git a/wgengine/router/router_openbsd.go b/wgengine/router/router_openbsd.go index b85992779..b71a01cc7 100644 --- a/wgengine/router/router_openbsd.go +++ b/wgengine/router/router_openbsd.go @@ -228,6 +228,13 @@ func (r *openbsdRouter) Set(cfg *Config) error { return errq } +// UpdateMagicsockPort implements the Router interface. This implementation +// does nothing and returns nil because this router does not currently need +// to know what the magicsock UDP port is. +func (r *openbsdRouter) UpdateMagicsockPort(_ uint16, _ string) error { + return nil +} + func (r *openbsdRouter) Close() error { cleanup(r.logf, r.tunname) return nil diff --git a/wgengine/router/router_userspace_bsd.go b/wgengine/router/router_userspace_bsd.go index 61c06037d..87d47115e 100644 --- a/wgengine/router/router_userspace_bsd.go +++ b/wgengine/router/router_userspace_bsd.go @@ -196,6 +196,13 @@ func (r *userspaceBSDRouter) Set(cfg *Config) (reterr error) { return reterr } +// UpdateMagicsockPort implements the Router interface. This implementation +// does nothing and returns nil because this router does not currently need +// to know what the magicsock UDP port is. +func (r *userspaceBSDRouter) UpdateMagicsockPort(_ uint16, _ string) error { + return nil +} + func (r *userspaceBSDRouter) Close() error { return nil } diff --git a/wgengine/router/router_windows.go b/wgengine/router/router_windows.go index d51f7a7c6..0f01194d3 100644 --- a/wgengine/router/router_windows.go +++ b/wgengine/router/router_windows.go @@ -102,6 +102,13 @@ func hasDefaultRoute(routes []netip.Prefix) bool { return false } +// UpdateMagicsockPort implements the Router interface. This implementation +// does nothing and returns nil because this router does not currently need +// to know what the magicsock UDP port is. +func (r *winRouter) UpdateMagicsockPort(_ uint16, _ string) error { + return nil +} + func (r *winRouter) Close() error { r.firewall.clear() diff --git a/wgengine/userspace.go b/wgengine/userspace.go index a49526e3b..12dd437aa 100644 --- a/wgengine/userspace.go +++ b/wgengine/userspace.go @@ -324,6 +324,13 @@ func NewUserspaceEngine(logf logger.Logf, conf Config) (_ Engine, reterr error) e.RequestStatus() } + onPortUpdate := func(port uint16, network string) { + e.logf("onPortUpdate(port=%v, network=%s)", port, network) + + if err := e.router.UpdateMagicsockPort(port, network); err != nil { + e.logf("UpdateMagicsockPort(port=%v, network=%s) failed: %w", port, network, err) + } + } magicsockOpts := magicsock.Options{ Logf: logf, Port: conf.ListenPort, @@ -333,6 +340,7 @@ func NewUserspaceEngine(logf logger.Logf, conf Config) (_ Engine, reterr error) NoteRecvActivity: e.noteRecvActivity, NetMon: e.netMon, ControlKnobs: conf.ControlKnobs, + OnPortUpdate: onPortUpdate, } var err error