Repository: google/go-safeweb
Pull request #154 (2020-09-10 06:45:12)

CSP plugin refactoring allowing Interceptor inspection.
Previously, `Policy` would contain a single function field. This made
it impossible to inspect the configuration of a given `Policy` object.

Now, `Policy` is an interface implemented by `StrictPolicy` (replaced
`StrictCSPBuilder`) and `FramingPolicy` (replaced `FramingPolicyBuilder`).

The package can now look at the Interceptor and make checks like:

```
if s := Interceptor.Enforce[0].(StrictPolicy); s.UnsafeEval {
  panic("unsafe-eval is dangerous!")
}
```

This enables implementing a conformance check in the nearest future
(a feature coming soon).


Patch:
diff --git a/safehttp/plugins/csp/csp.go b/safehttp/plugins/csp/csp.go
index 1a39a3e..481460f 100644
--- a/safehttp/plugins/csp/csp.go
+++ b/safehttp/plugins/csp/csp.go
@@ -42,11 +42,11 @@ func generateNonce() string {
 }
 
 // Policy defines a CSP policy.
-type Policy struct {
-	// serialize serializes this policy for use in a Content-Security-Policy header
+type Policy interface {
+	// Serialize serializes this policy for use in a Content-Security-Policy header
 	// or in a Content-Security-Policy-Report-Only header. A nonce will be provided
 	// to serialize which can be used in 'nonce-{random-nonce}' values in directives.
-	serialize func(nonce string) string
+	Serialize(nonce string) string
 }
 
 type ctxKey struct{}
@@ -61,10 +61,10 @@ func Nonce(ctx context.Context) (string, error) {
 	return v.(string), nil
 }
 
-// StrictCSPBuilder can be used to build a strict, nonce-based CSP.
+// StrictPolicy can be used to build a strict, nonce-based CSP.
 //
 // See https://csp.withgoogle.com/docs/strict-csp.html for more info.
-type StrictCSPBuilder struct {
+type StrictPolicy struct {
 	// NoStrictDynamic controls whether script-src should contain the 'strict-dynamic'
 	// value.
 	//
@@ -92,77 +92,77 @@ type StrictCSPBuilder struct {
 	Hashes []string
 }
 
-// Build creates a Policy based on the specified options.
-func (s StrictCSPBuilder) Build() Policy {
-	return Policy{
-		serialize: func(nonce string) string {
-			var b strings.Builder
-
-			// object-src 'none'; script-src 'unsafe-inline' 'nonce-{random}'
-			b.WriteString("object-src 'none'; script-src 'unsafe-inline' 'nonce-")
-			b.WriteString(nonce)
-			b.WriteByte('\'')
-
-			if !s.NoStrictDynamic {
-				b.WriteString(" 'strict-dynamic' https: http:")
-			}
-
-			if s.UnsafeEval {
-				b.WriteString(" 'unsafe-eval'")
-			}
-
-			for _, h := range s.Hashes {
-				b.WriteString(" '")
-				b.WriteString(h)
-				b.WriteByte('\'')
-			}
-
-			b.WriteString("; base-uri ")
-			if s.BaseURI == "" {
-				b.WriteString("'none'")
-			} else {
-				b.WriteString(s.BaseURI)
-			}
-
-			if s.ReportURI != "" {
-				b.WriteString("; report-uri ")
-				b.WriteString(s.ReportURI)
-			}
-
-			return b.String()
-		},
+// Serialize serializes this policy for use in a Content-Security-Policy header
+// or in a Content-Security-Policy-Report-Only header. A nonce will be provided
+// to Serialize which can be used in 'nonce-{random-nonce}' values in directives.
+func (s StrictPolicy) Serialize(nonce string) string {
+	var b strings.Builder
+
+	// object-src 'none'; script-src 'unsafe-inline' 'nonce-{random}'
+	b.WriteString("object-src 'none'; script-src 'unsafe-inline' 'nonce-")
+	b.WriteString(nonce)
+	b.WriteByte('\'')
+
+	if !s.NoStrictDynamic {
+		b.WriteString(" 'strict-dynamic' https: http:")
+	}
+
+	if s.UnsafeEval {
+		b.WriteString(" 'unsafe-eval'")
+	}
+
+	for _, h := range s.Hashes {
+		b.WriteString(" '")
+		b.WriteString(h)
+		b.WriteByte('\'')
+	}
+
+	b.WriteString("; base-uri ")
+	if s.BaseURI == "" {
+		b.WriteString("'none'")
+	} else {
+		b.WriteString(s.BaseURI)
 	}
+
+	if s.ReportURI != "" {
+		b.WriteString("; report-uri ")
+		b.WriteString(s.ReportURI)
+	}
+
+	return b.String()
+
 }
 
-// FramingPolicyBuilder can be used to create a new CSP policy with frame-ancestors
+// FramingPolicy can be used to create a new CSP policy with frame-ancestors
 // set to 'self'.
 //
 // TODO: allow relaxation on specific endpoints according to #77.
-type FramingPolicyBuilder struct {
+type FramingPolicy struct {
 	// ReportURI controls the report-uri directive. If ReportUri is empty, no report-uri
 	// directive will be set.
 	ReportURI string
 }
 
-// Build creates a Policy based on the specified options.
-func (f FramingPolicyBuilder) Build() Policy {
-	return Policy{
-		serialize: func(_ string) string {
-			var b strings.Builder
-			b.WriteString("frame-ancestors 'self'")
+// Serialize serializes this policy for use in a Content-Security-Policy header
+// or in a Content-Security-Policy-Report-Only header. A nonce will be provided
+// to Serialize which can be used in 'nonce-{random-nonce}' values in directives.
+func (f FramingPolicy) Serialize(nonce string) string {
+	var b strings.Builder
+	b.WriteString("frame-ancestors 'self'")
 
-			if f.ReportURI != "" {
-				b.WriteString("; report-uri ")
-				b.WriteString(f.ReportURI)
-			}
-
-			return b.String()
-		},
+	if f.ReportURI != "" {
+		b.WriteString("; report-uri ")
+		b.WriteString(f.ReportURI)
 	}
+
+	return b.String()
 }
 
 // Interceptor intercepts requests and applies CSP policies.
 type Interceptor struct {
+	// TODO: move it to a safehttp.Config implementation to enable per-handler
+	// configuration.
+
 	// Enforce specifies which policies will be set as the Content-Security-Policy
 	// header.
 	Enforce []Policy
@@ -176,8 +176,8 @@ type Interceptor struct {
 func Default(reportURI string) Interceptor {
 	return Interceptor{
 		Enforce: []Policy{
-			StrictCSPBuilder{ReportURI: reportURI}.Build(),
-			FramingPolicyBuilder{ReportURI: reportURI}.Build(),
+			StrictPolicy{ReportURI: reportURI},
+			FramingPolicy{ReportURI: reportURI},
 		},
 	}
 }
@@ -190,11 +190,11 @@ func (it Interceptor) Before(w *safehttp.ResponseWriter, r *safehttp.IncomingReq
 
 	var CSPs []string
 	for _, p := range it.Enforce {
-		CSPs = append(CSPs, p.serialize(nonce))
+		CSPs = append(CSPs, p.Serialize(nonce))
 	}
 	var reportCSPs []string
 	for _, p := range it.ReportOnly {
-		reportCSPs = append(reportCSPs, p.serialize(nonce))
+		reportCSPs = append(reportCSPs, p.Serialize(nonce))
 	}
 
 	h := w.Header()


Test patch:
diff --git a/safehttp/plugins/csp/csp_test.go b/safehttp/plugins/csp/csp_test.go
index 2d13568..f55a1e1 100644
--- a/safehttp/plugins/csp/csp_test.go
+++ b/safehttp/plugins/csp/csp_test.go
@@ -48,62 +48,62 @@ func TestSerialize(t *testing.T) {
 	}{
 		{
 			name:       "StrictCSP",
-			policy:     StrictCSPBuilder{}.Build(),
+			policy:     StrictPolicy{},
 			wantString: "object-src 'none'; script-src 'unsafe-inline' 'nonce-super-secret' 'strict-dynamic' https: http:; base-uri 'none'",
 		},
 		{
 			name:       "StrictCSP with no strict-dynamic",
-			policy:     StrictCSPBuilder{NoStrictDynamic: true}.Build(),
+			policy:     StrictPolicy{NoStrictDynamic: true},
 			wantString: "object-src 'none'; script-src 'unsafe-inline' 'nonce-super-secret'; base-uri 'none'",
 		},
 		{
 			name:       "StrictCSP with unsafe-eval",
-			policy:     StrictCSPBuilder{UnsafeEval: true}.Build(),
+			policy:     StrictPolicy{UnsafeEval: true},
 			wantString: "object-src 'none'; script-src 'unsafe-inline' 'nonce-super-secret' 'strict-dynamic' https: http: 'unsafe-eval'; base-uri 'none'",
 		},
 		{
 			name:       "StrictCSP with set base-uri",
-			policy:     StrictCSPBuilder{BaseURI: "https://example.com"}.Build(),
+			policy:     StrictPolicy{BaseURI: "https://example.com"},
 			wantString: "object-src 'none'; script-src 'unsafe-inline' 'nonce-super-secret' 'strict-dynamic' https: http:; base-uri https://example.com",
 		},
 		{
 			name:       "StrictCSP with report-uri",
-			policy:     StrictCSPBuilder{ReportURI: "https://example.com/collector"}.Build(),
+			policy:     StrictPolicy{ReportURI: "https://example.com/collector"},
 			wantString: "object-src 'none'; script-src 'unsafe-inline' 'nonce-super-secret' 'strict-dynamic' https: http:; base-uri 'none'; report-uri https://example.com/collector",
 		},
 		{
 			name: "StrictCSP with one hash",
-			policy: StrictCSPBuilder{Hashes: []string{
+			policy: StrictPolicy{Hashes: []string{
 				"sha256-CihokcEcBW4atb/CW/XWsvWwbTjqwQlE9nj9ii5ww5M=",
-			}}.Build(),
+			}},
 			wantString: "object-src 'none'; script-src 'unsafe-inline' 'nonce-super-secret' 'strict-dynamic' https: http: 'sha256-CihokcEcBW4atb/CW/XWsvWwbTjqwQlE9nj9ii5ww5M='; base-uri 'none'",
 		},
 		{
 			name: "StrictCSP with multiple hashes",
-			policy: StrictCSPBuilder{Hashes: []string{
+			policy: StrictPolicy{Hashes: []string{
 				"sha256-CihokcEcBW4atb/CW/XWsvWwbTjqwQlE9nj9ii5ww5M=",
 				"sha256-CihokcEcBW4atb/CW/XWsvWwbTjqwQlE9nj9ii5ww5M=",
-			}}.Build(),
+			}},
 			wantString: "object-src 'none'; script-src 'unsafe-inline' 'nonce-super-secret' 'strict-dynamic' https: http: 'sha256-CihokcEcBW4atb/CW/XWsvWwbTjqwQlE9nj9ii5ww5M=' 'sha256-CihokcEcBW4atb/CW/XWsvWwbTjqwQlE9nj9ii5ww5M='; base-uri 'none'",
 		},
 		{
 			name:       "FramingCSP",
-			policy:     FramingPolicyBuilder{}.Build(),
+			policy:     FramingPolicy{},
 			wantString: "frame-ancestors 'self'",
 		},
 		{
 			name:       "FramingCSP with report-uri",
-			policy:     FramingPolicyBuilder{ReportURI: "httsp://example.com/collector"}.Build(),
+			policy:     FramingPolicy{ReportURI: "httsp://example.com/collector"},
 			wantString: "frame-ancestors 'self'; report-uri httsp://example.com/collector",
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			s := tt.policy.serialize("super-secret")
+			s := tt.policy.Serialize("super-secret")
 
 			if s != tt.wantString {
-				t.Errorf("tt.policy.serialize() got: %q want: %q", s, tt.wantString)
+				t.Errorf("tt.policy.Serialize() got: %q want: %q", s, tt.wantString)
 			}
 		})
 	}
@@ -144,7 +144,7 @@ func TestBefore(t *testing.T) {
 			name: "StrictCSP Report Only",
 			interceptor: Interceptor{
 				ReportOnly: []Policy{
-					StrictCSPBuilder{ReportURI: "https://example.com/collector"}.Build(),
+					StrictPolicy{ReportURI: "https://example.com/collector"},
 				},
 			},
 			wantReportOnlyPolicy: []string{
@@ -156,7 +156,7 @@ func TestBefore(t *testing.T) {
 			name: "FramingCSP Report Only",
 			interceptor: Interceptor{
 				ReportOnly: []Policy{
-					FramingPolicyBuilder{ReportURI: "https://example.com/collector"}.Build(),
+					FramingPolicy{ReportURI: "https://example.com/collector"},
 				},
 			},
 			wantReportOnlyPolicy: []string{"frame-ancestors 'self'; report-uri https://example.com/collector"},