From bd90781b34d2558f27196194ba871f1cda60ffe6 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Mon, 31 Jan 2022 09:20:22 -0800 Subject: [PATCH] ipn/ipnlocal, wgengine/netstack: use netstack for peerapi server We're finding a bunch of host operating systems/firewalls interact poorly with peerapi. We either get ICMP errors from the host or users need to run commands to allow the peerapi port: https://github.com/tailscale/tailscale/issues/3842#issuecomment-1025133727 ... even though the peerapi should be an internal implementation detail. Rather than fight the host OS & firewalls, this change handles the server side of peerapi entirely in netstack (except on iOS), so it never makes its way to the host OS where it might be messed with. Two main downsides are: 1) netstack isn't as fast, but we don't really need speed for peerapi. And actually, with fewer trips to/from the kernel, we might actually make up for some of the netstack performance loss by staying in userspace. 2) tcpdump / Wireshark etc packet captures will no longer see the peerapi traffic. Oh well. Crawshaw's been wanting to add packet capture server support to tailscaled, so we'll probably do that sooner now. A future change might also then use peerapi for the client-side (except on iOS). Updates #3842 (probably fixes, as well as many exit node issues I bet) Change-Id: Ibc25edbb895dc083d1f07bd3cab614134705aa39 Signed-off-by: Brad Fitzpatrick --- ipn/ipnlocal/local.go | 27 +++++++++++++++++++-- ipn/ipnlocal/peerapi.go | 45 +++++++++++++++++++---------------- wgengine/netstack/netstack.go | 43 ++++++++++++++++++++++++++++++--- 3 files changed, 90 insertions(+), 25 deletions(-) diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 6ffc59891..b071120ce 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -215,7 +215,7 @@ func NewLocalBackend(logf logger.Logf, logid string, store ipn.StateStore, diale wiredPeerAPIPort := false if ig, ok := e.(wgengine.InternalsGetter); ok { if tunWrap, _, ok := ig.GetInternals(); ok { - tunWrap.PeerAPIPort = b.getPeerAPIPortForTSMPPing + tunWrap.PeerAPIPort = b.GetPeerAPIPort wiredPeerAPIPort = true } } @@ -1788,7 +1788,9 @@ func (b *LocalBackend) setPrefsLockedOnEntry(caller string, newp *ipn.Prefs) { b.send(ipn.Notify{Prefs: newp}) } -func (b *LocalBackend) getPeerAPIPortForTSMPPing(ip netaddr.IP) (port uint16, ok bool) { +// GetPeerAPIPort returns the port number for the peerapi server +// running on the provided IP. +func (b *LocalBackend) GetPeerAPIPort(ip netaddr.IP) (port uint16, ok bool) { b.mu.Lock() defer b.mu.Unlock() for _, pln := range b.peerAPIListeners { @@ -1799,6 +1801,27 @@ func (b *LocalBackend) getPeerAPIPortForTSMPPing(ip netaddr.IP) (port uint16, ok return 0, false } +// ServePeerAPIConnection serves an already-accepted connection c. +// +// The remote parameter is the remote address. +// The local paramater is the local address (either a Tailscale IPv4 +// or IPv6 IP and the peerapi port for that address). +// +// The connection will be closed by ServePeerAPIConnection. +func (b *LocalBackend) ServePeerAPIConnection(remote, local netaddr.IPPort, c net.Conn) { + b.mu.Lock() + defer b.mu.Unlock() + for _, pln := range b.peerAPIListeners { + if pln.ip == local.IP() { + go pln.ServeConn(remote, c) + return + } + } + b.logf("[unexpected] no peerAPI listener found for %v", local) + c.Close() + return +} + func (b *LocalBackend) peerAPIServicesLocked() (ret []tailcfg.Service) { for _, pln := range b.peerAPIListeners { proto := tailcfg.PeerAPI4 diff --git a/ipn/ipnlocal/peerapi.go b/ipn/ipnlocal/peerapi.go index 19aeb2240..8c2fe946b 100644 --- a/ipn/ipnlocal/peerapi.go +++ b/ipn/ipnlocal/peerapi.go @@ -481,27 +481,32 @@ func (pln *peerAPIListener) serve() { c.Close() continue } - peerNode, peerUser, ok := pln.lb.WhoIs(ipp) - if !ok { - logf("peerapi: unknown peer %v", ipp) - c.Close() - continue - } - h := &peerAPIHandler{ - ps: pln.ps, - isSelf: pln.ps.selfNode.User == peerNode.User, - remoteAddr: ipp, - peerNode: peerNode, - peerUser: peerUser, - } - httpServer := &http.Server{ - Handler: h, - } - if addH2C != nil { - addH2C(httpServer) - } - go httpServer.Serve(&oneConnListener{Listener: pln.ln, conn: c}) + pln.ServeConn(ipp, c) + } +} + +func (pln *peerAPIListener) ServeConn(src netaddr.IPPort, c net.Conn) { + logf := pln.lb.logf + peerNode, peerUser, ok := pln.lb.WhoIs(src) + if !ok { + logf("peerapi: unknown peer %v", src) + c.Close() + return + } + h := &peerAPIHandler{ + ps: pln.ps, + isSelf: pln.ps.selfNode.User == peerNode.User, + remoteAddr: src, + peerNode: peerNode, + peerUser: peerUser, + } + httpServer := &http.Server{ + Handler: h, + } + if addH2C != nil { + addH2C(httpServer) } + go httpServer.Serve(&oneConnListener{Listener: pln.ln, conn: c}) } type oneConnListener struct { diff --git a/wgengine/netstack/netstack.go b/wgengine/netstack/netstack.go index 72e1808ed..1501f4722 100644 --- a/wgengine/netstack/netstack.go +++ b/wgengine/netstack/netstack.go @@ -82,9 +82,12 @@ type Impl struct { mc *magicsock.Conn logf logger.Logf dialer *tsdial.Dialer - ctx context.Context // alive until Close - ctxCancel context.CancelFunc // called on Close - lb *ipnlocal.LocalBackend + ctx context.Context // alive until Close + ctxCancel context.CancelFunc // called on Close + lb *ipnlocal.LocalBackend // or nil + + peerapiPort4Atomic uint32 // uint16 port number for IPv4 peerapi + peerapiPort6Atomic uint32 // uint16 port number for IPv6 peerapi // atomicIsLocalIPFunc holds a func that reports whether an IP // is a local (non-subnet) Tailscale IP address of this @@ -407,9 +410,33 @@ func (ns *Impl) processSSH() bool { return ns.lb != nil && ns.lb.ShouldRunSSH() } +func (ns *Impl) peerAPIPortAtomic(ip netaddr.IP) *uint32 { + if ip.Is4() { + return &ns.peerapiPort4Atomic + } else { + return &ns.peerapiPort6Atomic + } +} + // shouldProcessInbound reports whether an inbound packet should be // handled by netstack. func (ns *Impl) shouldProcessInbound(p *packet.Parsed, t *tstun.Wrapper) bool { + // Handle incoming peerapi connections in netstack. + if ns.lb != nil && p.IPProto == ipproto.TCP { + var peerAPIPort uint16 + dstIP := p.Dst.IP() + if p.TCPFlags&packet.TCPSynAck == packet.TCPSyn && ns.isLocalIP(dstIP) { + if port, ok := ns.lb.GetPeerAPIPort(p.Dst.IP()); ok { + peerAPIPort = port + atomic.StoreUint32(ns.peerAPIPortAtomic(dstIP), uint32(port)) + } + } else { + peerAPIPort = uint16(atomic.LoadUint32(ns.peerAPIPortAtomic(dstIP))) + } + if p.IPProto == ipproto.TCP && p.Dst.Port() == peerAPIPort { + return true + } + } if ns.isInboundTSSH(p) && ns.processSSH() { return true } @@ -621,6 +648,16 @@ func (ns *Impl) acceptTCP(r *tcp.ForwarderRequest) { } return } + if ns.lb != nil { + if port, ok := ns.lb.GetPeerAPIPort(dialIP); ok { + if reqDetails.LocalPort == port && ns.isLocalIP(dialIP) { + src := netaddr.IPPortFrom(clientRemoteIP, reqDetails.RemotePort) + dst := netaddr.IPPortFrom(dialIP, port) + ns.lb.ServePeerAPIConnection(src, dst, c) + return + } + } + } if ns.ForwardTCPIn != nil { ns.ForwardTCPIn(c, reqDetails.LocalPort) return