cmd/browser: strengthen cross-site request forgery prevention

This extends XSRF token checking to the startup method and when fetching
Upspin content. The latter is particularly important as tricking someone
into retrieving a file from Upspin space could lead them to trigger some
dynamic interaction with an Upspin server, which could be bad.

We now generate a single access token for all browser XHR requests, and
individual file tokens for accessing specific Upspin paths.

We provide the access token in the URL that the browser opens, so that
it's not possible to obtain the token by sending an HTTP request to the
server. This makes it less likely that a malicious actor could make
requests to the server and therefore act as the Upspin user.

Change-Id: Ie63f66dfd137d3364993d5427de6a4c3c07aafd1
Reviewed-on: https://upspin-review.googlesource.com/12420
Reviewed-by: Rob Pike <r@golang.org>
diff --git a/cmd/browser/main.go b/cmd/browser/main.go
index 65b2b17..b2e3cfe 100644
--- a/cmd/browser/main.go
+++ b/cmd/browser/main.go
@@ -59,7 +59,7 @@
 	if err != nil {
 		exit(err)
 	}
-	url := fmt.Sprintf("http://%s/", *httpAddr)
+	url := fmt.Sprintf("http://%s/#key=%s", *httpAddr, s.key)
 	if !startBrowser(url) {
 		fmt.Printf("Open %s in your web browser.\n", url)
 	} else {
@@ -76,15 +76,18 @@
 // server implements an http.Handler that performs various Upspin operations
 // using a config. It is the back end for the JavaScript Upspin browser.
 type server struct {
-	xsrfKey string       // Random secret key for generating XSRF tokens.
-	static  http.Handler // Handler for serving static content (HTML, JS, etc).
+	// key to prevent request forgery; static for server's lifetime.
+	key string
+
+	// Handler for serving static content (HTML, JS, etc).
+	static http.Handler
 
 	mu  sync.Mutex
 	cfg upspin.Config // Non-nil if signup flow has been completed.
 	cli upspin.Client
 }
 
-func newServer() (http.Handler, error) {
+func newServer() (*server, error) {
 	key, err := generateKey()
 	if err != nil {
 		return nil, err
@@ -96,11 +99,17 @@
 	}
 
 	return &server{
-		xsrfKey: key,
-		static:  http.FileServer(http.Dir(pkg.Dir)),
+		key:    key,
+		static: http.FileServer(http.Dir(pkg.Dir)),
 	}, nil
 }
 
+func (s *server) hasConfig() bool {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	return s.cfg != nil && s.cli != nil
+}
+
 func (s *server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	p := r.URL.Path
 	if p == "/_upspin" {
@@ -115,7 +124,17 @@
 }
 
 func (s *server) serveContent(w http.ResponseWriter, r *http.Request) {
+	if !s.hasConfig() {
+		http.Error(w, "No configuration", http.StatusServiceUnavailable)
+		return
+	}
+
 	p := r.URL.Path[1:]
+	if !xsrftoken.Valid(r.FormValue("token"), s.key, string(s.cfg.UserName()), p) {
+		http.Error(w, "Invalid XSRF token", http.StatusForbidden)
+		return
+	}
+
 	name := upspin.PathName(p)
 	de, err := s.cli.Lookup(name, true)
 	if err != nil {
@@ -133,25 +152,22 @@
 
 func (s *server) serveAPI(w http.ResponseWriter, r *http.Request) {
 	if r.Method != "POST" {
-		http.Error(w, "bad request", http.StatusBadRequest)
+		http.Error(w, "Expected POST request", http.StatusMethodNotAllowed)
 		return
 	}
+
+	// Require a valid key.
+	if r.FormValue("key") != s.key {
+		http.Error(w, "Invalid key", http.StatusForbidden)
+		return
+	}
+
 	method := r.FormValue("method")
 
-	s.mu.Lock()
-	hasConfig := s.cfg != nil
-	s.mu.Unlock()
-
-	// Don't permit accesses of non-signup methods if there is no config
+	// Don't permit accesses of non-startup methods if there is no config
 	// nor client; those methods need them.
-	if method != "startup" && !hasConfig {
-		http.Error(w, "no config", http.StatusBadRequest)
-		return
-	}
-
-	// Require a valid XSRF token for any requests except "startup".
-	if method != "startup" && !xsrftoken.Valid(r.FormValue("token"), s.xsrfKey, "magic", "") {
-		http.Error(w, "invalid xsrf token", http.StatusForbidden)
+	if method != "startup" && !s.hasConfig() {
+		http.Error(w, "No configuration", http.StatusServiceUnavailable)
 		return
 	}
 
@@ -167,13 +183,11 @@
 		if cfg != nil {
 			user = string(cfg.UserName())
 		}
-		token := xsrftoken.Generate(s.xsrfKey, "magic", "")
 		resp = struct {
 			Startup  *startupResponse
 			UserName string
-			Token    string
 			Error    string
-		}{sResp, user, token, errString}
+		}{sResp, user, errString}
 	case "list":
 		path := upspin.PathName(r.FormValue("path"))
 		des, err := s.cli.Glob(upspin.AllFilesGlob(path))
@@ -181,10 +195,18 @@
 		if err != nil {
 			errString = err.Error()
 		}
+		var entries []entryWithToken
+		for _, de := range des {
+			tok := xsrftoken.Generate(s.key, string(s.cfg.UserName()), string(de.Name))
+			entries = append(entries, entryWithToken{
+				DirEntry:  de,
+				FileToken: tok,
+			})
+		}
 		resp = struct {
-			Entries []*upspin.DirEntry
+			Entries []entryWithToken
 			Error   string
-		}{des, errString}
+		}{entries, errString}
 	case "mkdir":
 		_, err := s.cli.MakeDirectory(upspin.PathName(r.FormValue("path")))
 		var errString string
@@ -227,8 +249,13 @@
 	w.Write(b)
 }
 
+type entryWithToken struct {
+	*upspin.DirEntry
+	FileToken string
+}
+
 func generateKey() (string, error) {
-	b := make([]byte, 8)
+	b := make([]byte, 32)
 	_, err := rand.Read(b)
 	if err != nil {
 		return "", err
diff --git a/cmd/browser/static/script.js b/cmd/browser/static/script.js
index bfb322b..835b633 100644
--- a/cmd/browser/static/script.js
+++ b/cmd/browser/static/script.js
@@ -278,16 +278,20 @@
 			var shortName = name.slice(name.lastIndexOf("/")+1);
 			var nameEl = entryEl.find(".up-entry-name");
 			if (isDir) {
-				nameEl.text(shortName);
-				nameEl.addClass("up-clickable");
-				nameEl.data("up-path", name);
-				nameEl.click(function(event) {
-					var p = $(this).data("up-path");
-					navigate(p);
-				});
+				nameEl
+					.text(shortName)
+					.addClass("up-clickable")
+					.data("up-path", name)
+					.click(function(event) {
+						var p = $(this).data("up-path");
+						navigate(p);
+					});
 			} else {
-				var nameLink = $("<a>").text(shortName).attr("href", "/" + name).attr("target", "_blank");
-				nameEl.append(nameLink);
+				$("<a>")
+					.text(shortName)
+					.attr("href", "/" + name + "?token=" + entry.FileToken)
+					.attr("target", "_blank")
+					.appendTo(nameEl);
 			}
 
 			var sizeEl = entryEl.find(".up-entry-size");
@@ -589,7 +593,7 @@
 function Page() {
 	var page = {
 		username: "",
-		token: ""
+		key: ""
 	};
 
 	// errorHandler returns an XHR error callback that invokes the given
@@ -609,7 +613,7 @@
 		$.ajax("/_upspin", {
 			method: "POST",
 			data: {
-				token: page.token,
+				key: page.key,
 				method: "list",
 				path: path,
 			},
@@ -629,7 +633,7 @@
 		$.ajax("/_upspin", {
 			method: "POST",
 			data: {
-				token: page.token,
+				key: page.key,
 				method: "rm",
 				paths: paths
 			},
@@ -649,7 +653,7 @@
 		$.ajax("/_upspin", {
 			method: "POST",
 			data: {
-				token: page.token,
+				key: page.key,
 				method: "copy",
 				paths: paths,
 				dest: dest
@@ -670,7 +674,7 @@
 		$.ajax("/_upspin", {
 			method: "POST",
 			data: {
-				token: page.token,
+				key: page.key,
 				method: "mkdir",
 				path: path
 			},
@@ -689,7 +693,10 @@
 	function startup(data, success, error) {
 		$.ajax("/_upspin", {
 			method: "POST",
-			data: $.extend({method: "startup"}, data),
+			data: $.extend({
+				key: page.key,
+				method: "startup"
+			}, data),
 			dataType: "json",
 			success: function(data) {
 				if (data.Error) {
@@ -723,12 +730,22 @@
 		browser2.navigate("augie@upspin.io");
 	}
 
+	// Obtain a request key.
+	var prefix = "#key="
+	if (!window.location.hash.startsWith(prefix)) {
+		// TODO(adg): make this more user-friendly,
+		// even though it will not happen often.
+		alert("No key provided in URL fragment.");
+		return;
+	}
+	page.key = window.location.hash.slice(prefix.length);
+	window.location.hash = "";
+
 	// Begin the Startup sequence.
 	Startup(startup, function(data) {
-		// When startup is complete note the user name and token and
-		// launch the browsers.
+		// When startup is complete, note the
+		// user name and launch the browsers.
 		page.username = data.UserName;
-		page.token = data.Token;
 		$("#headerUsername").text(page.username);
 		startBrowsers();
 	});