many: change CSP to use hashes instead of nonces
Change our content security policy (CSP) for scripts.
Instead of using a nonce, which lends itself poorly to caching, use
hashes. See https://csp.withgoogle.com/docs/faq.html, search for "CSP
hashes".
To make hashes work, the hash of every inline script must appear in
our Content-Security-Policy header.
Also, not all browsers support hashing with scripts loaded from files,
so we must dynamically load the files by using an inline script that
builds a script tag with a src attribute. (We need to do this anyway
for the Google Tag Manager script.) See the link above for a
description of the technique. It works because the CSP header mentions
'strict-dynamic', which trusts everything loaded from a trusted
script.
Ideally, we would both generate all these hashes automatically,
and check that they are all correct. This CL doesn't do that.
A followup CL will.
List of changes:
- Replace script tags with scr attributes with inline scripts that
load from the files.
- In internal/middleware/secureheaders.go, add the list of script
hashes to the CSP header.
- Remove all references to nonces.
Updates b/159711607.
Change-Id: Ia9b78ecd85e24619e758f2580a370778708b9e71
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/239897
Reviewed-by: Roberto Clapis <robclap8@gmail.com>
Reviewed-by: Julie Qiu <julie@golang.org>
diff --git a/content/static/html/base.tmpl b/content/static/html/base.tmpl
index 92195d7..b3edc9a 100644
--- a/content/static/html/base.tmpl
+++ b/content/static/html/base.tmpl
@@ -179,14 +179,38 @@
</div>
</div>
</footer>
+
+<script>
+ function loadScript(src) {
+ let s = document.createElement("script");
+ s.src = src;
+ document.head.appendChild(s);
+ }
+
+ loadScript("/static/js/base.min.js");
+</script>
+
{{block "post_content" .}}{{end}}
+
{{if not .DevMode}}
- <script nonce="{{.Nonce}}" async src="https://www.googletagmanager.com/gtm.js?id={{.GoogleTagManagerContainerID}}"></script>
- <noscript><iframe nonce="{{.Nonce}}" src="https://www.googletagmanager.com/ns.html?id={{.GoogleTagManagerContainerID}}"
- height="0" width="0" style="display:none;visibility:hidden"></iframe></noscript>
+<script async>
+ var gtmURL = "https://www.googletagmanager.com/gtm.js?id=GTM-W8MVQXG";
+ loadScript(gtmURL);
+
+ f = document.createElement("iframe")
+ f.src = gtmURL;
+ f.height = "0";
+ f.width = "0";
+ f.style = "display:none;visibility:hidden"
+ s = document.createElement("noscript");
+ s.appendChild(f);
+ document.head.appendChild(s);
+</script>
{{end}}
-<script nonce="{{.Nonce}}" src="/static/js/base.min.js?version={{.AppVersionLabel}}"></script>
+
{{if (.Experiments.IsActive "autocomplete")}}
- <script nonce="{{.Nonce}}" src="/third_party/autoComplete.js/autoComplete.min.js?version={{.AppVersionLabel}}"></script>
- <script nonce="{{.Nonce}}" src="/static/js/completion.min.js?version={{.AppVersionLabel}}"></script>
+<script>
+ loadScript("/third_party/autoComplete.js/autoComplete.min.js");
+ loadScript("/static/js/completion.min.js");
+</script>
{{end}}
diff --git a/content/static/html/pages/details.tmpl b/content/static/html/pages/details.tmpl
index 50c48e2..096ce0f 100644
--- a/content/static/html/pages/details.tmpl
+++ b/content/static/html/pages/details.tmpl
@@ -120,7 +120,12 @@
{{end}}
{{define "post_content"}}
-<script nonce="{{.Nonce}}">
+
+<!-- inputEl.blur() below prevents a jump to the focused element in some browsers.
+ Do not add comments to the script below: they are stripped by html/template
+ (https://golang.org/issue/28628), which messes up the CSP hash.
+-->
+<script>
const navEl = document.querySelector('.js-modulesNav');
const selectedEl = navEl.querySelector(`[aria-selected='true']`);
if (selectedEl.offsetLeft + selectedEl.offsetWidth > navEl.offsetWidth) {
@@ -134,7 +139,7 @@
const inputEl = document.querySelector('.js-detailsHeaderPathInput');
inputEl.select();
document.execCommand('copy');
- inputEl.blur(); // prevents jump to focused element in some browsers
+ inputEl.blur();
});
}
</script>
diff --git a/content/static/html/pages/notfound.tmpl b/content/static/html/pages/notfound.tmpl
index 3c71936..b51399b 100644
--- a/content/static/html/pages/notfound.tmpl
+++ b/content/static/html/pages/notfound.tmpl
@@ -15,7 +15,12 @@
</div>
</div>
-<script nonce="{{.Nonce}}">
+<!-- TODO: update middleware.AcceptMethods so that the GET at the end of the
+ script can be a POST instead.
+ Do not add comments to the script below: they are stripped by html/template
+ (https://golang.org/issue/28628), which messes up the CSP hash.
+-->
+<script>
const fetchButton = document.querySelector('.js-notFoundButton');
if (fetchButton) {
fetchButton.addEventListener('click', e => {
@@ -45,7 +50,6 @@
};
document.querySelector('.js-notFoundMessage').innerHTML = "Fetching... Feel free to navigate away and check back later, we'll keep working on it!";
btn.innerHTML = "Fetching...";
- // TODO: update middleware.AcceptMethods so that this is POST instead of a GET request.
httpRequest.open('GET', "/fetch" + window.location.pathname);
httpRequest.send();
}
diff --git a/content/static/html/pages/pkg_doc.tmpl b/content/static/html/pages/pkg_doc.tmpl
index 18e5794..81aa73a 100644
--- a/content/static/html/pages/pkg_doc.tmpl
+++ b/content/static/html/pages/pkg_doc.tmpl
@@ -49,5 +49,7 @@
{{end}}
{{define "details_post_content"}}
- <script nonce="{{.Nonce}}" src="/static/js/jump.min.js?version={{.AppVersionLabel}}"></script>
+<script>
+loadScript("/static/js/jump.min.js");
+</script>
{{end}}
diff --git a/internal/frontend/server.go b/internal/frontend/server.go
index 4db0c78..a72ef01 100644
--- a/internal/frontend/server.go
+++ b/internal/frontend/server.go
@@ -186,7 +186,6 @@
type basePage struct {
HTMLTitle string
Query string
- Nonce string
Experiments *experiment.Set
GodocURL string
DevMode bool
@@ -217,7 +216,6 @@
return basePage{
HTMLTitle: title,
Query: searchQuery(r),
- Nonce: middleware.NoncePlaceholder,
Experiments: experiment.FromContext(r.Context()),
GodocURL: middleware.GodocURLPlaceholder,
DevMode: s.devMode,
@@ -225,11 +223,6 @@
}
}
-// GoogleTagManagerContainerID returns the container ID from GoogleTagManager.
-func (b basePage) GoogleTagManagerContainerID() string {
- return "GTM-W8MVQXG"
-}
-
// errorPage contains fields for rendering a HTTP error page.
type errorPage struct {
basePage
@@ -320,9 +313,6 @@
if page == nil {
page = &errorPage{}
}
- if page.Nonce == "" {
- page.Nonce = middleware.NoncePlaceholder
- }
if page.messageTemplate == "" {
page.messageTemplate = `<h3 class="Error-message">{{.}}</h3>`
}
diff --git a/internal/middleware/godoc.go b/internal/middleware/godoc.go
index 8920442..e93b828 100644
--- a/internal/middleware/godoc.go
+++ b/internal/middleware/godoc.go
@@ -102,3 +102,18 @@
result.RawQuery = q.Encode()
return result.String()
}
+
+// capturingResponseWriter is an http.ResponseWriter that captures
+// the body for later processing.
+type capturingResponseWriter struct {
+ http.ResponseWriter
+ buf bytes.Buffer
+}
+
+func (c *capturingResponseWriter) Write(b []byte) (int, error) {
+ return c.buf.Write(b)
+}
+
+func (c *capturingResponseWriter) bytes() []byte {
+ return c.buf.Bytes()
+}
diff --git a/internal/middleware/nonce.go b/internal/middleware/nonce.go
deleted file mode 100644
index d12b48a..0000000
--- a/internal/middleware/nonce.go
+++ /dev/null
@@ -1,27 +0,0 @@
-// Copyright 2019 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-// Package middleware implements a simple middleware pattern for http handlers,
-// along with implementations for some common middlewares.
-package middleware
-
-import (
- "crypto/rand"
- "encoding/base64"
- "fmt"
- "io"
-)
-
-const (
- nonceCtxKey = "CSPNonce"
- nonceLen = 20
-)
-
-func generateNonce() (string, error) {
- nonceBytes := make([]byte, nonceLen)
- if _, err := io.ReadAtLeast(rand.Reader, nonceBytes, nonceLen); err != nil {
- return "", fmt.Errorf("io.ReadAtLeast(rand.Reader, nonceBytes, %d): %v", nonceLen, err)
- }
- return base64.StdEncoding.EncodeToString(nonceBytes), nil
-}
diff --git a/internal/middleware/secureheaders.go b/internal/middleware/secureheaders.go
index 959c36b..ee36bc2 100644
--- a/internal/middleware/secureheaders.go
+++ b/internal/middleware/secureheaders.go
@@ -5,27 +5,29 @@
package middleware
import (
- "bytes"
"fmt"
"net/http"
"strings"
-
- "golang.org/x/pkgsite/internal/log"
)
-// NoncePlaceholder should be used as the value for nonces in rendered content.
-// It is substituted for the actual nonce value by the SecureHeaders middleware.
-const NoncePlaceholder = "$$GODISCOVERYNONCE$$"
+var scriptHashes = []string{
+ // From content/static/html/pages/base.tmpl
+ "'sha256-d6W7MwuGWbguTHRzQhf5QN1jXmNo9Ao218saZkWLWZI='",
+ "'sha256-CCu0fuIQFBHSCEpfR6ZRzzcczJIS/VGMGrez8LR49WY='",
+ "'sha256-qPGTOKPn+niRiNKQIEX0Ktwuj+D+iPQWIxnlhPicw58='",
+ // From content/static/html/pages/notfound.tmpl
+ "'sha256-h5L4TV5GzTaBQYCnA8tDw9+9/AIdK9dwgkwlqFjVqEI='",
+ // From content/static/html/pages/details.tmpl
+ "'sha256-s16e7aT7Gsajq5UH1DbaEFEnNx2VjvS5Xixcxwm4+F8='",
+ // From content/static/html/pages/pkg_doc.tmpl
+ "'sha256-AvMTqQ+22BA0Nsht+ajju4EQseFQsoG1RxW3Nh6M+wc='",
+}
// SecureHeaders adds a content-security-policy and other security-related
// headers to all responses.
func SecureHeaders() Middleware {
return func(h http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
- nonce, err := generateNonce()
- if err != nil {
- log.Infof(r.Context(), "generateNonce(): %v", err)
- }
csp := []string{
// Disallow plugin content: pkg.go.dev does not use it.
"object-src 'none'",
@@ -33,9 +35,8 @@
// locations of scripts loaded from relative URLs. The site doesn’t have
// a <base> tag anyway.
"base-uri 'none'",
-
- fmt.Sprintf("script-src 'nonce-%s' 'unsafe-inline' 'strict-dynamic' https: http:",
- nonce),
+ fmt.Sprintf("script-src 'unsafe-inline' 'strict-dynamic' https: http: %s",
+ strings.Join(scriptHashes, " ")),
}
w.Header().Set("Content-Security-Policy", strings.Join(csp, "; "))
// Don't allow frame embedding.
@@ -43,27 +44,7 @@
// Prevent MIME sniffing.
w.Header().Set("X-Content-Type-Options", "nosniff")
- crw := &capturingResponseWriter{ResponseWriter: w}
- h.ServeHTTP(crw, r)
- body := bytes.ReplaceAll(crw.bytes(), []byte(NoncePlaceholder), []byte(nonce))
- if _, err := w.Write(body); err != nil {
- log.Errorf(r.Context(), "SecureHeaders, writing: %v", err)
- }
+ h.ServeHTTP(w, r)
})
}
}
-
-// capturingResponseWriter is an http.ResponseWriter that captures
-// the body for later processing.
-type capturingResponseWriter struct {
- http.ResponseWriter
- buf bytes.Buffer
-}
-
-func (c *capturingResponseWriter) Write(b []byte) (int, error) {
- return c.buf.Write(b)
-}
-
-func (c *capturingResponseWriter) bytes() []byte {
- return c.buf.Bytes()
-}
diff --git a/internal/middleware/secureheaders_test.go b/internal/middleware/secureheaders_test.go
index d5bc075..ebb0566 100644
--- a/internal/middleware/secureheaders_test.go
+++ b/internal/middleware/secureheaders_test.go
@@ -5,36 +5,13 @@
package middleware
import (
- "fmt"
- "io/ioutil"
"net/http"
"net/http/httptest"
- "regexp"
"testing"
)
func TestSecureHeaders(t *testing.T) {
- const origBody = `
- <link foo>
- <script nonce="$$GODISCOVERYNONCE$$" async src="bar"></script>
- blah blah blah
- <script nonce="$$GODISCOVERYNONCE$$">js</script>
- bloo bloo bloo
- <iframe nonce="$$GODISCOVERYNONCE$$" src="baz"></iframe>
-`
-
- const wantBodyFmt = `
- <link foo>
- <script nonce="%[1]s" async src="bar"></script>
- blah blah blah
- <script nonce="%[1]s">js</script>
- bloo bloo bloo
- <iframe nonce="%[1]s" src="baz"></iframe>
-`
-
- handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
- fmt.Fprint(w, origBody)
- })
+ handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})
mw := SecureHeaders()
ts := httptest.NewServer(mw(handler))
defer ts.Close()
@@ -43,7 +20,7 @@
t.Errorf("GET returned error %v", err)
}
defer resp.Body.Close()
- // Simply test that the expected headers are set.
+ // Test that the expected headers are set.
expectedHeaders := []string{
"content-security-policy",
"x-frame-options",
@@ -54,21 +31,4 @@
t.Errorf("GET returned empty %s", header)
}
}
-
- // Check that the nonce was substituted correctly.
- // We need to extract it from the header.
- nonceRE := regexp.MustCompile(`'nonce-([^']+)'`)
- matches := nonceRE.FindStringSubmatch(resp.Header.Get("content-security-policy"))
- if matches == nil {
- t.Fatal("cannot extract nonce")
- }
- body, err := ioutil.ReadAll(resp.Body)
- if err != nil {
- t.Fatal(err)
- }
- gotBody := string(body)
- wantBody := fmt.Sprintf(wantBodyFmt, matches[1])
- if gotBody != wantBody {
- t.Errorf("got body %s\nwant body %s", gotBody, wantBody)
- }
}
