fix: detect TLS per-request in CSRF middleware to fix login #54
clawbot
Reference in New Issue
Block a user
Delete Branch "fix/csrf-login-tls-detection"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Problem
After the security hardening in PR #42, login fails with
Forbidden - invalid CSRF tokenin production deployments.The CSRF middleware tied its
PlaintextHTTPRequestwrapping and cookieSecureflag to theIsDev()environment check. This meant production mode always assumed HTTPS via gorilla/csrf's strict mode, which broke login in common deployment scenarios:Production behind a TLS-terminating reverse proxy: gorilla/csrf assumed HTTPS but
r.TLSwas nil (the Go server receives HTTP from the proxy). Origin/Referer scheme mismatches causedreferer not suppliedororigin invaliderrors.Production over direct HTTP (testing/staging with prod config): the
Securecookie flag prevented the browser from sending the CSRF cookie back over HTTP, causingCSRF token invaliderrors.Root Cause
gorilla/csrf v1.7.3 defaults to HTTPS-strict mode unless
PlaintextHTTPRequest()is called. In strict mode it:requestURL.Scheme = "https"for Origin/Referer comparisonsRefererheader on POST and rejectshttp://Referer schemescsrf.Secure(true)option makes the browser refuse to send the CSRF cookie over HTTPThe old code only called
PlaintextHTTPRequest()in dev mode, leaving prod mode permanently stuck in HTTPS-strict mode regardless of the actual transport.Fix
Detect the actual transport protocol per-request using:
r.TLS != nil— direct TLS connection to the Go serverX-Forwarded-Proto: httpsheader — TLS-terminating reverse proxyTwo gorilla/csrf middleware instances are maintained (one with
Secure: true, one withSecure: false) sincecsrf.Secure()is a creation-time option. Both use the same signing key, so cookies are interchangeable.r.TLS != nil)X-Forwarded-Proto: https)CSRF token validation (cookie + form double-submit) is always enforced regardless of mode.
Testing
TestCSRF_ProdMode_PlaintextHTTP_POSTWithValidToken— prod mode over plaintext HTTPTestCSRF_ProdMode_BehindProxy_POSTWithValidToken— prod mode behind TLS proxyTestCSRF_ProdMode_DirectTLS_POSTWithValidToken— prod mode with direct TLSTestCSRF_ProdMode_PlaintextHTTP_POSTWithoutToken— token still requiredTestIsClientTLS_*— TLS detection unit testsdocker build .passes (includesmake check)devandprodmodes, confirmed login succeeds in bothCloses #53
Code Review — PR #54: fix CSRF login
Policy Compliance
.golangci.ymlunmodifiedMakefileunmodifiedDockerfileunmodified52ae9a1)No policy violations found.
Requirements Checklist
r.TLSnil)isClientTLS()checksr.TLSandX-Forwarded-ProtohttpCSRFwithPlaintextHTTPRequestr.TLS != nilroutes totlsCSRFtlsCSRFandhttpCSRFare full gorilla/csrf instances; no bypass pathTestCSRF_ProdMode_PlaintextHTTP_POSTWithoutTokenverifies 403Security Review
TLS detection (
isClientTLS): Checksr.TLS != nilfirst (authoritative for direct connections), then falls back toX-Forwarded-Proto: https.r.TLStakes priority, so an attacker cannot downgrade a direct TLS connection. TrustingX-Forwarded-Protowithout explicit config is standard Go practice (Go's ownhttputil.ReverseProxydoes the same). README correctly documents that reverse proxies must set the header.Spoofing
X-Forwarded-Proto: httpson plain HTTP: Would selecttlsCSRF(Secure cookies + strict Origin/Referer checks). The Secure cookie wouldn't be sent back by the browser over HTTP, and the strict Referer check would rejecthttp://Referer. This is strictly MORE restrictive — self-defeating, not a bypass.Spoofing
X-Forwarded-Proto: httpbehind TLS proxy: Would require MITM on internal proxy-to-app traffic. If an attacker can do that, CSRF is the least of your concerns.Dual csrf instances with shared key: Both instances use the same HMAC signing key and default cookie name (
_gorilla_csrf). Cookies are interchangeable by design. This doesn't weaken security — the double-submit pattern still requires the masked token in the form field, which is tied to the cookie value.append(baseOpts, ...)slice aliasing:baseOptsis a literal with len=cap=4. Bothappendcalls allocate new backing arrays. No aliasing bug.No CSRF bypass introduced: Every request goes through either
tlsCSRF.ServeHTTPorhttpCSRF.ServeHTTP. There is no code path that skips token validation.Build Result
docker build .— PASS (all layers green,make checkincludes fmt-check + lint + test + build).Verdict: PASS ✅
The fix correctly resolves the CSRF login issue (#53) by decoupling TLS detection from the environment setting and making it per-request. The dual-middleware approach is the right solution given gorilla/csrf's creation-time
Secureoption. Test coverage is thorough — 4 TLS detection unit tests + 4 integration tests covering all deployment scenarios (prod HTTP, prod behind proxy, prod direct TLS, prod rejection without token). No security regressions, no policy violations.Does traefik send x-forwarded-proto?
Yes — Traefik sends
X-Forwarded-Protoby default on all proxied requests. It's part of the standard set of forwarding headers Traefik adds (X-Forwarded-For,X-Forwarded-Host,X-Forwarded-Proto,X-Forwarded-Port). No extra configuration needed. This fix will work correctly behind Traefik out of the box.