Allow entirely disabling CSP headers (jenkinsci#23915)

Co-authored-by: Daniel Beck <[email protected]>

Author: daniel-beck

core/src/main/java/jenkins/security/csp/CspHeader.java

@@ -24,6 +24,7 @@
 
 package jenkins.security.csp;
 
+import edu.umd.cs.findbugs.annotations.CheckForNull;
 import org.kohsuke.accmod.Restricted;
 import org.kohsuke.accmod.restrictions.Beta;
 
@@ -35,14 +36,16 @@
 @Restricted(Beta.class)
 public enum CspHeader {
     ContentSecurityPolicy("Content-Security-Policy"),
-    ContentSecurityPolicyReportOnly("Content-Security-Policy-Report-Only");
+    ContentSecurityPolicyReportOnly("Content-Security-Policy-Report-Only"),
+    None(null);
 
     private final String headerName;
 
     CspHeader(String headerName) {
         this.headerName = headerName;
     }
 
+    @CheckForNull
     public String getHeaderName() {
         return headerName;
     }

core/src/main/java/jenkins/security/csp/impl/CspDecorator.java

@@ -105,10 +105,11 @@ public String getReportingEndpointsHeaderValue(HttpServletRequest req) {
     }
 
     /**
-     * Determines the name of the HTTP header to set.
+     * Determines the name of the HTTP header to set, or {@code null} if none.
      *
      * @return the name of the HTTP header to set.
      */
+    @CheckForNull
     public String getContentSecurityPolicyHeaderName() {
         final Optional<CspHeaderDecider> decider = CspHeaderDecider.getCurrentDecider();
         if (decider.isPresent()) {

core/src/main/java/jenkins/security/csp/impl/CspFilter.java

@@ -61,12 +61,14 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
 
         CspDecorator cspDecorator = ExtensionList.lookupSingleton(CspDecorator.class);
         final String headerName = cspDecorator.getContentSecurityPolicyHeaderName();
+        final boolean headerShouldBeSet = headerName != null;
 
         // This is the preliminary value outside Stapler request handling (and providing a context object)
         final String headerValue = cspDecorator.getContentSecurityPolicyHeaderValue(req);
 
         final boolean isResourceRequest = ResourceDomainConfiguration.isResourceRequest(req);
-        if (!isResourceRequest) {
+
+        if (headerShouldBeSet && !isResourceRequest) {
             // The Filter/Decorator approach needs us to "set" headers rather than "add", so no additional endpoints are supported at the moment.
             final String reportingEndpoints = cspDecorator.getReportingEndpointsHeaderValue(req);
             if (reportingEndpoints != null) {
@@ -78,14 +80,16 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
         try {
             chain.doFilter(req, rsp);
         } finally {
-            try {
-                final String actualHeader = rsp.getHeader(headerName);
-                if (!isResourceRequest && hasUnexpectedDifference(headerValue, actualHeader)) {
-                    LOGGER.log(Level.FINE, "CSP header has unexpected differences: Expected '" + headerValue + "' but got '" + actualHeader + "'");
+            if (headerShouldBeSet) {
+                try {
+                    final String actualHeader = rsp.getHeader(headerName);
+                    if (!isResourceRequest && hasUnexpectedDifference(headerValue, actualHeader)) {
+                        LOGGER.log(Level.FINE, "CSP header has unexpected differences: Expected '" + headerValue + "' but got '" + actualHeader + "'");
+                    }
+                } catch (RuntimeException e) {
+                    // Be defensive just in case
+                    LOGGER.log(Level.FINER, "Error checking CSP header after request processing", e);
                 }
-            } catch (RuntimeException e) {
-                // Be defensive just in case
-                LOGGER.log(Level.FINER, "Error checking CSP header after request processing", e);
             }
         }
     }

core/src/main/java/jenkins/security/csp/impl/SystemPropertyHeaderDecider.java

@@ -25,7 +25,9 @@
 package jenkins.security.csp.impl;
 
 import hudson.Extension;
+import hudson.Util;
 import java.util.Arrays;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -50,7 +52,8 @@ public Optional<CspHeader> decide() {
         final String systemProperty = SystemProperties.getString(SYSTEM_PROPERTY_NAME);
         if (systemProperty != null) {
             LOGGER.log(Level.FINEST, "Using system property: {0}", new Object[]{ systemProperty });
-            return Arrays.stream(CspHeader.values()).filter(h -> h.getHeaderName().equals(systemProperty)).findFirst();
+            final String expected = Util.fixEmptyAndTrim(systemProperty);
+            return Arrays.stream(CspHeader.values()).filter(h -> Objects.equals(expected, h.getHeaderName())).findFirst();
         }
         return Optional.empty();
     }

core/src/main/resources/jenkins/security/csp/impl/CspDecorator/httpHeaders.jelly

@@ -23,6 +23,9 @@ THE SOFTWARE.
 -->
 <?jelly escape-by-default='true'?>
 <j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler">
-    <st:setHeader name="${it.getContentSecurityPolicyHeaderName()}" value="${it.getContentSecurityPolicyHeaderValue(request2)}" />
-    <st:setHeader name="Reporting-Endpoints" value="${it.getReportingEndpointsHeaderValue(request2)}" />
+    <j:set var="cspHeaderName" value="${it.getContentSecurityPolicyHeaderName()}"/>
+    <j:if test="${cspHeaderName != null}">
+        <st:setHeader name="${cspHeaderName}" value="${it.getContentSecurityPolicyHeaderValue(request2)}" />
+        <st:setHeader name="Reporting-Endpoints" value="${it.getReportingEndpointsHeaderValue(request2)}" />
+    </j:if>
 </j:jelly>

core/src/main/resources/jenkins/security/csp/impl/SystemPropertyHeaderDecider/message.jelly

@@ -23,5 +23,14 @@ THE SOFTWARE.
 -->
 <?jelly escape-by-default='true'?>
 <j:jelly xmlns:j="jelly:core" xmlns:f="/lib/form">
-    <f:description>${%blurb(it.headerName)}</f:description>
+    <f:description>
+        <j:choose>
+            <j:when test="${it.headerName != null}">
+                ${%blurb(it.headerName)}
+            </j:when>
+            <j:otherwise>
+                ${%blurbUnset}
+            </j:otherwise>
+        </j:choose>
+    </f:description>
 </j:jelly>

core/src/main/resources/jenkins/security/csp/impl/SystemPropertyHeaderDecider/message.properties

@@ -1,2 +1,4 @@
 blurb = Content Security Policy is configured to use the HTTP header <code>{0}</code> based on the system property <code>jenkins.security.csp.CspHeader.headerName</code>. \
-    It cannot be configured through the UI while this system property specifies a header name.
+    It cannot be configured through the UI while this system property is set.
+blurbUnset = Content Security Policy is disabled based on the system property <code>jenkins.security.csp.CspHeader.headerName</code>. \
+    It cannot be configured through the UI while this system property is set.

test/src/test/java/jenkins/security/csp/impl/CspHeaderDeciderTest.java

@@ -104,6 +104,31 @@ public void testDefaultWithSystemPropertyUnenforce(JenkinsRule j) throws IOExcep
         }
     }
 
+    @Test
+    public void testDefaultWithSystemPropertyNone(JenkinsRule j) throws IOException, SAXException {
+        System.setProperty(SystemPropertyHeaderDecider.SYSTEM_PROPERTY_NAME, "");
+        try (JenkinsRule.WebClient webClient = j.createWebClient()) {
+            final Optional<CspHeaderDecider> decider = CspHeaderDecider.getCurrentDecider();
+            assertTrue(decider.isPresent());
+            assertThat(decider.get(), instanceOf(SystemPropertyHeaderDecider.class));
+
+            assertFalse(ExtensionList.lookupSingleton(CspRecommendation.class).isActivated());
+
+            final HtmlPage htmlPage = webClient.goTo("configureSecurity");
+            assertThat(
+                    htmlPage.getWebResponse().getContentAsString(),
+                    hasMessage(jellyResource(SystemPropertyHeaderDecider.class, "message.properties"), "blurbUnset"));
+            assertThat(htmlPage.getWebResponse().getResponseHeaderValue("Content-Security-Policy"), nullValue());
+            assertThat(htmlPage.getWebResponse().getResponseHeaderValue("Content-Security-Policy-Report-Only"), nullValue());
+
+            // submit form and confirm this didn't create a config file
+            htmlPage.getFormByName("config").submit(htmlPage.getFormByName("config").getButtonByName("Submit"));
+            assertFalse(ExtensionList.lookupSingleton(CspConfiguration.class).getConfigFile().exists());
+        } finally {
+            System.clearProperty(SystemPropertyHeaderDecider.SYSTEM_PROPERTY_NAME);
+        }
+    }
+
     @Test
     public void testDefaultWithSystemPropertyWrong(JenkinsRule j) throws IOException, SAXException {
         System.setProperty(SystemPropertyHeaderDecider.SYSTEM_PROPERTY_NAME, "Some-Other-Value");
@@ -219,7 +244,11 @@ public void testFallbackAdminMonitorAndSetup(JenkinsRule j) throws IOException,
     }
 
     private static Matcher<String> hasBlurb(Properties props) {
-        return containsString(props.getProperty("blurb"));
+        return hasMessage(props, "blurb");
+    }
+
+    private static Matcher<String> hasMessage(Properties props, String key) {
+        return containsString(props.getProperty(key));
     }
 
     private static Properties jellyResource(Class<?> clazz, String filename) throws IOException {