From 420684f60034a2a5097b6b1ef756fc9746db93da Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 19 Dec 2023 11:24:24 -0600 Subject: [PATCH 1/7] feat: enable csrf token header --- coderd/httpmw/csrf.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/coderd/httpmw/csrf.go b/coderd/httpmw/csrf.go index 2a1f383a7490a..a2db4086a0038 100644 --- a/coderd/httpmw/csrf.go +++ b/coderd/httpmw/csrf.go @@ -32,12 +32,6 @@ func CSRF(secureCookie bool) func(next http.Handler) http.Handler { mw.ExemptRegexp(regexp.MustCompile("derp/*")) mw.ExemptFunc(func(r *http.Request) bool { - // Enable CSRF in November 2022 by deleting this "return true" line. - // CSRF is not enforced to ensure backwards compatibility with older - // cli versions. - //nolint:revive - return true - // CSRF only affects requests that automatically attach credentials via a cookie. // If no cookie is present, then there is no risk of CSRF. //nolint:govet From a91a99229b8b895dc90a1c3be42bc11767a3e7df Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 19 Dec 2023 13:15:32 -0600 Subject: [PATCH 2/7] Exempt external auth requets --- coderd/httpmw/csrf.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/coderd/httpmw/csrf.go b/coderd/httpmw/csrf.go index a2db4086a0038..2d3f47fc961d5 100644 --- a/coderd/httpmw/csrf.go +++ b/coderd/httpmw/csrf.go @@ -19,9 +19,8 @@ func CSRF(secureCookie bool) func(next http.Handler) http.Handler { mw.SetFailureHandler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { http.Error(w, "Something is wrong with your CSRF token. Please refresh the page. If this error persists, try clearing your cookies.", http.StatusBadRequest) })) - // Exempt all requests that do not require CSRF protection. - // All GET requests are exempt by default. + // All GET requests are exempt by default and no not need to be added here. mw.ExemptPath("/api/v2/csp/reports") // Top level agent routes. @@ -30,6 +29,9 @@ func CSRF(secureCookie bool) func(next http.Handler) http.Handler { mw.ExemptRegexp(regexp.MustCompile("api/v2/workspaceagents/me/*")) // Derp routes mw.ExemptRegexp(regexp.MustCompile("derp/*")) + // Some extra non-auth + mw.ExemptRegexp(regexp.MustCompile("/externa-auth/*")) + mw.ExemptRegexp(regexp.MustCompile("/github/*")) mw.ExemptFunc(func(r *http.Request) bool { // CSRF only affects requests that automatically attach credentials via a cookie. From b1c560fff6caf25a1fbdaddd7f79480df9ff4cd3 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 19 Dec 2023 14:02:03 -0600 Subject: [PATCH 3/7] ensure dev server bypasses CSRF --- site/vite.config.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/site/vite.config.ts b/site/vite.config.ts index b0fa3e8be5168..6f79d3c6d4ce4 100644 --- a/site/vite.config.ts +++ b/site/vite.config.ts @@ -38,6 +38,17 @@ export default defineConfig({ }, server: { port: process.env.PORT ? Number(process.env.PORT) : 8080, + headers: { + // This header corresponds to "src/api/api.ts"'s hardcoded FE token. + // This is the secret side of the CSRF double cookie submit method. + // This should be sent on **every** response from the webserver. + // + // This is required because in production, the Golang webserver generates + // this "Set-Cookie" header. The Vite webserver needs to replicate this + // behavior. Instead of implementing CSRF though, we just use static + // values for simplicity. + "Set-Cookie": "csrf_token=JXm9hOUdZctWt0ZZGAy9xiS/gxMKYOThdxjjMnMUyn4=; Path=/; HttpOnly; SameSite=Lax", + }, proxy: { "/api": { ws: true, From 8b3da6e71490b7b4c6236dccff75de4849ab9c34 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 19 Dec 2023 14:05:30 -0600 Subject: [PATCH 4/7] fmt --- site/vite.config.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/site/vite.config.ts b/site/vite.config.ts index 6f79d3c6d4ce4..82d32c63a655f 100644 --- a/site/vite.config.ts +++ b/site/vite.config.ts @@ -47,7 +47,8 @@ export default defineConfig({ // this "Set-Cookie" header. The Vite webserver needs to replicate this // behavior. Instead of implementing CSRF though, we just use static // values for simplicity. - "Set-Cookie": "csrf_token=JXm9hOUdZctWt0ZZGAy9xiS/gxMKYOThdxjjMnMUyn4=; Path=/; HttpOnly; SameSite=Lax", + "Set-Cookie": + "csrf_token=JXm9hOUdZctWt0ZZGAy9xiS/gxMKYOThdxjjMnMUyn4=; Path=/; HttpOnly; SameSite=Lax", }, proxy: { "/api": { From 7760ff19e1651c1e0c45c2150b15217c4232ccb8 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 19 Dec 2023 14:55:19 -0600 Subject: [PATCH 5/7] external auth is just get requests --- coderd/httpmw/csrf.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/coderd/httpmw/csrf.go b/coderd/httpmw/csrf.go index 2d3f47fc961d5..4761646dedaf7 100644 --- a/coderd/httpmw/csrf.go +++ b/coderd/httpmw/csrf.go @@ -20,7 +20,7 @@ func CSRF(secureCookie bool) func(next http.Handler) http.Handler { http.Error(w, "Something is wrong with your CSRF token. Please refresh the page. If this error persists, try clearing your cookies.", http.StatusBadRequest) })) // Exempt all requests that do not require CSRF protection. - // All GET requests are exempt by default and no not need to be added here. + // All GET requests are exempt by default. mw.ExemptPath("/api/v2/csp/reports") // Top level agent routes. @@ -29,9 +29,6 @@ func CSRF(secureCookie bool) func(next http.Handler) http.Handler { mw.ExemptRegexp(regexp.MustCompile("api/v2/workspaceagents/me/*")) // Derp routes mw.ExemptRegexp(regexp.MustCompile("derp/*")) - // Some extra non-auth - mw.ExemptRegexp(regexp.MustCompile("/externa-auth/*")) - mw.ExemptRegexp(regexp.MustCompile("/github/*")) mw.ExemptFunc(func(r *http.Request) bool { // CSRF only affects requests that automatically attach credentials via a cookie. From bc6ba7c840aa0cf99335e7db50dc2aefa10c9cbf Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 19 Dec 2023 15:13:12 -0600 Subject: [PATCH 6/7] Add some more routes --- coderd/httpmw/csrf.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/coderd/httpmw/csrf.go b/coderd/httpmw/csrf.go index 4761646dedaf7..2c64451b59bed 100644 --- a/coderd/httpmw/csrf.go +++ b/coderd/httpmw/csrf.go @@ -23,12 +23,17 @@ func CSRF(secureCookie bool) func(next http.Handler) http.Handler { // All GET requests are exempt by default. mw.ExemptPath("/api/v2/csp/reports") - // Top level agent routes. - mw.ExemptRegexp(regexp.MustCompile("api/v2/workspaceagents/[^/]*$")) // Agent authenticated routes mw.ExemptRegexp(regexp.MustCompile("api/v2/workspaceagents/me/*")) + mw.ExemptRegexp(regexp.MustCompile("api/v2/workspaceagents/*")) + // Workspace Proxy routes + mw.ExemptRegexp(regexp.MustCompile("api/v2/workspaceproxies/me/*")) // Derp routes mw.ExemptRegexp(regexp.MustCompile("derp/*")) + // Scim + mw.ExemptRegexp(regexp.MustCompile("api/v2/scim/*")) + // Provisioner daemon routes + mw.ExemptRegexp(regexp.MustCompile("/organizations/[^/]+/provisionerdaemons/*")) mw.ExemptFunc(func(r *http.Request) bool { // CSRF only affects requests that automatically attach credentials via a cookie. From ce380580b79ace6b3f955db21b9ccd41c590a044 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 19 Dec 2023 15:24:47 -0600 Subject: [PATCH 7/7] Extra assurance nothing breaks --- coderd/httpmw/csrf.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/coderd/httpmw/csrf.go b/coderd/httpmw/csrf.go index 2c64451b59bed..7888365741873 100644 --- a/coderd/httpmw/csrf.go +++ b/coderd/httpmw/csrf.go @@ -57,6 +57,13 @@ func CSRF(secureCookie bool) func(next http.Handler) http.Handler { return true } + if r.Header.Get(codersdk.ProvisionerDaemonPSK) != "" { + // If present, the provisioner daemon also is providing an api key + // that will make them exempt from CSRF. But this is still useful + // for enumerating the external auths. + return true + } + // If the X-CSRF-TOKEN header is set, we can exempt the func if it's valid. // This is the CSRF check. sent := r.Header.Get("X-CSRF-TOKEN")