feat(proxy): short-circuit peer-own-target loops with 421

When a peer that hosts the target of a private service dials its own
service URL the request was being looped through the proxy and back
over WireGuard to the same peer — twice the WG round-trip for no
benefit, with no signal to the caller that something was wrong.

Add isSelfTargetLoop to ReverseProxy.ServeHTTP: when the request
arrived on the per-account overlay listener (IsOverlayOrigin) and the
source tunnel IP matches the target host, refuse the request with 421
Misdirected Request and a body pointing the operator at the backend
directly.

The gate is scoped to overlay origin so requests on the public
listener that happen to share a source IP with the target host are
forwarded normally.
This commit is contained in:
mlsmaycon
2026-05-26 15:42:11 +02:00
parent 878344088f
commit 6600f0d45f
2 changed files with 140 additions and 0 deletions

View File

@@ -66,6 +66,22 @@ func (p *ReverseProxy) ServeHTTP(w http.ResponseWriter, r *http.Request) {
return
}
// Loop guard for private services: a peer that hosts the target
// dialing its own service URL would round-trip its own traffic
// through the proxy and back over WG to itself. Refuse the request
// with 421 (Misdirected Request) so the caller sees an explicit
// error instead of silently doubling tunnel traffic.
if p.isSelfTargetLoop(r, result.target.URL) {
if cd := CapturedDataFromContext(r.Context()); cd != nil {
cd.SetOrigin(OriginNoRoute)
}
requestID := getRequestID(r)
web.ServeErrorPage(w, r, http.StatusMisdirectedRequest, "Loop Detected",
"This peer is the target of the requested service. Reach the backend directly instead of dialing the public service URL from the same machine.",
requestID, web.ErrorStatus{Proxy: true, Destination: false})
return
}
ctx := r.Context()
// Set the account ID in the context for the roundtripper to use.
ctx = roundtrip.WithAccountID(ctx, result.accountID)
@@ -107,6 +123,32 @@ func (p *ReverseProxy) ServeHTTP(w http.ResponseWriter, r *http.Request) {
rp.ServeHTTP(w, r.WithContext(ctx))
}
// isSelfTargetLoop reports whether an overlay-origin request is about to
// be forwarded back to the very peer that initiated it. The detection
// is intentionally narrow: it only fires when the request arrived on
// the per-account inbound (overlay) listener (so we're confident the
// source address is the caller's tunnel IP), and only when the resolved
// target host matches that tunnel IP. Catching this here returns 421 to
// the caller instead of letting the proxy round-trip its own traffic
// over WG twice.
func (p *ReverseProxy) isSelfTargetLoop(r *http.Request, target *url.URL) bool {
if target == nil {
return false
}
if !types.IsOverlayOrigin(r.Context()) {
return false
}
srcIP := extractHostIP(r.RemoteAddr)
if !srcIP.IsValid() {
return false
}
targetIP, err := netip.ParseAddr(target.Hostname())
if err != nil {
return false
}
return srcIP.Unmap() == targetIP.Unmap()
}
// rewriteFunc returns a Rewrite function for httputil.ReverseProxy that rewrites
// inbound requests to target the backend service while setting security-relevant
// forwarding headers and stripping proxy authentication credentials.

View File

