diff --git a/backend/proxy/proxy.go b/backend/proxy/proxy.go index 393a5eb..372b9d0 100644 --- a/backend/proxy/proxy.go +++ b/backend/proxy/proxy.go @@ -515,17 +515,20 @@ func (m *ProxyHandler) prepareRequestWithoutSession(req *http.Request, reqCtx *R req.URL.Host = reqCtx.TargetDomain req.URL.Scheme = reqCtx.TargetScheme - // create a dummy session for header normalization (no campaign/session data) + // build the phish to target host mapping the same way the response path + // does (createMinimalConfig) so it includes the primary target to phish + // entry, not only the statically declared hosts. this mapping reverses + // phishing hosts back to the upstream hosts before the request leaves the + // proxy, both in query parameters and in the Origin and Referer headers. + config := m.createMinimalConfig(reqCtx.PhishDomain, reqCtx.TargetDomain) + reqCtx.ConfigMap = config + + // dummy session carries the same mapping for header normalization dummySession := &service.ProxySession{ Config: sync.Map{}, } - // populate dummy config for normalization - need to map phishing domains to target domains - if reqCtx.ProxyConfig != nil && reqCtx.ProxyConfig.Hosts != nil { - for targetDomain, hostConfig := range reqCtx.ProxyConfig.Hosts { - if hostConfig != nil { - dummySession.Config.Store(targetDomain, *hostConfig) - } - } + for host, hostConfig := range config { + dummySession.Config.Store(host, hostConfig) } // normalize headers diff --git a/backend/proxy/proxy_test.go b/backend/proxy/proxy_test.go new file mode 100644 index 0000000..3f77843 --- /dev/null +++ b/backend/proxy/proxy_test.go @@ -0,0 +1,110 @@ +package proxy + +import ( + "net/http" + "testing" + + "github.com/phishingclub/phishingclub/service" +) + +const ( + testOriginalHost = "login.microsoftonline.com" + testPhishHost = "login.phish.example.com" +) + +func testHostConfig() map[string]service.ProxyServiceDomainConfig { + return map[string]service.ProxyServiceDomainConfig{ + testOriginalHost: {To: testPhishHost}, + } +} + +// patchUrls must reverse a phishing host back to the upstream host when a +// request travels toward the upstream server. +func TestPatchUrls_ReverseMapsPhishHostToOriginal(t *testing.T) { + m := &ProxyHandler{} + in := []byte("https://" + testPhishHost + "/oauth2/authorize?scope=openid") + want := "https://" + testOriginalHost + "/oauth2/authorize?scope=openid" + + got := string(m.patchUrls(testHostConfig(), in, CONVERT_TO_ORIGINAL_URLS)) + if got != want { + t.Fatalf("reverse map failed\n got: %s\nwant: %s", got, want) + } +} + +// A request without a session must still reverse the phishing host embedded in +// a query parameter such as redirect_uri, so the phishing domain is not sent to +// the upstream server. This is the concrete leak the fix closes. +func TestPatchQueryParameters_ReverseMapsRedirectURI(t *testing.T) { + m := &ProxyHandler{} + reqCtx := &RequestContext{ConfigMap: testHostConfig()} + + req, err := http.NewRequest( + http.MethodGet, + "https://"+testPhishHost+"/authorize?client_id=abc&redirect_uri=https%3A%2F%2F"+testPhishHost+"%2Fcb", + nil, + ) + if err != nil { + t.Fatal(err) + } + + m.patchQueryParametersWithContext(req, reqCtx) + + got := req.URL.Query().Get("redirect_uri") + want := "https://" + testOriginalHost + "/cb" + if got != want { + t.Fatalf("redirect_uri not reverse mapped\n got: %s\nwant: %s", got, want) + } + if cid := req.URL.Query().Get("client_id"); cid != "abc" { + t.Fatalf("unrelated parameter altered: client_id=%s", cid) + } +} + +// With no mapping the query patch is a safe no-op and must not panic. This +// documents that populating the mapping is what enables the reverse map. +func TestPatchQueryParameters_EmptyConfigIsNoOp(t *testing.T) { + m := &ProxyHandler{} + reqCtx := &RequestContext{} + + req, err := http.NewRequest( + http.MethodGet, + "https://"+testPhishHost+"/authorize?redirect_uri=https%3A%2F%2F"+testPhishHost+"%2Fcb", + nil, + ) + if err != nil { + t.Fatal(err) + } + + m.patchQueryParametersWithContext(req, reqCtx) + + if got := req.URL.Query().Get("redirect_uri"); got != "https://"+testPhishHost+"/cb" { + t.Fatalf("unexpected rewrite with empty config: %s", got) + } +} + +// normalizeRequestHeaders must reverse the phishing host in the Origin and +// Referer headers. This guards against regressing to an unpopulated header +// config, which would leak the phishing domain to the upstream server. +func TestNormalizeRequestHeaders_ReverseMapsOriginAndReferer(t *testing.T) { + m := &ProxyHandler{} + + session := &service.ProxySession{} + for host, cfg := range testHostConfig() { + session.Config.Store(host, cfg) + } + + req, err := http.NewRequest(http.MethodGet, "https://"+testPhishHost+"/authorize", nil) + if err != nil { + t.Fatal(err) + } + req.Header.Set("Origin", "https://"+testPhishHost) + req.Header.Set("Referer", "https://"+testPhishHost+"/foo") + + m.normalizeRequestHeaders(req, session) + + if got, want := req.Header.Get("Origin"), "https://"+testOriginalHost; got != want { + t.Fatalf("Origin not reverse mapped\n got: %s\nwant: %s", got, want) + } + if got, want := req.Header.Get("Referer"), "https://"+testOriginalHost+"/foo"; got != want { + t.Fatalf("Referer not reverse mapped\n got: %s\nwant: %s", got, want) + } +} diff --git a/makefile b/makefile index 18c5c59..c1b6f79 100644 --- a/makefile +++ b/makefile @@ -1,4 +1,4 @@ -.PHONY: build down up up-low-mem fix-tls backend-purge backend-down purge logs backend-password dbgate-down dbgate-up geoip-fetch govulncheck +.PHONY: build down up up-low-mem fix-tls backend-purge backend-down purge logs backend-password dbgate-down dbgate-up geoip-fetch govulncheck test-proxy test-proxy-fast up: sudo docker compose up -d backend frontend api-test-server pebble dbgate mailer dns test mitmproxy; \ sudo docker compose logs -f --tail 1000 backend frontend; @@ -170,6 +170,26 @@ geoip-fetch: @echo "Fetching GeoIP data from ipverse/rir-ip..."; \ cd backend/scripts && go run fetch-geoip-data.go +# tests +# run the proxy package tests in a standalone go container using the vendored +# dependencies, so it works without the dev stack running. the named volume +# keeps the go build cache between runs so only the first run does a full +# compile. the first run also pulls golang:1.25.10 and can take a few minutes; +# go test prints nothing until the build finishes, so silence is expected. +test-proxy: + sudo docker run --rm \ + -e GOCACHE=/gocache \ + -v $(CURDIR)/backend:/app \ + -v phishingclub_gocache:/gocache \ + -w /app \ + golang:1.25.10 \ + go test ./proxy/... -v + +# same tests but inside the already running backend container, which has a warm +# build cache. much faster, but requires the dev stack to be up (make up) +test-proxy-fast: + sudo docker compose exec -w /app backend go test ./proxy/... -v + # security govulncheck: sudo docker run --rm \