From 71dcc0ae5b3d9e86178ac5180ff4299f8fd74dd4 Mon Sep 17 00:00:00 2001 From: eltonkl Date: Fri, 15 May 2026 16:51:56 +0000 Subject: [PATCH 01/15] lib: add /connect-ipip endpoint for IPIP tunnel peers Adds a companion to /connect that returns an inner IP from the same WgCidr pool but provisions a Linux IPIP tunnel keyed on the HTTPS source address instead of a WireGuard peer. Each request creates vp- with local=srv.BindAddr, remote=, and installs a /32 host route so decapsulated return traffic exits the right tunnel. Wildcard FORWARD/TCP-MSS rules cover every IPIP interface this server creates. Idle peers (no observed rx for PeerIdleTimeout) are pruned in a background loop and their IPs returned to the shared allocator. Co-authored-by: Codesmith --- lib/ipip.go | 362 ++++++++++++++++++++++++++++++++++++++++++ lib/server.go | 25 +++ lib/server_manager.go | 4 + 3 files changed, 391 insertions(+) create mode 100644 lib/ipip.go diff --git a/lib/ipip.go b/lib/ipip.go new file mode 100644 index 0000000..7e33b43 --- /dev/null +++ b/lib/ipip.go @@ -0,0 +1,362 @@ +package lib + +import ( + "encoding/json" + "fmt" + "log" + "net" + "net/http" + "net/netip" + "time" + + "github.com/vishvananda/netlink" +) + +// ipipPeer tracks server-side state for a single IPIP tunnel peer. +// +// One ipipPeer corresponds to exactly one Linux IPIP interface created on the +// server, with the peer's HTTPS source address as the remote tunnel endpoint +// and an inner IP allocated from srv.WgCidr. +type ipipPeer struct { + clientIP netip.Addr // outer (HTTPS) source address of the client + peerIP netip.Addr // inner address allocated from srv.WgCidr + ifname string // Linux interface name (e.g. vp0-1) + + lastSeen time.Time // last time we observed activity from this peer + rxBytes uint64 // rx bytes observed at lastSeen, for activity detection +} + +type connectIpipResponse struct { + AssignedAddr string +} + +// ipipIfname returns the Linux interface name used for the IPIP tunnel to the +// peer at peerIP. +// +// The name is "vp-" where host is the low 16 bits of peerIP. +// This stays within IFNAMSIZ (15 visible chars) for all valid server indices +// and host portions of the WgCidr, and makes it possible to install one +// iptables wildcard rule per server (vp-+) covering every IPIP +// peer attached to that server. +func (srv *Server) ipipIfname(peerIP netip.Addr) string { + b := peerIP.As4() + host := uint16(b[2])<<8 | uint16(b[3]) + return fmt.Sprintf("vp%d-%d", srv.Index, host) +} + +// ipipIfaceWildcard returns the iptables-style wildcard that matches every +// IPIP interface created for this server. +func (srv *Server) ipipIfaceWildcard() string { + return fmt.Sprintf("vp%d-+", srv.Index) +} + +// connectIpipHandler handles POST /connect-ipip. +// +// It authenticates the request with the shared Bearer password (matching the +// existing /connect handler), allocates an inner IP from srv.ipAllocator, +// creates a Linux IPIP tunnel whose remote is the HTTPS source address of the +// request, installs a host route so that return traffic destined for the +// inner IP exits via that tunnel, and returns the assigned inner address. +// +// Repeated calls from the same client IP are idempotent: the existing peer +// is reused and its lastSeen timestamp refreshed. +func (srv *Server) connectIpipHandler(w http.ResponseWriter, r *http.Request) { + if r.Method != "POST" { + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } + if r.Header.Get("Authorization") != "Bearer "+srv.Password { + http.Error(w, "unauthorized", http.StatusUnauthorized) + return + } + + host, _, err := net.SplitHostPort(r.RemoteAddr) + if err != nil { + http.Error(w, "invalid remote address", http.StatusBadRequest) + return + } + clientIP, err := netip.ParseAddr(host) + if err != nil { + http.Error(w, "invalid remote address", http.StatusBadRequest) + return + } + if !clientIP.Is4() { + http.Error(w, "ipv4 client address required", http.StatusBadRequest) + return + } + + // Idempotent path: if we already have an IPIP tunnel for this client IP, + // refresh its lastSeen and return the same assignment. + srv.ipipMu.Lock() + if existing, ok := srv.ipipPeers[clientIP]; ok { + existing.lastSeen = time.Now() + assigned := fmt.Sprintf("%v/%d", existing.peerIP, srv.WgCidr.Bits()) + srv.ipipMu.Unlock() + writeIpipResponse(w, assigned) + return + } + srv.ipipMu.Unlock() + + peerIP := srv.ipAllocator.Allocate() + if peerIP.IsUnspecified() { + log.Printf("no more ip addresses available in %v", srv.WgCidr) + http.Error(w, "no more IP addresses available", http.StatusServiceUnavailable) + return + } + + ifname := srv.ipipIfname(peerIP) + if err := srv.createIpipLink(ifname, clientIP, peerIP); err != nil { + srv.ipAllocator.Free(peerIP) + log.Printf("[%v] failed to create IPIP tunnel for %v: %v", + srv.BindAddr, clientIP, err) + http.Error(w, "failed to create IPIP tunnel", http.StatusInternalServerError) + return + } + + peer := &ipipPeer{ + clientIP: clientIP, + peerIP: peerIP, + ifname: ifname, + lastSeen: time.Now(), + } + + // Another request from the same client IP could have raced us. If so, + // drop the one we just built and reuse the winner so we don't leak an + // allocation or an interface. + srv.ipipMu.Lock() + if winner, ok := srv.ipipPeers[clientIP]; ok { + srv.ipipMu.Unlock() + srv.tearDownIpipLink(ifname) + srv.ipAllocator.Free(peerIP) + writeIpipResponse(w, fmt.Sprintf("%v/%d", winner.peerIP, srv.WgCidr.Bits())) + return + } + srv.ipipPeers[clientIP] = peer + srv.ipipMu.Unlock() + + log.Printf("[%v] new ipip peer %v at %v (iface %s)", + srv.BindAddr, clientIP, peerIP, ifname) + + writeIpipResponse(w, fmt.Sprintf("%v/%d", peerIP, srv.WgCidr.Bits())) +} + +func writeIpipResponse(w http.ResponseWriter, assigned string) { + resp := &connectIpipResponse{AssignedAddr: assigned} + respBuf, err := json.Marshal(resp) + if err != nil { + http.Error(w, "failed to serialize response", http.StatusInternalServerError) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write(respBuf) +} + +// createIpipLink creates the Linux IPIP interface and installs a host route +// pointing peerIP at it so return traffic finds the right tunnel. +// +// We intentionally do NOT add an IP address to the IPIP interface: the +// equivalent assignment in the WireGuard path puts srv.WgCidr.Addr() on the +// WireGuard interface, and adding the same local address to a second +// interface would either be rejected or break routing. A more-specific /32 +// route per peer is enough to deliver decapsulated return traffic to the +// correct tunnel, and the MASQUERADE rule already in place on srv.BindIface +// handles outbound NAT. +func (srv *Server) createIpipLink(ifname string, remote, peerIP netip.Addr) error { + link := &netlink.Iptun{ + LinkAttrs: netlink.LinkAttrs{Name: ifname}, + Local: addrToIp(srv.BindAddr), + Remote: addrToIp(remote), + } + // Best-effort cleanup of any stale interface with the same name. + _ = netlink.LinkDel(link) + + if err := netlink.LinkAdd(link); err != nil { + return fmt.Errorf("add ipip link: %v", err) + } + + if err := netlink.LinkSetUp(link); err != nil { + _ = netlink.LinkDel(link) + return fmt.Errorf("bring up ipip link: %v", err) + } + + // Pin the peer's inner IP to this tunnel so decapsulated return packets + // go back out the right interface (a /32 wins over the /16 connected + // route on the WireGuard interface). + dst := prefixToIPNet(netip.PrefixFrom(peerIP, 32)) + route := &netlink.Route{ + LinkIndex: link.Attrs().Index, + Dst: &dst, + Scope: netlink.SCOPE_LINK, + } + if err := netlink.RouteReplace(route); err != nil { + _ = netlink.LinkDel(link) + return fmt.Errorf("add host route for %v: %v", peerIP, err) + } + + return nil +} + +// tearDownIpipLink removes the IPIP interface and any associated host route. +// The interface deletion is what carries the kernel state; the explicit +// RouteDel is belt-and-braces in case the route somehow outlives the link. +func (srv *Server) tearDownIpipLink(ifname string) { + link, err := netlink.LinkByName(ifname) + if err != nil { + // Already gone; nothing to do. + return + } + if err := netlink.LinkDel(link); err != nil { + log.Printf("[%v] failed to delete ipip link %s: %v", + srv.BindAddr, ifname, err) + } +} + +func (srv *Server) removeIdleIpipPeersLoop() { + for { + select { + case <-srv.Ctx.Done(): + return + case <-time.After(5 * time.Second): + } + + if err := srv.removeIdleIpipPeers(); err != nil { + log.Printf("error removing idle ipip peers: %v", err) + } + } +} + +// removeIdleIpipPeers prunes IPIP peers whose tunnel has seen no inbound +// traffic for longer than PeerIdleTimeout. Activity is detected by polling +// the interface's rx_bytes counter via netlink. +func (srv *Server) removeIdleIpipPeers() error { + srv.ipipMu.Lock() + type snapshot struct { + clientIP netip.Addr + ifname string + peer *ipipPeer + } + snaps := make([]snapshot, 0, len(srv.ipipPeers)) + for clientIP, peer := range srv.ipipPeers { + snaps = append(snaps, snapshot{clientIP: clientIP, ifname: peer.ifname, peer: peer}) + } + srv.ipipMu.Unlock() + + now := time.Now() + var toRemove []snapshot + for _, s := range snaps { + link, err := netlink.LinkByName(s.ifname) + if err != nil { + // The interface vanished out from under us. Treat as removable. + toRemove = append(toRemove, s) + continue + } + stats := link.Attrs().Statistics + var rx uint64 + if stats != nil { + rx = stats.RxBytes + } + + srv.ipipMu.Lock() + if rx != s.peer.rxBytes { + s.peer.rxBytes = rx + s.peer.lastSeen = now + } + idle := now.Sub(s.peer.lastSeen) > PeerIdleTimeout + srv.ipipMu.Unlock() + + if idle { + toRemove = append(toRemove, s) + } + } + + for _, s := range toRemove { + srv.ipipMu.Lock() + // Re-check inside the lock in case a fresh /connect-ipip from the + // same client IP just bumped lastSeen. + current, ok := srv.ipipPeers[s.clientIP] + if !ok || current != s.peer { + srv.ipipMu.Unlock() + continue + } + if now.Sub(current.lastSeen) <= PeerIdleTimeout { + srv.ipipMu.Unlock() + continue + } + delete(srv.ipipPeers, s.clientIP) + srv.ipipMu.Unlock() + + log.Printf("[%v] removing idle ipip peer %v at %v", + srv.BindAddr, s.clientIP, s.peer.peerIP) + srv.tearDownIpipLink(s.peer.ifname) + srv.ipAllocator.Free(s.peer.peerIP) + } + return nil +} + +// CleanupIpip tears down every IPIP interface this server created. It is +// safe to call multiple times. +func (srv *Server) CleanupIpip() { + srv.ipipMu.Lock() + peers := make([]*ipipPeer, 0, len(srv.ipipPeers)) + for _, p := range srv.ipipPeers { + peers = append(peers, p) + } + srv.ipipPeers = make(map[netip.Addr]*ipipPeer) + srv.ipipMu.Unlock() + + for _, p := range peers { + srv.tearDownIpipLink(p.ifname) + srv.ipAllocator.Free(p.peerIP) + } +} + +// iptablesIpipForwardRule adds or removes the FORWARD ACCEPT rule that lets +// decapsulated traffic from any of this server's IPIP interfaces transit. +func (srv *Server) iptablesIpipForwardRule(enabled bool) error { + rule := []string{ + "-i", srv.ipipIfaceWildcard(), + "-j", "ACCEPT", + "-m", "comment", "--comment", + fmt.Sprintf("vprox ipip forward rule for %s", srv.Ifname()), + } + if enabled { + return srv.Ipt.AppendUnique("filter", "FORWARD", rule...) + } + return srv.Ipt.Delete("filter", "FORWARD", rule...) +} + +// iptablesIpipMssRules adds or removes TCP MSS clamping for the IPIP +// interfaces (inbound and outbound) so traffic fits the tunnel MTU. +func (srv *Server) iptablesIpipMssRules(enabled bool) error { + out := []string{ + "-o", srv.ipipIfaceWildcard(), + "-p", "tcp", + "--tcp-flags", "SYN,RST", "SYN", + "-j", "TCPMSS", + "--clamp-mss-to-pmtu", + "-m", "comment", "--comment", + fmt.Sprintf("vprox ipip TCP MSS outbound rule for %s", srv.Ifname()), + } + in := []string{ + "-i", srv.ipipIfaceWildcard(), + "-p", "tcp", + "--tcp-flags", "SYN,RST", "SYN", + "-j", "TCPMSS", + "--clamp-mss-to-pmtu", + "-m", "comment", "--comment", + fmt.Sprintf("vprox ipip TCP MSS inbound rule for %s", srv.Ifname()), + } + apply := srv.Ipt.AppendUnique + op := "append" + if !enabled { + apply = srv.Ipt.Delete + op = "delete" + } + if err := apply("mangle", "FORWARD", out...); err != nil { + return fmt.Errorf("%s ipip outbound MSS rule: %v", op, err) + } + if err := apply("mangle", "FORWARD", in...); err != nil { + return fmt.Errorf("%s ipip inbound MSS rule: %v", op, err) + } + return nil +} diff --git a/lib/server.go b/lib/server.go index e775fc3..0d07dd5 100644 --- a/lib/server.go +++ b/lib/server.go @@ -107,6 +107,11 @@ type Server struct { // kernel device: an entry is added when a peer is allocated an IP and // removed when the reaper deletes an idle peer. peerIPs map[wgtypes.Key]netip.Addr + + // ipipMu protects ipipPeers. It is separate from mu so that the IPIP + // peer bookkeeping does not contend with the WireGuard peer state. + ipipMu sync.Mutex + ipipPeers map[netip.Addr]*ipipPeer } // InitState initializes the private server state. @@ -134,6 +139,7 @@ func (srv *Server) InitState() error { } srv.newPeers = make(map[wgtypes.Key]time.Time) srv.peerIPs = make(map[wgtypes.Key]netip.Addr) + srv.ipipPeers = make(map[netip.Addr]*ipipPeer) return nil } @@ -620,6 +626,16 @@ func (srv *Server) StartIptables() error { return fmt.Errorf("failed to add inbound TCP MSS rule: %v", err) } + // Rules covering this server's IPIP peer interfaces. The interfaces are + // created lazily by /connect-ipip, but the FORWARD/MSS rules can be + // installed upfront via a wildcard so they are ready when peers attach. + if err := srv.iptablesIpipForwardRule(true); err != nil { + return fmt.Errorf("failed to add ipip forward rule: %v", err) + } + if err := srv.iptablesIpipMssRules(true); err != nil { + return fmt.Errorf("failed to add ipip MSS rules: %v", err) + } + // SNAT rule for internal network traffic. This is currently only applicable for boxes in // the US. if srv.Region == "us-west" { @@ -719,6 +735,13 @@ func (srv *Server) CleanupIptables() { log.Printf("failed to remove inbound TCP MSS rule: %v", err) } + if err := srv.iptablesIpipForwardRule(false); err != nil { + log.Printf("failed to remove ipip forward rule: %v", err) + } + if err := srv.iptablesIpipMssRules(false); err != nil { + log.Printf("failed to remove ipip MSS rules: %v", err) + } + if srv.Region == "us-west" { // Remove SNAT rule for internal traffic rule = []string{ @@ -872,6 +895,7 @@ func (srv *Server) ListenForHttps() error { } go srv.removeIdlePeersLoop() + go srv.removeIdleIpipPeersLoop() // Some bind addresses may not have been added to the network interface. If // that is the case, we need to add it (transiently). @@ -881,6 +905,7 @@ func (srv *Server) ListenForHttps() error { mux := http.NewServeMux() mux.HandleFunc("/", srv.indexHandler) mux.HandleFunc("/connect", srv.connectHandler) + mux.HandleFunc("/connect-ipip", srv.connectIpipHandler) cert, err := loadServerTls() if err != nil { diff --git a/lib/server_manager.go b/lib/server_manager.go index 9218518..2846d75 100644 --- a/lib/server_manager.go +++ b/lib/server_manager.go @@ -151,6 +151,10 @@ func (sm *ServerManager) Start(ip netip.Addr) error { log.Printf("[%v] failed to start iptables: %v", ip, err) return } + // IPIP tunnels have no kernel-adoption path yet (unlike WireGuard, + // which RestorePeersFromKernel re-adopts), so tear them down on + // shutdown to keep the allocator and kernel state consistent. + defer srv.CleanupIpip() if err := srv.ListenForHttps(); err != nil { log.Printf("[%v] https server failed: %v", ip, err) From de222083d8e2e6974319b7793462066cd2953ae6 Mon Sep 17 00:00:00 2001 From: eltonkl Date: Fri, 15 May 2026 16:58:02 +0000 Subject: [PATCH 02/15] lib: resolve ipip link by name before using its ifindex netlink.LinkAdd does not populate Index on the passed Iptun struct, so the host route was being installed with LinkIndex 0 and never bound to the tunnel. Look up the link by name after creation and use the resolved attrs for both LinkSetUp and RouteReplace. Co-authored-by: Codesmith --- lib/ipip.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/lib/ipip.go b/lib/ipip.go index 7e33b43..9d90e76 100644 --- a/lib/ipip.go +++ b/lib/ipip.go @@ -174,8 +174,17 @@ func (srv *Server) createIpipLink(ifname string, remote, peerIP netip.Addr) erro return fmt.Errorf("add ipip link: %v", err) } - if err := netlink.LinkSetUp(link); err != nil { + // netlink.LinkAdd does not populate the struct's Index field, so look up + // the link by name to get the kernel-assigned attributes (including the + // real ifindex) before we install routes that depend on it. + resolved, err := netlink.LinkByName(ifname) + if err != nil { _ = netlink.LinkDel(link) + return fmt.Errorf("resolve ipip link %s: %v", ifname, err) + } + + if err := netlink.LinkSetUp(resolved); err != nil { + _ = netlink.LinkDel(resolved) return fmt.Errorf("bring up ipip link: %v", err) } @@ -184,12 +193,12 @@ func (srv *Server) createIpipLink(ifname string, remote, peerIP netip.Addr) erro // route on the WireGuard interface). dst := prefixToIPNet(netip.PrefixFrom(peerIP, 32)) route := &netlink.Route{ - LinkIndex: link.Attrs().Index, + LinkIndex: resolved.Attrs().Index, Dst: &dst, Scope: netlink.SCOPE_LINK, } if err := netlink.RouteReplace(route); err != nil { - _ = netlink.LinkDel(link) + _ = netlink.LinkDel(resolved) return fmt.Errorf("add host route for %v: %v", peerIP, err) } From 85b7c21ade1171fa3ff8bddbdafd5a10db9194ae Mon Sep 17 00:00:00 2001 From: eltonkl Date: Fri, 15 May 2026 17:03:34 +0000 Subject: [PATCH 03/15] lib: tighten ipip footguns (rp_filter + per-peer FORWARD filter) Defense in depth against inner-source spoofing on the ipip data plane. Once a tunnel is registered, anyone who can deliver an ipip packet to us with the registered client's outer source IP can inject inner traffic. Two cheap server-side mitigations: - Set rp_filter=1 on each ipip iface as it's created. With the per-peer /32 route we install, the kernel drops decapsulated packets whose inner source IP wouldn't route back through the same iface they arrived on. - Replace the broad 'FORWARD ACCEPT for vp-+' wildcard with per-peer iptables rules: ACCEPT -i -s /32, then DROP -i . Each tunnel only forwards traffic with the inner source we assigned to it. Wildcard TCP MSS clamping stays as-is; it doesn't depend on the inner source. tearDownIpipLink now takes peerIP so it can clean up the per-peer rules before deleting the interface. Co-authored-by: Codesmith --- lib/ipip.go | 117 ++++++++++++++++++++++++++++++++++++++++---------- lib/server.go | 14 +++--- 2 files changed, 100 insertions(+), 31 deletions(-) diff --git a/lib/ipip.go b/lib/ipip.go index 9d90e76..49aeae9 100644 --- a/lib/ipip.go +++ b/lib/ipip.go @@ -7,6 +7,7 @@ import ( "net" "net/http" "net/netip" + "os" "time" "github.com/vishvananda/netlink" @@ -126,7 +127,7 @@ func (srv *Server) connectIpipHandler(w http.ResponseWriter, r *http.Request) { srv.ipipMu.Lock() if winner, ok := srv.ipipPeers[clientIP]; ok { srv.ipipMu.Unlock() - srv.tearDownIpipLink(ifname) + srv.tearDownIpipLink(ifname, peerIP) srv.ipAllocator.Free(peerIP) writeIpipResponse(w, fmt.Sprintf("%v/%d", winner.peerIP, srv.WgCidr.Bits())) return @@ -202,13 +203,41 @@ func (srv *Server) createIpipLink(ifname string, remote, peerIP netip.Addr) erro return fmt.Errorf("add host route for %v: %v", peerIP, err) } + // Strict reverse-path filtering: the kernel drops decapsulated packets + // whose inner source IP wouldn't route back through the same interface + // they arrived on. Combined with the per-peer /32 route installed + // above, this means an attacker who can send IPIP packets to us as + // some registered client's outer IP still can't inject inner traffic + // claiming to be from a different peer's inner IP. + // + // Linux uses max(conf.all.rp_filter, conf..rp_filter), so + // setting it to 1 here is enough regardless of the global default. + // Best-effort: log and continue if the sysctl is missing. + if err := setRpFilter(ifname, 1); err != nil { + log.Printf("[%v] failed to enable rp_filter on %s: %v", + srv.BindAddr, ifname, err) + } + + // Per-peer iptables filter: only accept forwarded traffic whose inner + // source IP matches the peer we assigned this tunnel to, and drop + // everything else arriving on this interface. This is defence in depth + // alongside rp_filter; it's enforced independently of the routing + // table and is more obvious at audit time. + if err := srv.addIpipPeerFilter(ifname, peerIP); err != nil { + _ = netlink.LinkDel(resolved) + return fmt.Errorf("install ipip peer filter: %v", err) + } + return nil } -// tearDownIpipLink removes the IPIP interface and any associated host route. -// The interface deletion is what carries the kernel state; the explicit -// RouteDel is belt-and-braces in case the route somehow outlives the link. -func (srv *Server) tearDownIpipLink(ifname string) { +// tearDownIpipLink removes the per-peer iptables filter and the IPIP +// interface. The interface deletion is what carries the kernel state; +// removing the iptables rules first keeps them from referencing a vanished +// interface for the brief window before they're cleaned up. +func (srv *Server) tearDownIpipLink(ifname string, peerIP netip.Addr) { + srv.removeIpipPeerFilter(ifname, peerIP) + link, err := netlink.LinkByName(ifname) if err != nil { // Already gone; nothing to do. @@ -220,6 +249,65 @@ func (srv *Server) tearDownIpipLink(ifname string) { } } +// setRpFilter writes the per-interface rp_filter sysctl. The /proc entry +// exists as soon as the interface is created. +func setRpFilter(ifname string, val int) error { + path := fmt.Sprintf("/proc/sys/net/ipv4/conf/%s/rp_filter", ifname) + return os.WriteFile(path, []byte(fmt.Sprintf("%d\n", val)), 0644) +} + +// ipipPeerAcceptRule is the iptables rule that permits forwarded traffic +// arriving on this peer's IPIP interface with the expected inner source IP. +func ipipPeerAcceptRule(ifname string, peerIP netip.Addr) []string { + return []string{ + "-i", ifname, + "-s", fmt.Sprintf("%v/32", peerIP), + "-j", "ACCEPT", + "-m", "comment", "--comment", + fmt.Sprintf("vprox ipip accept peer %v on %s", peerIP, ifname), + } +} + +// ipipPeerDropRule is the iptables rule that drops anything else arriving +// on this peer's IPIP interface (i.e. inner source spoofing). +func ipipPeerDropRule(ifname string) []string { + return []string{ + "-i", ifname, + "-j", "DROP", + "-m", "comment", "--comment", + fmt.Sprintf("vprox ipip drop spoofed on %s", ifname), + } +} + +func (srv *Server) addIpipPeerFilter(ifname string, peerIP netip.Addr) error { + accept := ipipPeerAcceptRule(ifname, peerIP) + if err := srv.Ipt.AppendUnique("filter", "FORWARD", accept...); err != nil { + return fmt.Errorf("add ipip accept rule: %v", err) + } + drop := ipipPeerDropRule(ifname) + if err := srv.Ipt.AppendUnique("filter", "FORWARD", drop...); err != nil { + _ = srv.Ipt.Delete("filter", "FORWARD", accept...) + return fmt.Errorf("add ipip drop rule: %v", err) + } + return nil +} + +func (srv *Server) removeIpipPeerFilter(ifname string, peerIP netip.Addr) { + // The interface is about to be deleted (or already is), so removal + // order doesn't matter for security; either rule alone matches + // nothing once the interface is gone. Use DeleteIfExists so a + // partially-installed filter (e.g. failed mid-add) cleans up + // without a noisy "rule does not exist" error. + if err := srv.Ipt.DeleteIfExists("filter", "FORWARD", ipipPeerDropRule(ifname)...); err != nil { + log.Printf("[%v] failed to remove ipip drop rule for %s: %v", + srv.BindAddr, ifname, err) + } + if err := srv.Ipt.DeleteIfExists("filter", "FORWARD", ipipPeerAcceptRule(ifname, peerIP)...); err != nil { + log.Printf("[%v] failed to remove ipip accept rule for %s: %v", + srv.BindAddr, ifname, err) + } +} + func (srv *Server) removeIdleIpipPeersLoop() { for { select { @@ -296,7 +384,7 @@ func (srv *Server) removeIdleIpipPeers() error { log.Printf("[%v] removing idle ipip peer %v at %v", srv.BindAddr, s.clientIP, s.peer.peerIP) - srv.tearDownIpipLink(s.peer.ifname) + srv.tearDownIpipLink(s.peer.ifname, s.peer.peerIP) srv.ipAllocator.Free(s.peer.peerIP) } return nil @@ -314,26 +402,11 @@ func (srv *Server) CleanupIpip() { srv.ipipMu.Unlock() for _, p := range peers { - srv.tearDownIpipLink(p.ifname) + srv.tearDownIpipLink(p.ifname, p.peerIP) srv.ipAllocator.Free(p.peerIP) } } -// iptablesIpipForwardRule adds or removes the FORWARD ACCEPT rule that lets -// decapsulated traffic from any of this server's IPIP interfaces transit. -func (srv *Server) iptablesIpipForwardRule(enabled bool) error { - rule := []string{ - "-i", srv.ipipIfaceWildcard(), - "-j", "ACCEPT", - "-m", "comment", "--comment", - fmt.Sprintf("vprox ipip forward rule for %s", srv.Ifname()), - } - if enabled { - return srv.Ipt.AppendUnique("filter", "FORWARD", rule...) - } - return srv.Ipt.Delete("filter", "FORWARD", rule...) -} - // iptablesIpipMssRules adds or removes TCP MSS clamping for the IPIP // interfaces (inbound and outbound) so traffic fits the tunnel MTU. func (srv *Server) iptablesIpipMssRules(enabled bool) error { diff --git a/lib/server.go b/lib/server.go index 0d07dd5..330d26f 100644 --- a/lib/server.go +++ b/lib/server.go @@ -626,12 +626,11 @@ func (srv *Server) StartIptables() error { return fmt.Errorf("failed to add inbound TCP MSS rule: %v", err) } - // Rules covering this server's IPIP peer interfaces. The interfaces are - // created lazily by /connect-ipip, but the FORWARD/MSS rules can be - // installed upfront via a wildcard so they are ready when peers attach. - if err := srv.iptablesIpipForwardRule(true); err != nil { - return fmt.Errorf("failed to add ipip forward rule: %v", err) - } + // Wildcard TCP MSS clamping for this server's IPIP interfaces. The + // interfaces themselves are created lazily by /connect-ipip; the + // FORWARD ACCEPT/DROP filter is installed per peer at that point so + // each tunnel only accepts traffic with the inner source IP we + // assigned to it (see addIpipPeerFilter). if err := srv.iptablesIpipMssRules(true); err != nil { return fmt.Errorf("failed to add ipip MSS rules: %v", err) } @@ -735,9 +734,6 @@ func (srv *Server) CleanupIptables() { log.Printf("failed to remove inbound TCP MSS rule: %v", err) } - if err := srv.iptablesIpipForwardRule(false); err != nil { - log.Printf("failed to remove ipip forward rule: %v", err) - } if err := srv.iptablesIpipMssRules(false); err != nil { log.Printf("failed to remove ipip MSS rules: %v", err) } From 1e5cebd3e3fd3ac291df3cf48a10fac9c09e9d38 Mon Sep 17 00:00:00 2001 From: eltonkl Date: Fri, 15 May 2026 17:10:58 +0000 Subject: [PATCH 04/15] lib: don't short-circuit ipip MSS cleanup on first delete failure When iptablesIpipMssRules ran with enabled=false, a failure to delete the outbound rule returned immediately and skipped the inbound delete, leaking that rule. Split the add and cleanup paths: add still fails fast, cleanup attempts both deletes independently and logs errors, matching the WG MSS cleanup pattern in CleanupIptables. Co-authored-by: Codesmith --- lib/ipip.go | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/lib/ipip.go b/lib/ipip.go index 49aeae9..cf8e4df 100644 --- a/lib/ipip.go +++ b/lib/ipip.go @@ -428,17 +428,24 @@ func (srv *Server) iptablesIpipMssRules(enabled bool) error { "-m", "comment", "--comment", fmt.Sprintf("vprox ipip TCP MSS inbound rule for %s", srv.Ifname()), } - apply := srv.Ipt.AppendUnique - op := "append" - if !enabled { - apply = srv.Ipt.Delete - op = "delete" + if enabled { + if err := srv.Ipt.AppendUnique("mangle", "FORWARD", out...); err != nil { + return fmt.Errorf("append ipip outbound MSS rule: %v", err) + } + if err := srv.Ipt.AppendUnique("mangle", "FORWARD", in...); err != nil { + return fmt.Errorf("append ipip inbound MSS rule: %v", err) + } + return nil } - if err := apply("mangle", "FORWARD", out...); err != nil { - return fmt.Errorf("%s ipip outbound MSS rule: %v", op, err) + + // Cleanup path: attempt both deletions independently so a failure on + // the first doesn't leak the second (matches the WG MSS cleanup + // pattern in CleanupIptables). + if err := srv.Ipt.Delete("mangle", "FORWARD", out...); err != nil { + log.Printf("failed to remove ipip outbound MSS rule: %v", err) } - if err := apply("mangle", "FORWARD", in...); err != nil { - return fmt.Errorf("%s ipip inbound MSS rule: %v", op, err) + if err := srv.Ipt.Delete("mangle", "FORWARD", in...); err != nil { + log.Printf("failed to remove ipip inbound MSS rule: %v", err) } return nil } From 1b568870450468203c392c42a45b5ce599c4364a Mon Sep 17 00:00:00 2001 From: Elton Leong Date: Fri, 15 May 2026 14:41:36 -0700 Subject: [PATCH 05/15] lib: address ipip review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - connectIpipHandler: refresh lastSeen on the winner in the race-loser path, matching the idempotent path - createIpipLink: correct the rp_filter comment — max(all, iface) means setting the per-iface value to 1 degrades to loose mode when conf.all.rp_filter is 2; the per-peer iptables filter is authoritative - removeIdleIpipPeers: count tx_bytes as well as rx_bytes so a peer in a one-way transfer isn't pruned mid-stream - connectIpipHandler: note that a stale idempotent assignment is healed by removeIdleIpipPeers within one poll interval - add ipip_test.go covering ifname/wildcard/iptables-rule helpers, including the IFNAMSIZ bound Co-Authored-By: Claude Opus 4.7 (1M context) --- lib/ipip.go | 42 ++++++++++++++++++++++++------------- lib/ipip_test.go | 54 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 14 deletions(-) create mode 100644 lib/ipip_test.go diff --git a/lib/ipip.go b/lib/ipip.go index cf8e4df..63ab217 100644 --- a/lib/ipip.go +++ b/lib/ipip.go @@ -25,6 +25,7 @@ type ipipPeer struct { lastSeen time.Time // last time we observed activity from this peer rxBytes uint64 // rx bytes observed at lastSeen, for activity detection + txBytes uint64 // tx bytes observed at lastSeen, for activity detection } type connectIpipResponse struct { @@ -88,6 +89,10 @@ func (srv *Server) connectIpipHandler(w http.ResponseWriter, r *http.Request) { // Idempotent path: if we already have an IPIP tunnel for this client IP, // refresh its lastSeen and return the same assignment. + // + // If the interface was destroyed out-of-band the stored assignment is + // briefly stale, but removeIdleIpipPeers notices the missing interface + // within one poll interval and drops the peer, so a retry rebuilds it. srv.ipipMu.Lock() if existing, ok := srv.ipipPeers[clientIP]; ok { existing.lastSeen = time.Now() @@ -126,6 +131,7 @@ func (srv *Server) connectIpipHandler(w http.ResponseWriter, r *http.Request) { // allocation or an interface. srv.ipipMu.Lock() if winner, ok := srv.ipipPeers[clientIP]; ok { + winner.lastSeen = time.Now() srv.ipipMu.Unlock() srv.tearDownIpipLink(ifname, peerIP) srv.ipAllocator.Free(peerIP) @@ -203,16 +209,20 @@ func (srv *Server) createIpipLink(ifname string, remote, peerIP netip.Addr) erro return fmt.Errorf("add host route for %v: %v", peerIP, err) } - // Strict reverse-path filtering: the kernel drops decapsulated packets - // whose inner source IP wouldn't route back through the same interface - // they arrived on. Combined with the per-peer /32 route installed - // above, this means an attacker who can send IPIP packets to us as - // some registered client's outer IP still can't inject inner traffic - // claiming to be from a different peer's inner IP. + // Reverse-path filtering: the kernel drops decapsulated packets whose + // inner source IP wouldn't route back through the same interface they + // arrived on. Combined with the per-peer /32 route installed above, + // this means an attacker who can send IPIP packets to us as some + // registered client's outer IP can't inject inner traffic claiming to + // be from a different peer's inner IP. // - // Linux uses max(conf.all.rp_filter, conf..rp_filter), so - // setting it to 1 here is enough regardless of the global default. - // Best-effort: log and continue if the sysctl is missing. + // Linux uses max(conf.all.rp_filter, conf..rp_filter): setting + // the per-interface value to 1 forces strict filtering unless + // conf.all.rp_filter is 2 (loose), in which case this layer degrades + // to loose mode and no longer enforces the same-interface check. The + // per-peer iptables filter installed below is the authoritative + // anti-spoof control regardless; rp_filter is a best-effort backstop. + // Log and continue if the sysctl is missing. if err := setRpFilter(ifname, 1); err != nil { log.Printf("[%v] failed to enable rp_filter on %s: %v", srv.BindAddr, ifname, err) @@ -322,9 +332,11 @@ func (srv *Server) removeIdleIpipPeersLoop() { } } -// removeIdleIpipPeers prunes IPIP peers whose tunnel has seen no inbound -// traffic for longer than PeerIdleTimeout. Activity is detected by polling -// the interface's rx_bytes counter via netlink. +// removeIdleIpipPeers prunes IPIP peers whose tunnel has seen no traffic for +// longer than PeerIdleTimeout. Activity is detected by polling the +// interface's rx_bytes and tx_bytes counters via netlink; counting both +// directions means a peer in the middle of a one-way transfer (e.g. a +// download with little return traffic) is not pruned mid-stream. func (srv *Server) removeIdleIpipPeers() error { srv.ipipMu.Lock() type snapshot struct { @@ -348,14 +360,16 @@ func (srv *Server) removeIdleIpipPeers() error { continue } stats := link.Attrs().Statistics - var rx uint64 + var rx, tx uint64 if stats != nil { rx = stats.RxBytes + tx = stats.TxBytes } srv.ipipMu.Lock() - if rx != s.peer.rxBytes { + if rx != s.peer.rxBytes || tx != s.peer.txBytes { s.peer.rxBytes = rx + s.peer.txBytes = tx s.peer.lastSeen = now } idle := now.Sub(s.peer.lastSeen) > PeerIdleTimeout diff --git a/lib/ipip_test.go b/lib/ipip_test.go new file mode 100644 index 0000000..ccdee2a --- /dev/null +++ b/lib/ipip_test.go @@ -0,0 +1,54 @@ +package lib + +import ( + "net/netip" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestIpipIfname(t *testing.T) { + srv := &Server{Index: 0} + assert.Equal(t, "vp0-1", srv.ipipIfname(netip.AddrFrom4([4]byte{10, 100, 0, 1}))) + assert.Equal(t, "vp0-258", srv.ipipIfname(netip.AddrFrom4([4]byte{10, 100, 1, 2}))) + + srv = &Server{Index: 7} + assert.Equal(t, "vp7-65535", srv.ipipIfname(netip.AddrFrom4([4]byte{10, 100, 255, 255}))) +} + +// TestIpipIfnameWithinIfnamsiz locks in the reasoning from ipipIfname's doc +// comment: the "vp-" name must stay within IFNAMSIZ (15 visible +// chars) even at the largest possible index and host values. +func TestIpipIfnameWithinIfnamsiz(t *testing.T) { + srv := &Server{Index: 65535} // max uint16 + name := srv.ipipIfname(netip.AddrFrom4([4]byte{10, 100, 255, 255})) + assert.Equal(t, "vp65535-65535", name) + assert.LessOrEqual(t, len(name), 15, "interface name exceeds IFNAMSIZ") +} + +func TestIpipIfaceWildcard(t *testing.T) { + assert.Equal(t, "vp0-+", (&Server{Index: 0}).ipipIfaceWildcard()) + assert.Equal(t, "vp42-+", (&Server{Index: 42}).ipipIfaceWildcard()) +} + +func TestIpipPeerAcceptRule(t *testing.T) { + peerIP := netip.AddrFrom4([4]byte{10, 100, 0, 5}) + rule := ipipPeerAcceptRule("vp0-5", peerIP) + assert.Equal(t, []string{ + "-i", "vp0-5", + "-s", "10.100.0.5/32", + "-j", "ACCEPT", + "-m", "comment", "--comment", + "vprox ipip accept peer 10.100.0.5 on vp0-5", + }, rule) +} + +func TestIpipPeerDropRule(t *testing.T) { + rule := ipipPeerDropRule("vp0-5") + assert.Equal(t, []string{ + "-i", "vp0-5", + "-j", "DROP", + "-m", "comment", "--comment", + "vprox ipip drop spoofed on vp0-5", + }, rule) +} From d2e89b73cc33dbee88df3cd35d766be93ee494b8 Mon Sep 17 00:00:00 2001 From: eltonkl Date: Fri, 15 May 2026 21:54:36 +0000 Subject: [PATCH 06/15] lib: reap ipip peers whose kernel iface has vanished If an ipip interface is destroyed out-of-band, the peer entry becomes unusable but two paths kept it alive forever: - The idempotent path in connectIpipHandler refreshed lastSeen on every retry without checking whether the interface still existed, handing the client back a broken assignment indefinitely. - removeIdleIpipPeers added vanished entries to toRemove but still applied the lastSeen <= PeerIdleTimeout guard in the final loop, so a freshly-refreshed (broken) peer never aged out. connectIpipHandler now verifies the interface with LinkByName before reusing an existing entry; if it's gone, the stale entry is dropped (iptables filter removed, IP returned to the allocator) and the request falls through to fresh allocation. removeIdleIpipPeers tracks a 'vanished' flag per snapshot and skips the freshness guard for vanished entries so they reap unconditionally. Co-authored-by: Codesmith --- lib/ipip.go | 77 ++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 52 insertions(+), 25 deletions(-) diff --git a/lib/ipip.go b/lib/ipip.go index 63ab217..955f298 100644 --- a/lib/ipip.go +++ b/lib/ipip.go @@ -87,21 +87,33 @@ func (srv *Server) connectIpipHandler(w http.ResponseWriter, r *http.Request) { return } - // Idempotent path: if we already have an IPIP tunnel for this client IP, - // refresh its lastSeen and return the same assignment. - // - // If the interface was destroyed out-of-band the stored assignment is - // briefly stale, but removeIdleIpipPeers notices the missing interface - // within one poll interval and drops the peer, so a retry rebuilds it. + // Idempotent path: if we already have an IPIP tunnel for this client + // IP AND its kernel interface still exists, refresh its lastSeen and + // return the same assignment. If the interface vanished out-of-band + // (manual `ip link del`, kernel reload, etc.) we cannot just refresh + // lastSeen: a retrying client would keep the stale entry alive + // forever, since removeIdleIpipPeers's freshness guard skips a peer + // whose lastSeen was just bumped. Drop the stale entry here and fall + // through to fresh allocation. srv.ipipMu.Lock() if existing, ok := srv.ipipPeers[clientIP]; ok { - existing.lastSeen = time.Now() - assigned := fmt.Sprintf("%v/%d", existing.peerIP, srv.WgCidr.Bits()) + if _, err := netlink.LinkByName(existing.ifname); err == nil { + existing.lastSeen = time.Now() + assigned := fmt.Sprintf("%v/%d", existing.peerIP, srv.WgCidr.Bits()) + srv.ipipMu.Unlock() + writeIpipResponse(w, assigned) + return + } + // Interface is gone; reclaim and rebuild. + log.Printf("[%v] ipip iface %s for %v vanished; rebuilding", + srv.BindAddr, existing.ifname, clientIP) + delete(srv.ipipPeers, clientIP) + srv.ipipMu.Unlock() + srv.removeIpipPeerFilter(existing.ifname, existing.peerIP) + srv.ipAllocator.Free(existing.peerIP) + } else { srv.ipipMu.Unlock() - writeIpipResponse(w, assigned) - return } - srv.ipipMu.Unlock() peerIP := srv.ipAllocator.Allocate() if peerIP.IsUnspecified() { @@ -351,12 +363,19 @@ func (srv *Server) removeIdleIpipPeers() error { srv.ipipMu.Unlock() now := time.Now() - var toRemove []snapshot + type removal struct { + snapshot + vanished bool + } + var toRemove []removal for _, s := range snaps { link, err := netlink.LinkByName(s.ifname) if err != nil { - // The interface vanished out from under us. Treat as removable. - toRemove = append(toRemove, s) + // The interface vanished out from under us (manual `ip link + // del`, kernel reload, etc.). Reap unconditionally; the + // freshness guard in the removal loop does not apply because + // the entry is unusable regardless of lastSeen. + toRemove = append(toRemove, removal{snapshot: s, vanished: true}) continue } stats := link.Attrs().Statistics @@ -376,30 +395,38 @@ func (srv *Server) removeIdleIpipPeers() error { srv.ipipMu.Unlock() if idle { - toRemove = append(toRemove, s) + toRemove = append(toRemove, removal{snapshot: s}) } } - for _, s := range toRemove { + for _, r := range toRemove { srv.ipipMu.Lock() // Re-check inside the lock in case a fresh /connect-ipip from the - // same client IP just bumped lastSeen. - current, ok := srv.ipipPeers[s.clientIP] - if !ok || current != s.peer { + // same client IP just replaced this entry. + current, ok := srv.ipipPeers[r.clientIP] + if !ok || current != r.peer { srv.ipipMu.Unlock() continue } - if now.Sub(current.lastSeen) <= PeerIdleTimeout { + // For idle-timeout removals, give a racing /connect-ipip that + // bumped lastSeen the benefit of the doubt. For vanished + // interfaces there's nothing to keep alive: the kernel state + // is gone, so reap regardless of lastSeen. + if !r.vanished && now.Sub(current.lastSeen) <= PeerIdleTimeout { srv.ipipMu.Unlock() continue } - delete(srv.ipipPeers, s.clientIP) + delete(srv.ipipPeers, r.clientIP) srv.ipipMu.Unlock() - log.Printf("[%v] removing idle ipip peer %v at %v", - srv.BindAddr, s.clientIP, s.peer.peerIP) - srv.tearDownIpipLink(s.peer.ifname, s.peer.peerIP) - srv.ipAllocator.Free(s.peer.peerIP) + reason := "idle" + if r.vanished { + reason = "vanished" + } + log.Printf("[%v] removing %s ipip peer %v at %v", + srv.BindAddr, reason, r.clientIP, r.peer.peerIP) + srv.tearDownIpipLink(r.peer.ifname, r.peer.peerIP) + srv.ipAllocator.Free(r.peer.peerIP) } return nil } From 0aca30d53c6b0aa0f596e5d5ba10ff2aff092c2d Mon Sep 17 00:00:00 2001 From: Elton Leong Date: Mon, 18 May 2026 15:59:11 -0400 Subject: [PATCH 07/15] lib: replace ipip rp_filter layer with a raw/PREROUTING host guard - drop the per-interface rp_filter layer. On these hosts conf.all.rp_filter is forced to 2 (loose) fleet-wide for the WireGuard path, so max(all, iface) can never reach strict mode and the per-interface setting was a no-op masquerading as a control. - add iptablesIpipHostGuardRule: a wildcard raw/PREROUTING DROP for IPIP traffic destined to a host-local address. The per-peer FORWARD filter only covers transit traffic, leaving packets that terminate on the host (notably the vprox HTTPS control plane) reachable with a forged inner source. raw/PREROUTING runs before conntrack and ufw's filter chains, so ufw's port accepts can't shadow it. - drop the always-nil error return from removeIdleIpipPeers. Co-Authored-By: Claude Opus 4.7 (1M context) --- lib/ipip.go | 82 +++++++++++++++++++++++++++++---------------------- lib/server.go | 11 +++++++ 2 files changed, 58 insertions(+), 35 deletions(-) diff --git a/lib/ipip.go b/lib/ipip.go index 955f298..88e8ba7 100644 --- a/lib/ipip.go +++ b/lib/ipip.go @@ -7,7 +7,6 @@ import ( "net" "net/http" "net/netip" - "os" "time" "github.com/vishvananda/netlink" @@ -221,30 +220,15 @@ func (srv *Server) createIpipLink(ifname string, remote, peerIP netip.Addr) erro return fmt.Errorf("add host route for %v: %v", peerIP, err) } - // Reverse-path filtering: the kernel drops decapsulated packets whose - // inner source IP wouldn't route back through the same interface they - // arrived on. Combined with the per-peer /32 route installed above, - // this means an attacker who can send IPIP packets to us as some - // registered client's outer IP can't inject inner traffic claiming to - // be from a different peer's inner IP. - // - // Linux uses max(conf.all.rp_filter, conf..rp_filter): setting - // the per-interface value to 1 forces strict filtering unless - // conf.all.rp_filter is 2 (loose), in which case this layer degrades - // to loose mode and no longer enforces the same-interface check. The - // per-peer iptables filter installed below is the authoritative - // anti-spoof control regardless; rp_filter is a best-effort backstop. - // Log and continue if the sysctl is missing. - if err := setRpFilter(ifname, 1); err != nil { - log.Printf("[%v] failed to enable rp_filter on %s: %v", - srv.BindAddr, ifname, err) - } - // Per-peer iptables filter: only accept forwarded traffic whose inner // source IP matches the peer we assigned this tunnel to, and drop - // everything else arriving on this interface. This is defence in depth - // alongside rp_filter; it's enforced independently of the routing - // table and is more obvious at audit time. + // everything else arriving on this interface. A decapsulated IPIP + // packet carries an attacker-controlled inner source, so this is what + // stops a peer from injecting traffic claiming to be from a different + // peer's inner IP. It covers transit (FORWARD) traffic only; packets + // terminating on the host itself are handled separately by the + // raw/PREROUTING guard installed in StartIptables (see + // iptablesIpipHostGuardRule). if err := srv.addIpipPeerFilter(ifname, peerIP); err != nil { _ = netlink.LinkDel(resolved) return fmt.Errorf("install ipip peer filter: %v", err) @@ -271,13 +255,6 @@ func (srv *Server) tearDownIpipLink(ifname string, peerIP netip.Addr) { } } -// setRpFilter writes the per-interface rp_filter sysctl. The /proc entry -// exists as soon as the interface is created. -func setRpFilter(ifname string, val int) error { - path := fmt.Sprintf("/proc/sys/net/ipv4/conf/%s/rp_filter", ifname) - return os.WriteFile(path, []byte(fmt.Sprintf("%d\n", val)), 0644) -} - // ipipPeerAcceptRule is the iptables rule that permits forwarded traffic // arriving on this peer's IPIP interface with the expected inner source IP. func ipipPeerAcceptRule(ifname string, peerIP netip.Addr) []string { @@ -338,9 +315,7 @@ func (srv *Server) removeIdleIpipPeersLoop() { case <-time.After(5 * time.Second): } - if err := srv.removeIdleIpipPeers(); err != nil { - log.Printf("error removing idle ipip peers: %v", err) - } + srv.removeIdleIpipPeers() } } @@ -349,7 +324,7 @@ func (srv *Server) removeIdleIpipPeersLoop() { // interface's rx_bytes and tx_bytes counters via netlink; counting both // directions means a peer in the middle of a one-way transfer (e.g. a // download with little return traffic) is not pruned mid-stream. -func (srv *Server) removeIdleIpipPeers() error { +func (srv *Server) removeIdleIpipPeers() { srv.ipipMu.Lock() type snapshot struct { clientIP netip.Addr @@ -428,7 +403,6 @@ func (srv *Server) removeIdleIpipPeers() error { srv.tearDownIpipLink(r.peer.ifname, r.peer.peerIP) srv.ipAllocator.Free(r.peer.peerIP) } - return nil } // CleanupIpip tears down every IPIP interface this server created. It is @@ -490,3 +464,41 @@ func (srv *Server) iptablesIpipMssRules(enabled bool) error { } return nil } + +// iptablesIpipHostGuardRule adds or removes the wildcard rule that drops +// traffic arriving on this server's IPIP interfaces and destined to a +// host-local address. +// +// IPIP peers route *through* the box and have no legitimate reason to +// reach the host itself. A decapsulated inner packet carries an +// attacker-controlled inner source, and the per-peer FORWARD filter +// (addIpipPeerFilter) only inspects transit traffic, not packets +// terminating on the host. Without this rule an authenticated peer could +// reach host-local services -- notably the vprox HTTPS control plane, +// whose /connect-ipip handler keys peer identity on the request source +// address -- with a forged source IP. +// +// It lives in raw/PREROUTING so it is evaluated before conntrack and +// before ufw's filter chains; ufw's accept rules for ports 22/443/etc. +// therefore cannot shadow it. The legitimate control-plane handshake is +// unaffected: it arrives on the physical bind interface, not vp-+. +func (srv *Server) iptablesIpipHostGuardRule(enabled bool) error { + rule := []string{ + "-i", srv.ipipIfaceWildcard(), + "-m", "addrtype", "--dst-type", "LOCAL", + "-j", "DROP", + "-m", "comment", "--comment", + fmt.Sprintf("vprox ipip host guard rule for %s", srv.Ifname()), + } + if enabled { + if err := srv.Ipt.AppendUnique("raw", "PREROUTING", rule...); err != nil { + return fmt.Errorf("append ipip host guard rule: %v", err) + } + return nil + } + + if err := srv.Ipt.Delete("raw", "PREROUTING", rule...); err != nil { + log.Printf("failed to remove ipip host guard rule: %v", err) + } + return nil +} diff --git a/lib/server.go b/lib/server.go index 330d26f..7e3c2cf 100644 --- a/lib/server.go +++ b/lib/server.go @@ -635,6 +635,13 @@ func (srv *Server) StartIptables() error { return fmt.Errorf("failed to add ipip MSS rules: %v", err) } + // Wildcard guard that drops IPIP traffic destined to the host itself, + // so peers can only transit the box and can't reach host-local + // services with a forged inner source (see iptablesIpipHostGuardRule). + if err := srv.iptablesIpipHostGuardRule(true); err != nil { + return fmt.Errorf("failed to add ipip host guard rule: %v", err) + } + // SNAT rule for internal network traffic. This is currently only applicable for boxes in // the US. if srv.Region == "us-west" { @@ -738,6 +745,10 @@ func (srv *Server) CleanupIptables() { log.Printf("failed to remove ipip MSS rules: %v", err) } + if err := srv.iptablesIpipHostGuardRule(false); err != nil { + log.Printf("failed to remove ipip host guard rule: %v", err) + } + if srv.Region == "us-west" { // Remove SNAT rule for internal traffic rule = []string{ From f0771afba99d199517af6137b575d674e4bfc5df Mon Sep 17 00:00:00 2001 From: eltonkl Date: Mon, 18 May 2026 21:05:44 +0000 Subject: [PATCH 08/15] lib: derive ipip ifname from CIDR offset to prevent collisions The old encoding used only the low 16 bits of peerIP for the suffix. For WgCidr widths wider than /16 (allowed: wgBlockPerIp can equal wgBlock.Bits(), which has no explicit lower bound), two distinct peers could produce the same ifname (e.g. 10.0.0.1 and 10.1.0.1 in a /8 both become vp0-1). createIpipLink does a best-effort LinkDel of any existing same-named interface before LinkAdd, so a collision would silently destroy a live peer's tunnel. Use the offset from srv.WgCidr.Addr() as the suffix instead, which is unique within any CIDR by construction, and length-check the result against IFNAMSIZ so a wildly oversized CIDR fails the /connect-ipip request loudly rather than producing a colliding or truncated name. Tests updated to set WgCidr on the test Server and cover both the wide-CIDR regression and the IFNAMSIZ rejection. Co-authored-by: Codesmith --- lib/ipip.go | 53 +++++++++++++++++++++++++++++++++++++----------- lib/ipip_test.go | 52 +++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 84 insertions(+), 21 deletions(-) diff --git a/lib/ipip.go b/lib/ipip.go index 88e8ba7..c3c8700 100644 --- a/lib/ipip.go +++ b/lib/ipip.go @@ -31,18 +31,41 @@ type connectIpipResponse struct { AssignedAddr string } -// ipipIfname returns the Linux interface name used for the IPIP tunnel to the -// peer at peerIP. +// ipipIfnameMaxLen is the maximum visible length of a Linux interface name +// (IFNAMSIZ is 16 including the null terminator). +const ipipIfnameMaxLen = 15 + +// ipipIfname returns the Linux interface name used for the IPIP tunnel to +// the peer at peerIP. +// +// The name is "vp-" where offset is the peer's distance +// from srv.WgCidr.Addr(). Using the full offset from the CIDR base (rather +// than e.g. the low 16 bits of peerIP) keeps the suffix globally unique +// across any allowed WgCidr width: for two distinct peers in the same +// server's CIDR, their offsets necessarily differ. The "vp-" prefix +// also gives us a stable wildcard (vp-+) for iptables. // -// The name is "vp-" where host is the low 16 bits of peerIP. -// This stays within IFNAMSIZ (15 visible chars) for all valid server indices -// and host portions of the WgCidr, and makes it possible to install one -// iptables wildcard rule per server (vp-+) covering every IPIP -// peer attached to that server. -func (srv *Server) ipipIfname(peerIP netip.Addr) string { - b := peerIP.As4() - host := uint16(b[2])<<8 | uint16(b[3]) - return fmt.Sprintf("vp%d-%d", srv.Index, host) +// The function returns an error if the resulting name would exceed +// IFNAMSIZ. In practice this never fires for sensibly-sized CIDRs (a /6 +// produces at most an 8-char decimal offset, which still fits alongside a +// 5-char server index), but it's a hard runtime guard against accidentally +// configuring a CIDR so wide that two peers would silently collide on the +// same interface name. +func (srv *Server) ipipIfname(peerIP netip.Addr) (string, error) { + base := srv.WgCidr.Addr().As4() + p := peerIP.As4() + baseInt := uint32(base[0])<<24 | uint32(base[1])<<16 | + uint32(base[2])<<8 | uint32(base[3]) + peerInt := uint32(p[0])<<24 | uint32(p[1])<<16 | + uint32(p[2])<<8 | uint32(p[3]) + offset := peerInt - baseInt + name := fmt.Sprintf("vp%d-%d", srv.Index, offset) + if len(name) > ipipIfnameMaxLen { + return "", fmt.Errorf( + "ipip ifname %q exceeds IFNAMSIZ (%d > %d); WgCidr is too wide for IPIP", + name, len(name), ipipIfnameMaxLen) + } + return name, nil } // ipipIfaceWildcard returns the iptables-style wildcard that matches every @@ -121,7 +144,13 @@ func (srv *Server) connectIpipHandler(w http.ResponseWriter, r *http.Request) { return } - ifname := srv.ipipIfname(peerIP) + ifname, err := srv.ipipIfname(peerIP) + if err != nil { + srv.ipAllocator.Free(peerIP) + log.Printf("[%v] %v", srv.BindAddr, err) + http.Error(w, "ipip ifname out of range", http.StatusInternalServerError) + return + } if err := srv.createIpipLink(ifname, clientIP, peerIP); err != nil { srv.ipAllocator.Free(peerIP) log.Printf("[%v] failed to create IPIP tunnel for %v: %v", diff --git a/lib/ipip_test.go b/lib/ipip_test.go index ccdee2a..97084f2 100644 --- a/lib/ipip_test.go +++ b/lib/ipip_test.go @@ -5,27 +5,61 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestIpipIfname(t *testing.T) { - srv := &Server{Index: 0} - assert.Equal(t, "vp0-1", srv.ipipIfname(netip.AddrFrom4([4]byte{10, 100, 0, 1}))) - assert.Equal(t, "vp0-258", srv.ipipIfname(netip.AddrFrom4([4]byte{10, 100, 1, 2}))) + srv := &Server{Index: 0, WgCidr: netip.MustParsePrefix("10.100.0.0/16")} + name, err := srv.ipipIfname(netip.AddrFrom4([4]byte{10, 100, 0, 1})) + require.NoError(t, err) + assert.Equal(t, "vp0-1", name) - srv = &Server{Index: 7} - assert.Equal(t, "vp7-65535", srv.ipipIfname(netip.AddrFrom4([4]byte{10, 100, 255, 255}))) + name, err = srv.ipipIfname(netip.AddrFrom4([4]byte{10, 100, 1, 2})) + require.NoError(t, err) + assert.Equal(t, "vp0-258", name) + + srv = &Server{Index: 7, WgCidr: netip.MustParsePrefix("10.100.0.0/16")} + name, err = srv.ipipIfname(netip.AddrFrom4([4]byte{10, 100, 255, 255})) + require.NoError(t, err) + assert.Equal(t, "vp7-65535", name) } // TestIpipIfnameWithinIfnamsiz locks in the reasoning from ipipIfname's doc -// comment: the "vp-" name must stay within IFNAMSIZ (15 visible -// chars) even at the largest possible index and host values. +// comment: the "vp-" name must stay within IFNAMSIZ (15 +// visible chars) even at the largest possible index and offset for any +// realistic CIDR width. func TestIpipIfnameWithinIfnamsiz(t *testing.T) { - srv := &Server{Index: 65535} // max uint16 - name := srv.ipipIfname(netip.AddrFrom4([4]byte{10, 100, 255, 255})) + srv := &Server{Index: 65535, WgCidr: netip.MustParsePrefix("10.100.0.0/16")} + name, err := srv.ipipIfname(netip.AddrFrom4([4]byte{10, 100, 255, 255})) + require.NoError(t, err) assert.Equal(t, "vp65535-65535", name) assert.LessOrEqual(t, len(name), 15, "interface name exceeds IFNAMSIZ") } +// TestIpipIfnameNoCollisionForWideCidr regression-tests the bug where two +// distinct peers inside a CIDR wider than /16 would produce the same +// interface name because only the low 16 bits of peerIP were used. +func TestIpipIfnameNoCollisionForWideCidr(t *testing.T) { + srv := &Server{Index: 0, WgCidr: netip.MustParsePrefix("10.0.0.0/8")} + a, err := srv.ipipIfname(netip.AddrFrom4([4]byte{10, 0, 0, 1})) + require.NoError(t, err) + b, err := srv.ipipIfname(netip.AddrFrom4([4]byte{10, 1, 0, 1})) + require.NoError(t, err) + assert.NotEqual(t, a, b, "names must differ for distinct peers in the same CIDR") +} + +// TestIpipIfnameRejectsOversizedNames verifies the hard IFNAMSIZ guard so an +// extreme CIDR + server index combination fails loudly instead of silently +// truncating or colliding. +func TestIpipIfnameRejectsOversizedNames(t *testing.T) { + // A /4 CIDR plus the maximum server index produces a 10-digit offset + // (4294967295 ÷ 16 ≈ 268M, 9 chars) on top of "vp65535-" (8 chars). + // 8 + 9 = 17 > 15, so this must error. + srv := &Server{Index: 65535, WgCidr: netip.MustParsePrefix("0.0.0.0/4")} + _, err := srv.ipipIfname(netip.AddrFrom4([4]byte{15, 255, 255, 255})) + assert.Error(t, err, "expected error for ifname exceeding IFNAMSIZ") +} + func TestIpipIfaceWildcard(t *testing.T) { assert.Equal(t, "vp0-+", (&Server{Index: 0}).ipipIfaceWildcard()) assert.Equal(t, "vp42-+", (&Server{Index: 42}).ipipIfaceWildcard()) From ea778dc0c002755d8cbd6dee03194a1583788d45 Mon Sep 17 00:00:00 2001 From: Elton Leong Date: Mon, 18 May 2026 17:13:59 -0400 Subject: [PATCH 09/15] lib: ipip follow-ups -- lock-free iface probe, startup ifname check connectIpipHandler's idempotent path held ipipMu across a netlink.LinkByName syscall and treated any lookup error as "interface vanished". Read the entry under ipipMu, release it, probe the interface lock-free, then re-acquire and re-check the entry's identity before mutating. Only netlink.LinkNotFoundError now counts as vanished; other errors log and reuse the tunnel. Also validate in InitState that WgCidr isn't wide enough for a peer's IPIP interface name to exceed IFNAMSIZ, so a misconfigured server fails fast instead of 500-ing every /connect-ipip request. The test helper now builds WgCidr the way server_manager.go does (network base + 1), so test ifnames match a real server's. Co-Authored-By: Claude Opus 4.7 (1M context) --- lib/ipip.go | 63 ++++++++++++++++++++++++++++++++---------------- lib/ipip_test.go | 36 ++++++++++++++++----------- lib/server.go | 16 ++++++++++++ 3 files changed, 80 insertions(+), 35 deletions(-) diff --git a/lib/ipip.go b/lib/ipip.go index c3c8700..5908aac 100644 --- a/lib/ipip.go +++ b/lib/ipip.go @@ -2,6 +2,7 @@ package lib import ( "encoding/json" + "errors" "fmt" "log" "net" @@ -46,11 +47,12 @@ const ipipIfnameMaxLen = 15 // also gives us a stable wildcard (vp-+) for iptables. // // The function returns an error if the resulting name would exceed -// IFNAMSIZ. In practice this never fires for sensibly-sized CIDRs (a /6 -// produces at most an 8-char decimal offset, which still fits alongside a -// 5-char server index), but it's a hard runtime guard against accidentally -// configuring a CIDR so wide that two peers would silently collide on the -// same interface name. +// IFNAMSIZ. This never fires for production-sized CIDRs -- a /16 yields at +// most a 5-digit offset, well within budget alongside a 5-digit server +// index -- but it is a hard runtime guard against a CIDR wide enough +// (roughly /8 or wider, paired with a large server index) that the offset +// pushes the name past IFNAMSIZ, where kernel truncation could make two +// distinct peers collide on the same interface name. func (srv *Server) ipipIfname(peerIP netip.Addr) (string, error) { base := srv.WgCidr.Addr().As4() p := peerIP.As4() @@ -110,31 +112,50 @@ func (srv *Server) connectIpipHandler(w http.ResponseWriter, r *http.Request) { } // Idempotent path: if we already have an IPIP tunnel for this client - // IP AND its kernel interface still exists, refresh its lastSeen and + // IP and its kernel interface still exists, refresh lastSeen and // return the same assignment. If the interface vanished out-of-band // (manual `ip link del`, kernel reload, etc.) we cannot just refresh // lastSeen: a retrying client would keep the stale entry alive // forever, since removeIdleIpipPeers's freshness guard skips a peer - // whose lastSeen was just bumped. Drop the stale entry here and fall + // whose lastSeen was just bumped. Drop the stale entry and fall // through to fresh allocation. srv.ipipMu.Lock() - if existing, ok := srv.ipipPeers[clientIP]; ok { - if _, err := netlink.LinkByName(existing.ifname); err == nil { - existing.lastSeen = time.Now() - assigned := fmt.Sprintf("%v/%d", existing.peerIP, srv.WgCidr.Bits()) + existing, ok := srv.ipipPeers[clientIP] + srv.ipipMu.Unlock() + + if ok { + // Probe the interface without holding ipipMu, so the netlink + // syscall doesn't block the idle loop or other requests. Only a + // genuine "not found" counts as vanished: a transient lookup + // failure must not tear down a working tunnel. + _, lookupErr := netlink.LinkByName(existing.ifname) + var notFound netlink.LinkNotFoundError + vanished := errors.As(lookupErr, ¬Found) + if lookupErr != nil && !vanished { + log.Printf("[%v] ipip iface %s lookup failed transiently (%v); reusing", + srv.BindAddr, existing.ifname, lookupErr) + } + + // Re-check identity under the lock: the entry may have been + // reaped or replaced while ipipMu was released for the probe. + srv.ipipMu.Lock() + if current, stillOurs := srv.ipipPeers[clientIP]; !stillOurs || current != existing { + // Entry changed under us; fall through to fresh allocation + // (the race re-check below reconciles with the winner). srv.ipipMu.Unlock() - writeIpipResponse(w, assigned) + } else if !vanished { + current.lastSeen = time.Now() + srv.ipipMu.Unlock() + writeIpipResponse(w, fmt.Sprintf("%v/%d", existing.peerIP, srv.WgCidr.Bits())) return + } else { + delete(srv.ipipPeers, clientIP) + srv.ipipMu.Unlock() + log.Printf("[%v] ipip iface %s for %v vanished; rebuilding", + srv.BindAddr, existing.ifname, clientIP) + srv.removeIpipPeerFilter(existing.ifname, existing.peerIP) + srv.ipAllocator.Free(existing.peerIP) } - // Interface is gone; reclaim and rebuild. - log.Printf("[%v] ipip iface %s for %v vanished; rebuilding", - srv.BindAddr, existing.ifname, clientIP) - delete(srv.ipipPeers, clientIP) - srv.ipipMu.Unlock() - srv.removeIpipPeerFilter(existing.ifname, existing.peerIP) - srv.ipAllocator.Free(existing.peerIP) - } else { - srv.ipipMu.Unlock() } peerIP := srv.ipAllocator.Allocate() diff --git a/lib/ipip_test.go b/lib/ipip_test.go index 97084f2..20eaf4a 100644 --- a/lib/ipip_test.go +++ b/lib/ipip_test.go @@ -8,20 +8,28 @@ import ( "github.com/stretchr/testify/require" ) +// testWgCidr builds a WgCidr the way server_manager.go does -- PrefixFrom on +// the network base + 1 (the server's reserved IP) -- so its .Addr() matches +// a real running server's rather than the masked network address. +func testWgCidr(cidr string) netip.Prefix { + p := netip.MustParsePrefix(cidr) + return netip.PrefixFrom(p.Masked().Addr().Next(), p.Bits()) +} + func TestIpipIfname(t *testing.T) { - srv := &Server{Index: 0, WgCidr: netip.MustParsePrefix("10.100.0.0/16")} - name, err := srv.ipipIfname(netip.AddrFrom4([4]byte{10, 100, 0, 1})) + srv := &Server{Index: 0, WgCidr: testWgCidr("10.100.0.0/16")} + name, err := srv.ipipIfname(netip.AddrFrom4([4]byte{10, 100, 0, 2})) require.NoError(t, err) assert.Equal(t, "vp0-1", name) - name, err = srv.ipipIfname(netip.AddrFrom4([4]byte{10, 100, 1, 2})) + name, err = srv.ipipIfname(netip.AddrFrom4([4]byte{10, 100, 1, 3})) require.NoError(t, err) assert.Equal(t, "vp0-258", name) - srv = &Server{Index: 7, WgCidr: netip.MustParsePrefix("10.100.0.0/16")} + srv = &Server{Index: 7, WgCidr: testWgCidr("10.100.0.0/16")} name, err = srv.ipipIfname(netip.AddrFrom4([4]byte{10, 100, 255, 255})) require.NoError(t, err) - assert.Equal(t, "vp7-65535", name) + assert.Equal(t, "vp7-65534", name) } // TestIpipIfnameWithinIfnamsiz locks in the reasoning from ipipIfname's doc @@ -29,10 +37,10 @@ func TestIpipIfname(t *testing.T) { // visible chars) even at the largest possible index and offset for any // realistic CIDR width. func TestIpipIfnameWithinIfnamsiz(t *testing.T) { - srv := &Server{Index: 65535, WgCidr: netip.MustParsePrefix("10.100.0.0/16")} + srv := &Server{Index: 65535, WgCidr: testWgCidr("10.100.0.0/16")} name, err := srv.ipipIfname(netip.AddrFrom4([4]byte{10, 100, 255, 255})) require.NoError(t, err) - assert.Equal(t, "vp65535-65535", name) + assert.Equal(t, "vp65535-65534", name) assert.LessOrEqual(t, len(name), 15, "interface name exceeds IFNAMSIZ") } @@ -40,10 +48,10 @@ func TestIpipIfnameWithinIfnamsiz(t *testing.T) { // distinct peers inside a CIDR wider than /16 would produce the same // interface name because only the low 16 bits of peerIP were used. func TestIpipIfnameNoCollisionForWideCidr(t *testing.T) { - srv := &Server{Index: 0, WgCidr: netip.MustParsePrefix("10.0.0.0/8")} - a, err := srv.ipipIfname(netip.AddrFrom4([4]byte{10, 0, 0, 1})) + srv := &Server{Index: 0, WgCidr: testWgCidr("10.0.0.0/8")} + a, err := srv.ipipIfname(netip.AddrFrom4([4]byte{10, 0, 0, 2})) require.NoError(t, err) - b, err := srv.ipipIfname(netip.AddrFrom4([4]byte{10, 1, 0, 1})) + b, err := srv.ipipIfname(netip.AddrFrom4([4]byte{10, 1, 0, 2})) require.NoError(t, err) assert.NotEqual(t, a, b, "names must differ for distinct peers in the same CIDR") } @@ -52,10 +60,10 @@ func TestIpipIfnameNoCollisionForWideCidr(t *testing.T) { // extreme CIDR + server index combination fails loudly instead of silently // truncating or colliding. func TestIpipIfnameRejectsOversizedNames(t *testing.T) { - // A /4 CIDR plus the maximum server index produces a 10-digit offset - // (4294967295 ÷ 16 ≈ 268M, 9 chars) on top of "vp65535-" (8 chars). - // 8 + 9 = 17 > 15, so this must error. - srv := &Server{Index: 65535, WgCidr: netip.MustParsePrefix("0.0.0.0/4")} + // A /4 CIDR (28 host bits) plus the maximum server index yields a + // 9-digit offset on top of "vp65535-" (8 chars): 8 + 9 = 17 > 15, so + // this must error. + srv := &Server{Index: 65535, WgCidr: testWgCidr("0.0.0.0/4")} _, err := srv.ipipIfname(netip.AddrFrom4([4]byte{15, 255, 255, 255})) assert.Error(t, err, "expected error for ifname exceeding IFNAMSIZ") } diff --git a/lib/server.go b/lib/server.go index 7e3c2cf..eb14351 100644 --- a/lib/server.go +++ b/lib/server.go @@ -137,6 +137,22 @@ func (srv *Server) InitState() error { if reservedIp != srv.WgCidr.Addr() { return fmt.Errorf("reserved IP address mistamches CIDR: %v != %v", reservedIp, srv.WgCidr.Addr()) } + + // Fail fast if WgCidr is wide enough that a peer's IPIP interface name + // would exceed IFNAMSIZ; otherwise every /connect-ipip request would + // 500 at runtime instead. The longest name comes from the last address + // of the block. + maskedWg := srv.WgCidr.Masked() + wgBase := maskedWg.Addr().As4() + lastInt := (uint32(wgBase[0])<<24 | uint32(wgBase[1])<<16 | + uint32(wgBase[2])<<8 | uint32(wgBase[3])) | (^uint32(0) >> uint(maskedWg.Bits())) + lastAddr := netip.AddrFrom4([4]byte{ + byte(lastInt >> 24), byte(lastInt >> 16), byte(lastInt >> 8), byte(lastInt), + }) + if _, err := srv.ipipIfname(lastAddr); err != nil { + return fmt.Errorf("WgCidr %v too wide for IPIP: %v", srv.WgCidr, err) + } + srv.newPeers = make(map[wgtypes.Key]time.Time) srv.peerIPs = make(map[wgtypes.Key]netip.Addr) srv.ipipPeers = make(map[netip.Addr]*ipipPeer) From e4d9ff61283b08af62fbc6a0bd675c9b2e950166 Mon Sep 17 00:00:00 2001 From: eltonkl Date: Mon, 18 May 2026 21:39:57 +0000 Subject: [PATCH 10/15] lib: don't tear down ipip tunnels on transient netlink errors removeIdleIpipPeers treated any LinkByName error as vanished and set vanished=true on the removal entry, which then bypasses the lastSeen freshness guard. A transient netlink failure (kernel resource pressure, brief glitch) would unconditionally tear down an actively-used tunnel. Mirror the discrimination already used in connectIpipHandler: only a genuine netlink.LinkNotFoundError counts as vanished. Any other error is logged and the peer is left in place for the next poll to retry. Co-authored-by: Codesmith --- lib/ipip.go | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/lib/ipip.go b/lib/ipip.go index 5908aac..0a71c85 100644 --- a/lib/ipip.go +++ b/lib/ipip.go @@ -396,11 +396,20 @@ func (srv *Server) removeIdleIpipPeers() { for _, s := range snaps { link, err := netlink.LinkByName(s.ifname) if err != nil { - // The interface vanished out from under us (manual `ip link - // del`, kernel reload, etc.). Reap unconditionally; the - // freshness guard in the removal loop does not apply because - // the entry is unusable regardless of lastSeen. - toRemove = append(toRemove, removal{snapshot: s, vanished: true}) + // Only a genuine "not found" counts as vanished. A + // transient lookup failure (kernel resource pressure, brief + // netlink glitch, etc.) must NOT tear down an + // actively-used tunnel, since vanished entries bypass the + // lastSeen freshness guard below. Log and re-check next + // poll instead. Matches the discrimination in + // connectIpipHandler. + var notFound netlink.LinkNotFoundError + if errors.As(err, ¬Found) { + toRemove = append(toRemove, removal{snapshot: s, vanished: true}) + } else { + log.Printf("[%v] ipip iface %s lookup failed transiently (%v); will retry", + srv.BindAddr, s.ifname, err) + } continue } stats := link.Attrs().Statistics From 941c9cfa115b49dac2892595fbaba30fefc7b3e4 Mon Sep 17 00:00:00 2001 From: Elton Leong Date: Mon, 18 May 2026 18:00:05 -0400 Subject: [PATCH 11/15] lib: correct stale mu comment The "Protects the fields below" comment on Server.mu predated ipipMu/ipipPeers being added below it. mu only guards newPeers; ipipPeers has its own ipipMu. Scope the comment to newPeers so it no longer over-claims. Co-Authored-By: Claude Opus 4.7 (1M context) --- lib/server.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/server.go b/lib/server.go index eb14351..1062197 100644 --- a/lib/server.go +++ b/lib/server.go @@ -93,7 +93,7 @@ type Server struct { ipAllocator *IpAllocator - mu sync.Mutex // Protects the fields below. + mu sync.Mutex // Protects newPeers and peerIPs. // newPeers records the time of each peer's most recent /connect, granting // a grace period during which the idle reaper won't remove the peer. This // protects peers that haven't completed a WireGuard handshake yet, and From d0d7397429d860e0d4aecb2173107d0f991fd004 Mon Sep 17 00:00:00 2001 From: Elton Leong Date: Thu, 21 May 2026 14:11:20 -0400 Subject: [PATCH 12/15] lib: replace ipip host guard with a handler-level check The raw/PREROUTING host guard was framed as anti-spoofing for host-local traffic from IPIP peers, but on closer inspection the threat it was meant to stop (peer-map poisoning via forged RemoteAddr in connectIpipHandler) is not practically exploitable: the SYN-ACK for a TCP handshake with a forged inner source routes to the forged address, not back to the attacker, so the TLS+HTTP exchange never completes. Meanwhile the guard breaks legitimate inner-to-host ICMP -- including the mac agent's `ping 10.100.0.1` health check. Drop the guard and instead encode the actual design invariant at the right layer: a requireExternalRemote middleware rejects control-plane requests whose r.RemoteAddr is inside srv.WgCidr, i.e. requests that arrived from inside a VPN tunnel. Both /connect and /connect-ipip key on RemoteAddr (for peer-map and log respectively), so the assumption "RemoteAddr is the client's real outer address" is now enforced once at the seam rather than each handler having to recheck. New control-plane endpoints inherit the protection by wrapping. Co-Authored-By: Claude Opus 4.7 (1M context) --- lib/ipip.go | 48 +++++------------------------------------------- lib/server.go | 39 ++++++++++++++++++++++++++------------- 2 files changed, 31 insertions(+), 56 deletions(-) diff --git a/lib/ipip.go b/lib/ipip.go index 0a71c85..13227d9 100644 --- a/lib/ipip.go +++ b/lib/ipip.go @@ -274,11 +274,11 @@ func (srv *Server) createIpipLink(ifname string, remote, peerIP netip.Addr) erro // source IP matches the peer we assigned this tunnel to, and drop // everything else arriving on this interface. A decapsulated IPIP // packet carries an attacker-controlled inner source, so this is what - // stops a peer from injecting traffic claiming to be from a different - // peer's inner IP. It covers transit (FORWARD) traffic only; packets - // terminating on the host itself are handled separately by the - // raw/PREROUTING guard installed in StartIptables (see - // iptablesIpipHostGuardRule). + // stops a peer from injecting transit traffic claiming to be from a + // different peer's inner IP. Inner packets that terminate on the host + // itself go through INPUT instead, where ufw's default deny and + // connectIpipHandler's check that the request did not originate from + // inside srv.WgCidr together cover the control-plane concern. if err := srv.addIpipPeerFilter(ifname, peerIP); err != nil { _ = netlink.LinkDel(resolved) return fmt.Errorf("install ipip peer filter: %v", err) @@ -523,41 +523,3 @@ func (srv *Server) iptablesIpipMssRules(enabled bool) error { } return nil } - -// iptablesIpipHostGuardRule adds or removes the wildcard rule that drops -// traffic arriving on this server's IPIP interfaces and destined to a -// host-local address. -// -// IPIP peers route *through* the box and have no legitimate reason to -// reach the host itself. A decapsulated inner packet carries an -// attacker-controlled inner source, and the per-peer FORWARD filter -// (addIpipPeerFilter) only inspects transit traffic, not packets -// terminating on the host. Without this rule an authenticated peer could -// reach host-local services -- notably the vprox HTTPS control plane, -// whose /connect-ipip handler keys peer identity on the request source -// address -- with a forged source IP. -// -// It lives in raw/PREROUTING so it is evaluated before conntrack and -// before ufw's filter chains; ufw's accept rules for ports 22/443/etc. -// therefore cannot shadow it. The legitimate control-plane handshake is -// unaffected: it arrives on the physical bind interface, not vp-+. -func (srv *Server) iptablesIpipHostGuardRule(enabled bool) error { - rule := []string{ - "-i", srv.ipipIfaceWildcard(), - "-m", "addrtype", "--dst-type", "LOCAL", - "-j", "DROP", - "-m", "comment", "--comment", - fmt.Sprintf("vprox ipip host guard rule for %s", srv.Ifname()), - } - if enabled { - if err := srv.Ipt.AppendUnique("raw", "PREROUTING", rule...); err != nil { - return fmt.Errorf("append ipip host guard rule: %v", err) - } - return nil - } - - if err := srv.Ipt.Delete("raw", "PREROUTING", rule...); err != nil { - log.Printf("failed to remove ipip host guard rule: %v", err) - } - return nil -} diff --git a/lib/server.go b/lib/server.go index 1062197..2fb17b7 100644 --- a/lib/server.go +++ b/lib/server.go @@ -651,13 +651,6 @@ func (srv *Server) StartIptables() error { return fmt.Errorf("failed to add ipip MSS rules: %v", err) } - // Wildcard guard that drops IPIP traffic destined to the host itself, - // so peers can only transit the box and can't reach host-local - // services with a forged inner source (see iptablesIpipHostGuardRule). - if err := srv.iptablesIpipHostGuardRule(true); err != nil { - return fmt.Errorf("failed to add ipip host guard rule: %v", err) - } - // SNAT rule for internal network traffic. This is currently only applicable for boxes in // the US. if srv.Region == "us-west" { @@ -761,10 +754,6 @@ func (srv *Server) CleanupIptables() { log.Printf("failed to remove ipip MSS rules: %v", err) } - if err := srv.iptablesIpipHostGuardRule(false); err != nil { - log.Printf("failed to remove ipip host guard rule: %v", err) - } - if srv.Region == "us-west" { // Remove SNAT rule for internal traffic rule = []string{ @@ -912,6 +901,30 @@ func (srv *Server) addBindAddr() error { }) } +// requireExternalRemote rejects requests whose source IP is inside +// srv.WgCidr -- i.e. requests that arrived from inside a VPN tunnel rather +// than from the public network. The control plane is meant to be reached +// from clients' outer addresses only; both connectHandler and +// connectIpipHandler treat r.RemoteAddr as identity-relevant input (for +// logging and for the IPIP peer-map key respectively), so a request whose +// apparent source is a VPN-internal IP is either spoofed or a +// misconfigured client routing everything into the tunnel. Either way it +// is not legitimate, and rejecting it here keeps the assumption that +// "RemoteAddr is the client's real outer address" honest for every +// control-plane handler without each having to recheck. +func (srv *Server) requireExternalRemote(h http.HandlerFunc) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + host, _, err := net.SplitHostPort(r.RemoteAddr) + if err == nil { + if ip, err := netip.ParseAddr(host); err == nil && srv.WgCidr.Contains(ip) { + http.Error(w, "control-plane requests must originate outside the VPN", http.StatusForbidden) + return + } + } + h(w, r) + } +} + func (srv *Server) ListenForHttps() error { if !srv.BindAddr.Is4() { return fmt.Errorf("invalid IPv4 bind address: %v", srv.BindAddr) @@ -927,8 +940,8 @@ func (srv *Server) ListenForHttps() error { mux := http.NewServeMux() mux.HandleFunc("/", srv.indexHandler) - mux.HandleFunc("/connect", srv.connectHandler) - mux.HandleFunc("/connect-ipip", srv.connectIpipHandler) + mux.HandleFunc("/connect", srv.requireExternalRemote(srv.connectHandler)) + mux.HandleFunc("/connect-ipip", srv.requireExternalRemote(srv.connectIpipHandler)) cert, err := loadServerTls() if err != nil { From 86b4b7ea1093318d0559a374822194123a744d29 Mon Sep 17 00:00:00 2001 From: pbardea Date: Tue, 4 Aug 2026 12:38:03 +0000 Subject: [PATCH 13/15] lib: drain in-flight connects before ipip cleanup Co-authored-by: Codesmith Staging --- lib/ipip.go | 12 +++++++++++- lib/server.go | 18 +++++++++++++++--- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/lib/ipip.go b/lib/ipip.go index 13227d9..a24edd5 100644 --- a/lib/ipip.go +++ b/lib/ipip.go @@ -189,8 +189,17 @@ func (srv *Server) connectIpipHandler(w http.ResponseWriter, r *http.Request) { // Another request from the same client IP could have raced us. If so, // drop the one we just built and reuse the winner so we don't leak an - // allocation or an interface. + // allocation or an interface. Likewise, if CleanupIpip already ran + // (this handler outlived the shutdown drain), registering the peer now + // would leak a tunnel that nothing will ever tear down. srv.ipipMu.Lock() + if srv.ipipClosed { + srv.ipipMu.Unlock() + srv.tearDownIpipLink(ifname, peerIP) + srv.ipAllocator.Free(peerIP) + http.Error(w, "server shutting down", http.StatusServiceUnavailable) + return + } if winner, ok := srv.ipipPeers[clientIP]; ok { winner.lastSeen = time.Now() srv.ipipMu.Unlock() @@ -468,6 +477,7 @@ func (srv *Server) removeIdleIpipPeers() { // safe to call multiple times. func (srv *Server) CleanupIpip() { srv.ipipMu.Lock() + srv.ipipClosed = true peers := make([]*ipipPeer, 0, len(srv.ipipPeers)) for _, p := range srv.ipipPeers { peers = append(peers, p) diff --git a/lib/server.go b/lib/server.go index 2fb17b7..8d51822 100644 --- a/lib/server.go +++ b/lib/server.go @@ -108,10 +108,15 @@ type Server struct { // removed when the reaper deletes an idle peer. peerIPs map[wgtypes.Key]netip.Addr - // ipipMu protects ipipPeers. It is separate from mu so that the IPIP - // peer bookkeeping does not contend with the WireGuard peer state. + // ipipMu protects ipipPeers and ipipClosed. It is separate from mu so + // that the IPIP peer bookkeeping does not contend with the WireGuard + // peer state. ipipMu sync.Mutex ipipPeers map[netip.Addr]*ipipPeer + // ipipClosed is set by CleanupIpip so that a /connect-ipip handler + // that outlives the shutdown drain cannot register a new tunnel after + // cleanup has already run. + ipipClosed bool } // InitState initializes the private server state. @@ -974,7 +979,14 @@ func (srv *Server) ListenForHttps() error { select { case <-srv.Ctx.Done(): log.Printf("server no longer listening on %v:443\n", srv.BindAddr) - return httpServer.Shutdown(srv.Ctx) + // srv.Ctx is already cancelled here, so passing it to Shutdown + // would return immediately without draining in-flight handlers. + // Drain with a fresh deadline so handlers (notably /connect-ipip, + // whose tunnels are torn down right after this function returns) + // finish before cleanup runs. + shutdownCtx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + return httpServer.Shutdown(shutdownCtx) case err = <-errCh: return err } From 933a1ec97d31b548f0876afa4b2f48fc9431bd01 Mon Sep 17 00:00:00 2001 From: pbardea Date: Sun, 9 Aug 2026 17:18:16 +0000 Subject: [PATCH 14/15] lib: serialize ipip creates, sweep stale tunnels on startup Co-authored-by: Codesmith Staging --- lib/ipip.go | 85 ++++++++++++++++++++++++++++++++++++++++--- lib/ipip_test.go | 23 ++++++++++++ lib/server.go | 6 +++ lib/server_manager.go | 8 +++- 4 files changed, 116 insertions(+), 6 deletions(-) diff --git a/lib/ipip.go b/lib/ipip.go index a24edd5..81159e7 100644 --- a/lib/ipip.go +++ b/lib/ipip.go @@ -8,6 +8,8 @@ import ( "net" "net/http" "net/netip" + "strconv" + "strings" "time" "github.com/vishvananda/netlink" @@ -76,6 +78,58 @@ func (srv *Server) ipipIfaceWildcard() string { return fmt.Sprintf("vp%d-+", srv.Index) } +// ipipPeerFromIfname is the inverse of ipipIfname: it recovers the peer's +// inner IP from a "vp-" interface name. The startup sweep +// uses it to remove the per-peer iptables rules of tunnels left over from a +// previous process. Returns false if the name does not belong to this +// server's IPIP interfaces. +func (srv *Server) ipipPeerFromIfname(ifname string) (netip.Addr, bool) { + suffix, found := strings.CutPrefix(ifname, fmt.Sprintf("vp%d-", srv.Index)) + if !found { + return netip.Addr{}, false + } + offset, err := strconv.ParseUint(suffix, 10, 32) + if err != nil { + return netip.Addr{}, false + } + base := srv.WgCidr.Addr().As4() + baseInt := uint32(base[0])<<24 | uint32(base[1])<<16 | + uint32(base[2])<<8 | uint32(base[3]) + peerInt := baseInt + uint32(offset) + return netip.AddrFrom4([4]byte{ + byte(peerInt >> 24), byte(peerInt >> 16), + byte(peerInt >> 8), byte(peerInt), + }), true +} + +// SweepStaleIpip removes IPIP interfaces (and their per-peer iptables rules) +// left over from a previous process. CleanupIpip only runs on a clean +// shutdown; after a crash or SIGKILL the interfaces survive while the new +// process starts with an empty allocator, so a leftover /32 host route could +// blackhole an IP the allocator later hands to a new WireGuard or IPIP peer. +// There is no adopt-on-restart path for IPIP (unlike WireGuard), so any +// surviving vp-* tunnel is stale by definition. +func (srv *Server) SweepStaleIpip() error { + links, err := netlink.LinkList() + if err != nil { + return fmt.Errorf("list links for ipip sweep: %v", err) + } + for _, link := range links { + ifname := link.Attrs().Name + peerIP, ok := srv.ipipPeerFromIfname(ifname) + if !ok { + continue + } + if _, isIptun := link.(*netlink.Iptun); !isIptun { + continue + } + log.Printf("[%v] sweeping stale ipip tunnel %s (peer %v)", + srv.BindAddr, ifname, peerIP) + srv.tearDownIpipLink(ifname, peerIP) + } + return nil +} + // connectIpipHandler handles POST /connect-ipip. // // It authenticates the request with the shared Bearer password (matching the @@ -158,6 +212,25 @@ func (srv *Server) connectIpipHandler(w http.ResponseWriter, r *http.Request) { } } + // Serialize tunnel creation. The kernel permits only one IPIP tunnel + // per (local, remote) pair, so two parallel first-time requests from + // the same client IP would race: both miss the peer map, both + // allocate, and the loser's LinkAdd fails with EEXIST even though a + // working tunnel exists. Holding the create lock across + // allocate+create+insert lets the second request observe the winner's + // entry below instead of colliding in the kernel. + srv.ipipCreateMu.Lock() + defer srv.ipipCreateMu.Unlock() + + srv.ipipMu.Lock() + if winner, ok := srv.ipipPeers[clientIP]; ok { + winner.lastSeen = time.Now() + srv.ipipMu.Unlock() + writeIpipResponse(w, fmt.Sprintf("%v/%d", winner.peerIP, srv.WgCidr.Bits())) + return + } + srv.ipipMu.Unlock() + peerIP := srv.ipAllocator.Allocate() if peerIP.IsUnspecified() { log.Printf("no more ip addresses available in %v", srv.WgCidr) @@ -187,11 +260,13 @@ func (srv *Server) connectIpipHandler(w http.ResponseWriter, r *http.Request) { lastSeen: time.Now(), } - // Another request from the same client IP could have raced us. If so, - // drop the one we just built and reuse the winner so we don't leak an - // allocation or an interface. Likewise, if CleanupIpip already ran - // (this handler outlived the shutdown drain), registering the peer now - // would leak a tunnel that nothing will ever tear down. + // Belt-and-braces: creation is serialized by ipipCreateMu, so no other + // request should have inserted an entry for this client IP since the + // re-check above, but if one somehow did, drop the tunnel we just + // built and reuse the winner so we don't leak an allocation or an + // interface. Likewise, if CleanupIpip already ran (this handler + // outlived the shutdown drain), registering the peer now would leak a + // tunnel that nothing will ever tear down. srv.ipipMu.Lock() if srv.ipipClosed { srv.ipipMu.Unlock() diff --git a/lib/ipip_test.go b/lib/ipip_test.go index 20eaf4a..e63e9e0 100644 --- a/lib/ipip_test.go +++ b/lib/ipip_test.go @@ -68,6 +68,29 @@ func TestIpipIfnameRejectsOversizedNames(t *testing.T) { assert.Error(t, err, "expected error for ifname exceeding IFNAMSIZ") } +// TestIpipPeerFromIfname verifies the ifname parser used by the startup +// sweep round-trips with ipipIfname and rejects names that don't belong to +// this server's IPIP interfaces. +func TestIpipPeerFromIfname(t *testing.T) { + srv := &Server{Index: 7, WgCidr: testWgCidr("10.100.0.0/16")} + for _, peer := range []netip.Addr{ + netip.AddrFrom4([4]byte{10, 100, 0, 2}), + netip.AddrFrom4([4]byte{10, 100, 1, 3}), + netip.AddrFrom4([4]byte{10, 100, 255, 255}), + } { + name, err := srv.ipipIfname(peer) + require.NoError(t, err) + got, ok := srv.ipipPeerFromIfname(name) + require.True(t, ok, "expected %s to parse", name) + assert.Equal(t, peer, got) + } + + for _, name := range []string{"vp8-1", "eth0", "vp7-", "vp7-x", "vproxy7"} { + _, ok := srv.ipipPeerFromIfname(name) + assert.False(t, ok, "expected %s to be rejected", name) + } +} + func TestIpipIfaceWildcard(t *testing.T) { assert.Equal(t, "vp0-+", (&Server{Index: 0}).ipipIfaceWildcard()) assert.Equal(t, "vp42-+", (&Server{Index: 42}).ipipIfaceWildcard()) diff --git a/lib/server.go b/lib/server.go index 8d51822..844432a 100644 --- a/lib/server.go +++ b/lib/server.go @@ -117,6 +117,12 @@ type Server struct { // that outlives the shutdown drain cannot register a new tunnel after // cleanup has already run. ipipClosed bool + // ipipCreateMu serializes the /connect-ipip create path. The kernel + // allows only one IPIP tunnel per (local, remote) pair, so two + // parallel requests from the same client would both miss the peer + // map and the loser's LinkAdd would fail with EEXIST; serializing + // creation lets the second request reuse the winner's tunnel. + ipipCreateMu sync.Mutex } // InitState initializes the private server state. diff --git a/lib/server_manager.go b/lib/server_manager.go index 2846d75..6f9594e 100644 --- a/lib/server_manager.go +++ b/lib/server_manager.go @@ -153,8 +153,14 @@ func (sm *ServerManager) Start(ip netip.Addr) error { } // IPIP tunnels have no kernel-adoption path yet (unlike WireGuard, // which RestorePeersFromKernel re-adopts), so tear them down on - // shutdown to keep the allocator and kernel state consistent. + // shutdown to keep the allocator and kernel state consistent, and + // sweep any tunnels a crashed predecessor left behind before their + // stale /32 routes can blackhole freshly allocated peer IPs. defer srv.CleanupIpip() + if err := srv.SweepStaleIpip(); err != nil { + log.Printf("[%v] failed to sweep stale ipip tunnels: %v", ip, err) + return + } if err := srv.ListenForHttps(); err != nil { log.Printf("[%v] https server failed: %v", ip, err) From 35d57aba04e5381da6f2b8658aa98bf3cfd99119 Mon Sep 17 00:00:00 2001 From: pbardea Date: Sun, 9 Aug 2026 17:25:47 +0000 Subject: [PATCH 15/15] lib: release ipip create lock before writing the response Co-authored-by: Codesmith Staging --- lib/ipip.go | 50 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/lib/ipip.go b/lib/ipip.go index 81159e7..dcf069b 100644 --- a/lib/ipip.go +++ b/lib/ipip.go @@ -212,13 +212,29 @@ func (srv *Server) connectIpipHandler(w http.ResponseWriter, r *http.Request) { } } - // Serialize tunnel creation. The kernel permits only one IPIP tunnel - // per (local, remote) pair, so two parallel first-time requests from - // the same client IP would race: both miss the peer map, both - // allocate, and the loser's LinkAdd fails with EEXIST even though a - // working tunnel exists. Holding the create lock across - // allocate+create+insert lets the second request observe the winner's - // entry below instead of colliding in the kernel. + assigned, errMsg, errStatus := srv.createIpipPeer(clientIP) + if errMsg != "" { + http.Error(w, errMsg, errStatus) + return + } + writeIpipResponse(w, assigned) +} + +// createIpipPeer allocates an inner IP for clientIP, creates the IPIP +// tunnel, and registers the peer. On success it returns the assigned inner +// address in CIDR notation; on failure it returns a non-empty errMsg and +// the HTTP status to report. Writing the response is left to the caller so +// ipipCreateMu is released before any client I/O: a slow client read must +// not stall every other /connect-ipip create. +// +// Tunnel creation is serialized by ipipCreateMu. The kernel permits only +// one IPIP tunnel per (local, remote) pair, so two parallel first-time +// requests from the same client IP would race: both miss the peer map, both +// allocate, and the loser's LinkAdd fails with EEXIST even though a working +// tunnel exists. Holding the create lock across allocate+create+insert lets +// the second request observe the winner's entry below instead of colliding +// in the kernel. +func (srv *Server) createIpipPeer(clientIP netip.Addr) (assigned, errMsg string, errStatus int) { srv.ipipCreateMu.Lock() defer srv.ipipCreateMu.Unlock() @@ -226,31 +242,27 @@ func (srv *Server) connectIpipHandler(w http.ResponseWriter, r *http.Request) { if winner, ok := srv.ipipPeers[clientIP]; ok { winner.lastSeen = time.Now() srv.ipipMu.Unlock() - writeIpipResponse(w, fmt.Sprintf("%v/%d", winner.peerIP, srv.WgCidr.Bits())) - return + return fmt.Sprintf("%v/%d", winner.peerIP, srv.WgCidr.Bits()), "", 0 } srv.ipipMu.Unlock() peerIP := srv.ipAllocator.Allocate() if peerIP.IsUnspecified() { log.Printf("no more ip addresses available in %v", srv.WgCidr) - http.Error(w, "no more IP addresses available", http.StatusServiceUnavailable) - return + return "", "no more IP addresses available", http.StatusServiceUnavailable } ifname, err := srv.ipipIfname(peerIP) if err != nil { srv.ipAllocator.Free(peerIP) log.Printf("[%v] %v", srv.BindAddr, err) - http.Error(w, "ipip ifname out of range", http.StatusInternalServerError) - return + return "", "ipip ifname out of range", http.StatusInternalServerError } if err := srv.createIpipLink(ifname, clientIP, peerIP); err != nil { srv.ipAllocator.Free(peerIP) log.Printf("[%v] failed to create IPIP tunnel for %v: %v", srv.BindAddr, clientIP, err) - http.Error(w, "failed to create IPIP tunnel", http.StatusInternalServerError) - return + return "", "failed to create IPIP tunnel", http.StatusInternalServerError } peer := &ipipPeer{ @@ -272,16 +284,14 @@ func (srv *Server) connectIpipHandler(w http.ResponseWriter, r *http.Request) { srv.ipipMu.Unlock() srv.tearDownIpipLink(ifname, peerIP) srv.ipAllocator.Free(peerIP) - http.Error(w, "server shutting down", http.StatusServiceUnavailable) - return + return "", "server shutting down", http.StatusServiceUnavailable } if winner, ok := srv.ipipPeers[clientIP]; ok { winner.lastSeen = time.Now() srv.ipipMu.Unlock() srv.tearDownIpipLink(ifname, peerIP) srv.ipAllocator.Free(peerIP) - writeIpipResponse(w, fmt.Sprintf("%v/%d", winner.peerIP, srv.WgCidr.Bits())) - return + return fmt.Sprintf("%v/%d", winner.peerIP, srv.WgCidr.Bits()), "", 0 } srv.ipipPeers[clientIP] = peer srv.ipipMu.Unlock() @@ -289,7 +299,7 @@ func (srv *Server) connectIpipHandler(w http.ResponseWriter, r *http.Request) { log.Printf("[%v] new ipip peer %v at %v (iface %s)", srv.BindAddr, clientIP, peerIP, ifname) - writeIpipResponse(w, fmt.Sprintf("%v/%d", peerIP, srv.WgCidr.Bits())) + return fmt.Sprintf("%v/%d", peerIP, srv.WgCidr.Bits()), "", 0 } func writeIpipResponse(w http.ResponseWriter, assigned string) {