mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-10 23:15:40 +00:00
xarmian
fc5a54dff7
* fix(server): read raw TCP peer for loopback check (TASK-662) TrustedProxyRealIP rewrites r.RemoteAddr when the peer is a trusted proxy. Without additional defense, an attacker reaching a trusted reverse proxy could set X-Forwarded-For: 127.0.0.1 and trick the bootstrap loopback check into accepting them as a local caller — reopening the full-instance-takeover path that TASK-660 closed at the spoof layer. Add CapturePeerAddr middleware that runs BEFORE TrustedProxyRealIP and stashes the untampered r.RemoteAddr in request context. Change requestIsLoopback to read via rawPeerAddr(r) (context-first, with a safe fallback for test paths that skip the middleware). r.RemoteAddr stays the rewritten value for the rate-limiter / audit-log paths that actually want the client's IP. Tests cover: direct loopback → true; direct LAN → false; trusted proxy forwarding spoofed 127.0.0.1 → false; untrusted peer with spoofed XFF=127.0.0.1 → false; and that rawPeerAddr falls back to r.RemoteAddr when CapturePeerAddr is absent. Parent: PLAN-643 (OSS Security Hardening). * fix(server): require loopback peer AND no proxy headers for bootstrap (Codex P1) Codex caught a regression in the initial PR: reading rawPeerAddr(r) made every request through a same-host reverse proxy look loopback, so a Caddy or nginx on 127.0.0.1 forwarding public traffic would let attackers reach the bootstrap endpoint from the internet. Tighten the rule to two independent conditions: 1. The untampered TCP peer is a loopback address. 2. Neither X-Forwarded-For nor X-Real-IP is set. A legitimate local CLI calling Pad directly satisfies both. A reverse proxy forwarding public traffic always sets the forwarding headers, so the presence of either disqualifies the request. The raw-peer check still defeats X-Forwarded-For spoofing from non-loopback attackers, and now also handles the Codex-flagged scenario where a local proxy is trusted or left misconfigured. Tests updated to cover: direct loopback no-headers allowed; loopback peer + XFF rejected; loopback peer + X-Real-IP rejected; IPv6 loopback allowed.
97 lines
2.7 KiB
Go
97 lines
2.7 KiB
Go
package server
|
|
|
|
import (
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"testing"
|
|
)
|
|
|
|
// TestRequestIsLoopback_IgnoresRealIPRewrite verifies that the bootstrap
|
|
// loopback check reads the untampered TCP peer captured by
|
|
// CapturePeerAddr, not r.RemoteAddr. A proxied attacker setting
|
|
// X-Forwarded-For: 127.0.0.1 must not be able to trick the check.
|
|
func TestRequestIsLoopback_IgnoresRealIPRewrite(t *testing.T) {
|
|
tests := []struct {
|
|
name string
|
|
peer string
|
|
spoofedXFF string
|
|
spoofedXRI string // X-Real-IP
|
|
trustedCIDRs string
|
|
wantLoopback bool
|
|
}{
|
|
{
|
|
name: "direct loopback peer, no proxy headers: allowed",
|
|
peer: "127.0.0.1:54321",
|
|
wantLoopback: true,
|
|
},
|
|
{
|
|
name: "direct LAN peer: rejected",
|
|
peer: "192.168.1.5:54321",
|
|
wantLoopback: false,
|
|
},
|
|
{
|
|
name: "loopback peer WITH X-Forwarded-For: rejected (proxy relay)",
|
|
peer: "127.0.0.1:54321",
|
|
spoofedXFF: "203.0.113.8",
|
|
wantLoopback: false,
|
|
},
|
|
{
|
|
name: "loopback peer WITH X-Real-IP: rejected (proxy relay)",
|
|
peer: "127.0.0.1:54321",
|
|
spoofedXRI: "203.0.113.8",
|
|
wantLoopback: false,
|
|
},
|
|
{
|
|
name: "loopback peer with spoofed XFF=127.0.0.1: rejected (any XFF rejects)",
|
|
peer: "127.0.0.1:54321",
|
|
spoofedXFF: "127.0.0.1",
|
|
wantLoopback: false,
|
|
},
|
|
{
|
|
name: "trusted proxy forwarding spoofed XFF=127.0.0.1: rejected",
|
|
peer: "10.0.0.5:54321",
|
|
spoofedXFF: "127.0.0.1",
|
|
trustedCIDRs: "10.0.0.0/8",
|
|
wantLoopback: false,
|
|
},
|
|
{
|
|
name: "untrusted peer with spoofed XFF=127.0.0.1: rejected",
|
|
peer: "203.0.113.7:54321",
|
|
spoofedXFF: "127.0.0.1",
|
|
trustedCIDRs: "10.0.0.0/8",
|
|
wantLoopback: false,
|
|
},
|
|
{
|
|
name: "IPv6 loopback peer, no proxy headers: allowed",
|
|
peer: "[::1]:54321",
|
|
wantLoopback: true,
|
|
},
|
|
}
|
|
|
|
for _, tt := range tests {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
var got bool
|
|
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
got = requestIsLoopback(r)
|
|
})
|
|
// Replicate the production middleware order: CapturePeerAddr, then TrustedProxyRealIP.
|
|
cidrs := ParseTrustedProxyCIDRs(tt.trustedCIDRs)
|
|
chain := CapturePeerAddr(TrustedProxyRealIP(cidrs)(handler))
|
|
|
|
req := httptest.NewRequest("GET", "/api/v1/auth/bootstrap", nil)
|
|
req.RemoteAddr = tt.peer
|
|
if tt.spoofedXFF != "" {
|
|
req.Header.Set("X-Forwarded-For", tt.spoofedXFF)
|
|
}
|
|
if tt.spoofedXRI != "" {
|
|
req.Header.Set("X-Real-IP", tt.spoofedXRI)
|
|
}
|
|
chain.ServeHTTP(httptest.NewRecorder(), req)
|
|
|
|
if got != tt.wantLoopback {
|
|
t.Fatalf("requestIsLoopback = %v, want %v", got, tt.wantLoopback)
|
|
}
|
|
})
|
|
}
|
|
}
|