Skip to content

Commit 3c5222f

Browse files
committed
golink: revert Sec-Fetch-Site xsrftoken replacement
Unfortunately Sec-Fetch-Site headers are not sent over plaintext HTTP making this unsuitable for non-TLS deployments.
1 parent d4fcd49 commit 3c5222f

File tree

8 files changed

+111
-137
lines changed

8 files changed

+111
-137
lines changed

flake.nix

+1-1
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939
"-X tailscale.com/version.longStamp=${tsVersion}"
4040
"-X tailscale.com/version.shortStamp=${tsVersion}"
4141
];
42-
vendorHash = "sha256-PUobfmZyn5YtiFTpxTtjnPhqijR5tsONk4HNlIs1oNk="; # SHA based on vendoring go.mod
42+
vendorHash = "sha256-RqKsIgwkCbIMQdCKtSHqW9eiZuNcZLCYeXFhPbgxjEU="; # SHA based on vendoring go.mod
4343
};
4444
});
4545

go.mod

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ go 1.24.0
44

55
require (
66
github.com/google/go-cmp v0.6.0
7+
golang.org/x/net v0.38.0
78
modernc.org/sqlite v1.19.4
89
tailscale.com v1.82.5
910
)
@@ -76,7 +77,6 @@ require (
7677
golang.org/x/crypto v0.36.0 // indirect
7778
golang.org/x/exp v0.0.0-20250210185358-939b2ce775ac // indirect
7879
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

+59-38
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"context"
1111
"crypto/rand"
1212
"embed"
13+
"encoding/base64"
1314
"encoding/json"
1415
"errors"
1516
"flag"
@@ -29,6 +30,7 @@ import (
2930
texttemplate "text/template"
3031
"time"
3132

33+
"golang.org/x/net/xsrftoken"
3234
"tailscale.com/client/tailscale"
3335
"tailscale.com/hostinfo"
3436
"tailscale.com/ipn"
@@ -39,8 +41,17 @@ import (
3941

4042
const (
4143
defaultHostname = "go"
42-
secFetchSite = "Sec-Fetch-Site"
43-
secGolink = "Sec-Golink"
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"
4455
)
4556

4657
var (
@@ -200,8 +211,6 @@ out:
200211
fqdn := strings.TrimSuffix(status.Self.DNSName, ".")
201212

202213
httpHandler := serveHandler()
203-
httpHandler = EnforceSecFetchSiteOrSecGolink(httpHandler)
204-
205214
if enableTLS {
206215
httpsHandler := HSTS(httpHandler)
207216
httpHandler = redirectHandler(fqdn)
@@ -266,15 +275,19 @@ type homeData struct {
266275
Short string
267276
Long string
268277
Clicks []visitData
278+
XSRF string
269279
ReadOnly bool
270280
}
271281

272282
// deleteData is the data used by deleteTmpl.
273283
type deleteData struct {
274284
Short string
275285
Long string
286+
XSRF string
276287
}
277288

289+
var xsrfKey string
290+
278291
func init() {
279292
homeTmpl = newTemplate("base.html", "home.html")
280293
detailTmpl = newTemplate("base.html", "detail.html")
@@ -286,6 +299,7 @@ func init() {
286299

287300
b := make([]byte, 24)
288301
rand.Read(b)
302+
xsrfKey = base64.StdEncoding.EncodeToString(b)
289303
}
290304

291305
var tmplFuncs = template.FuncMap{
@@ -402,34 +416,6 @@ func HSTS(h http.Handler) http.Handler {
402416
})
403417
}
404418

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-
433419
// serverHandler returns the main http.Handler for serving all requests.
434420
func serveHandler() http.Handler {
435421
mux := http.NewServeMux()
@@ -490,10 +476,16 @@ func serveHome(w http.ResponseWriter, r *http.Request, short string) {
490476
}
491477
}
492478

479+
cu, err := currentUser(r)
480+
if err != nil {
481+
http.Error(w, err.Error(), http.StatusInternalServerError)
482+
return
483+
}
493484
homeTmpl.Execute(w, homeData{
494485
Short: short,
495486
Long: long,
496487
Clicks: clicks,
488+
XSRF: xsrftoken.Generate(xsrfKey, cu.login, newShortName),
497489
ReadOnly: *readonly,
498490
})
499491
}
@@ -605,6 +597,7 @@ type detailData struct {
605597
// Editable indicates whether the current user can edit the link.
606598
Editable bool
607599
Link *Link
600+
XSRF string
608601
}
609602

610603
func serveDetail(w http.ResponseWriter, r *http.Request) {
@@ -648,6 +641,7 @@ func serveDetail(w http.ResponseWriter, r *http.Request) {
648641
data := detailData{
649642
Link: link,
650643
Editable: canEdit,
644+
XSRF: xsrftoken.Generate(xsrfKey, cu.login, link.Short),
651645
}
652646
if canEdit && !ownerExists {
653647
data.Link.Owner = cu.login
@@ -835,6 +829,16 @@ func serveDelete(w http.ResponseWriter, r *http.Request) {
835829
return
836830
}
837831

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+
838842
if err := db.Delete(short); err != nil {
839843
http.Error(w, err.Error(), http.StatusInternalServerError)
840844
return
@@ -844,6 +848,7 @@ func serveDelete(w http.ResponseWriter, r *http.Request) {
844848
deleteTmpl.Execute(w, deleteData{
845849
Short: link.Short,
846850
Long: link.Long,
851+
XSRF: xsrftoken.Generate(xsrfKey, cu.login, newShortName),
847852
})
848853
}
849854

@@ -881,15 +886,20 @@ func serveSave(w http.ResponseWriter, r *http.Request) {
881886
return
882887
}
883888

884-
// Prevent accidental overwrites of existing links.
885-
// If the link already exists, make sure this request is an intentional update.
886-
if link != nil && r.FormValue("update") == "" {
887-
http.Error(w, "link already exists", http.StatusForbidden)
889+
if !canEditLink(r.Context(), link, cu) {
890+
http.Error(w, fmt.Sprintf("cannot update link owned by %q", link.Owner), http.StatusForbidden)
888891
return
889892
}
890893

891-
if !canEditLink(r.Context(), link, cu) {
892-
http.Error(w, fmt.Sprintf("cannot update link owned by %q", link.Owner), http.StatusForbidden)
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)
893903
return
894904
}
895905

@@ -1067,3 +1077,14 @@ func resolveLink(link *url.URL) (*url.URL, error) {
10671077
}
10681078
return dst, err
10691079
}
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)