From 07e783c7be4d497a342648b761f4439211969328 Mon Sep 17 00:00:00 2001
From: Percy Wegmann <percy@tailscale.com>
Date: Wed, 1 May 2024 15:57:06 -0500
Subject: [PATCH] drive: use secret token to authenticate access to file server
 on localhost

This prevents Mark-of-the-Web bypass attacks in case someone visits the
localhost WebDAV server directly.

Fixes tailscale/corp#19592

Signed-off-by: Percy Wegmann <percy@tailscale.com>
---
 drive/driveimpl/drive_test.go |  6 +++++-
 drive/driveimpl/fileserver.go | 37 ++++++++++++++++++++++++++---------
 2 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/drive/driveimpl/drive_test.go b/drive/driveimpl/drive_test.go
index fe9986433..e245ac791 100644
--- a/drive/driveimpl/drive_test.go
+++ b/drive/driveimpl/drive_test.go
@@ -131,7 +131,11 @@ func TestSecretTokenAuth(t *testing.T) {
 		Transport: &http.Transport{DisableKeepAlives: true},
 	}
 	addr := strings.Split(fileserverAddr, "|")[1]
-	u := fmt.Sprintf("http://%s/%s/%s", addr, "fakesecret", url.PathEscape(file111))
+	wrongSecret, err := generateSecretToken()
+	if err != nil {
+		t.Fatal(err)
+	}
+	u := fmt.Sprintf("http://%s/%s/%s", addr, wrongSecret, url.PathEscape(file111))
 	resp, err := client.Get(u)
 	if err != nil {
 		t.Fatal(err)
diff --git a/drive/driveimpl/fileserver.go b/drive/driveimpl/fileserver.go
index 9a4a2d323..f3c77eb54 100644
--- a/drive/driveimpl/fileserver.go
+++ b/drive/driveimpl/fileserver.go
@@ -50,19 +50,28 @@ func NewFileServer() (*FileServer, error) {
 	}
 	// }
 
-	tokenBytes := make([]byte, 32)
-	_, err = rand.Read(tokenBytes)
+	secretToken, err := generateSecretToken()
 	if err != nil {
-		return nil, fmt.Errorf("generate token bytes: %w", err)
+		return nil, err
 	}
 
 	return &FileServer{
 		l:             l,
-		secretToken:   hex.EncodeToString(tokenBytes),
+		secretToken:   secretToken,
 		shareHandlers: make(map[string]http.Handler),
 	}, nil
 }
 
+// generateSecretToken generates a hex-encoded 256 bit secet.
+func generateSecretToken() (string, error) {
+	tokenBytes := make([]byte, 32)
+	_, err := rand.Read(tokenBytes)
+	if err != nil {
+		return "", fmt.Errorf("generateSecretToken: %w", err)
+	}
+	return hex.EncodeToString(tokenBytes), nil
+}
+
 // Addr returns the address at which this FileServer is listening. This
 // includes the secret token in front of the address, delimited by a pipe |.
 func (s *FileServer) Addr() string {
@@ -109,11 +118,21 @@ func (s *FileServer) SetShares(shares map[string]string) {
 	}
 }
 
-// ServeHTTP implements the http.Handler interface. In order to prevent
-// Mark-of-the-Web bypass attacks if someone visits this fileserver directly
-// within a browser, it requires that the first path element be this file
-// server's secret token. Without knowing the secret token, it's impossible
-// to construct a URL that passes validation.
+// ServeHTTP implements the http.Handler interface. This requires a secret
+// token in the path in order to prevent Mark-of-the-Web (MOTW) bypass attacks
+// of the below sort:
+//
+//  1. Attacker with write access to the share puts a malicious file via
+//     http://100.100.100.100:8080/<tailnet>/<machine>/</share>/bad.exe
+//  2. Attacker then induces victim to visit
+//     http://localhost:[PORT]/<share>/bad.exe
+//  3. Because that is loaded from localhost, it does not get the MOTW
+//     thereby bypasses some OS-level security.
+//
+// The path on this file server is actually not as above, but rather
+// http://localhost:[PORT]/<secretToken>/<share>/bad.exe. Unless the attacker
+// can discover the secretToken, the attacker cannot craft a localhost URL that
+// will work.
 func (s *FileServer) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	parts := shared.CleanAndSplit(r.URL.Path)