#610 Add frame-ancestors 'self' to the content security policy

docs/technical-stakeholders-functional-overview.md

@@ -204,7 +204,7 @@ Dev-only tooling (`Dev*Controller`s incl. impersonation used by E2E): `[AllowAno
 - The app is single-organisation-per-session; org selection happens inside DfE Sign-In. LA-Estab is composed as `{localAuthority.code}/{establishmentNumber}`, with a hardcoded `DSI/TEST` override for the DSI sandbox org (URN 990082).
 - **DataProtection** key ring persisted to blob (`data-protection-keys/keys.xml`) so OIDC state cookies survive multi-replica load balancing.
 - Antiforgery header name `X-XSRF-TOKEN` — all `fetch()` POSTs send it (`wwwroot/js/site.js` `getToken()` pattern).
-- Security headers: CSP (script/style/img/connect/frame allow-list for self + GTM/GA/Clarity, `object-src 'none'`, `form-action 'self'`), HSTS outside dev, HTTPS redirect, forwarded headers for the AKS ingress.
+- Security headers: CSP (script/style/img/connect/frame allow-list for self + GTM/GA/Clarity, `object-src 'none'`, `base-uri 'self'`, `frame-ancestors 'self'` so no other site can frame the service (#610), `form-action 'self'` plus the DfE Sign-in origin), HSTS outside dev, HTTPS redirect, forwarded headers for the AKS ingress.
 
 ## 10. Data model
 

src/DfE.CheckPerformanceData.Web/Middleware/SecurityHeadersMiddleware.cs

@@ -9,8 +9,11 @@ namespace DfE.CheckPerformanceData.Web.Middleware;
 /// Sets the response security headers, and refuses TRACE.
 /// </summary>
 /// <remarks>
-/// The header set was already most of the way there — HSTS, a content-security policy and
-/// X-Frame-Options were in place — with a few gaps. These headers only do anything if they are on
+/// The header set was already most of the way there — HSTS and a content-security policy were
+/// in place — with a few gaps. (An earlier version of this remark said X-Frame-Options was set
+/// too. It never was, by this app, and the policy carried no <c>frame-ancestors</c> either until
+/// #610, so whether a page could be framed depended on whatever sat in front of the app.) These
+/// headers only do anything if they are on
 /// every response, so they belong in one middleware rather than spread across controllers where a
 /// new endpoint silently misses them.
 ///
@@ -49,7 +52,15 @@ public sealed class SecurityHeadersMiddleware(
         "connect-src 'self' https://*.googletagmanager.com https://*.google-analytics.com https://*.analytics.google.com https://*.clarity.ms; " +
         "frame-src 'self' https://*.googletagmanager.com; " +
         "object-src 'none'; " +
-        "base-uri 'self'; ";
+        "base-uri 'self'; " +
+        // Which sites may put this service in a frame: this one only. frame-ancestors replaced
+        // X-Frame-Options in current browsers. Our code has never set X-Frame-Options; the only
+        // one the service sends is the SAMEORIGIN that ASP.NET Core's antiforgery adds to pages
+        // that render a token, which agrees with this directive but leaves every other response
+        // (and anything in front of the app: the AKS ingress, Front Door) uncovered — so without
+        // this directive the clickjacking surface was whatever the platform happened to set
+        // (#610). 'self' rather than 'none' so a same-origin frame stays possible.
+        "frame-ancestors 'self'; ";
 
     // form-action also governs every redirect a form submission follows, not only the form's own
     // URL. A POST made after the sign-in has expired is answered with a 302 to DfE Sign-in; with

tests/DfE.CheckPerformanceData.UnitTests/Web/SecurityHeadersMiddlewareTests.cs

@@ -6,8 +6,9 @@
 
 namespace DfE.CheckPerformanceData.Application.UnitTests.Web;
 
-// The response-header set was most of a good one — HSTS, a content-security policy and
-// X-Frame-Options were already there — with a few gaps a security assessment picked up. These
+// The response-header set was most of a good one — HSTS and a content-security policy were
+// already there — with a few gaps a security assessment picked up (and, until #610, no
+// frame-ancestors; the app has never sent X-Frame-Options, whatever this comment once said). These
 // are cheap headers that only help if they are on every response, so they are set in one place
 // rather than per-controller, and asserted here rather than left to a scan to notice.
 public sealed class SecurityHeadersMiddlewareTests
@@ -133,6 +134,22 @@ public async Task ContentSecurityPolicy_IsStillSet()
         Assert.Contains("form-action 'self'", csp);
     }
 
+    // frame-ancestors is the CSP directive that decides which sites may put this service in a
+    // frame; it replaced X-Frame-Options in every current browser. The policy had none, and the
+    // app has never set X-Frame-Options (the comment at the top of this file used to say it
+    // did), so until #610 whether a page could be framed — the clickjacking surface — depended
+    // on whatever sat in front of the app. Asserted as a whole directive rather than a
+    // substring: "frame-ancestors 'self' https://other.example" would pass a Contains.
+    [Fact]
+    public async Task FrameAncestors_AllowsOnlyThisSite()
+    {
+        var context = await Run();
+
+        var directives = context.Response.Headers["Content-Security-Policy"].ToString()
+            .Split(';', StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries);
+        Assert.Contains("frame-ancestors 'self'", directives);
+    }
+
     // A POST made after the sign-in has expired is answered with a 302 to DfE Sign-in. Chrome
     // applies form-action to every redirect a form submission follows, so with 'self' alone the
     // browser cancelled that navigation and the page simply did nothing — on every form in the