Repository: google/go-safeweb
Pull request #257 (2021-03-31 14:29:52)

Extend FramingPolicy to include allowed sources.
This is the first step to add a full support for framing protection (described in #77)

> It's a good idea to open an issue first for discussion.

- [ ] Tests pass
- [ ] Appropriate changes to README are included in PR

kele: Thank you! :)

Patch:
diff --git a/safehttp/plugins/csp/csp.go b/safehttp/plugins/csp/csp.go
index bfdc360..e1ac56f 100644
--- a/safehttp/plugins/csp/csp.go
+++ b/safehttp/plugins/csp/csp.go
@@ -147,6 +147,9 @@ func (s StrictPolicy) Serialize(nonce string) string {
 //
 // TODO: allow relaxation on specific endpoints according to #77.
 type FramingPolicy struct {
+	// Specifies valid parents that may embed a page using: <frame>, <iframe>, <object>, <embed>, or <applet>.
+	// More details: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/frame-ancestors#sources
+	Sources []string
 	// ReportURI controls the report-uri directive. If ReportUri is empty, no report-uri
 	// directive will be set.
 	ReportURI string
@@ -157,11 +160,33 @@ type FramingPolicy struct {
 // 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(frameAncestors(f.Sources))
+	b.WriteString(report(f.ReportURI))
+
+	return strings.TrimSpace(b.String())
+}
+
+func frameAncestors(sources []string) string {
+	var b strings.Builder
 	b.WriteString("frame-ancestors 'self'")
 
-	if f.ReportURI != "" {
-		b.WriteString("; report-uri ")
-		b.WriteString(f.ReportURI)
+	for _, s := range sources {
+		b.WriteString(" ")
+		b.WriteString(s)
+	}
+	b.WriteString("; ")
+
+	return b.String()
+}
+
+func report(reportURI string) string {
+	var b strings.Builder
+
+	if reportURI != "" {
+		b.WriteString("report-uri ")
+		b.WriteString(reportURI)
+		b.WriteString("; ")
 	}
 
 	return b.String()


Test patch:
diff --git a/safehttp/plugins/csp/csp_test.go b/safehttp/plugins/csp/csp_test.go
index 7272b31..1eefd20 100644
--- a/safehttp/plugins/csp/csp_test.go
+++ b/safehttp/plugins/csp/csp_test.go
@@ -89,12 +89,27 @@ func TestSerialize(t *testing.T) {
 		{
 			name:       "FramingCSP",
 			policy:     FramingPolicy{},
-			wantString: "frame-ancestors 'self'",
+			wantString: "frame-ancestors 'self';",
 		},
 		{
 			name:       "FramingCSP with report-uri",
 			policy:     FramingPolicy{ReportURI: "httsp://example.com/collector"},
-			wantString: "frame-ancestors 'self'; report-uri httsp://example.com/collector",
+			wantString: "frame-ancestors 'self'; report-uri httsp://example.com/collector;",
+		},
+		{
+			name: "FramingCSP with one source",
+			policy: FramingPolicy{Sources: []string{
+				"https://www.example.org",
+			}},
+			wantString: "frame-ancestors 'self' https://www.example.org;",
+		},
+		{
+			name: "FramingCSP with multiple sources",
+			policy: FramingPolicy{Sources: []string{
+				"https://a.example.org",
+				"https://b.example.org",
+			}},
+			wantString: "frame-ancestors 'self' https://a.example.org https://b.example.org;",
 		},
 		{
 			name:       "TrustedTypesCSP",
@@ -137,7 +152,7 @@ func TestBefore(t *testing.T) {
 			interceptor: Default(""),
 			wantEnforcePolicy: []string{
 				"object-src 'none'; script-src 'unsafe-inline' 'nonce-KSkpKSkpKSkpKSkpKSkpKSkpKSk=' 'strict-dynamic' https: http:; base-uri 'none'",
-				"frame-ancestors 'self'",
+				"frame-ancestors 'self';",
 				"require-trusted-types-for 'script'",
 			},
 			wantNonce: "KSkpKSkpKSkpKSkpKSkpKSkpKSk=",
@@ -147,7 +162,7 @@ func TestBefore(t *testing.T) {
 			interceptor: Default("https://example.com/collector"),
 			wantEnforcePolicy: []string{
 				"object-src 'none'; script-src 'unsafe-inline' 'nonce-KSkpKSkpKSkpKSkpKSkpKSkpKSk=' 'strict-dynamic' https: http:; base-uri 'none'; report-uri https://example.com/collector",
-				"frame-ancestors 'self'; report-uri https://example.com/collector",
+				"frame-ancestors 'self'; report-uri https://example.com/collector;",
 				"require-trusted-types-for 'script'; report-uri https://example.com/collector",
 			},
 			wantNonce: "KSkpKSkpKSkpKSkpKSkpKSkpKSk=",
@@ -171,7 +186,7 @@ func TestBefore(t *testing.T) {
 					FramingPolicy{ReportURI: "https://example.com/collector"},
 				},
 			},
-			wantReportOnlyPolicy: []string{"frame-ancestors 'self'; report-uri https://example.com/collector"},
+			wantReportOnlyPolicy: []string{"frame-ancestors 'self'; report-uri https://example.com/collector;"},
 			wantNonce:            "KSkpKSkpKSkpKSkpKSkpKSkpKSk=",
 		},
 	}
diff --git a/tests/integration/csp/csp_test.go b/tests/integration/csp/csp_test.go
index 22024e1..214ce32 100644
--- a/tests/integration/csp/csp_test.go
+++ b/tests/integration/csp/csp_test.go
@@ -72,7 +72,7 @@ func TestServeMuxInstallCSP(t *testing.T) {
 		"Content-Type": {"text/html; charset=utf-8"},
 		"Content-Security-Policy": {
 			"object-src 'none'; script-src 'unsafe-inline' 'nonce-" + nonce + "' 'strict-dynamic' https: http:; base-uri 'none'",
-			"frame-ancestors 'self'",
+			"frame-ancestors 'self';",
 			"require-trusted-types-for 'script'"},
 	}
 	if diff := cmp.Diff(wantHeaders, map[string][]string(rr.Header())); diff != "" {