Protect save and update from CSRF (#117)

Also, fix a lint.

Fixes #116

Signed-off-by: Chris Palmer <cpalmer@tailscale.com>
This commit is contained in:
Chris Palmer
2024-04-01 18:20:58 -07:00
committed by GitHub
parent 6c79b503e1
commit c66cbb8eca
6 changed files with 89 additions and 8 deletions
+63 -5
View File
@@ -39,7 +39,20 @@ import (
"tailscale.com/util/dnsname"
)
const defaultHostname = "go"
const (
defaultHostname = "go"
// Used as a placeholder short name for generating the XSRF defense token,
// when creating new links.
newShortName = ".new"
// If the caller sends this header set to a non-empty value, we will allow
// them to make the call even without an XSRF token. JavaScript in browser
// cannot set this header, per the [Fetch Spec].
//
// [Fetch Spec]: https://fetch.spec.whatwg.org
secHeaderName = "Sec-Golink"
)
var (
verbose = flag.Bool("verbose", false, "be verbose")
@@ -254,11 +267,19 @@ type visitData struct {
NumClicks int
}
// homeData is the data used by the homeTmpl template.
// homeData is the data used by homeTmpl.
type homeData struct {
Short string
Long string
Clicks []visitData
XSRF string
}
// deleteData is the data used by deleteTmpl.
type deleteData struct {
Short string
Long string
XSRF string
}
var xsrfKey string
@@ -416,10 +437,16 @@ func serveHome(w http.ResponseWriter, r *http.Request, short string) {
}
}
cu, err := currentUser(r)
if err != nil {
http.Error(w, err.Error(), http.StatusInternalServerError)
return
}
homeTmpl.Execute(w, homeData{
Short: short,
Long: long,
Clicks: clicks,
XSRF: xsrftoken.Generate(xsrfKey, cu.login, newShortName),
})
}
@@ -739,6 +766,11 @@ func serveDelete(w http.ResponseWriter, r *http.Request) {
return
}
// Deletion by CLI has never worked because it has always required the XSRF
// token. (Refer to commit c7ac33d04c33743606f6224009a5c73aa0b8dec0.) If we
// want to enable deletion via CLI and to honor allowUnknownUsers for
// deletion, we could change the below to a call to isRequestAuthorized. For
// now, always require the XSRF token, thus maintaining the status quo.
if !xsrftoken.Valid(r.PostFormValue("xsrf"), xsrfKey, cu.login, short) {
http.Error(w, "invalid XSRF token", http.StatusBadRequest)
return
@@ -750,7 +782,11 @@ func serveDelete(w http.ResponseWriter, r *http.Request) {
}
deleteLinkStats(link)
deleteTmpl.Execute(w, link)
deleteTmpl.Execute(w, deleteData{
Short: link.Short,
Long: link.Long,
XSRF: xsrftoken.Generate(xsrfKey, cu.login, newShortName),
})
}
// serveSave handles requests to save or update a Link. Both short name and
@@ -787,6 +823,11 @@ func serveSave(w http.ResponseWriter, r *http.Request) {
return
}
if !isRequestAuthorized(r, cu, short) {
http.Error(w, "invalid XSRF token", http.StatusBadRequest)
return
}
// allow transferring ownership to valid users. If empty, set owner to current user.
owner := r.FormValue("owner")
if owner != "" {
@@ -886,8 +927,7 @@ func restoreLastSnapshot() error {
_, err := db.Load(link.Short)
if err == nil {
continue // exists
}
if err != nil && !errors.Is(err, fs.ErrNotExist) {
} else if !errors.Is(err, fs.ErrNotExist) {
return err
}
if err := db.Save(link); err != nil {
@@ -923,3 +963,21 @@ func resolveLink(link *url.URL) (*url.URL, error) {
}
return dst, err
}
func isRequestAuthorized(r *http.Request, u user, short string) bool {
if *allowUnknownUsers {
return true
}
if r.Header.Get(secHeaderName) != "" {
return true
}
// If the request is to create a new link, test the XSRF token against
// newShortName instead of short.
tokenShortName := short
_, err := db.Load(short)
if r.URL.Path == "/" && r.Method == http.MethodPost && errors.Is(err, fs.ErrNotExist) {
tokenShortName = newShortName
}
return xsrftoken.Valid(r.PostFormValue("xsrf"), xsrfKey, u.login, tokenShortName)
}
+22 -2
View File
@@ -129,12 +129,19 @@ func TestServeSave(t *testing.T) {
if err != nil {
t.Fatal(err)
}
db.Save(&Link{Short: "link-owned-by-tagged-devices", Long: "/before", Owner: "tagged-devices"})
fooXSRF := func(short string) string {
return xsrftoken.Generate(xsrfKey, "foo@example.com", short)
}
barXSRF := func(short string) string {
return xsrftoken.Generate(xsrfKey, "bar@example.com", short)
}
tests := []struct {
name string
short string
xsrf string
long string
allowUnknownUsers bool
currentUser func(*http.Request) (user, error)
@@ -155,12 +162,14 @@ func TestServeSave(t *testing.T) {
{
name: "save simple link",
short: "who",
xsrf: fooXSRF(newShortName),
long: "http://who/",
wantStatus: http.StatusOK,
},
{
name: "disallow editing another's link",
short: "who",
xsrf: barXSRF("who"),
long: "http://who/",
currentUser: func(*http.Request) (user, error) { return user{login: "bar@example.com"}, nil },
wantStatus: http.StatusForbidden,
@@ -168,6 +177,7 @@ func TestServeSave(t *testing.T) {
{
name: "allow editing link owned by tagged-devices",
short: "link-owned-by-tagged-devices",
xsrf: barXSRF("link-owned-by-tagged-devices"),
long: "/after",
currentUser: func(*http.Request) (user, error) { return user{login: "bar@example.com"}, nil },
wantStatus: http.StatusOK,
@@ -175,6 +185,7 @@ func TestServeSave(t *testing.T) {
{
name: "admins can edit any link",
short: "who",
xsrf: barXSRF("who"),
long: "http://who/",
currentUser: func(*http.Request) (user, error) { return user{login: "bar@example.com", isAdmin: true}, nil },
wantStatus: http.StatusOK,
@@ -182,6 +193,7 @@ func TestServeSave(t *testing.T) {
{
name: "disallow unknown users",
short: "who2",
xsrf: fooXSRF("who2"),
long: "http://who/",
currentUser: func(*http.Request) (user, error) { return user{}, errors.New("") },
wantStatus: http.StatusInternalServerError,
@@ -194,6 +206,13 @@ func TestServeSave(t *testing.T) {
currentUser: func(*http.Request) (user, error) { return user{}, nil },
wantStatus: http.StatusOK,
},
{
name: "invalid xsrf",
short: "goat",
xsrf: fooXSRF("sheep"),
long: "https://goat.example.com/goat.php?goat=true",
wantStatus: http.StatusBadRequest,
},
}
for _, tt := range tests {
@@ -213,6 +232,7 @@ func TestServeSave(t *testing.T) {
r := httptest.NewRequest("POST", "/", strings.NewReader(url.Values{
"short": {tt.short},
"long": {tt.long},
"xsrf": {tt.xsrf},
}.Encode()))
r.Header.Set("Content-Type", "application/x-www-form-urlencoded")
w := httptest.NewRecorder()
@@ -252,7 +272,7 @@ func TestServeDelete(t *testing.T) {
wantStatus: http.StatusBadRequest,
},
{
name: "non-existant link",
name: "nonexistent link",
short: "does-not-exist",
wantStatus: http.StatusNotFound,
},
+1
View File
@@ -4,6 +4,7 @@
<p class="py-4">Deleted this by mistake? You can recreate the same link below.</p>
<form method="POST" action="/">
<input type="hidden" name="xsrf" value="{{ .XSRF }}" />
<div class="flex flex-wrap">
<div class="flex">
<label for=short class="flex my-2 px-2 items-center bg-gray-100 border border-r-0 border-gray-300 rounded-l-md text-gray-700">http://go/</label>
+1
View File
@@ -3,6 +3,7 @@
{{ if .Editable }}
<form method="POST" action="/">
<input type="hidden" name="xsrf" value="{{ .XSRF }}" />
<div class="flex flex-wrap">
<div class="flex">
<label for=short class="flex my-2 px-2 items-center bg-gray-100 border border-r-0 border-gray-300 rounded-l-md text-gray-700">http://go/</label>
+1 -1
View File
@@ -122,7 +122,7 @@ This is useful to create data snapshots that can be restored later.
<p>
Create a new link by sending a POST request with a <code>short</code> and <code>long</code> value:
<pre>{{`$ curl -L -d short=cs -d long=https://cs.github.com/ go
<pre>{{`$ curl -L -H Sec-Golink:1 -d short=cs -d long=https://cs.github.com/ go
{"Short":"cs","Long":"https://cs.github.com/","Created":"2022-06-03T22:15:29.993978392Z","LastEdit":"2022-06-03T22:15:29.993978392Z","Owner":"amelie@example.com"}`}}
</pre>
+1
View File
@@ -5,6 +5,7 @@
<p class="">Did you mean <a class="text-blue-600 hover:underline" href="{{.}}">{{.}}</a> ? Create a go link for it now:</p>
{{ end }}
<form method="POST" action="/" class="flex flex-wrap">
<input type="hidden" name="xsrf" value="{{ .XSRF }}" />
<div class="flex">
<label for=short class="flex my-2 px-2 items-center bg-gray-100 border border-r-0 border-gray-300 rounded-l-md text-gray-700">http://go/</label>
<input id=short name=short required type=text size=15 placeholder="shortname" value="{{.Short}}" pattern="\w[\w\-\.]*" title="Must start with letter or number; may contain letters, numbers, dashes, and periods."