Skip to content

Commit 9b5ee8f

Browse files
Emyrkpresleyp
Emyrk
and
presleyp
authored
feat: Implement (but not enforce) CSRF for FE requests (#3786)
Future work is to enforce CSRF Co-authored-by: Presley Pizzo <presley@coder.com>
1 parent 9ab437d commit 9b5ee8f

22 files changed

+211
-115
lines changed

‎coderd/coderd.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,7 @@ func New(options *Options) *API {
187187
next.ServeHTTP(w, r)
188188
})
189189
},
190+
httpmw.CSRF(options.SecureAuthCookie),
190191
)
191192

192193
apps := func(r chi.Router) {

‎coderd/httpapi/cookie_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,13 @@ func TestStripCoderCookies(t *testing.T) {
1717
"testing=hello; wow=test",
1818
"testing=hello; wow=test",
1919
}, {
20-
"session_token=moo; wow=test",
20+
"coder_session_token=moo; wow=test",
2121
"wow=test",
2222
}, {
23-
"another_token=wow; session_token=ok",
23+
"another_token=wow; coder_session_token=ok",
2424
"another_token=wow",
2525
}, {
26-
"session_token=ok; oauth_state=wow; oauth_redirect=/",
26+
"coder_session_token=ok; oauth_state=wow; oauth_redirect=/",
2727
"",
2828
}} {
2929
tc := tc

‎coderd/httpmw/apikey.go

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -123,13 +123,7 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool
123123
httpapi.Write(rw, code, response)
124124
}
125125

126-
var cookieValue string
127-
cookie, err := r.Cookie(codersdk.SessionTokenKey)
128-
if err != nil {
129-
cookieValue = r.URL.Query().Get(codersdk.SessionTokenKey)
130-
} else {
131-
cookieValue = cookie.Value
132-
}
126+
cookieValue := apiTokenFromRequest(r)
133127
if cookieValue == "" {
134128
write(http.StatusUnauthorized, codersdk.Response{
135129
Message: signedOutErrorMessage,
@@ -335,3 +329,36 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool
335329
})
336330
}
337331
}
332+
333+
// apiTokenFromRequest returns the api token from the request.
334+
// Find the session token from:
335+
// 1: The cookie
336+
// 2: The old cookie
337+
// 3. The coder_session_token query parameter
338+
// 4. The custom auth header
339+
func apiTokenFromRequest(r *http.Request) string {
340+
cookie, err := r.Cookie(codersdk.SessionTokenKey)
341+
if err == nil && cookie.Value != "" {
342+
return cookie.Value
343+
}
344+
345+
// TODO: @emyrk in October 2022, remove this oldCookie check.
346+
// This is just to support the old cli for 1 release. Then everyone
347+
// must update.
348+
oldCookie, err := r.Cookie("session_token")
349+
if err == nil && oldCookie.Value != "" {
350+
return oldCookie.Value
351+
}
352+
353+
urlValue := r.URL.Query().Get(codersdk.SessionTokenKey)
354+
if urlValue != "" {
355+
return urlValue
356+
}
357+
358+
headerValue := r.Header.Get(codersdk.SessionCustomHeader)
359+
if headerValue != "" {
360+
return headerValue
361+
}
362+
363+
return ""
364+
}

‎coderd/httpmw/apikey_test.go

Lines changed: 12 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,7 @@ func TestAPIKey(t *testing.T) {
7474
r = httptest.NewRequest("GET", "/", nil)
7575
rw = httptest.NewRecorder()
7676
)
77-
r.AddCookie(&http.Cookie{
78-
Name: codersdk.SessionTokenKey,
79-
Value: "test-wow-hello",
80-
})
77+
r.Header.Set(codersdk.SessionCustomHeader, "test-wow-hello")
8178