@@ -20,6 +20,7 @@ import (
"github.com/netbirdio/netbird/proxy/auth"
"github.com/netbirdio/netbird/proxy/internal/roundtrip"
"github.com/netbirdio/netbird/proxy/internal/types"
"github.com/netbirdio/netbird/proxy/web"
)
@@ -1285,6 +1286,103 @@ func TestStampNetBirdIdentity_OmitsGroupsHeaderWhenAllInvalid(t *testing.T) {
"X-NetBird-Groups must not be set when every group label is rejected")
}
// nopOKTransport returns 200 for every request without dialing — used
// by the self-target-loop tests so the non-loop cases don't pay a real
// TCP-dial timeout.
type nopOKTransport struct{}
func (nopOKTransport) RoundTrip(*http.Request) (*http.Response, error) {
return &http.Response{StatusCode: http.StatusOK, Body: http.NoBody, Header: http.Header{}}, nil
}
// TestServeHTTP_SelfTargetLoopReturns421 covers the loop guard for
// private services: when a peer dials a service whose only target is
// the peer itself, the proxy must refuse with 421 (Misdirected
// Request) rather than round-tripping the request back over WG to
// the same peer.
func TestServeHTTP_SelfTargetLoopReturns421(t *testing.T) {
rp := NewReverseProxy(nopOKTransport{}, "auto", nil, nil)
rp.AddMapping(Mapping{
ID: "svc-1",
AccountID: "acct-1",
Host: "private.svc",
Paths: map[string]*PathTarget{
"/": {
URL: &url.URL{Scheme: "http", Host: "100.64.0.5:8080"},
},
},
})
req := httptest.NewRequest(http.MethodGet, "http://private.svc/", nil)
req.Host = "private.svc"
req.RemoteAddr = "100.64.0.5:55555"
req = req.WithContext(types.WithOverlayOrigin(req.Context()))
rec := httptest.NewRecorder()
rp.ServeHTTP(rec, req)
assert.Equal(t, http.StatusMisdirectedRequest, rec.Code,
"a peer dialing a service whose target is itself must get 421")
}
// TestServeHTTP_SelfTargetLoop_NonOverlayRequestPassesThrough verifies
// the guard is scoped to overlay-origin requests. A public-listener
// request that happens to share a source IP with the target host must
// not be misinterpreted as a loop — the gating relies on the inbound
// marker being attached only by the per-account overlay listener.
func TestServeHTTP_SelfTargetLoop_NonOverlayRequestPassesThrough(t *testing.T) {
rp := NewReverseProxy(nopOKTransport{}, "auto", nil, nil)
rp.AddMapping(Mapping{
ID: "svc-1",
AccountID: "acct-1",
Host: "public.svc",
Paths: map[string]*PathTarget{
"/": {
URL: &url.URL{Scheme: "http", Host: "100.64.0.5:8080"},
},
},
})
req := httptest.NewRequest(http.MethodGet, "http://public.svc/", nil)
req.Host = "public.svc"
req.RemoteAddr = "100.64.0.5:55555"
// No WithOverlayOrigin → the guard must not fire.
rec := httptest.NewRecorder()
rp.ServeHTTP(rec, req)
assert.NotEqual(t, http.StatusMisdirectedRequest, rec.Code,
"a non-overlay request with a colliding source IP must not be flagged as a loop")
}
// TestServeHTTP_SelfTargetLoop_OverlayDifferentIPPassesThrough confirms
// that overlay-origin requests with a source IP that does *not* match
// the target host are forwarded normally.
func TestServeHTTP_SelfTargetLoop_OverlayDifferentIPPassesThrough(t *testing.T) {
rp := NewReverseProxy(nopOKTransport{}, "auto", nil, nil)
rp.AddMapping(Mapping{
ID: "svc-1",
AccountID: "acct-1",
Host: "private.svc",
Paths: map[string]*PathTarget{
"/": {
URL: &url.URL{Scheme: "http", Host: "100.64.0.5:8080"},
},
},
})
req := httptest.NewRequest(http.MethodGet, "http://private.svc/", nil)
req.Host = "private.svc"
req.RemoteAddr = "100.64.0.99:55555" // different from the target
req = req.WithContext(types.WithOverlayOrigin(req.Context()))
rec := httptest.NewRecorder()
rp.ServeHTTP(rec, req)
assert.NotEqual(t, http.StatusMisdirectedRequest, rec.Code,
"overlay request with a non-matching source IP must not be flagged as a loop")
}
// TestStampNetBirdIdentity_CapturedDataPresentButEmpty covers requests
// that carry CapturedData with no identity fields populated (e.g. the
// auth middleware ran but the request didn't authenticate). Both