Fix migrate lock scope and stop RealIP from bypassing proxy trust.
Hold a session advisory lock for the full migration apply path, and remove Chi RealIP so clientIP can validate the TCP peer before walking X-Forwarded-For.
This commit is contained in:
@@ -148,9 +148,9 @@ func New(st store.Store, sessionStore scs.Store, templateFS fs.FS, staticFS fs.F
|
||||
func (s *Server) Handler() http.Handler {
|
||||
r := chi.NewRouter()
|
||||
r.Use(middleware.RequestID)
|
||||
if len(s.cfg.TrustedProxies) > 0 {
|
||||
r.Use(middleware.RealIP)
|
||||
}
|
||||
// Do not use middleware.RealIP: it rewrites RemoteAddr from client-controlled
|
||||
// forwarding headers before clientIP can validate the TCP peer against
|
||||
// TrustedProxies. clientIP walks X-Forwarded-For itself when the peer is trusted.
|
||||
r.Use(middleware.Logger)
|
||||
r.Use(middleware.Recoverer)
|
||||
r.Use(func(next http.Handler) http.Handler {
|
||||
|
||||
@@ -6,10 +6,12 @@ import (
|
||||
"image"
|
||||
"image/png"
|
||||
"mime/multipart"
|
||||
"net"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/alexedwards/scs/v2/memstore"
|
||||
"github.com/google/uuid"
|
||||
@@ -663,3 +665,52 @@ func csrfFrom(html string) string {
|
||||
}
|
||||
return html[:j]
|
||||
}
|
||||
|
||||
// TestRegisterThrottleUsesTCPPeerThroughRouter ensures forged X-Forwarded-For
|
||||
// cannot bypass rate limits when the direct peer is outside TrustedProxies.
|
||||
// This must go through Handler() so middleware ordering bugs are caught.
|
||||
func TestRegisterThrottleUsesTCPPeerThroughRouter(t *testing.T) {
|
||||
_, proxyNet, err := net.ParseCIDR("10.0.0.0/8")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
srv, _ := newTestServer(t, Config{TrustedProxies: []*net.IPNet{proxyNet}})
|
||||
// Tight window so the test stays fast.
|
||||
srv.registerIP = newThrottle(3, time.Minute, 100)
|
||||
h := srv.Handler()
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/register", nil))
|
||||
cookies := rec.Result().Cookies()
|
||||
csrf := csrfFrom(rec.Body.String())
|
||||
if csrf == "" {
|
||||
t.Fatal("missing csrf")
|
||||
}
|
||||
|
||||
post := func(xff string) int {
|
||||
form := strings.NewReader("_csrf=" + csrf + "&username=ab&password=hunter22")
|
||||
req := httptest.NewRequest(http.MethodPost, "/register", form)
|
||||
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
|
||||
req.RemoteAddr = "203.0.113.50:9"
|
||||
req.Header.Set("X-Forwarded-For", xff)
|
||||
for _, c := range cookies {
|
||||
req.AddCookie(c)
|
||||
}
|
||||
w := httptest.NewRecorder()
|
||||
h.ServeHTTP(w, req)
|
||||
return w.Code
|
||||
}
|
||||
|
||||
if code := post("198.51.100.1"); code != http.StatusOK {
|
||||
t.Fatalf("attempt 1: got %d want 200 (validation error page)", code)
|
||||
}
|
||||
if code := post("198.51.100.2"); code != http.StatusOK {
|
||||
t.Fatalf("attempt 2: got %d want 200", code)
|
||||
}
|
||||
if code := post("198.51.100.3"); code != http.StatusOK {
|
||||
t.Fatalf("attempt 3: got %d want 200", code)
|
||||
}
|
||||
if code := post("198.51.100.4"); code != http.StatusTooManyRequests {
|
||||
t.Fatalf("forged XFF must not bypass peer throttle, got %d want 429", code)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user