safeweb: add support for custom CSP (#13975)

To allow more flexibility with CSPs, add a fully customizable `CSP` type
that can be provided in `Config` and encodes itself into the correct
format. Preserve the `CSPAllowInlineStyles` option as is today, but
maybe that'll get deprecated later in favor of the new CSP field.

In particular, this allows for pages loading external JS, or inline JS
with nonces or hashes (see
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/script-src#unsafe_inline_script)

Updates https://github.com/tailscale/corp/issues/8027

Signed-off-by: Andrew Lytvynov <[email protected]>

safeweb/http.go

@@ -74,25 +74,74 @@ import (
 	crand "crypto/rand"
 	"fmt"
 	"log"
+	"maps"
 	"net"
 	"net/http"
 	"net/url"
 	"path"
+	"slices"
 	"strings"
 
 	"github.com/gorilla/csrf"
 )
 
-// The default Content-Security-Policy header.
-var defaultCSP = strings.Join([]string{
-	`default-src 'self'`,      // origin is the only valid source for all content types
-	`script-src 'self'`,       // disallow inline javascript
-	`frame-ancestors 'none'`,  // disallow framing of the page
-	`form-action 'self'`,      // disallow form submissions to other origins
-	`base-uri 'self'`,         // disallow base URIs from other origins
-	`block-all-mixed-content`, // disallow mixed content when serving over HTTPS
-	`object-src 'self'`,       // disallow embedding of resources from other origins
-}, "; ")
+// CSP is the value of a Content-Security-Policy header. Keys are CSP
+// directives (like "default-src") and values are source expressions (like
+// "'self'" or "https://tailscale.com"). A nil slice value is allowed for some
+// directives like "upgrade-insecure-requests" that don't expect a list of
+// source definitions.
+type CSP map[string][]string
+
+// DefaultCSP is the recommended CSP to use when not loading resources from
+// other domains and not embedding the current website. If you need to tweak
+// the CSP, it is recommended to extend DefaultCSP instead of writing your own
+// from scratch.
+func DefaultCSP() CSP {
+	return CSP{
+		"default-src":     {"self"}, // origin is the only valid source for all content types
+		"frame-ancestors": {"none"}, // disallow framing of the page
+		"form-action":     {"self"}, // disallow form submissions to other origins
+		"base-uri":        {"self"}, // disallow base URIs from other origins
+		// TODO(awly): consider upgrade-insecure-requests in SecureContext
+		// instead, as this is deprecated.
+		"block-all-mixed-content": nil, // disallow mixed content when serving over HTTPS
+	}
+}
+
+// Set sets the values for a given directive. Empty values are allowed, if the
+// directive doesn't expect any (like "upgrade-insecure-requests").
+func (csp CSP) Set(directive string, values ...string) {
+	csp[directive] = values
+}
+
+// Add adds a source expression to an existing directive.
+func (csp CSP) Add(directive, value string) {
+	csp[directive] = append(csp[directive], value)
+}
+
+// Del deletes a directive and all its values.
+func (csp CSP) Del(directive string) {
+	delete(csp, directive)
+}
+
+func (csp CSP) String() string {
+	keys := slices.Collect(maps.Keys(csp))
+	slices.Sort(keys)
+	var s strings.Builder
+	for _, k := range keys {
+		s.WriteString(k)
+		for _, v := range csp[k] {
+			// Special values like 'self', 'none', 'unsafe-inline', etc., must
+			// be quoted. Do it implicitly as a convenience here.
+			if !strings.Contains(v, ".") && len(v) > 1 && v[0] != '\'' && v[len(v)-1] != '\'' {
+				v = "'" + v + "'"
+			}
+			s.WriteString(" " + v)
+		}
+		s.WriteString("; ")
+	}
+	return strings.TrimSpace(s.String())
+}
 
 // The default Strict-Transport-Security header. This header tells the browser
 // to exclusively use HTTPS for all requests to the origin for the next year.
@@ -130,6 +179,9 @@ type Config struct {
 	// startup.
 	CSRFSecret []byte
 
+	// CSP is the Content-Security-Policy header to return with BrowserMux
+	// responses.
+	CSP CSP
 	// CSPAllowInlineStyles specifies whether to include `style-src:
 	// unsafe-inline` in the Content-Security-Policy header to permit the use of
 	// inline CSS.
@@ -168,6 +220,10 @@ func (c *Config) setDefaults() error {
 		}
 	}
 
+	if c.CSP == nil {
+		c.CSP = DefaultCSP()
+	}
+
 	return nil
 }
 
@@ -199,16 +255,20 @@ func NewServer(config Config) (*Server, error) {
 	if config.CookiesSameSiteLax {
 		sameSite = csrf.SameSiteLaxMode
 	}
+	if config.CSPAllowInlineStyles {
+		if _, ok := config.CSP["style-src"]; ok {
+			config.CSP.Add("style-src", "unsafe-inline")
+		} else {
+			config.CSP.Set("style-src", "self", "unsafe-inline")
+		}
+	}
 	s := &Server{
 		Config: config,
-		csp:    defaultCSP,
+		csp:    config.CSP.String(),
 		// only set Secure flag on CSRF cookies if we are in a secure context
 		// as otherwise the browser will reject the cookie
 		csrfProtect: csrf.Protect(config.CSRFSecret, csrf.Secure(config.SecureContext), csrf.SameSite(sameSite)),
 	}
-	if config.CSPAllowInlineStyles {
-		s.csp = defaultCSP + `; style-src 'self' 'unsafe-inline'`
-	}
 	s.h = cmp.Or(config.HTTPServer, &http.Server{})
 	if s.h.Handler != nil {
 		return nil, fmt.Errorf("use safeweb.Config.APIMux and safeweb.Config.BrowserMux instead of http.Server.Handler")

safeweb/http_test.go

@@ -241,18 +241,26 @@ func TestCSRFProtection(t *testing.T) {
 func TestContentSecurityPolicyHeader(t *testing.T) {
 	tests := []struct {
 		name     string
+		csp      CSP
 		apiRoute bool
-		wantCSP  bool
+		wantCSP  string
 	}{
 		{
-			name:     "default routes get CSP headers",
-			apiRoute: false,
-			wantCSP:  true,
+			name:    "default CSP",
+			wantCSP: `base-uri 'self'; block-all-mixed-content; default-src 'self'; form-action 'self'; frame-ancestors 'none';`,
+		},
+		{
+			name: "custom CSP",
+			csp: CSP{
+				"default-src":               {"'self'", "https://tailscale.com"},
+				"upgrade-insecure-requests": nil,
+			},
+			wantCSP: `default-src 'self' https://tailscale.com; upgrade-insecure-requests;`,
 		},
 		{
 			name:     "`/api/*` routes do not get CSP headers",
 			apiRoute: true,
-			wantCSP:  false,
+			wantCSP:  "",
 		},
 	}
 
@@ -265,9 +273,9 @@ func TestContentSecurityPolicyHeader(t *testing.T) {
 			var s *Server
 			var err error
 			if tt.apiRoute {
-				s, err = NewServer(Config{APIMux: h})
+				s, err = NewServer(Config{APIMux: h, CSP: tt.csp})
 			} else {
-				s, err = NewServer(Config{BrowserMux: h})
+				s, err = NewServer(Config{BrowserMux: h, CSP: tt.csp})
 			}
 			if err != nil {
 				t.Fatal(err)
@@ -279,8 +287,8 @@ func TestContentSecurityPolicyHeader(t *testing.T) {
 			s.h.Handler.ServeHTTP(w, req)
 			resp := w.Result()
 
-			if (resp.Header.Get("Content-Security-Policy") == "") == tt.wantCSP {
-				t.Fatalf("content security policy want: %v; got: %v", tt.wantCSP, resp.Header.Get("Content-Security-Policy"))
+			if got := resp.Header.Get("Content-Security-Policy"); got != tt.wantCSP {
+				t.Fatalf("content security policy want: %q; got: %q", tt.wantCSP, got)
 			}
 		})
 	}
@@ -397,7 +405,7 @@ func TestCSPAllowInlineStyles(t *testing.T) {
 			csp := resp.Header.Get("Content-Security-Policy")
 			allowsStyles := strings.Contains(csp, "style-src 'self' 'unsafe-inline'")
 			if allowsStyles != allow {
-				t.Fatalf("CSP inline styles want: %v; got: %v", allow, allowsStyles)
+				t.Fatalf("CSP inline styles want: %v, got: %v in %q", allow, allowsStyles, csp)
 			}
 		})
 	}