From 603e93355b7eaa3229e14cfdae07984f188bb21f Mon Sep 17 00:00:00 2001 From: Pierre-Olivier Mercier Date: Mon, 27 Apr 2026 11:50:42 +0700 Subject: [PATCH] Deepen CSP, Permissions-Policy and cookie audits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Detect CSP weaknesses individually (unsafe-inline, unsafe-eval, missing default-src/script-src, permissive sources on script-src or its default-src fallback) instead of a single catch-all "unsafe" code, and honour CSP3 fetch-directive fallback via EffectiveSources/WildcardSource helpers. Validate Permissions-Policy values: warn when a powerful feature (camera, microphone, geolocation, payment, sensors, …) is granted to all origins. Add a SameSite aggregate state on cookie audits so callers get the global ratio alongside per-cookie diagnostics. --- checker/header_rule.go | 200 ++++++++----- checker/headers.go | 144 +++++++-- checker/rules_cookies.go | 12 + checker/rules_cookies_test.go | 43 ++- checker/rules_modern_headers.go | 128 +++++++- checker/rules_modern_headers_test.go | 10 +- checker/rules_security_headers.go | 399 +++++++++++++------------ checker/rules_security_headers_test.go | 123 ++++++-- 8 files changed, 746 insertions(+), 313 deletions(-) diff --git a/checker/header_rule.go b/checker/header_rule.go index b57f5a7..4a08ab1 100644 --- a/checker/header_rule.go +++ b/checker/header_rule.go @@ -11,50 +11,71 @@ import ( sdk "git.happydns.org/checker-sdk-go/checker" ) -// HeaderRuleSpec declares a "presence + value validation" rule for one -// HTTP response header. It covers the most common shape of security -// header rule (one of Referrer-Policy, Permissions-Policy, COOP, COEP, -// CORP, X-Content-Type-Options, …) without forcing the author to write -// the load/iterate/build-state scaffolding. +// HeaderResult is one observation produced by a HeaderRuleSpec callback. +// Suffix is appended to spec.Code (with a dot separator) to form the +// final CheckState code, e.g. "http.hsts" + "short_max_age" → +// "http.hsts.short_max_age". An empty Suffix uses spec.Code verbatim. +type HeaderResult struct { + Status sdk.Status + Suffix string + Message string + Meta map[string]any +} + +// HeaderRuleSpec declares a per-HTTPS-probe rule built around a single +// response header. It supersedes the per-rule Evaluate boilerplate that +// every "load HTTPData → iterate successful HTTPS probes → inspect one +// header → emit one CheckState" rule used to repeat. // -// The DSL emits three CheckState codes derived from Code: -// - Code+".missing" when the header is absent -// - Code+".invalid" when Validate returns a non-OK status -// - Code+".ok" when Validate accepts the value +// Three callbacks cover the spectrum, from simplest to most expressive: // -// Rules with richer semantics (HSTS quality thresholds, CSP directive -// inspection, cookie flag aggregation, legacy headers with reversed -// "absent is fine" semantics) keep implementing sdk.CheckRule directly. +// - Validate: the header is present and a single boolean verdict is +// enough. Returns (Status, message); the rule emits ".ok" on +// StatusOK or ".invalid" otherwise. Used by the modern privacy +// headers (Referrer-Policy, COOP/COEP/CORP, Permissions-Policy). +// +// - Inspect: the header is present and may produce any number of +// findings with arbitrary suffixes. Used by HSTS (".short_max_age"), +// CSP (".unsafe_inline" / ".wildcard_script_src" / …) and the +// legacy X-XSS-Protection rule which reports custom suffixes +// (".disabled", ".enabled"). +// +// - OnMissing: the header is absent and the default ".missing" +// emitter is wrong — either an alternative satisfies the +// requirement (CSP frame-ancestors standing in for X-Frame-Options), +// or absence has non-default severity (X-XSS-Protection emits +// Info ".absent", not Warn ".missing"), or the severity depends +// on a CheckerOption (HSTS/CSP gate "missing" on a configurable +// "required" flag). +// +// Validate and Inspect are mutually exclusive. OnMissing can be combined +// with either. Specs that omit all three behave as a pure presence check +// (".ok" when set, default ".missing" when not). type HeaderRuleSpec struct { - // Code is the rule's Name() and the prefix for every CheckState - // code it emits. - Code string - - // Description is returned by Description(). + Code string Description string + Header string - // Header is the response header to inspect. Lookups go through the - // lowercased map populated by the collector, so casing is flexible. - Header string - - // Required toggles the severity of an absent header: Warn when true, - // Info when false. + // Required toggles the severity of the default ".missing" emitter + // (Warn when true, Info when false). Ignored when OnMissing is set. Required bool - // Validate, when set, inspects the trimmed header value. Return - // (StatusOK, msg) to accept the value (emits ".ok" with msg) or any - // other status to flag it (emits ".invalid" with msg). When nil, - // presence alone is treated as OK with a generic message. - Validate func(value string) (sdk.Status, string) - - // FixHint, when set, is attached as Meta.fix on the ".missing" - // state. + // FixHint, when set, populates Meta.fix on the default ".missing" + // emitter. Ignored when OnMissing is set (callbacks must build + // their own Meta). FixHint string + + Validate func(value string) (sdk.Status, string) + Inspect func(value string, p HTTPProbe, opts sdk.CheckerOptions) []HeaderResult + OnMissing func(p HTTPProbe, opts sdk.CheckerOptions) []HeaderResult } // HeaderRule constructs a self-contained sdk.CheckRule from a spec. // Intended to be wired in init() via RegisterRule. func HeaderRule(spec HeaderRuleSpec) sdk.CheckRule { + if spec.Validate != nil && spec.Inspect != nil { + panic("checker: HeaderRuleSpec " + spec.Code + " sets both Validate and Inspect") + } return &headerRule{spec: spec} } @@ -63,49 +84,90 @@ type headerRule struct{ spec HeaderRuleSpec } func (r *headerRule) Name() string { return r.spec.Code } func (r *headerRule) Description() string { return r.spec.Description } -func (r *headerRule) Evaluate(ctx context.Context, obs sdk.ObservationGetter, _ sdk.CheckerOptions) []sdk.CheckState { +func (r *headerRule) Evaluate(ctx context.Context, obs sdk.ObservationGetter, opts sdk.CheckerOptions) []sdk.CheckState { data, errSt := loadHTTPData(ctx, obs) if errSt != nil { return []sdk.CheckState{*errSt} } + probes := successfulHTTPSProbes(data.Probes) + if len(probes) == 0 { + return []sdk.CheckState{unknownState(r.spec.Code+".no_https", "No successful HTTPS probe to evaluate.")} + } headerKey := strings.ToLower(r.spec.Header) - return EvalPerHTTPS(data, r.spec.Code, func(p HTTPProbe) sdk.CheckState { - v := strings.TrimSpace(p.Headers[headerKey]) - if v == "" { - status := sdk.StatusWarn - if !r.spec.Required { - status = sdk.StatusInfo - } - st := sdk.CheckState{ - Status: status, - Code: r.spec.Code + ".missing", - Subject: p.Address, - Message: r.spec.Header + " is not set.", - } - if r.spec.FixHint != "" { - st.Meta = map[string]any{"fix": r.spec.FixHint} - } - return st + out := make([]sdk.CheckState, 0, len(probes)) + for _, p := range probes { + for _, res := range r.evaluateProbe(p, opts, headerKey) { + out = append(out, r.toCheckState(p, res)) } - if r.spec.Validate == nil { - return sdk.CheckState{ - Status: sdk.StatusOK, - Code: r.spec.Code + ".ok", - Subject: p.Address, - Message: r.spec.Header + " is set.", - } - } - status, msg := r.spec.Validate(v) - suffix := ".invalid" - if status == sdk.StatusOK { - suffix = ".ok" - } - return sdk.CheckState{ - Status: status, - Code: r.spec.Code + suffix, - Subject: p.Address, - Message: msg, - } - }) + } + return out +} + +func (r *headerRule) evaluateProbe(p HTTPProbe, opts sdk.CheckerOptions, headerKey string) []HeaderResult { + v := strings.TrimSpace(p.Headers[headerKey]) + if v == "" { + if r.spec.OnMissing != nil { + return ensureNonEmpty(r.spec.OnMissing(p, opts), r.defaultPresent()) + } + return []HeaderResult{r.defaultMissing()} + } + switch { + case r.spec.Inspect != nil: + return ensureNonEmpty(r.spec.Inspect(v, p, opts), r.defaultPresent()) + case r.spec.Validate != nil: + status, msg := r.spec.Validate(v) + suffix := "invalid" + if status == sdk.StatusOK { + suffix = "ok" + } + return []HeaderResult{{Status: status, Suffix: suffix, Message: msg}} + default: + return []HeaderResult{r.defaultPresent()} + } +} + +func (r *headerRule) defaultMissing() HeaderResult { + status := sdk.StatusInfo + if r.spec.Required { + status = sdk.StatusWarn + } + res := HeaderResult{ + Status: status, + Suffix: "missing", + Message: r.spec.Header + " is not set.", + } + if r.spec.FixHint != "" { + res.Meta = map[string]any{"fix": r.spec.FixHint} + } + return res +} + +func (r *headerRule) defaultPresent() HeaderResult { + return HeaderResult{ + Status: sdk.StatusOK, + Suffix: "ok", + Message: r.spec.Header + " is set.", + } +} + +func (r *headerRule) toCheckState(p HTTPProbe, res HeaderResult) sdk.CheckState { + code := r.spec.Code + if res.Suffix != "" { + code = code + "." + res.Suffix + } + return sdk.CheckState{ + Status: res.Status, + Code: code, + Subject: p.Address, + Message: res.Message, + Meta: res.Meta, + } +} + +func ensureNonEmpty(results []HeaderResult, fallback HeaderResult) []HeaderResult { + if len(results) == 0 { + return []HeaderResult{fallback} + } + return results } diff --git a/checker/headers.go b/checker/headers.go index 92ca13d..b975842 100644 --- a/checker/headers.go +++ b/checker/headers.go @@ -5,21 +5,30 @@ package checker import ( + "fmt" "strconv" "strings" ) // HSTSDirectives is the parsed form of a Strict-Transport-Security header -// (RFC 6797 §6.1). +// (RFC 6797 §6.1). MaxAgeSet distinguishes an explicit max-age=0 from a +// header that omitted the (mandatory) directive entirely. Errors lists +// per-directive parse problems so callers can surface them instead of +// silently treating malformed values as max-age=0. type HSTSDirectives struct { MaxAge int64 + MaxAgeSet bool IncludeSub bool Preload bool + Errors []string } // ParseHSTS pulls max-age, includeSubDomains and preload out of an HSTS // value. Returns nil for an empty value so callers can distinguish "header -// absent" from "header present with max-age=0". +// absent" from "header present with max-age=0". Per RFC 6797 §6.1.1 +// max-age is REQUIRED, MUST appear exactly once, and its value is a +// non-negative integer (optionally quoted); violations are reported via +// the Errors slice. func ParseHSTS(v string) *HSTSDirectives { v = strings.TrimSpace(v) if v == "" { @@ -28,21 +37,57 @@ func ParseHSTS(v string) *HSTSDirectives { h := &HSTSDirectives{} for _, part := range strings.Split(v, ";") { part = strings.TrimSpace(part) + if part == "" { + continue + } + lower := strings.ToLower(part) switch { - case strings.HasPrefix(strings.ToLower(part), "max-age="): - val := strings.Trim(part[len("max-age="):], "\"") - if n, err := strconv.ParseInt(val, 10, 64); err == nil { + case strings.HasPrefix(lower, "max-age="): + raw := strings.TrimSpace(part[len("max-age="):]) + val, quoted := unquoteHSTS(raw) + if h.MaxAgeSet { + h.Errors = append(h.Errors, "max-age specified more than once") + continue + } + h.MaxAgeSet = true + n, err := strconv.ParseInt(val, 10, 64) + switch { + case err != nil: + h.Errors = append(h.Errors, fmt.Sprintf("max-age value %q is not a valid integer", raw)) + case n < 0: + h.Errors = append(h.Errors, fmt.Sprintf("max-age value %d is negative", n)) + case quoted && val == "": + h.Errors = append(h.Errors, "max-age value is empty") + default: h.MaxAge = n } - case strings.EqualFold(part, "includeSubDomains"): + case lower == "max-age": + h.Errors = append(h.Errors, "max-age directive has no value") + h.MaxAgeSet = true + case lower == "includesubdomains": h.IncludeSub = true - case strings.EqualFold(part, "preload"): + case lower == "preload": h.Preload = true } + // Unknown directives are ignored per RFC 6797 §6.1. + } + if !h.MaxAgeSet { + h.Errors = append(h.Errors, "max-age directive is missing") } return h } +// unquoteHSTS strips a surrounding pair of double quotes from a directive +// value (RFC 6797 allows the quoted-string form). Returns the inner value +// and whether quotes were present, so callers can distinguish `max-age=""` +// from `max-age=`. +func unquoteHSTS(s string) (string, bool) { + if len(s) >= 2 && s[0] == '"' && s[len(s)-1] == '"' { + return s[1 : len(s)-1], true + } + return s, false +} + // CSPDirectives is the parsed form of a Content-Security-Policy header // (W3C CSP3). Directive names are lowercased; source tokens keep their // original casing because keywords like 'unsafe-inline' must round-trip @@ -95,23 +140,88 @@ func (c *CSPDirectives) HasSource(directive, source string) bool { return false } -// HasUnsafe reports whether any directive uses 'unsafe-inline' or -// 'unsafe-eval' — the two keywords that nullify most of CSP's value. -func (c *CSPDirectives) HasUnsafe() bool { +// cspFetchFallback maps CSP fetch directives to default-src per CSP3 +// §6.1: when a directive is absent, the user agent falls back to +// default-src. Non-fetch directives (frame-ancestors, form-action, +// base-uri, …) have no fallback and are deliberately omitted. +var cspFetchFallback = map[string]string{ + "child-src": "default-src", + "connect-src": "default-src", + "font-src": "default-src", + "frame-src": "default-src", + "img-src": "default-src", + "manifest-src": "default-src", + "media-src": "default-src", + "object-src": "default-src", + "prefetch-src": "default-src", + "script-src": "default-src", + "script-src-attr": "default-src", + "script-src-elem": "default-src", + "style-src": "default-src", + "style-src-attr": "default-src", + "style-src-elem": "default-src", + "worker-src": "default-src", +} + +// EffectiveSources returns the source list that browsers will enforce +// for directive: the directive's own list when declared, otherwise its +// default-src fallback for fetch directives. The second return is true +// iff the policy explicitly declares the directive (or its fallback). +func (c *CSPDirectives) EffectiveSources(directive string) ([]string, bool) { if c == nil { - return false + return nil, false } - for _, sources := range c.Directives { - for _, s := range sources { - ls := strings.ToLower(s) - if ls == "'unsafe-inline'" || ls == "'unsafe-eval'" { - return true - } + name := strings.ToLower(directive) + if s, ok := c.Directives[name]; ok { + return s, true + } + if fb, ok := cspFetchFallback[name]; ok { + if s, ok := c.Directives[fb]; ok { + return s, true + } + } + return nil, false +} + +func (c *CSPDirectives) effectiveHasSource(directive, source string) bool { + srcs, _ := c.EffectiveSources(directive) + for _, s := range srcs { + if strings.EqualFold(s, source) { + return true } } return false } +// HasUnsafeInline reports whether the effective script-src or style-src +// allows 'unsafe-inline'. +func (c *CSPDirectives) HasUnsafeInline() bool { + return c.effectiveHasSource("script-src", "'unsafe-inline'") || + c.effectiveHasSource("style-src", "'unsafe-inline'") +} + +// HasUnsafeEval reports whether the effective script-src allows +// 'unsafe-eval' (style-src does not enforce script execution, so we +// look at scripts only). +func (c *CSPDirectives) HasUnsafeEval() bool { + return c.effectiveHasSource("script-src", "'unsafe-eval'") +} + +// WildcardSource returns a permissive token (the literal `*`, or one of +// the schemes `http:`, `https:`, `data:`, `blob:`) found in the +// effective sources of directive, or "" if none. These tokens +// effectively neutralise the directive. +func (c *CSPDirectives) WildcardSource(directive string) string { + srcs, _ := c.EffectiveSources(directive) + for _, s := range srcs { + switch strings.ToLower(s) { + case "*", "http:", "https:", "data:", "blob:": + return s + } + } + return "" +} + // ParsedHeaders bundles the structured headers we parse repeatedly. Fields // are nil when the underlying header is absent on the probe; rules can // nil-check or rely on the typed accessors which already handle nil. diff --git a/checker/rules_cookies.go b/checker/rules_cookies.go index 8087ac7..07c5071 100644 --- a/checker/rules_cookies.go +++ b/checker/rules_cookies.go @@ -35,6 +35,7 @@ func (r *cookieFlagsRule) Evaluate(ctx context.Context, obs sdk.ObservationGette var states []sdk.CheckState totalCookies := 0 + samesiteMissing := 0 for _, p := range probes { for _, c := range p.Cookies { totalCookies++ @@ -47,6 +48,7 @@ func (r *cookieFlagsRule) Evaluate(ctx context.Context, obs sdk.ObservationGette } if c.SameSite == "" { issues = append(issues, "missing SameSite") + samesiteMissing++ } else if strings.EqualFold(c.SameSite, "None") && !c.Secure { issues = append(issues, "SameSite=None requires Secure") } @@ -63,6 +65,16 @@ func (r *cookieFlagsRule) Evaluate(ctx context.Context, obs sdk.ObservationGette if totalCookies == 0 { return []sdk.CheckState{passState("http.cookie_flags.none", "No cookies were set on the inspected responses.")} } + if samesiteMissing > 0 { + // Aggregate alongside per-cookie diagnostics so callers see the + // global ratio at a glance — mirrors what Mozilla Observatory + // reports as a single cookies test outcome. + states = append(states, sdk.CheckState{ + Status: sdk.StatusWarn, + Code: "http.cookie_flags.samesite_missing", + Message: fmt.Sprintf("%d of %d cookies do not set SameSite.", samesiteMissing, totalCookies), + }) + } if len(states) == 0 { return []sdk.CheckState{passState("http.cookie_flags.ok", fmt.Sprintf("All %d cookies have proper Secure/HttpOnly/SameSite flags.", totalCookies))} } diff --git a/checker/rules_cookies_test.go b/checker/rules_cookies_test.go index c556257..890ec60 100644 --- a/checker/rules_cookies_test.go +++ b/checker/rules_cookies_test.go @@ -48,12 +48,26 @@ func TestCookieFlagsRule_Issues(t *testing.T) { {Name: "none-without-secure", Secure: false, HttpOnly: true, SameSite: "None"}, } states := runRule(t, &cookieFlagsRule{}, &HTTPData{Probes: []HTTPProbe{p}}, nil) - if len(states) != len(p.Cookies) { - t.Fatalf("got %d states, want %d", len(states), len(p.Cookies)) + // Per-cookie diagnostics + a single SameSite aggregate (1 cookie out + // of 4 is missing SameSite). + if len(states) != len(p.Cookies)+1 { + t.Fatalf("got %d states, want %d", len(states), len(p.Cookies)+1) } mustStatus(t, states, sdk.StatusWarn) - // Check each diagnostic mentions the cookie name and a relevant phrase. + if !hasCode(states, "http.cookie_flags.samesite_missing") { + t.Errorf("missing samesite_missing aggregate: %+v", states) + } + for _, st := range states { + if st.Code == "http.cookie_flags.samesite_missing" { + if !strings.Contains(st.Message, "1 of 4") { + t.Errorf("aggregate message %q should mention 1 of 4", st.Message) + } + } + } + + // Check each per-cookie diagnostic mentions the cookie name and a + // relevant phrase. wantSubstr := map[string]string{ "no-secure": "missing Secure", "no-httponly": "missing HttpOnly", @@ -61,6 +75,9 @@ func TestCookieFlagsRule_Issues(t *testing.T) { "none-without-secure": "SameSite=None requires Secure", } for _, st := range states { + if st.Code != "http.cookie_flags.weak" { + continue + } matched := false for name, phrase := range wantSubstr { if strings.Contains(st.Message, name) && strings.Contains(st.Message, phrase) { @@ -74,6 +91,26 @@ func TestCookieFlagsRule_Issues(t *testing.T) { } } +func TestCookieFlagsRule_SameSiteAggregateOnly(t *testing.T) { + // Two cookies, both otherwise compliant but missing SameSite. We + // expect 2 per-cookie warnings + 1 aggregate. + p := httpsProbe("a:443") + p.Cookies = []CookieInfo{ + {Name: "a", Secure: true, HttpOnly: true, SameSite: ""}, + {Name: "b", Secure: true, HttpOnly: true, SameSite: ""}, + } + states := runRule(t, &cookieFlagsRule{}, &HTTPData{Probes: []HTTPProbe{p}}, nil) + mustStatus(t, states, sdk.StatusWarn) + if !hasCode(states, "http.cookie_flags.samesite_missing") { + t.Fatalf("missing aggregate state: %+v", states) + } + for _, st := range states { + if st.Code == "http.cookie_flags.samesite_missing" && !strings.Contains(st.Message, "2 of 2") { + t.Errorf("aggregate should report 2 of 2, got %q", st.Message) + } + } +} + func TestCookieFlagsRule_SameSiteNoneCaseInsensitive(t *testing.T) { p := httpsProbe("a:443") p.Cookies = []CookieInfo{{Name: "x", Secure: false, HttpOnly: true, SameSite: "none"}} diff --git a/checker/rules_modern_headers.go b/checker/rules_modern_headers.go index e813ea6..7690b03 100644 --- a/checker/rules_modern_headers.go +++ b/checker/rules_modern_headers.go @@ -5,6 +5,8 @@ package checker import ( + "fmt" + "sort" "strings" sdk "git.happydns.org/checker-sdk-go/checker" @@ -38,10 +40,11 @@ func init() { RegisterRule(HeaderRule(HeaderRuleSpec{ Code: "http.permissions_policy", - Description: "Reports the presence of a Permissions-Policy header (W3C Permissions Policy, replaces Feature-Policy).", + Description: "Verifies that the Permissions-Policy header restricts powerful APIs (camera, microphone, geolocation, …).", Header: "Permissions-Policy", Required: false, FixHint: "Define a Permissions-Policy that disables APIs the site does not use, e.g. `Permissions-Policy: camera=(), microphone=(), geolocation=()`.", + Validate: validatePermissionsPolicy, })) RegisterRule(HeaderRule(HeaderRuleSpec{ @@ -146,6 +149,129 @@ func validateCORP(v string) (sdk.Status, string) { return sdk.StatusWarn, "Cross-Origin-Resource-Policy has an unrecognised value: " + v } +// dangerousPermissionsPolicyFeatures lists features whose default +// (browser-level) allowlist is permissive enough to warrant an explicit +// restriction. Sources: W3C Permissions Policy registry + the +// "powerful features" list (camera, microphone, geolocation, payment, +// usb, midi, sensors, screen-wake-lock, fullscreen, autoplay, …). +// Tracking-related features (interest-cohort, browsing-topics) are +// included for privacy. +var dangerousPermissionsPolicyFeatures = map[string]struct{}{ + "accelerometer": {}, + "ambient-light-sensor": {}, + "autoplay": {}, + "battery": {}, + "browsing-topics": {}, + "camera": {}, + "display-capture": {}, + "document-domain": {}, + "encrypted-media": {}, + "fullscreen": {}, + "geolocation": {}, + "gyroscope": {}, + "hid": {}, + "identity-credentials-get": {}, + "idle-detection": {}, + "interest-cohort": {}, + "magnetometer": {}, + "microphone": {}, + "midi": {}, + "otp-credentials": {}, + "payment": {}, + "picture-in-picture": {}, + "publickey-credentials-create": {}, + "publickey-credentials-get": {}, + "screen-wake-lock": {}, + "serial": {}, + "storage-access": {}, + "usb": {}, + "window-management": {}, + "xr-spatial-tracking": {}, +} + +// validatePermissionsPolicy parses a Permissions-Policy header +// (RFC 8941 structured fields, dictionary form) and warns when any +// dangerous feature is granted to all origins (`*`) or when the value +// is syntactically broken. A header that only restricts features (e.g. +// `camera=()`) is accepted even if it does not enumerate every +// dangerous one — listing every feature would be noisy and +// most browsers default-deny powerful features in cross-origin frames +// already. +func validatePermissionsPolicy(v string) (sdk.Status, string) { + entries, err := parsePermissionsPolicy(v) + if err != nil { + return sdk.StatusWarn, "Permissions-Policy is malformed: " + err.Error() + } + if len(entries) == 0 { + return sdk.StatusWarn, "Permissions-Policy is empty." + } + var permissive []string + for feature, allowlist := range entries { + if _, dangerous := dangerousPermissionsPolicyFeatures[feature]; !dangerous { + continue + } + if isPermissionsAllowlistWildcard(allowlist) { + permissive = append(permissive, feature) + } + } + if len(permissive) > 0 { + sort.Strings(permissive) + return sdk.StatusWarn, + "Permissions-Policy grants " + strings.Join(permissive, ", ") + + " to all origins (`*`); restrict these to (), self or specific origins." + } + return sdk.StatusOK, "Permissions-Policy restricts powerful features." +} + +// parsePermissionsPolicy splits the header into a feature → allowlist +// map. It tolerates the two forms in the wild: the spec'd +// structured-field form (`camera=()`, `geolocation=(self "https://x")`) +// and the legacy comma form (`camera=()`). Allowlist tokens are kept +// verbatim minus surrounding parentheses so the caller can detect `*`. +func parsePermissionsPolicy(v string) (map[string]string, error) { + v = strings.TrimSpace(v) + if v == "" { + return nil, nil + } + out := map[string]string{} + for _, raw := range strings.Split(v, ",") { + entry := strings.TrimSpace(raw) + if entry == "" { + continue + } + eq := strings.IndexByte(entry, '=') + if eq < 0 { + return nil, fmt.Errorf("entry %q is missing `=`", entry) + } + feature := strings.ToLower(strings.TrimSpace(entry[:eq])) + allowlist := strings.TrimSpace(entry[eq+1:]) + if feature == "" { + return nil, fmt.Errorf("entry %q has an empty feature name", entry) + } + out[feature] = allowlist + } + return out, nil +} + +// isPermissionsAllowlistWildcard reports whether an allowlist grants +// the feature to every origin. The two equivalent forms are the bare +// `*` and the parenthesised list `(*)`. +func isPermissionsAllowlistWildcard(allowlist string) bool { + a := strings.TrimSpace(allowlist) + if a == "*" { + return true + } + if strings.HasPrefix(a, "(") && strings.HasSuffix(a, ")") { + inner := strings.TrimSpace(a[1 : len(a)-1]) + for _, tok := range strings.Fields(inner) { + if tok == "*" { + return true + } + } + } + return false +} + // splitCSV splits on commas, trims whitespace, lowercases, and drops // empty fragments. Used for header values that are comma-separated lists // of tokens (Referrer-Policy, Accept-Encoding, …). diff --git a/checker/rules_modern_headers_test.go b/checker/rules_modern_headers_test.go index 9ca4e07..ad7255c 100644 --- a/checker/rules_modern_headers_test.go +++ b/checker/rules_modern_headers_test.go @@ -57,8 +57,6 @@ func TestReferrerPolicyRule(t *testing.T) { } func TestPermissionsPolicyRule(t *testing.T) { - // Permissions-Policy has no Validate function: presence alone is OK, - // absence is Info (Required=false). cases := []struct { name string value string @@ -66,8 +64,14 @@ func TestPermissionsPolicyRule(t *testing.T) { code string }{ {"missing", "", sdk.StatusInfo, "http.permissions_policy.missing"}, - {"present", "camera=(), microphone=()", sdk.StatusOK, "http.permissions_policy.ok"}, + {"restrictive", "camera=(), microphone=()", sdk.StatusOK, "http.permissions_policy.ok"}, + {"self only", "geolocation=(self)", sdk.StatusOK, "http.permissions_policy.ok"}, {"empty value treated as missing", " ", sdk.StatusInfo, "http.permissions_policy.missing"}, + {"camera wildcard", "camera=*", sdk.StatusWarn, "http.permissions_policy.invalid"}, + {"microphone parenthesised wildcard", "microphone=(*)", sdk.StatusWarn, "http.permissions_policy.invalid"}, + {"non-dangerous wildcard ignored", "fullscreen=(self), accelerometer=*", sdk.StatusWarn, "http.permissions_policy.invalid"}, + {"unknown feature wildcard ignored", "totally-made-up=*", sdk.StatusOK, "http.permissions_policy.ok"}, + {"malformed entry", "camera", sdk.StatusWarn, "http.permissions_policy.invalid"}, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { diff --git a/checker/rules_security_headers.go b/checker/rules_security_headers.go index 92ade0c..0d1a877 100644 --- a/checker/rules_security_headers.go +++ b/checker/rules_security_headers.go @@ -5,166 +5,43 @@ package checker import ( - "context" "fmt" "strings" sdk "git.happydns.org/checker-sdk-go/checker" ) -func init() { - RegisterRule(&hstsRule{}) - RegisterRule(&cspRule{}) - RegisterRule(&xFrameOptionsRule{}) - RegisterRule(&xXSSProtectionRule{}) -} - -// hstsRule checks the Strict-Transport-Security header on HTTPS responses. -type hstsRule struct{} - -func (r *hstsRule) Name() string { return "http.hsts" } -func (r *hstsRule) Description() string { - return "Verifies the presence and quality of the Strict-Transport-Security header on HTTPS responses." -} - -func (r *hstsRule) Evaluate(ctx context.Context, obs sdk.ObservationGetter, opts sdk.CheckerOptions) []sdk.CheckState { - data, errSt := loadHTTPData(ctx, obs) - if errSt != nil { - return []sdk.CheckState{*errSt} - } - require := sdk.GetBoolOption(opts, OptionRequireHSTS, true) - minDays := sdk.GetIntOption(opts, OptionMinHSTSMaxAgeDays, DefaultMinHSTSMaxAge) - minSeconds := int64(minDays) * 86400 - - return EvalPerHTTPS(data, "http.hsts", func(p HTTPProbe) sdk.CheckState { - h := ParseHSTS(p.Headers["strict-transport-security"]) - if h == nil { - status := sdk.StatusWarn - if !require { - status = sdk.StatusInfo - } - return sdk.CheckState{ - Status: status, - Code: "http.hsts.missing", - Subject: p.Address, - Message: "Strict-Transport-Security header is missing.", - Meta: map[string]any{"fix": "Send `Strict-Transport-Security: max-age=15552000; includeSubDomains` from HTTPS responses."}, - } - } - if h.MaxAge < minSeconds { - return sdk.CheckState{ - Status: sdk.StatusWarn, - Code: "http.hsts.short_max_age", - Subject: p.Address, - Message: fmt.Sprintf("HSTS max-age=%d is below the recommended %d seconds (%d days).", h.MaxAge, minSeconds, minDays), - } - } - return sdk.CheckState{ - Status: sdk.StatusOK, - Code: "http.hsts.ok", - Subject: p.Address, - Message: fmt.Sprintf("HSTS present (max-age=%d, includeSubDomains=%v, preload=%v).", h.MaxAge, h.IncludeSub, h.Preload), - } - }) -} - -// cspRule checks for the presence of a Content-Security-Policy header. -type cspRule struct{} - -func (r *cspRule) Name() string { return "http.csp" } -func (r *cspRule) Description() string { - return "Verifies the presence of a Content-Security-Policy header on HTTPS responses." -} - -func (r *cspRule) Evaluate(ctx context.Context, obs sdk.ObservationGetter, opts sdk.CheckerOptions) []sdk.CheckState { - data, errSt := loadHTTPData(ctx, obs) - if errSt != nil { - return []sdk.CheckState{*errSt} - } - require := sdk.GetBoolOption(opts, OptionRequireCSP, false) - - return EvalPerHTTPS(data, "http.csp", func(p HTTPProbe) sdk.CheckState { - csp := ParseCSP(p.Headers["content-security-policy"]) - if csp == nil { - status := sdk.StatusInfo - if require { - status = sdk.StatusWarn - } - return sdk.CheckState{ - Status: status, - Code: "http.csp.missing", - Subject: p.Address, - Message: "Content-Security-Policy header is missing.", - Meta: map[string]any{"fix": "Define a CSP appropriate for your application (e.g. default-src 'self')."}, - } - } - if csp.HasUnsafe() { - return sdk.CheckState{ - Status: sdk.StatusWarn, - Code: "http.csp.unsafe", - Subject: p.Address, - Message: "Content-Security-Policy uses 'unsafe-inline' or 'unsafe-eval'.", - } - } - return sdk.CheckState{ - Status: sdk.StatusOK, - Code: "http.csp.ok", - Subject: p.Address, - Message: "Content-Security-Policy is set.", - } - }) -} - -// xFrameOptionsRule checks X-Frame-Options (or frame-ancestors in CSP as -// an acceptable substitute). -type xFrameOptionsRule struct{} - -func (r *xFrameOptionsRule) Name() string { return "http.x_frame_options" } -func (r *xFrameOptionsRule) Description() string { - return "Verifies that responses set X-Frame-Options or a CSP frame-ancestors directive." -} - -func (r *xFrameOptionsRule) Evaluate(ctx context.Context, obs sdk.ObservationGetter, _ sdk.CheckerOptions) []sdk.CheckState { - data, errSt := loadHTTPData(ctx, obs) - if errSt != nil { - return []sdk.CheckState{*errSt} - } - - return EvalPerHTTPS(data, "http.x_frame_options", func(p HTTPProbe) sdk.CheckState { - xfo := strings.ToUpper(strings.TrimSpace(p.Headers["x-frame-options"])) - hasFrameAncestors := ParseCSP(p.Headers["content-security-policy"]).HasDirective("frame-ancestors") - - switch { - case xfo == "DENY" || xfo == "SAMEORIGIN" || hasFrameAncestors: - return sdk.CheckState{ - Status: sdk.StatusOK, - Code: "http.x_frame_options.ok", - Subject: p.Address, - Message: "Clickjacking protection is in place.", - } - case xfo != "": - return sdk.CheckState{ - Status: sdk.StatusWarn, - Code: "http.x_frame_options.invalid", - Subject: p.Address, - Message: "X-Frame-Options has an unrecognised value: " + xfo, - } - default: - return sdk.CheckState{ - Status: sdk.StatusWarn, - Code: "http.x_frame_options.missing", - Subject: p.Address, - Message: "Neither X-Frame-Options nor CSP frame-ancestors is set.", - Meta: map[string]any{"fix": "Send `X-Frame-Options: DENY` (or SAMEORIGIN) or use CSP frame-ancestors."}, - } - } - }) -} +// All five "core" security-header rules are wired through the HeaderRule +// DSL. The richer ones (HSTS, CSP, X-Frame-Options, X-XSS-Protection) +// use Inspect / OnMissing to express thresholds, multi-finding outputs, +// alternative-source fallbacks and reversed "absent is fine" semantics +// without re-implementing the load/iterate/build-state scaffolding. func init() { - // Showcase: a rule expressed entirely as a HeaderRuleSpec. Compare - // with the hand-rolled rules above — the boilerplate vanishes once - // the only logic is "is this header present and well-formed?". + RegisterRule(HeaderRule(HeaderRuleSpec{ + Code: "http.hsts", + Description: "Verifies the presence and quality of the Strict-Transport-Security header on HTTPS responses.", + Header: "Strict-Transport-Security", + Inspect: inspectHSTS, + OnMissing: missingHSTS, + })) + + RegisterRule(HeaderRule(HeaderRuleSpec{ + Code: "http.csp", + Description: "Verifies the presence and quality of the Content-Security-Policy header on HTTPS responses.", + Header: "Content-Security-Policy", + Inspect: inspectCSP, + OnMissing: missingCSP, + })) + + RegisterRule(HeaderRule(HeaderRuleSpec{ + Code: "http.x_frame_options", + Description: "Verifies that responses set X-Frame-Options or a CSP frame-ancestors directive.", + Header: "X-Frame-Options", + Inspect: inspectXFrameOptions, + OnMissing: missingXFrameOptions, + })) + RegisterRule(HeaderRule(HeaderRuleSpec{ Code: "http.x_content_type_options", Description: "Verifies that responses set X-Content-Type-Options: nosniff.", @@ -178,54 +55,188 @@ func init() { return sdk.StatusWarn, "X-Content-Type-Options has an unexpected value: " + strings.ToLower(v) }, })) + + RegisterRule(HeaderRule(HeaderRuleSpec{ + Code: "http.x_xss_protection", + Description: "Reports the value of the legacy X-XSS-Protection header (disabled is preferred on modern browsers; CSP is the proper replacement).", + Header: "X-XSS-Protection", + Inspect: inspectXXSSProtection, + OnMissing: func(_ HTTPProbe, _ sdk.CheckerOptions) []HeaderResult { + return []HeaderResult{{ + Status: sdk.StatusInfo, + Suffix: "absent", + Message: "X-XSS-Protection is not set; CSP is the recommended replacement.", + }} + }, + })) } -// xXSSProtectionRule checks the legacy X-XSS-Protection header. Modern -// browsers ignore it, but if present we want it to be sane. -type xXSSProtectionRule struct{} +// HSTS ---------------------------------------------------------------- -func (r *xXSSProtectionRule) Name() string { return "http.x_xss_protection" } -func (r *xXSSProtectionRule) Description() string { - return "Reports the value of the legacy X-XSS-Protection header (disabled is preferred on modern browsers; CSP is the proper replacement)." +func missingHSTS(_ HTTPProbe, opts sdk.CheckerOptions) []HeaderResult { + status := sdk.StatusWarn + if !sdk.GetBoolOption(opts, OptionRequireHSTS, true) { + status = sdk.StatusInfo + } + return []HeaderResult{{ + Status: status, + Suffix: "missing", + Message: "Strict-Transport-Security header is missing.", + Meta: map[string]any{"fix": "Send `Strict-Transport-Security: max-age=15552000; includeSubDomains` from HTTPS responses."}, + }} } -func (r *xXSSProtectionRule) Evaluate(ctx context.Context, obs sdk.ObservationGetter, _ sdk.CheckerOptions) []sdk.CheckState { - data, errSt := loadHTTPData(ctx, obs) - if errSt != nil { - return []sdk.CheckState{*errSt} +func inspectHSTS(value string, _ HTTPProbe, opts sdk.CheckerOptions) []HeaderResult { + h := ParseHSTS(value) + if h == nil { + // Defensive: ParseHSTS only returns nil on empty input, which the + // DSL has already routed to OnMissing. + return []HeaderResult{{ + Status: sdk.StatusWarn, Suffix: "invalid", + Message: "Strict-Transport-Security header is malformed.", + }} + } + if len(h.Errors) > 0 { + return []HeaderResult{{ + Status: sdk.StatusWarn, + Suffix: "invalid", + Message: fmt.Sprintf("Strict-Transport-Security header is malformed: %s.", strings.Join(h.Errors, "; ")), + Meta: map[string]any{"fix": "Send `Strict-Transport-Security: max-age=15552000; includeSubDomains` with a non-negative integer max-age."}, + }} + } + minDays := sdk.GetIntOption(opts, OptionMinHSTSMaxAgeDays, DefaultMinHSTSMaxAge) + minSeconds := int64(minDays) * 86400 + if h.MaxAge < minSeconds { + return []HeaderResult{{ + Status: sdk.StatusWarn, + Suffix: "short_max_age", + Message: fmt.Sprintf("HSTS max-age=%d is below the recommended %d seconds (%d days).", h.MaxAge, minSeconds, minDays), + }} + } + return []HeaderResult{{ + Status: sdk.StatusOK, + Suffix: "ok", + Message: fmt.Sprintf("HSTS present (max-age=%d, includeSubDomains=%v, preload=%v).", h.MaxAge, h.IncludeSub, h.Preload), + }} +} + +// CSP ----------------------------------------------------------------- + +func missingCSP(_ HTTPProbe, opts sdk.CheckerOptions) []HeaderResult { + status := sdk.StatusInfo + if sdk.GetBoolOption(opts, OptionRequireCSP, false) { + status = sdk.StatusWarn + } + return []HeaderResult{{ + Status: status, + Suffix: "missing", + Message: "Content-Security-Policy header is missing.", + Meta: map[string]any{"fix": "Define a CSP appropriate for your application (e.g. default-src 'self')."}, + }} +} + +// inspectCSP surfaces multiple weakness suffixes per probe — see the +// historical docstring on evaluateCSP for the rationale (unsafe-inline / +// unsafe-eval split, missing default-src, permissive script-src). +func inspectCSP(value string, _ HTTPProbe, _ sdk.CheckerOptions) []HeaderResult { + csp := ParseCSP(value) + if csp == nil { + return []HeaderResult{{ + Status: sdk.StatusWarn, Suffix: "invalid", + Message: "Content-Security-Policy header is empty.", + }} + } + var out []HeaderResult + add := func(suffix, msg string) { + out = append(out, HeaderResult{Status: sdk.StatusWarn, Suffix: suffix, Message: msg}) } - return EvalPerHTTPS(data, "http.x_xss_protection", func(p HTTPProbe) sdk.CheckState { - v := strings.TrimSpace(p.Headers["x-xss-protection"]) - switch { - case v == "": - return sdk.CheckState{ - Status: sdk.StatusInfo, - Code: "http.x_xss_protection.absent", - Subject: p.Address, - Message: "X-XSS-Protection is not set; CSP is the recommended replacement.", - } - case strings.HasPrefix(v, "0"): - return sdk.CheckState{ - Status: sdk.StatusOK, - Code: "http.x_xss_protection.disabled", - Subject: p.Address, - Message: "X-XSS-Protection is explicitly disabled (recommended).", - } - case strings.Contains(strings.ToLower(v), "mode=block"): - return sdk.CheckState{ - Status: sdk.StatusInfo, - Code: "http.x_xss_protection.enabled", - Subject: p.Address, - Message: "X-XSS-Protection is set to the historically recommended `1; mode=block`. Modern browsers ignore this header; CSP is the proper replacement.", - } - default: - return sdk.CheckState{ - Status: sdk.StatusInfo, - Code: "http.x_xss_protection.enabled", - Subject: p.Address, - Message: "X-XSS-Protection is enabled. Modern browsers ignore this header; CSP is the proper replacement.", - } + hasDefault := csp.HasDirective("default-src") + hasScript := csp.HasDirective("script-src") + if !hasDefault && !hasScript { + add("missing_default", + "Content-Security-Policy declares neither default-src nor script-src; script execution is not constrained.") + } + if csp.HasUnsafeInline() { + add("unsafe_inline", + "Content-Security-Policy allows 'unsafe-inline' for scripts or styles, which negates most XSS protection.") + } + if csp.HasUnsafeEval() { + add("unsafe_eval", + "Content-Security-Policy allows 'unsafe-eval' in script-src, enabling eval()/new Function().") + } + switch { + case hasScript: + if w := csp.WildcardSource("script-src"); w != "" { + add("wildcard_script_src", + "Content-Security-Policy script-src includes the permissive source "+w+", allowing scripts from arbitrary origins.") } - }) + case hasDefault: + if w := csp.WildcardSource("default-src"); w != "" { + add("wildcard_default_src", + "Content-Security-Policy default-src includes the permissive source "+w+" and no script-src overrides it.") + } + } + + if len(out) == 0 { + return []HeaderResult{{ + Status: sdk.StatusOK, + Suffix: "ok", + Message: "Content-Security-Policy is set with no detected weaknesses.", + }} + } + return out +} + +// X-Frame-Options ----------------------------------------------------- + +func inspectXFrameOptions(value string, _ HTTPProbe, _ sdk.CheckerOptions) []HeaderResult { + xfo := strings.ToUpper(value) + if xfo == "DENY" || xfo == "SAMEORIGIN" { + return []HeaderResult{{ + Status: sdk.StatusOK, Suffix: "ok", + Message: "Clickjacking protection is in place.", + }} + } + return []HeaderResult{{ + Status: sdk.StatusWarn, Suffix: "invalid", + Message: "X-Frame-Options has an unrecognised value: " + xfo, + }} +} + +func missingXFrameOptions(p HTTPProbe, _ sdk.CheckerOptions) []HeaderResult { + if ParseCSP(p.Headers["content-security-policy"]).HasDirective("frame-ancestors") { + return []HeaderResult{{ + Status: sdk.StatusOK, Suffix: "ok", + Message: "Clickjacking protection is in place.", + }} + } + return []HeaderResult{{ + Status: sdk.StatusWarn, + Suffix: "missing", + Message: "Neither X-Frame-Options nor CSP frame-ancestors is set.", + Meta: map[string]any{"fix": "Send `X-Frame-Options: DENY` (or SAMEORIGIN) or use CSP frame-ancestors."}, + }} +} + +// X-XSS-Protection ---------------------------------------------------- + +func inspectXXSSProtection(value string, _ HTTPProbe, _ sdk.CheckerOptions) []HeaderResult { + switch { + case strings.HasPrefix(value, "0"): + return []HeaderResult{{ + Status: sdk.StatusOK, Suffix: "disabled", + Message: "X-XSS-Protection is explicitly disabled (recommended).", + }} + case strings.Contains(strings.ToLower(value), "mode=block"): + return []HeaderResult{{ + Status: sdk.StatusInfo, Suffix: "enabled", + Message: "X-XSS-Protection is set to the historically recommended `1; mode=block`. Modern browsers ignore this header; CSP is the proper replacement.", + }} + default: + return []HeaderResult{{ + Status: sdk.StatusInfo, Suffix: "enabled", + Message: "X-XSS-Protection is enabled. Modern browsers ignore this header; CSP is the proper replacement.", + }} + } } diff --git a/checker/rules_security_headers_test.go b/checker/rules_security_headers_test.go index 539a656..00c6c27 100644 --- a/checker/rules_security_headers_test.go +++ b/checker/rules_security_headers_test.go @@ -17,16 +17,21 @@ func TestParseHSTS(t *testing.T) { maxAge int64 includeSub bool preload bool + wantErr bool }{ - {"empty", "", 0, false, false}, - {"max-age only", "max-age=31536000", 31536000, false, false}, - {"includeSubDomains", "max-age=15552000; includeSubDomains", 15552000, true, false}, - {"all flags", "max-age=63072000; includeSubDomains; preload", 63072000, true, true}, - {"quoted max-age", `max-age="3600"`, 3600, false, false}, - {"case-insensitive directive", "MAX-AGE=42; INCLUDESUBDOMAINS; PRELOAD", 42, true, true}, - {"messy spaces", " max-age=10 ; includeSubDomains ", 10, true, false}, - {"unparseable max-age", "max-age=not-a-number", 0, false, false}, - {"no max-age, only flags", "includeSubDomains; preload", 0, true, true}, + {"empty", "", 0, false, false, false}, + {"max-age only", "max-age=31536000", 31536000, false, false, false}, + {"includeSubDomains", "max-age=15552000; includeSubDomains", 15552000, true, false, false}, + {"all flags", "max-age=63072000; includeSubDomains; preload", 63072000, true, true, false}, + {"quoted max-age", `max-age="3600"`, 3600, false, false, false}, + {"case-insensitive directive", "MAX-AGE=42; INCLUDESUBDOMAINS; PRELOAD", 42, true, true, false}, + {"messy spaces", " max-age=10 ; includeSubDomains ", 10, true, false, false}, + {"unparseable max-age", "max-age=not-a-number", 0, false, false, true}, + {"no max-age, only flags", "includeSubDomains; preload", 0, true, true, true}, + {"negative max-age", "max-age=-1", 0, false, false, true}, + {"empty quoted max-age", `max-age=""`, 0, false, false, true}, + {"max-age without value", "max-age; includeSubDomains", 0, true, false, true}, + {"duplicate max-age", "max-age=10; max-age=20", 10, false, false, true}, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { @@ -44,13 +49,17 @@ func TestParseHSTS(t *testing.T) { t.Errorf("ParseHSTS(%q) = (%d, %v, %v), want (%d, %v, %v)", c.in, h.MaxAge, h.IncludeSub, h.Preload, c.maxAge, c.includeSub, c.preload) } + if got := len(h.Errors) > 0; got != c.wantErr { + t.Errorf("ParseHSTS(%q) errors = %v (%v), want wantErr=%v", + c.in, h.Errors, got, c.wantErr) + } }) } } func TestHSTSRule_NoHTTPSProbes(t *testing.T) { data := &HTTPData{Probes: []HTTPProbe{httpProbe("a:80")}} - states := runRule(t, &hstsRule{}, data, nil) + states := runRule(t, ruleByName(t, "http.hsts"), data, nil) mustStatus(t, states, sdk.StatusUnknown) if !hasCode(states, "http.hsts.no_https") { t.Errorf("missing no_https code: %+v", states) @@ -59,7 +68,7 @@ func TestHSTSRule_NoHTTPSProbes(t *testing.T) { func TestHSTSRule_MissingRequired(t *testing.T) { data := &HTTPData{Probes: []HTTPProbe{httpsProbe("a:443")}} - states := runRule(t, &hstsRule{}, data, sdk.CheckerOptions{OptionRequireHSTS: true}) + states := runRule(t, ruleByName(t, "http.hsts"), data, sdk.CheckerOptions{OptionRequireHSTS: true}) mustStatus(t, states, sdk.StatusWarn) if !hasCode(states, "http.hsts.missing") { t.Errorf("missing 'http.hsts.missing': %+v", states) @@ -68,7 +77,7 @@ func TestHSTSRule_MissingRequired(t *testing.T) { func TestHSTSRule_MissingNotRequired(t *testing.T) { data := &HTTPData{Probes: []HTTPProbe{httpsProbe("a:443")}} - states := runRule(t, &hstsRule{}, data, sdk.CheckerOptions{OptionRequireHSTS: false}) + states := runRule(t, ruleByName(t, "http.hsts"), data, sdk.CheckerOptions{OptionRequireHSTS: false}) mustStatus(t, states, sdk.StatusInfo) } @@ -76,7 +85,7 @@ func TestHSTSRule_ShortMaxAge(t *testing.T) { p := httpsProbe("a:443") p.Headers["strict-transport-security"] = "max-age=60" data := &HTTPData{Probes: []HTTPProbe{p}} - states := runRule(t, &hstsRule{}, data, nil) + states := runRule(t, ruleByName(t, "http.hsts"), data, nil) mustStatus(t, states, sdk.StatusWarn) if !hasCode(states, "http.hsts.short_max_age") { t.Errorf("missing short_max_age code: %+v", states) @@ -87,7 +96,7 @@ func TestHSTSRule_OK(t *testing.T) { p := httpsProbe("a:443") p.Headers["strict-transport-security"] = "max-age=63072000; includeSubDomains; preload" data := &HTTPData{Probes: []HTTPProbe{p}} - states := runRule(t, &hstsRule{}, data, nil) + states := runRule(t, ruleByName(t, "http.hsts"), data, nil) mustStatus(t, states, sdk.StatusOK) if !hasCode(states, "http.hsts.ok") { t.Errorf("missing ok code: %+v", states) @@ -95,7 +104,7 @@ func TestHSTSRule_OK(t *testing.T) { } func TestHSTSRule_LoadFailure(t *testing.T) { - states := (&hstsRule{}).Evaluate(t.Context(), &fakeObs{failGet: true}, nil) + states := ruleByName(t, "http.hsts").Evaluate(t.Context(), &fakeObs{failGet: true}, nil) if len(states) != 1 || states[0].Status != sdk.StatusError { t.Fatalf("expected single error state, got %+v", states) } @@ -104,22 +113,78 @@ func TestHSTSRule_LoadFailure(t *testing.T) { func TestCSPRule_Missing(t *testing.T) { data := &HTTPData{Probes: []HTTPProbe{httpsProbe("a:443")}} // Default: not required → Info. - states := runRule(t, &cspRule{}, data, nil) + states := runRule(t, ruleByName(t, "http.csp"), data, nil) mustStatus(t, states, sdk.StatusInfo) // Required → Warn. - states = runRule(t, &cspRule{}, data, sdk.CheckerOptions{OptionRequireCSP: true}) + states = runRule(t, ruleByName(t, "http.csp"), data, sdk.CheckerOptions{OptionRequireCSP: true}) mustStatus(t, states, sdk.StatusWarn) } func TestCSPRule_Unsafe(t *testing.T) { - for _, csp := range []string{"default-src 'self'; script-src 'unsafe-inline'", "default-src 'unsafe-eval'"} { + cases := []struct { + csp string + code string + }{ + {"default-src 'self'; script-src 'self' 'unsafe-inline'", "http.csp.unsafe_inline"}, + {"default-src 'self'; script-src 'self' 'unsafe-eval'", "http.csp.unsafe_eval"}, + // unsafe-eval on default-src falls back to script-src. + {"default-src 'self' 'unsafe-eval'", "http.csp.unsafe_eval"}, + } + for _, c := range cases { p := httpsProbe("a:443") - p.Headers["content-security-policy"] = csp + p.Headers["content-security-policy"] = c.csp data := &HTTPData{Probes: []HTTPProbe{p}} - states := runRule(t, &cspRule{}, data, nil) + states := runRule(t, ruleByName(t, "http.csp"), data, nil) mustStatus(t, states, sdk.StatusWarn) - if !hasCode(states, "http.csp.unsafe") { - t.Errorf("csp=%q: missing unsafe code: %+v", csp, states) + if !hasCode(states, c.code) { + t.Errorf("csp=%q: missing code %q in %+v", c.csp, c.code, states) + } + } +} + +func TestCSPRule_MissingDefault(t *testing.T) { + p := httpsProbe("a:443") + p.Headers["content-security-policy"] = "frame-ancestors 'none'" + data := &HTTPData{Probes: []HTTPProbe{p}} + states := runRule(t, ruleByName(t, "http.csp"), data, nil) + mustStatus(t, states, sdk.StatusWarn) + if !hasCode(states, "http.csp.missing_default") { + t.Errorf("missing_default not emitted: %+v", states) + } +} + +func TestCSPRule_WildcardScriptSrc(t *testing.T) { + cases := []struct { + csp string + code string + }{ + {"default-src 'self'; script-src *", "http.csp.wildcard_script_src"}, + {"default-src 'self'; script-src https:", "http.csp.wildcard_script_src"}, + // No script-src declared → wildcard on default-src is reported. + {"default-src *", "http.csp.wildcard_default_src"}, + } + for _, c := range cases { + p := httpsProbe("a:443") + p.Headers["content-security-policy"] = c.csp + data := &HTTPData{Probes: []HTTPProbe{p}} + states := runRule(t, ruleByName(t, "http.csp"), data, nil) + mustStatus(t, states, sdk.StatusWarn) + if !hasCode(states, c.code) { + t.Errorf("csp=%q: missing code %q in %+v", c.csp, c.code, states) + } + } +} + +func TestCSPRule_TightScriptSrcMasksDefaultWildcard(t *testing.T) { + // default-src is permissive but script-src locks scripts down — we + // should not emit the default-src wildcard warning. + p := httpsProbe("a:443") + p.Headers["content-security-policy"] = "default-src *; script-src 'self'" + data := &HTTPData{Probes: []HTTPProbe{p}} + states := runRule(t, ruleByName(t, "http.csp"), data, nil) + for _, s := range states { + if s.Code == "http.csp.wildcard_default_src" { + t.Errorf("unexpected wildcard_default_src when script-src tightens scripts: %+v", states) } } } @@ -128,7 +193,7 @@ func TestCSPRule_OK(t *testing.T) { p := httpsProbe("a:443") p.Headers["content-security-policy"] = "default-src 'self'" data := &HTTPData{Probes: []HTTPProbe{p}} - states := runRule(t, &cspRule{}, data, nil) + states := runRule(t, ruleByName(t, "http.csp"), data, nil) mustStatus(t, states, sdk.StatusOK) } @@ -156,7 +221,7 @@ func TestXFrameOptionsRule(t *testing.T) { p.Headers["content-security-policy"] = c.csp } data := &HTTPData{Probes: []HTTPProbe{p}} - states := runRule(t, &xFrameOptionsRule{}, data, nil) + states := runRule(t, ruleByName(t, "http.x_frame_options"), data, nil) mustStatus(t, states, c.want) if !hasCode(states, c.wantSub) { t.Errorf("missing code %q in %+v", c.wantSub, states) @@ -204,7 +269,7 @@ func TestXXSSProtectionRule(t *testing.T) { if c.val != "" { p.Headers["x-xss-protection"] = c.val } - states := runRule(t, &xXSSProtectionRule{}, &HTTPData{Probes: []HTTPProbe{p}}, nil) + states := runRule(t, ruleByName(t, "http.x_xss_protection"), &HTTPData{Probes: []HTTPProbe{p}}, nil) mustStatus(t, states, c.want) if !hasCode(states, c.code) { t.Errorf("val=%q: want code %q, got %+v", c.val, c.code, states) @@ -214,7 +279,13 @@ func TestXXSSProtectionRule(t *testing.T) { func TestSecurityHeaders_NoHTTPS(t *testing.T) { // Each header rule must emit Unknown when there are no successful HTTPS probes. - rules := []sdk.CheckRule{&hstsRule{}, &cspRule{}, &xFrameOptionsRule{}, ruleByName(t, "http.x_content_type_options"), &xXSSProtectionRule{}} + rules := []sdk.CheckRule{ + ruleByName(t, "http.hsts"), + ruleByName(t, "http.csp"), + ruleByName(t, "http.x_frame_options"), + ruleByName(t, "http.x_content_type_options"), + ruleByName(t, "http.x_xss_protection"), + } data := &HTTPData{Probes: []HTTPProbe{httpProbe("a:80")}} for _, r := range rules { states := runRule(t, r, data, nil)