Jelajahi Sumber

feat(template): extract CSP to a function, systematically use nonces, and use `default-src 'none'` instead of `self`

Having the CSP built in a function instead of in the template makes it easier
to properly construct it. This was also the opportunity to switch from
default-src 'self' to default-src 'none', to deny everything that isn't
explicitly allowed, instead of allowing everything coming from 'self'.

Moreover, as Miniflux is shoving the content of feeds in the same origin as
itself, using self doesn't do much security-wise. It's much better to
systematically use a nonce-based policy, so that an attacker able to bypass the
sanitization will have to guess the nonce to gain arbitrary javascript
execution.
Julien Voisin 6 bulan lalu
induk
melakukan
bf7f55e28a

+ 35 - 0
internal/template/functions.go

@@ -33,6 +33,7 @@ type funcMap struct {
 func (f *funcMap) Map() template.FuncMap {
 	return template.FuncMap{
 		"contains":         strings.Contains,
+		"csp":              csp,
 		"startsWith":       strings.HasPrefix,
 		"formatFileSize":   formatFileSize,
 		"dict":             dict,
@@ -116,6 +117,40 @@ func (f *funcMap) Map() template.FuncMap {
 	}
 }
 
+func csp(user *model.User, nonce string) string {
+	policies := map[string]string{
+		"default-src":               "'none'",
+		"frame-src":                 "*",
+		"img-src":                   "* data:",
+		"manifest-src":              "'self'",
+		"media-src":                 "*",
+		"require-trusted-types-for": "'script'",
+		"script-src":                "'nonce-" + nonce + "' 'strict-dynamic'",
+		"style-src":                 "'nonce-" + nonce + "'",
+		"trusted-types":             "html url",
+		"connect-src":               "'self'",
+	}
+
+	if user != nil {
+		if user.ExternalFontHosts != "" {
+			policies["font-src"] = user.ExternalFontHosts
+			if user.Stylesheet != "" {
+				policies["style-src"] += " " + user.ExternalFontHosts
+			}
+		}
+	}
+
+	var policy strings.Builder
+	for key, value := range policies {
+		policy.WriteString(key)
+		policy.WriteString(" ")
+		policy.WriteString(value)
+		policy.WriteString("; ")
+	}
+
+	return `<meta http-equiv="Content-Security-Policy" content="` + policy.String() + `">`
+}
+
 func dict(values ...any) (map[string]any, error) {
 	if len(values)%2 != 0 {
 		return nil, fmt.Errorf("dict expects an even number of arguments")

+ 91 - 0
internal/template/functions_test.go

@@ -4,10 +4,12 @@
 package template // import "miniflux.app/v2/internal/template"
 
 import (
+	"strings"
 	"testing"
 	"time"
 
 	"miniflux.app/v2/internal/locale"
+	"miniflux.app/v2/internal/model"
 )
 
 func TestDict(t *testing.T) {
@@ -159,3 +161,92 @@ func TestFormatFileSize(t *testing.T) {
 		}
 	}
 }
+
+func TestCSPExternalFont(t *testing.T) {
+	want := []string{
+		`default-src 'none';`,
+		`img-src * data:;`,
+		`media-src *;`,
+		`frame-src *;`,
+		`style-src 'nonce-1234';`,
+		`script-src 'nonce-1234'`,
+		`'strict-dynamic';`,
+		`font-src test.com;`,
+		`require-trusted-types-for 'script';`,
+		`trusted-types html url;`,
+		`manifest-src 'self';`,
+	}
+	got := csp(&model.User{ExternalFontHosts: "test.com"}, "1234")
+
+	for _, value := range want {
+		if !strings.Contains(got, value) {
+			t.Errorf(`Unexpected result, didn't find %q in %q`, value, got)
+		}
+	}
+}
+
+func TestCSPNoUser(t *testing.T) {
+	want := []string{
+		`default-src 'none';`,
+		`img-src * data:;`,
+		`media-src *;`,
+		`frame-src *;`,
+		`style-src 'nonce-1234';`,
+		`script-src 'nonce-1234'`,
+		`'strict-dynamic';`,
+		`require-trusted-types-for 'script';`,
+		`trusted-types html url;`,
+		`manifest-src 'self';`,
+	}
+	got := csp(nil, "1234")
+
+	for _, value := range want {
+		if !strings.Contains(got, value) {
+			t.Errorf(`Unexpected result, didn't find %q in %q`, value, got)
+		}
+	}
+}
+
+func TestCSPCustomJSExternalFont(t *testing.T) {
+	want := []string{
+		`default-src 'none';`,
+		`img-src * data:;`,
+		`media-src *;`,
+		`frame-src *;`,
+		`style-src 'nonce-1234';`,
+		`script-src 'nonce-1234'`,
+		`'strict-dynamic';`,
+		`require-trusted-types-for 'script';`,
+		`trusted-types html url;`,
+		`manifest-src 'self';`,
+	}
+	got := csp(&model.User{ExternalFontHosts: "test.com", CustomJS: "alert(1)"}, "1234")
+
+	for _, value := range want {
+		if !strings.Contains(got, value) {
+			t.Errorf(`Unexpected result, didn't find %q in %q`, value, got)
+		}
+	}
+}
+
+func TestCSPExternalFontStylesheet(t *testing.T) {
+	want := []string{
+		`default-src 'none';`,
+		`img-src * data:;`,
+		`media-src *;`,
+		`frame-src *;`,
+		`style-src 'nonce-1234' test.com;`,
+		`script-src 'nonce-1234'`,
+		`'strict-dynamic';`,
+		`require-trusted-types-for 'script';`,
+		`trusted-types html url;`,
+		`manifest-src 'self';`,
+	}
+	got := csp(&model.User{ExternalFontHosts: "test.com", Stylesheet: "a {color: red;}"}, "1234")
+
+	for _, value := range want {
+		if !strings.Contains(got, value) {
+			t.Errorf(`Unexpected result, didn't find %q in %q`, value, got)
+		}
+	}
+}

+ 5 - 11
internal/template/templates/common/layout.html

@@ -25,24 +25,18 @@
     <link rel="apple-touch-icon" sizes="167x167" href="{{ route "appIcon" "filename" "icon-167.png" }}">
     <link rel="apple-touch-icon" sizes="180x180" href="{{ route "appIcon" "filename" "icon-180.png" }}">
 
-    <link rel="stylesheet" type="text/css" href="{{ route "stylesheet" "name" .theme "checksum" .theme_checksum }}">
-
-    {{ if .user }}
-        {{ $cspNonce := nonce }}
-        <meta http-equiv="Content-Security-Policy" content="default-src 'self'; img-src * data:; media-src *; frame-src *; {{ if .user.ExternalFontHosts }}font-src {{ .user.ExternalFontHosts }}; {{ end }}style-src 'self'{{ if .user.Stylesheet }}{{ if .user.ExternalFontHosts }} {{ .user.ExternalFontHosts }}{{ end }} 'nonce-{{ $cspNonce }}'{{ end }}{{ if .user.CustomJS }}; script-src 'self' 'nonce-{{ $cspNonce }}'{{ end }}; require-trusted-types-for 'script'; trusted-types html url;">
-
+    {{ $cspNonce := nonce }}
+    {{ csp .user $cspNonce | safeHTML }}
+    <link rel="stylesheet" nonce="{{ $cspNonce }}" type="text/css" href="{{ route "stylesheet" "name" .theme "checksum" .theme_checksum }}">
+    <script nonce="{{ $cspNonce }}" src="{{ route "javascript" "name" "app" "checksum" .app_js_checksum }}" type="module"></script>
+    {{ if .user -}}
         {{ if .user.Stylesheet -}}
         <style nonce="{{ $cspNonce }}">{{ .user.Stylesheet | safeCSS }}</style>
         {{ end -}}
-
         {{ if .user.CustomJS -}}
         <script type="module" nonce="{{ $cspNonce }}">{{ .user.CustomJS | safeJS }}</script>
         {{ end -}}
-    {{ else -}}
-        <meta http-equiv="Content-Security-Policy" content="default-src 'self'; img-src * data:; media-src *; frame-src *; require-trusted-types-for 'script'; trusted-types html url;">
     {{ end -}}
-
-    <script src="{{ route "javascript" "name" "app" "checksum" .app_js_checksum }}" type="module"></script>
 </head>
 <body
     data-service-worker-url="{{ route "javascript" "name" "service-worker" "checksum" .sw_js_checksum }}"