From e5f64eb21ddb818b00b8e2bebcf105ce246ec73a Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 20 Feb 2026 09:01:00 -0600 Subject: [PATCH] chore: optionally prefix authentication related cookies (#22148) When the deployment option is enabled auth cookies are prefixed with `__HOST-` ([info](https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Set-Cookie)). This is all done in a middleware that intercepts all requests and strips the prefix on incoming request cookies. --- cli/testdata/coder_server_--help.golden | 4 + cli/testdata/server-config.yaml.golden | 4 + coderd/apidoc/docs.go | 3 + coderd/apidoc/swagger.json | 3 + coderd/coderd.go | 1 + coderd/userauth.go | 2 +- coderd/userauth_test.go | 7 +- codersdk/deployment.go | 93 +++++++- codersdk/deployment_test.go | 224 ++++++++++++++++++ docs/reference/api/general.md | 1 + docs/reference/api/schemas.md | 4 + docs/reference/cli/server.md | 11 + .../cli/testdata/coder_server_--help.golden | 4 + enterprise/wsproxy/wsproxy.go | 1 + site/src/api/typesGenerated.ts | 1 + 15 files changed, 357 insertions(+), 6 deletions(-) diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden index c0231d9129..19aaa546dd 100644 --- a/cli/testdata/coder_server_--help.golden +++ b/cli/testdata/coder_server_--help.golden @@ -390,6 +390,10 @@ NETWORKING OPTIONS: Specifies the wildcard hostname to use for workspace applications in the form "*.example.com". + --host-prefix-cookie bool, $CODER_HOST_PREFIX_COOKIE (default: false) + Recommended to be enabled. Enables `__Host-` prefix for cookies to + guarantee they are only set by the right domain. + NETWORKING / DERP OPTIONS: Most Coder deployments never have to think about DERP because all connections between workspaces and users are peer-to-peer. However, when Coder cannot diff --git a/cli/testdata/server-config.yaml.golden b/cli/testdata/server-config.yaml.golden index e5a6b6427e..c7b1098a6c 100644 --- a/cli/testdata/server-config.yaml.golden +++ b/cli/testdata/server-config.yaml.golden @@ -181,6 +181,10 @@ networking: # Controls the 'SameSite' property is set on browser session cookies. # (default: lax, type: enum[lax\|none]) sameSiteAuthCookie: lax + # Recommended to be enabled. Enables `__Host-` prefix for cookies to guarantee + # they are only set by the right domain. + # (default: false, type: bool) + hostPrefixCookie: false # Whether Coder only allows connections to workspaces via the browser. # (default: , type: bool) browserOnly: false diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index db59eec279..d08c0d3ea0 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -15571,6 +15571,9 @@ const docTemplate = `{ "codersdk.HTTPCookieConfig": { "type": "object", "properties": { + "host_prefix": { + "type": "boolean" + }, "same_site": { "type": "string" }, diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 14b4552027..b8fbb24d47 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -14092,6 +14092,9 @@ "codersdk.HTTPCookieConfig": { "type": "object", "properties": { + "host_prefix": { + "type": "boolean" + }, "same_site": { "type": "string" }, diff --git a/coderd/coderd.go b/coderd/coderd.go index 3594956bbe..c439722f20 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -900,6 +900,7 @@ func New(options *Options) *API { sharedhttpmw.Recover(api.Logger), httpmw.WithProfilingLabels, tracing.StatusWriterMiddleware, + options.DeploymentValues.HTTPCookies.Middleware, tracing.Middleware(api.TracerProvider), httpmw.AttachRequestID, httpmw.ExtractRealIP(api.RealIPConfig), diff --git a/coderd/userauth.go b/coderd/userauth.go index 0a189f991e..42a8f3871b 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -704,7 +704,7 @@ func (api *API) postLogout(rw http.ResponseWriter, r *http.Request) { Name: codersdk.SessionTokenCookie, Path: "/", } - http.SetCookie(rw, cookie) + http.SetCookie(rw, api.DeploymentValues.HTTPCookies.Apply(cookie)) // Delete the session token from database. apiKey := httpmw.APIKey(r) diff --git a/coderd/userauth_test.go b/coderd/userauth_test.go index f41fb65ee1..d78c77f45e 100644 --- a/coderd/userauth_test.go +++ b/coderd/userauth_test.go @@ -1905,10 +1905,13 @@ func TestUserLogout(t *testing.T) { // Create a custom database so it's easier to make scoped tokens for // testing. db, pubSub := dbtestutil.NewDB(t) + dv := coderdtest.DeploymentValues(t) + dv.HTTPCookies.EnableHostPrefix = true client := coderdtest.New(t, &coderdtest.Options{ - Database: db, - Pubsub: pubSub, + DeploymentValues: dv, + Database: db, + Pubsub: pubSub, }) firstUser := coderdtest.CreateFirstUser(t, client) diff --git a/codersdk/deployment.go b/codersdk/deployment.go index ef375e8647..bd1f8c6063 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -848,14 +848,87 @@ type TraceConfig struct { DataDog serpent.Bool `json:"data_dog" typescript:",notnull"` } +const cookieHostPrefix = "__Host-" + type HTTPCookieConfig struct { - Secure serpent.Bool `json:"secure_auth_cookie,omitempty" typescript:",notnull"` - SameSite string `json:"same_site,omitempty" typescript:",notnull"` + Secure serpent.Bool `json:"secure_auth_cookie,omitempty" typescript:",notnull"` + SameSite string `json:"same_site,omitempty" typescript:",notnull"` + EnableHostPrefix bool `json:"host_prefix,omitempty" typescript:",notnull"` +} + +// cookiesToPrefix is the set of cookies that should be prefixed with the host prefix if EnableHostPrefix is true. +// This is a constant, do not ever mutate it. +var cookiesToPrefix = map[string]struct{}{ + SessionTokenCookie: {}, +} + +// Middleware handles some cookie mutation the requests. +// +// For performance of this, see 'BenchmarkHTTPCookieConfigMiddleware' +// This code is executed on every request, so efficiency is important. +// If making changes, please consider the performance implications and run benchmarks. +func (cfg *HTTPCookieConfig) Middleware(next http.Handler) http.Handler { + prefixed := make(map[string]struct{}) + for name := range cookiesToPrefix { + prefixed[cookieHostPrefix+name] = struct{}{} + } + return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + if !cfg.EnableHostPrefix { + // If a deployment has this config on, then turned it off. Then some old __Host- + // cookies could exist on the browsers of the clients. These cookies have no + // impact, so we are going to ignore them if they exist (niche scenario) + next.ServeHTTP(rw, r) + return + } + + // When 'EnableHostPrefix', some cookies are set with a `__Host-` prefix. This + // middleware will strip any prefixes, so the backend is unaware of this security + // feature. + // + // This code also handles any unprefixed cookies that are now invalid. + cookies := r.Cookies() + for i, c := range cookies { + // If any cookies that should be prefixed are found without the prefix, remove + // them from the client and the request. This is usually from a migration where + // the prefix was just turned on. In any case, these cookies MUST be dropped + if _, ok := cookiesToPrefix[c.Name]; ok { + // Remove the cookie from the client to prevent any future requests from sending it. + http.SetCookie(rw, &http.Cookie{ + MaxAge: -1, // Delete + Name: c.Name, + Path: "/", + }) + // And remove it from the request so the rest of the code doesn't see it. + cookies[i] = nil + } + + // Only strip prefix's from the cookies we care about. Let other `__Host-` cookies be + if _, ok := prefixed[c.Name]; ok { + c.Name = strings.TrimPrefix(c.Name, cookieHostPrefix) + } + } + + // r.Cookies() returns copies, so we need to rebuild the header. + r.Header.Del("Cookie") + for _, c := range cookies { + if c != nil { + r.AddCookie(c) + } + } + + next.ServeHTTP(rw, r) + }) } func (cfg *HTTPCookieConfig) Apply(c *http.Cookie) *http.Cookie { c.Secure = cfg.Secure.Value() c.SameSite = cfg.HTTPSameSite() + if cfg.EnableHostPrefix { + // Only prefix the cookies we want to be prefixed. + if _, ok := cookiesToPrefix[c.Name]; ok { + c.Name = cookieHostPrefix + c.Name + } + } return c } @@ -1375,7 +1448,8 @@ func (c *DeploymentValues) Options() serpent.OptionSet { Value: &c.HTTPAddress, Group: &deploymentGroupNetworkingHTTP, YAML: "httpAddress", - Annotations: serpent.Annotations{}.Mark(annotationExternalProxies, "true"), + Annotations: serpent.Annotations{}. + Mark(annotationExternalProxies, "true"), } tlsBindAddress := serpent.Option{ Name: "TLS Address", @@ -2813,6 +2887,19 @@ func (c *DeploymentValues) Options() serpent.OptionSet { YAML: "sameSiteAuthCookie", Annotations: serpent.Annotations{}.Mark(annotationExternalProxies, "true"), }, + { + Name: "__Host Prefix Cookies", + Description: "Recommended to be enabled. Enables `__Host-` prefix for cookies to guarantee they are only set by the right domain.", + Flag: "host-prefix-cookie", + Env: "CODER_HOST_PREFIX_COOKIE", + Value: serpent.BoolOf(&c.HTTPCookies.EnableHostPrefix), + // Ideally this is true, however any frontend interactions with the coder api would be broken. + // So for compatibility reasons, this is set to false. + Default: "false", + Group: &deploymentGroupNetworking, + YAML: "hostPrefixCookie", + Annotations: serpent.Annotations{}.Mark(annotationExternalProxies, "true"), + }, { Name: "Terms of Service URL", Description: "A URL to an external Terms of Service that must be accepted by users when logging in.", diff --git a/codersdk/deployment_test.go b/codersdk/deployment_test.go index 11f6734b3a..24476d4a52 100644 --- a/codersdk/deployment_test.go +++ b/codersdk/deployment_test.go @@ -5,6 +5,8 @@ import ( "embed" "encoding/json" "fmt" + "net/http" + "net/http/httptest" "runtime" "strings" "testing" @@ -882,3 +884,225 @@ func TestComputeMaxIdleConns(t *testing.T) { }) } } + +func TestHTTPCookieConfigMiddleware(t *testing.T) { + t.Parallel() + + // Realistic cookies that are always present in production. + // These cookies are added to every test. + baseCookies := []*http.Cookie{ + {Name: "_ga", Value: "GA1.1.661026807.1770083336"}, + {Name: "_ga_G0Q1B9GRC0", Value: "GS2.1.s1771343727$o49$g1$t1771343993$j48$l0$h0"}, + {Name: "csrf_token", Value: "gDiKk8GjTM2iCUHAPfN9GlC+DGjzAprlLi2vJ+5TBU0="}, + } + + cases := []struct { + name string + cfg codersdk.HTTPCookieConfig + extraCookies []*http.Cookie + expectedCookies map[string]string // cookie name -> value that handler should see + expectedDeleted []string // if any cookies are supposed to be deleted via Set-Cookie + }{ + { + name: "Disabled_PassesThrough", + cfg: codersdk.HTTPCookieConfig{}, + extraCookies: []*http.Cookie{ + {Name: codersdk.SessionTokenCookie, Value: "token123"}, + }, + expectedCookies: map[string]string{ + codersdk.SessionTokenCookie: "token123", + }, + }, + { + name: "Enabled_StripsPrefixFromCookie", + cfg: codersdk.HTTPCookieConfig{EnableHostPrefix: true}, + extraCookies: []*http.Cookie{ + {Name: "__Host-" + codersdk.SessionTokenCookie, Value: "token123"}, + }, + expectedCookies: map[string]string{ + codersdk.SessionTokenCookie: "token123", + }, + }, + { + name: "Enabled_DeletesUnprefixedCookie", + cfg: codersdk.HTTPCookieConfig{EnableHostPrefix: true}, + extraCookies: []*http.Cookie{ + // Unprefixed cookie that should be in the "to prefix" list. + {Name: codersdk.SessionTokenCookie, Value: "unprefixed-token"}, + }, + expectedCookies: map[string]string{ + // Session token should NOT be present - it was deleted. + }, + expectedDeleted: []string{codersdk.SessionTokenCookie}, + }, + { + name: "Enabled_BothPrefixedAndUnprefixed", + cfg: codersdk.HTTPCookieConfig{EnableHostPrefix: true}, + extraCookies: []*http.Cookie{ + // Browser might send both during migration. + {Name: codersdk.SessionTokenCookie, Value: "unprefixed-token"}, + {Name: "__Host-" + codersdk.SessionTokenCookie, Value: "prefixed-token"}, + }, + expectedCookies: map[string]string{ + codersdk.SessionTokenCookie: "prefixed-token", // Prefixed wins. + }, + expectedDeleted: []string{codersdk.SessionTokenCookie}, + }, + { + name: "Enabled_MultiplePrefixedCookies", + cfg: codersdk.HTTPCookieConfig{EnableHostPrefix: true}, + extraCookies: []*http.Cookie{ + {Name: "__Host-" + codersdk.SessionTokenCookie, Value: "session"}, + {Name: "__Host-SomeOtherCookie", Value: "other-cookie"}, + {Name: "__Host-Santa", Value: "santa"}, + }, + expectedCookies: map[string]string{ + codersdk.SessionTokenCookie: "session", + "__Host-SomeOtherCookie": "other-cookie", + "__Host-Santa": "santa", + }, + }, + { + name: "Enabled_UnrelatedCookiesUnchanged", + cfg: codersdk.HTTPCookieConfig{EnableHostPrefix: true}, + extraCookies: []*http.Cookie{ + {Name: "custom_cookie", Value: "custom-value"}, + {Name: "__Host-" + codersdk.SessionTokenCookie, Value: "session"}, + {Name: "__Host-foobar", Value: "do-not-change-me"}, + }, + expectedCookies: map[string]string{ + "custom_cookie": "custom-value", + codersdk.SessionTokenCookie: "session", + "__Host-foobar": "do-not-change-me", + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + var handlerCookies []*http.Cookie + handler := tc.cfg.Middleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + handlerCookies = r.Cookies() + })) + + req := httptest.NewRequest("GET", "/", nil) + for _, c := range baseCookies { + req.AddCookie(c) + } + for _, c := range tc.extraCookies { + req.AddCookie(c) + } + + rw := httptest.NewRecorder() + handler.ServeHTTP(rw, req) + + // Verify cookies seen by handler. + gotCookies := make(map[string]string) + for _, c := range handlerCookies { + gotCookies[c.Name] = c.Value + } + + for _, v := range baseCookies { + tc.expectedCookies[v.Name] = v.Value + } + assert.Equal(t, tc.expectedCookies, gotCookies) + + // Verify Set-Cookie header for deletion. + setCookies := rw.Result().Cookies() + if len(tc.expectedDeleted) > 0 { + assert.NotEmpty(t, setCookies, "expected Set-Cookie header for cookie deletion") + expDel := make(map[string]struct{}) + for _, name := range tc.expectedDeleted { + expDel[name] = struct{}{} + } + // Verify it's a deletion (MaxAge < 0). + for _, c := range setCookies { + assert.Less(t, c.MaxAge, 0, "Set-Cookie should have MaxAge < 0 for deletion") + delete(expDel, c.Name) + } + require.Empty(t, expDel, "expected Set-Cookie header for deletion") + } else { + assert.Empty(t, setCookies, "did not expect Set-Cookie header") + } + }) + } +} + +func BenchmarkHTTPCookieConfigMiddleware(b *testing.B) { + noop := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}) + + // Realistic cookies that are always present in production. + baseCookies := []*http.Cookie{ + {Name: "_ga", Value: "GA1.1.661026807.1770083336"}, + {Name: "_ga_G0Q1B9GRC0", Value: "GS2.1.s1771343727$o49$g1$t1771343993$j48$l0$h0"}, + {Name: "csrf_token", Value: "gDiKk8GjTM2iCUHAPfN9GlC+DGjzAprlLi2vJ+5TBU0="}, + } + + cases := []struct { + name string + cfg codersdk.HTTPCookieConfig + extraCookies []*http.Cookie + }{ + { + name: "Disabled", + cfg: codersdk.HTTPCookieConfig{}, + extraCookies: []*http.Cookie{ + {Name: codersdk.SessionTokenCookie, Value: "KybJV9fNul-u11vlll9wiF6eLQDxBVucD"}, + }, + }, + { + name: "Enabled_NoPrefixedCookies", + cfg: codersdk.HTTPCookieConfig{EnableHostPrefix: true}, + extraCookies: []*http.Cookie{ + {Name: codersdk.SessionTokenCookie, Value: "KybJV9fNul-u11vlll9wiF6eLQDxBVucD"}, + }, + }, + { + name: "Enabled_WithPrefixedCookie", + cfg: codersdk.HTTPCookieConfig{EnableHostPrefix: true}, + extraCookies: []*http.Cookie{ + {Name: "__Host-" + codersdk.SessionTokenCookie, Value: "KybJV9fNul-u11vlll9wiF6eLQDxBVucD"}, + }, + }, + { + name: "Enabled_MultiplePrefixedCookies", + cfg: codersdk.HTTPCookieConfig{EnableHostPrefix: true}, + extraCookies: []*http.Cookie{ + {Name: "__Host-" + codersdk.SessionTokenCookie, Value: "KybJV9fNul-u11vlll9wiF6eLQDxBVucD"}, + {Name: "__Host-" + codersdk.PathAppSessionTokenCookie, Value: "xyz123"}, + {Name: "__Host-" + codersdk.SubdomainAppSessionTokenCookie, Value: "abc456"}, + {Name: "__Host-" + "foobar", Value: "do-not-change-me"}, + }, + }, + { + name: "Enabled_NonSessionPrefixedCookies", + cfg: codersdk.HTTPCookieConfig{EnableHostPrefix: true}, + extraCookies: []*http.Cookie{ + {Name: "__Host-" + codersdk.SessionTokenCookie, Value: "KybJV9fNul-u11vlll9wiF6eLQDxBVucD"}, + }, + }, + } + + for _, tc := range cases { + b.Run(tc.name, func(b *testing.B) { + handler := tc.cfg.Middleware(noop) + rw := httptest.NewRecorder() + + allCookies := make([]*http.Cookie, 1, len(baseCookies)) + copy(allCookies, baseCookies) + // Combine base cookies with test-specific cookies. + allCookies = append(allCookies, tc.extraCookies...) + + b.ResetTimer() + for i := 0; i < b.N; i++ { + req := httptest.NewRequest("GET", "/", nil) + for _, c := range allCookies { + req.AddCookie(c) + } + handler.ServeHTTP(rw, req) + } + }) + } +} diff --git a/docs/reference/api/general.md b/docs/reference/api/general.md index f0ac2d64c9..f3499b3930 100644 --- a/docs/reference/api/general.md +++ b/docs/reference/api/general.md @@ -314,6 +314,7 @@ curl -X GET http://coder-server:8080/api/v2/deployment/config \ "hide_ai_tasks": true, "http_address": "string", "http_cookies": { + "host_prefix": true, "same_site": "string", "secure_auth_cookie": true }, diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index b05abbc2c1..db674a94f0 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -2815,6 +2815,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "hide_ai_tasks": true, "http_address": "string", "http_cookies": { + "host_prefix": true, "same_site": "string", "secure_auth_cookie": true }, @@ -3369,6 +3370,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "hide_ai_tasks": true, "http_address": "string", "http_cookies": { + "host_prefix": true, "same_site": "string", "secure_auth_cookie": true }, @@ -4488,6 +4490,7 @@ Only certain features set these fields: - FeatureManagedAgentLimit| ```json { + "host_prefix": true, "same_site": "string", "secure_auth_cookie": true } @@ -4497,6 +4500,7 @@ Only certain features set these fields: - FeatureManagedAgentLimit| | Name | Type | Required | Restrictions | Description | |----------------------|---------|----------|--------------|-------------| +| `host_prefix` | boolean | false | | | | `same_site` | string | false | | | | `secure_auth_cookie` | boolean | false | | | diff --git a/docs/reference/cli/server.md b/docs/reference/cli/server.md index 122e05fcd1..f7a26fceaa 100644 --- a/docs/reference/cli/server.md +++ b/docs/reference/cli/server.md @@ -1058,6 +1058,17 @@ Controls if the 'Secure' property is set on browser session cookies. Controls the 'SameSite' property is set on browser session cookies. +### --host-prefix-cookie + +| | | +|-------------|------------------------------------------| +| Type | bool | +| Environment | $CODER_HOST_PREFIX_COOKIE | +| YAML | networking.hostPrefixCookie | +| Default | false | + +Recommended to be enabled. Enables `__Host-` prefix for cookies to guarantee they are only set by the right domain. + ### --terms-of-service-url | | | diff --git a/enterprise/cli/testdata/coder_server_--help.golden b/enterprise/cli/testdata/coder_server_--help.golden index 45bb9b6277..485b493b35 100644 --- a/enterprise/cli/testdata/coder_server_--help.golden +++ b/enterprise/cli/testdata/coder_server_--help.golden @@ -391,6 +391,10 @@ NETWORKING OPTIONS: Specifies the wildcard hostname to use for workspace applications in the form "*.example.com". + --host-prefix-cookie bool, $CODER_HOST_PREFIX_COOKIE (default: false) + Recommended to be enabled. Enables `__Host-` prefix for cookies to + guarantee they are only set by the right domain. + NETWORKING / DERP OPTIONS: Most Coder deployments never have to think about DERP because all connections between workspaces and users are peer-to-peer. However, when Coder cannot diff --git a/enterprise/wsproxy/wsproxy.go b/enterprise/wsproxy/wsproxy.go index cbf8faab91..2b033115f5 100644 --- a/enterprise/wsproxy/wsproxy.go +++ b/enterprise/wsproxy/wsproxy.go @@ -332,6 +332,7 @@ func New(ctx context.Context, opts *Options) (*Server, error) { sharedhttpmw.Recover(s.Logger), httpmw.WithProfilingLabels, tracing.StatusWriterMiddleware, + opts.CookieConfig.Middleware, tracing.Middleware(s.TracerProvider), httpmw.AttachRequestID, httpmw.ExtractRealIP(s.Options.RealIPConfig), diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 997d65adda..32a4876240 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -2335,6 +2335,7 @@ export interface GroupSyncSettings { export interface HTTPCookieConfig { readonly secure_auth_cookie?: boolean; readonly same_site?: string; + readonly host_prefix?: boolean; } // From health/model.go