Skip to content

Commit 676a032

Browse files
committed
golink: replace xsrftoken with Sec-Fetch-Site validation
Deprecate the use of xsrftokens entirely and replace them with requiring clients to send [Sec-Fetch-Site](https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Sec-Fetch-Site) fetch metadata request headers that clarify the relationship between the initating and requested origin. Add tests for Sec-Golink behavior which continues to be permitted as a substitute for the new CSRF protection as was for the previous one. Signed-off-by: Patrick O'Doherty <[email protected]>
1 parent bfae094 commit 676a032

File tree

3 files changed

+103
-105
lines changed

3 files changed

+103
-105
lines changed

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ go 1.24.0
44

55
require (
66
github.com/google/go-cmp v0.6.0
7-
golang.org/x/net v0.38.0
87
modernc.org/sqlite v1.19.4
98
tailscale.com v1.82.5
109
)
@@ -77,6 +76,7 @@ require (
7776
golang.org/x/crypto v0.36.0 // indirect
7877
golang.org/x/exp v0.0.0-20250210185358-939b2ce775ac // indirect
7978
golang.org/x/mod v0.23.0 // indirect
79+
golang.org/x/net v0.38.0 // indirect
8080
golang.org/x/sync v0.12.0 // indirect
8181
golang.org/x/sys v0.31.0 // indirect
8282
golang.org/x/term v0.30.0 // indirect

golink.go

Lines changed: 32 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010
"context"
1111
"crypto/rand"
1212
"embed"
13-
"encoding/base64"
1413
"encoding/json"
1514
"errors"
1615
"flag"
@@ -30,7 +29,6 @@ import (
3029
texttemplate "text/template"
3130
"time"
3231

33-
"golang.org/x/net/xsrftoken"
3432
"tailscale.com/client/tailscale"
3533
"tailscale.com/hostinfo"
3634
"tailscale.com/ipn"
@@ -41,17 +39,8 @@ import (
4139

4240
const (
4341
defaultHostname = "go"
44-
45-
// Used as a placeholder short name for generating the XSRF defense token,
46-
// when creating new links.
47-
newShortName = ".new"
48-
49-
// If the caller sends this header set to a non-empty value, we will allow
50-
// them to make the call even without an XSRF token. JavaScript in browser
51-
// cannot set this header, per the [Fetch Spec].
52-
//
53-
// [Fetch Spec]: https://fetch.spec.whatwg.org
54-
secHeaderName = "Sec-Golink"
42+
secFetchSite = "Sec-Fetch-Site"
43+
secGolink = "Sec-Golink"
5544
)
5645

5746
var (
@@ -211,6 +200,8 @@ out:
211200
fqdn := strings.TrimSuffix(status.Self.DNSName, ".")
212201

213202
httpHandler := serveHandler()
203+
httpHandler = EnforceSecFetchSiteOrSecGolink(httpHandler)
204+
214205
if enableTLS {
215206
httpsHandler := HSTS(httpHandler)
216207
httpHandler = redirectHandler(fqdn)
@@ -275,19 +266,15 @@ type homeData struct {
275266
Short string
276267
Long string
277268
Clicks []visitData
278-
XSRF string
279269
ReadOnly bool
280270
}
281271

282272
// deleteData is the data used by deleteTmpl.
283273
type deleteData struct {
284274
Short string
285275
Long string
286-
XSRF string
287276
}
288277

289-
var xsrfKey string
290-
291278
func init() {
292279
homeTmpl = newTemplate("base.html", "home.html")
293280
detailTmpl = newTemplate("base.html", "detail.html")
@@ -299,7 +286,6 @@ func init() {
299286

300287
b := make([]byte, 24)
301288
rand.Read(b)
302-
xsrfKey = base64.StdEncoding.EncodeToString(b)
303289
}
304290

305291
var tmplFuncs = template.FuncMap{
@@ -416,6 +402,34 @@ func HSTS(h http.Handler) http.Handler {
416402
})
417403
}
418404

405+
// EnforceSecFetchSiteOrSecGolink is a Cross-Site Request Forgery protection
406+
// middleware that validates the Sec-Fetch-Site header for non-idempotent
407+
// requests. It requires clients to send Sec-Fetch-Site set to "same-origin".
408+
//
409+
// It alternatively allows for clients to send the header "Sec-Golink" set to
410+
// any value to maintain compatibility with clients developed against earlier
411+
// versions of golink that relied on xsrf token based CSRF protection.
412+
func EnforceSecFetchSiteOrSecGolink(h http.Handler) http.Handler {
413+
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
414+
switch r.Method {
415+
case "GET", "HEAD", "OPTIONS": // allow idempotent methods
416+
h.ServeHTTP(w, r)
417+
return
418+
}
419+
420+
// Check for Sec-Fetch-Site header set to "same-origin"
421+
// or Sec-Golink header set to any value for backwards compatibility.
422+
sameOrigin := r.Header.Get(secFetchSite) == "same-origin"
423+
secGolink := r.Header.Get(secGolink) != ""
424+
if sameOrigin || secGolink {
425+
h.ServeHTTP(w, r)
426+
return
427+
}
428+
429+
http.Error(w, "invalid non `Sec-Fetch-Site: same-origin` request", http.StatusBadRequest)
430+
})
431+
}
432+
419433
// serverHandler returns the main http.Handler for serving all requests.
420434
func serveHandler() http.Handler {
421435
mux := http.NewServeMux()
@@ -476,16 +490,10 @@ func serveHome(w http.ResponseWriter, r *http.Request, short string) {
476490
}
477491
}
478492

479-
cu, err := currentUser(r)
480-
if err != nil {
481-
http.Error(w, err.Error(), http.StatusInternalServerError)
482-
return
483-
}
484493
homeTmpl.Execute(w, homeData{
485494
Short: short,
486495
Long: long,
487496
Clicks: clicks,
488-
XSRF: xsrftoken.Generate(xsrfKey, cu.login, newShortName),
489497
ReadOnly: *readonly,
490498
})
491499
}
@@ -597,7 +605,6 @@ type detailData struct {
597605
// Editable indicates whether the current user can edit the link.
598606
Editable bool
599607
Link *Link
600-
XSRF string
601608
}
602609

603610
func serveDetail(w http.ResponseWriter, r *http.Request) {
@@ -641,7 +648,6 @@ func serveDetail(w http.ResponseWriter, r *http.Request) {
641648
data := detailData{
642649
Link: link,
643650
Editable: canEdit,
644-
XSRF: xsrftoken.Generate(xsrfKey, cu.login, link.Short),
645651
}
646652
if canEdit && !ownerExists {
647653
data.Link.Owner = cu.login
@@ -829,16 +835,6 @@ func serveDelete(w http.ResponseWriter, r *http.Request) {
829835
return
830836
}
831837

832-
// Deletion by CLI has never worked because it has always required the XSRF
833-
// token. (Refer to commit c7ac33d04c33743606f6224009a5c73aa0b8dec0.) If we
834-
// want to enable deletion via CLI and to honor allowUnknownUsers for
835-
// deletion, we could change the below to a call to isRequestAuthorized. For
836-
// now, always require the XSRF token, thus maintaining the status quo.
837-
if !xsrftoken.Valid(r.PostFormValue("xsrf"), xsrfKey, cu.login, link.Short) {
838-
http.Error(w, "invalid XSRF token", http.StatusBadRequest)
839-
return
840-
}
841-
842838
if err := db.Delete(short); err != nil {
843839
http.Error(w, err.Error(), http.StatusInternalServerError)
844840
return
@@ -848,7 +844,6 @@ func serveDelete(w http.ResponseWriter, r *http.Request) {
848844
deleteTmpl.Execute(w, deleteData{
849845
Short: link.Short,
850846
Long: link.Long,
851-
XSRF: xsrftoken.Generate(xsrfKey, cu.login, newShortName),
852847
})
853848
}
854849

@@ -891,18 +886,6 @@ func serveSave(w http.ResponseWriter, r *http.Request) {
891886
return
892887
}
893888

894-
// short name to use for XSRF token.
895-
// For new link creation, the special newShortName value is used.
896-
tokenShortName := newShortName
897-
if link != nil {
898-
tokenShortName = link.Short
899-
}
900-
901-
if !isRequestAuthorized(r, cu, tokenShortName) {
902-
http.Error(w, "invalid XSRF token", http.StatusBadRequest)
903-
return
904-
}
905-
906889
// allow transferring ownership to valid users. If empty, set owner to current user.
907890
owner := r.FormValue("owner")
908891
if owner != "" {
@@ -1077,14 +1060,3 @@ func resolveLink(link *url.URL) (*url.URL, error) {
10771060
}
10781061
return dst, err
10791062
}
1080-
1081-
func isRequestAuthorized(r *http.Request, u user, short string) bool {
1082-
if *allowUnknownUsers {
1083-
return true
1084-
}
1085-
if r.Header.Get(secHeaderName) != "" {
1086-
return true
1087-
}
1088-
1089-
return xsrftoken.Valid(r.PostFormValue("xsrf"), xsrfKey, u.login, short)
1090-
}

0 commit comments

Comments
 (0)