diff --git a/cmd/hushd/handlers.go b/cmd/hushd/handlers.go index 30bdc89..04ed1ad 100644 --- a/cmd/hushd/handlers.go +++ b/cmd/hushd/handlers.go @@ -20,15 +20,14 @@ type Server struct { metrics *Metrics } -// pageData is the same for every render: the limits, so the browser enforces -// what the server enforces. It carries nothing request-specific, which is why -// both pages are safely static. +// pageData is the same for every render: the ciphertext cap, so the browser +// enforces what the server enforces, and the default lifetime, so the page +// states the TTL the server will apply. It carries nothing request-specific, +// which is why both pages are safely static. func (s *Server) pageData() web.Data { return web.Data{ MaxCiphertextBytes: secret.MaxCiphertextBytes, DefaultTTLSeconds: int(secret.DefaultTTL.Seconds()), - MinTTLSeconds: int(secret.MinTTL.Seconds()), - MaxTTLSeconds: int(secret.MaxTTL.Seconds()), } } diff --git a/cmd/hushd/handlers_test.go b/cmd/hushd/handlers_test.go index 869c6d1..3a4375e 100644 --- a/cmd/hushd/handlers_test.go +++ b/cmd/hushd/handlers_test.go @@ -131,6 +131,11 @@ func TestGettingTheRevealPageNeverConsumesTheSecret(t *testing.T) { // The reveal page must be identical for every id, including ids that were never // minted. If it 404'd on an unknown id it would become an oracle for whether a // link was ever real. +// +// Compared with the CSP nonce masked out: it is fresh per RESPONSE, so it makes +// two loads of the same id differ too. Masking it keeps the assertion on the +// property that matters — that nothing in the page varies with the id — instead +// of weakening to a substring check. func TestTheRevealPageDoesNotDiscloseWhetherASecretExists(t *testing.T) { h, _ := testApp(t) id := create(t, h, ciphertext("real")) @@ -139,10 +144,32 @@ func TestTheRevealPageDoesNotDiscloseWhetherASecretExists(t *testing.T) { _, _, fake := do(t, h, http.MethodGet, "/s/"+strings.Repeat("A", 43), "") _, _, junk := do(t, h, http.MethodGet, "/s/not-an-id", "") - if real != fake || real != junk { + if maskNonce(real) != maskNonce(fake) || maskNonce(real) != maskNonce(junk) { t.Fatal("the reveal page differs between a real id, a well-formed unknown id, and junk — " + "it must not disclose existence") } + // Equal after masking AND equal in length: a variable-length nonce would + // leak nothing about the id, but it would make Content-Length vary, and the + // masking above would hide that. + if len(real) != len(fake) || len(real) != len(junk) { + t.Fatalf("the reveal page's length varies with the id: %d, %d, %d", len(real), len(fake), len(junk)) + } +} + +// maskNonce replaces every occurrence of the page's own CSP nonce with a fixed +// token, so two responses can be compared for everything else. +func maskNonce(page string) string { + const marker = `nonce="` + i := strings.Index(page, marker) + if i < 0 { + return page + } + rest := page[i+len(marker):] + j := strings.IndexByte(rest, '"') + if j < 0 { + return page + } + return strings.ReplaceAll(page, rest[:j], "NONCE") } // A malformed id must produce the SAME 410 as a missing one. A 400 here would @@ -292,3 +319,92 @@ func TestPagesAreNotCacheable(t *testing.T) { } } } + +// The pages carry inline script and inline style, and the chassis sets a JSON +// API policy (`default-src 'none'`) on every response. Two policies on one +// response INTERSECT: when this override regresses, the browser blocks the +// page's own crypto and its fetch to /api, and every other test here still +// passes because the HTML is byte-identical. This is that regression. +func TestPagesSendOneNonceCSPThatPermitsTheirOwnInlineCode(t *testing.T) { + h, _ := testApp(t) + + for _, path := range []string{"/", "/s/" + strings.Repeat("A", 43)} { + r := httptest.NewRequest(http.MethodGet, path, nil) + w := httptest.NewRecorder() + h.ServeHTTP(w, r) + + got := w.Header().Values("Content-Security-Policy") + if len(got) != 1 { + t.Fatalf("%s sent %d CSP headers %q, want exactly 1 — policies intersect, so a second one only subtracts", path, len(got), got) + } + policy := got[0] + + nonce := cspNonce(t, path, policy) + body := w.Body.String() + + // Every inline block must carry this response's nonce. A single + // unnonced diff --git a/internal/web/web.go b/internal/web/web.go index 7630d70..6b94c7b 100644 --- a/internal/web/web.go +++ b/internal/web/web.go @@ -3,10 +3,13 @@ package web import ( + "crypto/rand" "embed" + "encoding/base64" "fmt" "html/template" "net/http" + "strconv" ) //go:embed templates/*.html @@ -19,14 +22,23 @@ type Pages struct { reveal *template.Template } -// Data is everything a page needs. The limits are passed through so the browser -// enforces the same caps the server does and a user learns their secret is too -// large before uploading it, not after. +// Data is everything a page needs. MaxCiphertextBytes is passed through so the +// browser enforces the same cap the server does and a user learns their secret +// is too large before uploading it, not after. DefaultTTLSeconds is the only +// other value: the UI does not offer a TTL choice, so the page states the +// lifetime the server will apply rather than asking for one. type Data struct { MaxCiphertextBytes int DefaultTTLSeconds int - MinTTLSeconds int - MaxTTLSeconds int +} + +// view is what the templates actually execute against: Data plus the two values +// that are derived per render. Nonce is NOT on Data on purpose — a caller that +// could set it could reuse one, and a reused nonce is the same as no nonce. +type view struct { + Data + Nonce string + DefaultTTL string } // New parses the embedded templates. It fails at boot rather than on first @@ -60,14 +72,97 @@ func (p *Pages) Reveal(w http.ResponseWriter, d Data) error { return render(w, p.reveal, d) } +// contentSecurityPolicy is the page policy, keyed to one per-response nonce. +// +// It is sent as a HEADER and the pages carry no CSP , which is not a +// style preference — both halves are load-bearing: +// +// - The chassis sets `default-src 'none'` for its JSON API surface. Two +// policies delivered on one response INTERSECT, so a loosening +// script-src cannot re-enable anything the header forbids; the browser +// blocked this page's own inline script and style, and its fetch to /api, +// while the looked permissive. Overriding the header here leaves +// exactly one policy on the response. +// - `frame-ancestors` is ignored entirely when delivered via , so the +// clickjacking half of the policy only exists as a header. +// +// A nonce rather than 'unsafe-inline': the whole guarantee is that no code +// except this reviewed, same-document script can reach the key in the +// fragment, and 'unsafe-inline' would extend that permission to any script an +// injection managed to place on the page. +func contentSecurityPolicy(nonce string) string { + return "default-src 'none'" + + "; script-src 'nonce-" + nonce + "'" + + "; style-src 'nonce-" + nonce + "'" + + // The pages fetch /api/secrets and /api/secrets/{id}/reveal. Same-origin + // only: there is no other host this page may ever talk to. + "; connect-src 'self'" + + // No image, font, media or frame is loaded by either page, so every + // remaining fetch directive stays at default-src 'none'. + "; form-action 'none'" + + "; base-uri 'none'" + + "; frame-ancestors 'none'" +} + func render(w http.ResponseWriter, t *template.Template, d Data) error { + nonce, err := newNonce() + if err != nil { + // No entropy means no nonce, and a page rendered without one is a page + // whose own script the browser will refuse. Fail loudly instead. + return fmt.Errorf("csp nonce: %w", err) + } + + h := w.Header() // no-store on both pages: a cached create page is harmless, but a cached // reveal page in a shared proxy would be a copy of a one-time URL. - w.Header().Set("Cache-Control", "no-store, max-age=0") - w.Header().Set("Content-Type", "text/html; charset=utf-8") + h.Set("Cache-Control", "no-store, max-age=0") + h.Set("Content-Type", "text/html; charset=utf-8") // Referrer-Policy is load-bearing here, not boilerplate: without it a click // on any link from the reveal page could send the full URL — including the // fragment-adjacent path — to a third party. - w.Header().Set("Referrer-Policy", "no-referrer") - return t.ExecuteTemplate(w, "base.html", d) + h.Set("Referrer-Policy", "no-referrer") + // Set, not Add: this REPLACES the chassis API policy for these two routes. + h.Set("Content-Security-Policy", contentSecurityPolicy(nonce)) + + return t.ExecuteTemplate(w, "base.html", view{ + Data: d, + Nonce: nonce, + DefaultTTL: humanSeconds(d.DefaultTTLSeconds), + }) +} + +// newNonce returns 128 bits of base64 for one response. CSP's nonce grammar is +// base64, so the encoding is part of the contract rather than a convenience — +// and the URL alphabet specifically, because '+' and '/' are escaped to +// character references inside an HTML attribute, leaving the nonce the browser +// parses to depend on entity decoding rather than on these bytes. +func newNonce() (string, error) { + b := make([]byte, 16) + if _, err := rand.Read(b); err != nil { + return "", err + } + return base64.RawURLEncoding.EncodeToString(b), nil +} + +// humanSeconds renders a TTL the way the page says it out loud. The server owns +// the number; this only decides whether to call it days, hours or minutes. +func humanSeconds(sec int) string { + switch { + case sec%86400 == 0 && sec >= 86400: + return plural(sec/86400, "day") + case sec%3600 == 0 && sec >= 3600: + return plural(sec/3600, "hour") + case sec >= 60: + return plural(sec/60, "minute") + default: + return plural(sec, "second") + } +} + +func plural(n int, unit string) string { + s := strconv.Itoa(n) + " " + unit + if n != 1 { + s += "s" + } + return s } diff --git a/scripts/release.sh b/scripts/release.sh index 3a2964f..f5a652c 100755 --- a/scripts/release.sh +++ b/scripts/release.sh @@ -1,14 +1,15 @@ #!/usr/bin/env bash # Build the current commit in-cluster and roll it out. No CI credential needed. # -# This exists because Woodpecker is not activated for this repo (its API token -# in rdev returns 401, and minting a new one needs a browser login). Rather than -# leave "git push does not deploy" as a trap for whoever pushes next, this does -# exactly what the pipeline's build+deploy steps do: a Kaniko Job for an amd64 -# image, then `kubectl set image`, then a real end-to-end check. +# Woodpecker IS activated for this repo, so a push to main builds and deploys. +# This is the path for when you do not want to wait for CI, when CI is down, or +# when you are rolling back — and it is how the first deploy happened, before +# activation. It does exactly what the pipeline's build and deploy steps do: a +# Kaniko Job for an amd64 image from the pushed git ref, then +# `kubectl set image`, then a real end-to-end check. # -# When Woodpecker is activated this becomes redundant, and that is fine — it is -# also the manual path for a rollback or a hotfix when CI is down. +# Credentials: none. The Gitea repo is public so the Kaniko git context needs no +# token, and the rollout uses your kubeconfig. # # ./scripts/release.sh set -euo pipefail