8279
httpmw.ExtractAPIKey(db, nil, false)(successHandler).ServeHTTP(rw, r)
8380
res := rw.Result()
@@ -92,10 +89,7 @@ func TestAPIKey(t *testing.T) {
9289
r = httptest.NewRequest("GET", "/", nil)
9390
rw = httptest.NewRecorder()
9491
)
95-
r.AddCookie(&http.Cookie{
96-
Name: codersdk.SessionTokenKey,
97-
Value: "test-wow",
98-
})
92+
r.Header.Set(codersdk.SessionCustomHeader, "test-wow")
9993

10094
httpmw.ExtractAPIKey(db, nil, false)(successHandler).ServeHTTP(rw, r)
10195
res := rw.Result()
@@ -110,10 +104,7 @@ func TestAPIKey(t *testing.T) {
110104
r = httptest.NewRequest("GET", "/", nil)
111105
rw = httptest.NewRecorder()
112106
)
113-
r.AddCookie(&http.Cookie{
114-
Name: codersdk.SessionTokenKey,
115-
Value: "testtestid-wow",
116-
})
107+
r.Header.Set(codersdk.SessionCustomHeader, "testtestid-wow")
117108

118109
httpmw.ExtractAPIKey(db, nil, false)(successHandler).ServeHTTP(rw, r)
119110
res := rw.Result()
@@ -129,10 +120,7 @@ func TestAPIKey(t *testing.T) {
129120
r = httptest.NewRequest("GET", "/", nil)
130121
rw = httptest.NewRecorder()
131122
)
132-
r.AddCookie(&http.Cookie{
133-
Name: codersdk.SessionTokenKey,
134-
Value: fmt.Sprintf("%s-%s", id, secret),
135-
})
123+
r.Header.Set(codersdk.SessionCustomHeader, fmt.Sprintf("%s-%s", id, secret))
136124

137125
httpmw.ExtractAPIKey(db, nil, false)(successHandler).ServeHTTP(rw, r)
138126
res := rw.Result()
@@ -149,10 +137,7 @@ func TestAPIKey(t *testing.T) {
149137
rw = httptest.NewRecorder()
150138
user = createUser(r.Context(), t, db)
151139
)
152-
r.AddCookie(&http.Cookie{
153-
Name: codersdk.SessionTokenKey,
154-
Value: fmt.Sprintf("%s-%s", id, secret),
155-
})
140+
r.Header.Set(codersdk.SessionCustomHeader, fmt.Sprintf("%s-%s", id, secret))
156141

157142
// Use a different secret so they don't match!
158143
hashed := sha256.Sum256([]byte("differentsecret"))
@@ -178,10 +163,7 @@ func TestAPIKey(t *testing.T) {
178163
rw = httptest.NewRecorder()
179164
user = createUser(r.Context(), t, db)
180165
)
181-
r.AddCookie(&http.Cookie{
182-
Name: codersdk.SessionTokenKey,
183-
Value: fmt.Sprintf("%s-%s", id, secret),
184-
})
166+
r.Header.Set(codersdk.SessionCustomHeader, fmt.Sprintf("%s-%s", id, secret))
185167

