From 077bbb840302e6f72a4df74d68066a60d64bd875 Mon Sep 17 00:00:00 2001 From: Sonia Appasamy Date: Wed, 16 Aug 2023 18:52:31 -0400 Subject: [PATCH] client/web: add csrf protection to web client api Adds csrf protection and hooks up an initial POST request from the React web client. Updates tailscale/corp#13775 Signed-off-by: Sonia Appasamy --- client/web/api.go | 41 +++++++++++ client/web/package.json | 2 + client/web/src/api.ts | 32 +++++++++ client/web/src/components/app.tsx | 6 +- client/web/src/components/legacy.tsx | 46 ++++++------ client/web/src/hooks/node-data.ts | 103 +++++++++++++++++++++++++-- client/web/web.go | 37 ++++++---- client/web/yarn.lock | 12 ++++ cmd/tailscale/depaware.txt | 6 +- go.mod | 3 + go.sum | 4 ++ 11 files changed, 245 insertions(+), 47 deletions(-) create mode 100644 client/web/api.go create mode 100644 client/web/src/api.ts diff --git a/client/web/api.go b/client/web/api.go new file mode 100644 index 000000000..be220580f --- /dev/null +++ b/client/web/api.go @@ -0,0 +1,41 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +package web + +import ( + "net/http" + "strings" + + "github.com/gorilla/csrf" + "tailscale.com/util/httpm" +) + +type api struct { + s *Server +} + +// ServeHTTP serves requests for the web client api. +// It should only be called by Server.ServeHTTP, via Server.apiHandler, +// which protects the handler using gorilla csrf. +func (a *api) ServeHTTP(w http.ResponseWriter, r *http.Request) { + w.Header().Set("X-CSRF-Token", csrf.Token(r)) + user, err := authorize(w, r) + if err != nil { + return + } + path := strings.TrimPrefix(r.URL.Path, "/api") + switch path { + case "/data": + switch r.Method { + case httpm.GET: + a.s.serveGetNodeDataJSON(w, r, user) + case httpm.POST: + a.s.servePostNodeUpdate(w, r) + default: + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + } + return + } + http.Error(w, "invalid endpoint", http.StatusNotFound) +} diff --git a/client/web/package.json b/client/web/package.json index c35bfdab1..2621442b4 100644 --- a/client/web/package.json +++ b/client/web/package.json @@ -8,10 +8,12 @@ }, "private": true, "dependencies": { + "classnames": "^2.3.1", "react": "^18.2.0", "react-dom": "^18.2.0" }, "devDependencies": { + "@types/classnames": "^2.2.10", "@types/react": "^18.0.20", "@types/react-dom": "^18.0.6", "@vitejs/plugin-react-swc": "^3.3.2", diff --git a/client/web/src/api.ts b/client/web/src/api.ts new file mode 100644 index 000000000..3009a1716 --- /dev/null +++ b/client/web/src/api.ts @@ -0,0 +1,32 @@ +let csrfToken: string + +// apiFetch wraps the standard JS fetch function +// with csrf header management. +export function apiFetch( + input: RequestInfo | URL, + init?: RequestInit | undefined +): Promise { + return fetch(input, { + ...init, + headers: withCsrfToken(init?.headers), + }).then((r) => { + updateCsrfToken(r) + if (!r.ok) { + return r.text().then((err) => { + throw new Error(err) + }) + } + return r + }) +} + +function withCsrfToken(h?: HeadersInit): HeadersInit { + return { ...h, "X-CSRF-Token": csrfToken } +} + +function updateCsrfToken(r: Response) { + const tok = r.headers.get("X-CSRF-Token") + if (tok) { + csrfToken = tok + } +} diff --git a/client/web/src/components/app.tsx b/client/web/src/components/app.tsx index a173feba5..8dd90c188 100644 --- a/client/web/src/components/app.tsx +++ b/client/web/src/components/app.tsx @@ -3,7 +3,9 @@ import { Footer, Header, IP, State } from "src/components/legacy" import useNodeData from "src/hooks/node-data" export default function App() { - const data = useNodeData() + // TODO(sonia): use isPosting value from useNodeData + // to fill loading states. + const { data, updateNode } = useNodeData() return (
@@ -15,7 +17,7 @@ export default function App() {
- +
diff --git a/client/web/src/components/legacy.tsx b/client/web/src/components/legacy.tsx index 01cd95c94..7c599d4a1 100644 --- a/client/web/src/components/legacy.tsx +++ b/client/web/src/components/legacy.tsx @@ -1,5 +1,6 @@ +import cx from "classnames" import React from "react" -import { NodeData } from "src/hooks/node-data" +import { NodeData, NodeUpdate } from "src/hooks/node-data" // TODO(tailscale/corp#13775): legacy.tsx contains a set of components // that (crudely) implement the pre-2023 web client. These are implemented @@ -162,9 +163,13 @@ export function IP(props: { data: NodeData }) { ) } -export function State(props: { data: NodeData }) { - const { data } = props - +export function State({ + data, + updateNode, +}: { + data: NodeData + updateNode: (update: NodeUpdate) => void +}) { switch (data.Status) { case "NeedsLogin": case "NoState": @@ -232,25 +237,20 @@ export function State(props: { data: NodeData }) { device name or IP address above.

-
- - {data.AdvertiseExitNode ? ( - - ) : ( - - )} - -
+ ) } diff --git a/client/web/src/hooks/node-data.ts b/client/web/src/hooks/node-data.ts index 3060721f9..444c9190d 100644 --- a/client/web/src/hooks/node-data.ts +++ b/client/web/src/hooks/node-data.ts @@ -1,4 +1,5 @@ -import { useEffect, useState } from "react" +import { useCallback, useEffect, useState } from "react" +import { apiFetch } from "src/api" export type NodeData = { Profile: UserProfile @@ -22,16 +23,104 @@ export type UserProfile = { ProfilePicURL: string } +export type NodeUpdate = { + AdvertiseRoutes?: string + AdvertiseExitNode?: boolean + Reauthenticate?: boolean + ForceLogout?: boolean +} + // useNodeData returns basic data about the current node. export default function useNodeData() { const [data, setData] = useState() + const [isPosting, setIsPosting] = useState(false) - useEffect(() => { - fetch("/api/data") - .then((response) => response.json()) - .then((json) => setData(json)) + const fetchNodeData = useCallback(() => { + apiFetch("/api/data") + .then((r) => r.json()) + .then((data) => setData(data)) .catch((error) => console.error(error)) - }, []) + }, [setData]) - return data + const updateNode = useCallback( + (update: NodeUpdate) => { + // The contents of this function are mostly copied over + // from the legacy client's web.html file. + // It makes all data updates through one API endpoint. + // As we build out the web client in React, + // this endpoint will eventually be deprecated. + + if (isPosting || !data) { + return + } + setIsPosting(true) + + update = { + // Default to current data value for any unset fields. + AdvertiseRoutes: + update.AdvertiseRoutes !== undefined + ? update.AdvertiseRoutes + : data.AdvertiseRoutes, + AdvertiseExitNode: + update.AdvertiseExitNode !== undefined + ? update.AdvertiseExitNode + : data.AdvertiseExitNode, + } + + const urlParams = new URLSearchParams(window.location.search) + const nextParams = new URLSearchParams({ up: "true" }) + const token = urlParams.get("SynoToken") + if (token) { + nextParams.set("SynoToken", token) + } + const search = nextParams.toString() + const url = `/api/data${search ? `?${search}` : ""}` + + var body, contentType: string + + if (data.IsUnraid) { + const params = new URLSearchParams() + params.append("csrf_token", data.UnraidToken) + params.append("ts_data", JSON.stringify(update)) + body = params.toString() + contentType = "application/x-www-form-urlencoded;charset=UTF-8" + } else { + body = JSON.stringify(update) + contentType = "application/json" + } + + apiFetch(url, { + method: "POST", + headers: { Accept: "application/json", "Content-Type": contentType }, + body: body, + }) + .then((r) => r.json()) + .then((r) => { + setIsPosting(false) + const err = r["error"] + if (err) { + throw new Error(err) + } + const url = r["url"] + if (url) { + if (data.IsUnraid) { + window.open(url, "_blank") + } else { + document.location.href = url + } + } + fetchNodeData() + }) + .catch((err) => alert("Failed operation: " + err.message)) + }, + [data] + ) + + useEffect( + fetchNodeData, + // Initial data load. + [] + ) + + return { data, updateNode, isPosting } } diff --git a/client/web/web.go b/client/web/web.go index c49cf404d..f08603ac0 100644 --- a/client/web/web.go +++ b/client/web/web.go @@ -7,6 +7,7 @@ package web import ( "bytes" "context" + "crypto/rand" "crypto/tls" _ "embed" "encoding/json" @@ -23,6 +24,7 @@ import ( "os/exec" "strings" + "github.com/gorilla/csrf" "tailscale.com/client/tailscale" "tailscale.com/envknob" "tailscale.com/ipn" @@ -31,7 +33,6 @@ import ( "tailscale.com/net/netutil" "tailscale.com/tailcfg" "tailscale.com/util/groupmember" - "tailscale.com/util/httpm" "tailscale.com/version/distro" ) @@ -52,6 +53,8 @@ type Server struct { devMode bool devProxy *httputil.ReverseProxy // only filled when devMode is on + + apiHandler http.Handler // csrf-protected api handler } // NewServer constructs a new Tailscale web client server. @@ -70,6 +73,11 @@ func NewServer(devMode bool, lc *tailscale.LocalClient) (s *Server, cleanup func if s.devMode { cleanup = s.startDevServer() s.addProxyToDevServer() + + // Create new handler for "/api" requests. + // And protect with gorilla csrf. + csrfProtect := csrf.Protect(csrfKey()) + s.apiHandler = csrfProtect(&api{s: s}) } return s, cleanup } @@ -271,19 +279,9 @@ req.send(null); // ServeHTTP processes all requests for the Tailscale web client. func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { if s.devMode { - if r.URL.Path == "/api/data" { - user, err := authorize(w, r) - if err != nil { - return - } - switch r.Method { - case httpm.GET: - s.serveGetNodeDataJSON(w, r, user) - case httpm.POST: - s.servePostNodeUpdate(w, r) - default: - http.Error(w, "method not allowed", http.StatusMethodNotAllowed) - } + if strings.HasPrefix(r.URL.Path, "/api/") { + // Pass through to other handlers via CSRF protection. + s.apiHandler.ServeHTTP(w, r) return } // When in dev mode, proxy to the Vite dev server. @@ -527,3 +525,14 @@ func (s *Server) tailscaleUp(ctx context.Context, st *ipnstate.Status, postData } } } + +// csrfKey creates a new random csrf token. +// If an error surfaces during key creation, +// the error is logged and the active process terminated. +func csrfKey() []byte { + key := make([]byte, 32) + if _, err := rand.Read(key); err != nil { + log.Fatal("error generating CSRF key: %w", err) + } + return key +} diff --git a/client/web/yarn.lock b/client/web/yarn.lock index e0bf046fb..c7d9d3743 100644 --- a/client/web/yarn.lock +++ b/client/web/yarn.lock @@ -543,6 +543,13 @@ resolved "https://registry.yarnpkg.com/@types/chai/-/chai-4.3.5.tgz#ae69bcbb1bebb68c4ac0b11e9d8ed04526b3562b" integrity sha512-mEo1sAde+UCE6b2hxn332f1g1E8WfYRu6p5SvTKr2ZKC1f7gFJXk4h5PyGP9Dt6gCaG8y8XhwnXWC6Iy2cmBng== +"@types/classnames@^2.2.10": + version "2.3.1" + resolved "https://registry.yarnpkg.com/@types/classnames/-/classnames-2.3.1.tgz#3c2467aa0f1a93f1f021e3b9bcf938bd5dfdc0dd" + integrity sha512-zeOWb0JGBoVmlQoznvqXbE0tEC/HONsnoUNH19Hc96NFsTAwTXbTqb8FMYkru1F/iqp7a18Ws3nWJvtA1sHD1A== + dependencies: + classnames "*" + "@types/estree@^1.0.0": version "1.0.1" resolved "https://registry.yarnpkg.com/@types/estree/-/estree-1.0.1.tgz#aa22750962f3bf0e79d753d3cc067f010c95f194" @@ -798,6 +805,11 @@ chokidar@^3.5.3: optionalDependencies: fsevents "~2.3.2" +classnames@*, classnames@^2.3.1: + version "2.3.2" + resolved "https://registry.yarnpkg.com/classnames/-/classnames-2.3.2.tgz#351d813bf0137fcc6a76a16b88208d2560a0d924" + integrity sha512-CSbhY4cFEJRe6/GQzIk5qXZ4Jeg5pcsP7b5peFSDpffpe1cqjASH/n9UTjBwOp6XpMSTwQ8Za2K5V02ueA7Tmw== + color-convert@^1.9.0: version "1.9.3" resolved "https://registry.yarnpkg.com/color-convert/-/color-convert-1.9.3.tgz#bb71850690e1f136567de629d2d5471deda4c1e8" diff --git a/cmd/tailscale/depaware.txt b/cmd/tailscale/depaware.txt index 7ce47f189..1c8260329 100644 --- a/cmd/tailscale/depaware.txt +++ b/cmd/tailscale/depaware.txt @@ -22,6 +22,8 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep L github.com/google/nftables/internal/parseexprfunc from github.com/google/nftables+ L github.com/google/nftables/xt from github.com/google/nftables/expr+ github.com/google/uuid from tailscale.com/util/quarantine+ + github.com/gorilla/csrf from tailscale.com/client/web + github.com/gorilla/securecookie from github.com/gorilla/csrf github.com/hdevalence/ed25519consensus from tailscale.com/tka L github.com/josharian/native from github.com/mdlayher/netlink+ L 💣 github.com/jsimonetti/rtnetlink from tailscale.com/net/interfaces+ @@ -38,6 +40,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep 💣 github.com/mitchellh/go-ps from tailscale.com/cmd/tailscale/cli+ github.com/peterbourgon/ff/v3 from github.com/peterbourgon/ff/v3/ffcli github.com/peterbourgon/ff/v3/ffcli from tailscale.com/cmd/tailscale/cli + github.com/pkg/errors from github.com/gorilla/csrf github.com/skip2/go-qrcode from tailscale.com/cmd/tailscale/cli github.com/skip2/go-qrcode/bitset from github.com/skip2/go-qrcode+ github.com/skip2/go-qrcode/reedsolomon from github.com/skip2/go-qrcode @@ -234,6 +237,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep encoding/base32 from tailscale.com/tka+ encoding/base64 from encoding/json+ encoding/binary from compress/gzip+ + encoding/gob from github.com/gorilla/securecookie encoding/hex from crypto/x509+ encoding/json from expvar+ encoding/pem from crypto/tls+ @@ -247,7 +251,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep hash/crc32 from compress/gzip+ hash/maphash from go4.org/mem html from tailscale.com/ipn/ipnstate+ - html/template from tailscale.com/client/web + html/template from tailscale.com/client/web+ image from github.com/skip2/go-qrcode+ image/color from github.com/skip2/go-qrcode+ image/png from github.com/skip2/go-qrcode diff --git a/go.mod b/go.mod index e99783767..47e8700c4 100644 --- a/go.mod +++ b/go.mod @@ -100,6 +100,8 @@ require ( software.sslmate.com/src/go-pkcs12 v0.2.0 ) +require github.com/gorilla/securecookie v1.1.1 // indirect + require ( 4d63.com/gocheckcompilerdirectives v1.2.1 // indirect 4d63.com/gochecknoglobals v0.2.1 // indirect @@ -208,6 +210,7 @@ require ( github.com/gordonklaus/ineffassign v0.0.0-20230107090616-13ace0543b28 // indirect github.com/goreleaser/chglog v0.5.0 // indirect github.com/goreleaser/fileglob v1.3.0 // indirect + github.com/gorilla/csrf v1.7.1 github.com/gostaticanalysis/analysisutil v0.7.1 // indirect github.com/gostaticanalysis/comment v1.4.2 // indirect github.com/gostaticanalysis/forcetypeassert v0.1.0 // indirect diff --git a/go.sum b/go.sum index 05fcaf293..0f1a632ec 100644 --- a/go.sum +++ b/go.sum @@ -478,6 +478,10 @@ github.com/goreleaser/fileglob v1.3.0 h1:/X6J7U8lbDpQtBvGcwwPS6OpzkNVlVEsFUVRx9+ github.com/goreleaser/fileglob v1.3.0/go.mod h1:Jx6BoXv3mbYkEzwm9THo7xbr5egkAraxkGorbJb4RxU= github.com/goreleaser/nfpm/v2 v2.32.1-0.20230803123630-24a43c5ad7cf h1:X8rzot0Te1TYSoADyMZfPt95Afhptpj0VqicKPAcmjM= github.com/goreleaser/nfpm/v2 v2.32.1-0.20230803123630-24a43c5ad7cf/go.mod h1:Z7rAxucnQGMGfAhpxm/UIrdH0/EcxEt91RW3mmVzx2U= +github.com/gorilla/csrf v1.7.1 h1:Ir3o2c1/Uzj6FBxMlAUB6SivgVMy1ONXwYgXn+/aHPE= +github.com/gorilla/csrf v1.7.1/go.mod h1:+a/4tCmqhG6/w4oafeAZ9pEa3/NZOWYVbD9fV0FwIQA= +github.com/gorilla/securecookie v1.1.1 h1:miw7JPhV+b/lAHSXz4qd/nN9jRiAFV5FwjeKyCS8BvQ= +github.com/gorilla/securecookie v1.1.1/go.mod h1:ra0sb63/xPlUeL+yeDciTfxMRAA+MP+HVt/4epWDjd4= github.com/gorilla/websocket v1.4.1/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE= github.com/gorilla/websocket v1.4.2 h1:+/TMaTYc4QFitKJxsQ7Yye35DkWvkdLcvGKqM+x0Ufc= github.com/gorilla/websocket v1.4.2/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=