186168
_, err := db.InsertAPIKey(r.Context(), database.InsertAPIKeyParams{
187169
ID: id,
@@ -206,10 +188,7 @@ func TestAPIKey(t *testing.T) {
206188
rw = httptest.NewRecorder()
207189
user = createUser(r.Context(), t, db)
208190
)
209-
r.AddCookie(&http.Cookie{
210-
Name: codersdk.SessionTokenKey,
211-
Value: fmt.Sprintf("%s-%s", id, secret),
212-
})
191+
r.Header.Set(codersdk.SessionCustomHeader, fmt.Sprintf("%s-%s", id, secret))
213192

214193
sentAPIKey, err := db.InsertAPIKey(r.Context(), database.InsertAPIKeyParams{
215194
ID: id,
@@ -280,10 +259,7 @@ func TestAPIKey(t *testing.T) {
280259
rw = httptest.NewRecorder()
281260
user = createUser(r.Context(), t, db)
282261
)
283-
r.AddCookie(&http.Cookie{
284-
Name: codersdk.SessionTokenKey,
285-
Value: fmt.Sprintf("%s-%s", id, secret),
286-
})
262+
r.Header.Set(codersdk.SessionCustomHeader, fmt.Sprintf("%s-%s", id, secret))
287263

288264
sentAPIKey, err := db.InsertAPIKey(r.Context(), database.InsertAPIKeyParams{
289265
ID: id,
@@ -316,10 +292,7 @@ func TestAPIKey(t *testing.T) {
316292
rw = httptest.NewRecorder()
317293
user = createUser(r.Context(), t, db)
318294
)
319-
r.AddCookie(&http.Cookie{
320-
Name: codersdk.SessionTokenKey,
321-
Value: fmt.Sprintf("%s-%s", id, secret),
322-
})
295+
r.Header.Set(codersdk.SessionCustomHeader, fmt.Sprintf("%s-%s", id, secret))
323296

324297
sentAPIKey, err := db.InsertAPIKey(r.Context(), database.InsertAPIKeyParams{
325298
ID: id,
@@ -352,10 +325,7 @@ func TestAPIKey(t *testing.T) {
352325
rw = httptest.NewRecorder()
353326
user = createUser(r.Context(), t, db)
354327
)
355-
r.AddCookie(&http.Cookie{
356-
Name: codersdk.SessionTokenKey,
357-
Value: fmt.Sprintf("%s-%s", id, secret),
358-
})
328+
r.Header.Set(codersdk.SessionCustomHeader, fmt.Sprintf("%s-%s", id, secret))
359329

360330
sentAPIKey, err := db.InsertAPIKey(r.Context(), database.InsertAPIKeyParams{
361331
ID: id,
@@ -395,10 +365,7 @@ func TestAPIKey(t *testing.T) {
395365
rw = httptest.NewRecorder()
396366
user = createUser(r.Context(), t, db)
397367
)
398-
r.AddCookie(&http.Cookie{
399-
Name: codersdk.SessionTokenKey,
400-
Value: fmt.Sprintf("%s-%s", id, secret),
401-
})
368+
r.Header.Set(codersdk.SessionCustomHeader, fmt.Sprintf("%s-%s", id, secret))
402369

403370
sentAPIKey, err := db.InsertAPIKey(r.Context(), database.InsertAPIKeyParams{
404371
ID: id,
@@ -449,10 +416,7 @@ func TestAPIKey(t *testing.T) {
449416
user = createUser(r.Context(), t, db)
450417
)
451418
r.RemoteAddr = "1.1.1.1:3555"
452-
r.AddCookie(&http.Cookie{
453-
Name: codersdk.SessionTokenKey,
454-
Value: fmt.Sprintf("%s-%s", id, secret),
455-
})
419+
r.Header.Set(codersdk.SessionCustomHeader, fmt.Sprintf("%s-%s", id, secret))
456420

457421
_, err := db.InsertAPIKey(r.Context(), database.InsertAPIKeyParams{
458422
ID: id,

‎coderd/httpmw/authorize_test.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,7 @@ func TestExtractUserRoles(t *testing.T) {
9393
})
9494

9595
req := httptest.NewRequest("GET", "/", nil)
96-
req.AddCookie(&http.Cookie{
97-
Name: codersdk.SessionTokenKey,
98-
Value: token,
99-
})
96+
req.Header.Set(codersdk.SessionCustomHeader, token)
10097

10198
rtr.ServeHTTP(rw, req)
10299
resp := rw.Result()

‎coderd/httpmw/csrf.go

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
package httpmw
2+
3+
import (
4+
"net/http"
5+
"regexp"
6+
7+
"github.com/justinas/nosurf"
8+
"golang.org/x/xerrors"
9+
10+
"github.com/coder/coder/codersdk"
11+
)
12+
13+
// CSRF is a middleware that verifies that a CSRF token is present in the request
14+
// for non-GET requests.
15+
func CSRF(secureCookie bool) func(next http.Handler) http.Handler {
16+
return func(next http.Handler) http.Handler {
17+
mw := nosurf.New(next)
18+
mw.SetBaseCookie(http.Cookie{Path: "/", HttpOnly: true, SameSite: http.SameSiteLaxMode, Secure: secureCookie})
19+
mw.SetFailureHandler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
20+
http.Error(w, "Something is wrong with your CSRF token. Please refresh the page. If this error persists, try clearing your cookies.", http.StatusBadRequest)
21+
}))
22+
23+
// Exempt all requests that do not require CSRF protection.
24+
// All GET requests are exempt by default.
25+
mw.ExemptPath("/api/v2/csp/reports")
26+
27+
// Top level agent routes.
28+
mw.ExemptRegexp(regexp.MustCompile("api/v2/workspaceagents/[^/]*$"))
29+
// Agent authenticated routes
30+
mw.ExemptRegexp(regexp.MustCompile("api/v2/workspaceagents/me/*"))
31+
// Derp routes
32+
mw.ExemptRegexp(regexp.MustCompile("derp/*"))
33+
34+
mw.ExemptFunc(func(r *http.Request) bool {
35+
// Enable CSRF in November 2022 by deleting this "return true" line.
36+
// CSRF is not enforced to ensure backwards compatibility with older
37+
// cli versions.
38+
//nolint:revive
39+
return true
40+
41+
// CSRF only affects requests that automatically attach credentials via a cookie.
42+
// If no cookie is present, then there is no risk of CSRF.
43+
//nolint:govet
44+
sessCookie, err := r.Cookie(codersdk.SessionTokenKey)
45+
if xerrors.Is(err, http.ErrNoCookie) {
46+
return true
47+
}
48+
49+
if token := r.Header.Get(codersdk.SessionCustomHeader); token == sessCookie.Value {
50+
// If the cookie and header match, we can assume this is the same as just using the
51+
// custom header auth. Custom header auth can bypass CSRF, as CSRF attacks
52+
// cannot add custom headers.
53+
return true
54+
}
55+
56+
if token := r.URL.Query().Get(codersdk.SessionTokenKey); token == sessCookie.Value {
57+
// If the auth is set in a url param and matches the cookie, it
58+
// is the same as just using the url param.
59+
return true
60+
}
61+
62+
// If the X-CSRF-TOKEN header is set, we can exempt the func if it's valid.
63+
// This is the CSRF check.
64+
sent := r.Header.Get("X-CSRF-TOKEN")
65+
if sent != "" {
66+
return nosurf.VerifyToken(nosurf.Token(r), sent)
67+
}
68+
return false
69+
})
70+
return mw
71+
}
72+
}

‎coderd/httpmw/organizationparam_test.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,7 @@ func TestOrganizationParam(t *testing.T) {
2929
r = httptest.NewRequest("GET", "/", nil)
3030
hashed = sha256.Sum256([]byte(secret))
3131
)
32-
r.AddCookie(&http.Cookie{
33-
Name: codersdk.SessionTokenKey,
34-
Value: fmt.Sprintf("%s-%s", id, secret),
35-
})
32+
r.Header.Set(codersdk.SessionCustomHeader, fmt.Sprintf("%s-%s", id, secret))
3633

3734
userID := uuid.New()
3835
username, err := cryptorand.String(8)

‎coderd/httpmw/templateparam_test.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,7 @@ func TestTemplateParam(t *testing.T) {
2929
hashed = sha256.Sum256([]byte(secret))
3030
)
3131
r := httptest.NewRequest("GET", "/", nil)
32-
r.AddCookie(&http.Cookie{
33-
Name: codersdk.SessionTokenKey,
34-
Value: fmt.Sprintf("%s-%s", id, secret),
35-
})
32+
r.Header.Set(codersdk.SessionCustomHeader, fmt.Sprintf("%s-%s", id, secret))
3633

3734
userID := uuid.New()
3835
username, err := cryptorand.String(8)

‎coderd/httpmw/templateversionparam_test.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,7 @@ func TestTemplateVersionParam(t *testing.T) {
2929
hashed = sha256.Sum256([]byte(secret))
3030
)
3131
r := httptest.NewRequest("GET", "/", nil)
32-
r.AddCookie(&http.Cookie{
33-
Name: codersdk.SessionTokenKey,
34-
Value: fmt.Sprintf("%s-%s", id, secret),
35-
})
32+
r.Header.Set(codersdk.SessionCustomHeader, fmt.Sprintf("%s-%s", id, secret))
3633

3734
userID := uuid.New()
3835
username, err := cryptorand.String(8)

‎coderd/httpmw/userparam_test.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,7 @@ func TestUserParam(t *testing.T) {
2929
r = httptest.NewRequest("GET", "/", nil)
3030
rw = httptest.NewRecorder()
3131
)
32-
r.AddCookie(&http.Cookie{
33-
Name: codersdk.SessionTokenKey,
34-
Value: fmt.Sprintf("%s-%s", id, secret),
35-
})
32+
r.Header.Set(codersdk.SessionCustomHeader, fmt.Sprintf("%s-%s", id, secret))
3633

3734
user, err := db.InsertUser(r.Context(), database.InsertUserParams{
3835
ID: uuid.New(),

‎coderd/httpmw/workspaceagent.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,14 @@ func WorkspaceAgent(r *http.Request) database.WorkspaceAgent {
2929
func ExtractWorkspaceAgent(db database.Store) func(http.Handler) http.Handler {
3030
return func(next http.Handler) http.Handler {
3131
return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
32-
cookie, err := r.Cookie(codersdk.SessionTokenKey)
33-
if err != nil {
32+
cookieValue := apiTokenFromRequest(r)
33+
if cookieValue == "" {
3434
httpapi.Write(rw, http.StatusUnauthorized, codersdk.Response{
3535
Message: fmt.Sprintf("Cookie %q must be provided.", codersdk.SessionTokenKey),
3636
})
3737
return
3838
}
39-
token, err := uuid.Parse(cookie.Value)
39+
token, err := uuid.Parse(cookieValue)
4040
if err != nil {
4141
httpapi.Write(rw, http.StatusUnauthorized, codersdk.Response{
4242
Message: "Agent token is invalid.",

‎coderd/httpmw/workspaceagent_test.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,7 @@ func TestWorkspaceAgent(t *testing.T) {
2222
setup := func(db database.Store) (*http.Request, uuid.UUID) {
2323
token := uuid.New()
2424
r := httptest.NewRequest("GET", "/", nil)
25-
r.AddCookie(&http.Cookie{
26-
Name: codersdk.SessionTokenKey,
27-
Value: token.String(),
28-
})
25+
r.Header.Set(codersdk.SessionCustomHeader, token.String())
2926
return r, token
3027
}
3128

‎coderd/httpmw/workspaceagentparam_test.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,7 @@ func TestWorkspaceAgentParam(t *testing.T) {
2929
hashed = sha256.Sum256([]byte(secret))
3030
)
3131
r := httptest.NewRequest("GET", "/", nil)
32-
r.AddCookie(&http.Cookie{
33-
Name: codersdk.SessionTokenKey,
34-
Value: fmt.Sprintf("%s-%s", id, secret),
35-
})
32+
r.Header.Set(codersdk.SessionCustomHeader, fmt.Sprintf("%s-%s", id, secret))
3633

3734
userID := uuid.New()
3835
username, err := cryptorand.String(8)

0 commit comments

Comments
 (0)