[JENKINS-76263] Add Content-Security-Policy header#11269
[JENKINS-76263] Add Content-Security-Policy header#11269daniel-beck merged 21 commits intojenkinsci:masterfrom
Content-Security-Policy header#11269Conversation
4b59ebe to
7009af6
Compare
7009af6 to
065c007
Compare
Wadeck
left a comment
There was a problem hiding this comment.
Superficial review, no manual testing.
| * Contribute to the Content-Security-Policy rules. | ||
| */ | ||
| @Restricted(Beta.class) | ||
| public interface Contributor extends ExtensionPoint { |
There was a problem hiding this comment.
| public interface Contributor extends ExtensionPoint { | |
| public interface CspContributor extends ExtensionPoint { |
nit: Otherwise the class seems very generic
- Add UI configurability - Admin monitor recommends setting up CSP - Add extension that automatically allows domains with current user's avatar - `script-src usage.jenkins.io` conditional on usage stats enabled - Robustness: Catch exceptions from `Contributor#apply` - Move internal classes to `jenkins.security.csp.impl` package - No longer `#initialize` all directives in BaseContributor - Do not use an internal class name for the header name system property - New extension point to decide which header name to use, incl. UI contribution - Remove `GitHubContributor`, new API is now demonstrated for user avatar
timja
left a comment
There was a problem hiding this comment.
I would expect this to be configured OOTB for new Jenkins instances, e.g. ones that go through setup wizard or aren't upgrading from another version (either should be possible).
Is that left out here to do it in a phased approach?
I've not reviewed the code in detail, but I've manually tested this PR a fair bit - not tried with CSP plugin
core/src/main/resources/jenkins/security/csp/impl/CspConfiguration/config.jelly
Show resolved
Hide resolved
core/src/main/resources/jenkins/security/csp/impl/CspRecommendation/index.properties
Outdated
Show resolved
Hide resolved
| # TODO Add better explanation | ||
| paragraph1 = Content Security Policy is a security mechanism that can reduce or eliminate the impact of web security vulnerabilities like cross-site-scripting (XSS). | ||
| paragraph2 = Most popular Jenkins plugins are compatible with Jenkins's default rule set, but not everything currently installed may be. \ | ||
| If you choose to enforce CSP and it causes the Jenkins UI to break, you can start Jenkins with the Java system property \ |
There was a problem hiding this comment.
This should be linked to the page https://www.jenkins.io/doc/book/managing/system-properties/ so that the person knows how to configure a system property.
There was a problem hiding this comment.
Thanks. Please note that the content of this page is still placeholder content. I'm currently debating either making it much more extensive, or alternatively linking to the docs on jenkins.io. In the latter case, this might end up becoming just a popup dialog instead.
| @@ -0,0 +1,6 @@ | |||
| # TODO Add better explanation | |||
| paragraph1 = Content Security Policy is a security mechanism that can reduce or eliminate the impact of web security vulnerabilities like cross-site-scripting (XSS). | |||
There was a problem hiding this comment.
Is the reason to not just enable from this page so that the CSP plugin can add more UI / customisation?
it just seems a bit over complicated right now. Having to click learn more. Read a bunch of content and then go to another page. Enable the checkbox and save it.
(not blocking)
There was a problem hiding this comment.
How would you remind admins they need to configure the RRURL? Additionally, if they get here from the admin monitor, a side benefit is that they'll know afterwards where the configuration is.
core/src/main/resources/jenkins/security/csp/impl/CspRecommendation/message.properties
Outdated
Show resolved
Hide resolved
core/src/main/java/jenkins/security/csp/impl/UserAvatarContributor.java
Outdated
Show resolved
Hide resolved
Phased approach for everyone. The initial draft here enabled it by default, but I got some pushback. The motivations here are roughly the following:
So while it's opt-in for everyone, this also advertises it through the admin monitor to hopefully get many admins to at least try it, and report back any problems. I expect it'll take at least one LTS cycle to gain more confidence / have plugins become compatible after user feedback (or |
I added my current plan to the PR description. |
- Change CspReceiver#report signature to take String instead of User - Change ReportingContext#encodeContext signature to take Class instead of Object - ReportingAction#doDynamic now actually limits how long reports can get - Better invalid input handling and related logging in ReportingAction#doDynamic - Fix type check in AdvancedConfiguration#getCurrent(Class) - Add a link from admin monitor page to the CSP compatibility section on jenkins.io - Remove extra space added after directive key without value - Add 'since TODO' Javadoc for Beta API - Add missing class Javadoc, minor improvements - Add a missed license header - Better logging in LoggingReceiver (the default) - Remove unnecessary Extension ordinal and related TODO comment - Fix log message in CspDecorator, logged the wrong type - Add more tests for CspBuilder (and remove some redundant ones) - Improve logging assertions in ContentSecurityPolicyTest - Remove some obsolete/unnecessary TODOs
…tives with 'none' value
- If CSP was previously enforced, but RRURL is disabled, show warning form validation (but not when it's newly enabled, let's make it clear what's the "good" choice) - Move LoggingReceiver from nested class to impl package (top level class) - Add link to RRURL configuration from form validation of CSP enforce flag
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
All implementations of |
| @@ -0,0 +1,2 @@ | |||
| blurb = The Content Security Policy header is currently set to <code>Content-Security-Policy</code> because Jenkins is running in development mode. \ | |||
| This behavior can be disabled by setting the Java system property <code>jenkins.security.csp.impl.DevelopmentHeaderDecider.DISABLED</code> to <code>true</code>. | |||
There was a problem hiding this comment.
How about this? It would be good to be able to just temporarily disable in a running instance, I was surprised following the supplied instructions didn't work in the script console.
| This behavior can be disabled by setting the Java system property <code>jenkins.security.csp.impl.DevelopmentHeaderDecider.DISABLED</code> to <code>true</code>. | |
| This behavior can be disabled by setting the Java system property <code>jenkins.security.csp.impl.DevelopmentHeaderDecider.DISABLED</code> to <code>true</code> before starting Jenkins or in the script console <code>jenkins.security.csp.impl.DevelopmentHeaderDecider.DISABLED = true</code> for a running Jenkins. |
There was a problem hiding this comment.
TBH I don't want to make the barrier to ignoring problems in a plugin too low. It is easy, and developing the CSP plugin requires you (well, me) to apply this, but most other plugins are long overdue making themselves compatible. If someone needs to quit and restart an additional time, maybe that nudges them into fixing things 😇
Regarding
the supplied instructions didn't work in the script console
this is consistent with most of https://www.jenkins.io/doc/book/managing/system-properties/
We generally do not support changing system properties after Jenkins has started. There are some (like the DBS CSP config), but those are usually documented as such and done like that for a reason.
| paragraph2 = Most popular Jenkins plugins are compatible with Jenkins''s default rule set, but not everything currently installed may be. \ | ||
| If you choose to enforce Content Security Policy and it causes the Jenkins UI to break, you can start Jenkins with the Java system property \ | ||
| <code>jenkins.security.csp.CspHeader.headerName</code> set to <code>Content-Security-Policy-Report-Only</code> to disable protections again. | ||
| paragraph3 = For resources on determining whether your current setup is compatible with Content Security Policy enforcement, visit <a href="https://www.jenkins.io/redirect/csp-compatibility" target="_blank">the documentation website</a>. |
There was a problem hiding this comment.
reads slightly better to me:
| paragraph3 = For resources on determining whether your current setup is compatible with Content Security Policy enforcement, visit <a href="https://www.jenkins.io/redirect/csp-compatibility" target="_blank">the documentation website</a>. | |
| paragraph3 = For resources on determining whether your current setup is compatible with Content Security Policy enforcement, see <a href="https://www.jenkins.io/redirect/csp-compatibility" target="_blank">the documentation</a>. |
Circular dependency with |
Kevin-CB
left a comment
There was a problem hiding this comment.
I retested with the latest changes and tried some avatar plugins without encountering any issues.
|
This PR is now ready for merge. We will merge it after approximately 24 hours if there is no negative feedback. Please see the merge process documentation for more information about the merge process. /label ready-for-merge |
MarkEWaite
left a comment
There was a problem hiding this comment.
My interactive testing showed only one already known issue (implied labels plugin) and that issue is now confirmed fixed in a release.
I explored all the "Manage Jenkins" pages on my controller and explored various jobs and operations on those jobs. List of plugins that I was using is in my pluigins.txt file.
| @CheckForNull | ||
| /* package */ static String getReportingEndpoint(HttpServletRequest req) { | ||
| Class<?> modelObjectClass = null; | ||
| String restOfPath = StringUtils.removeStart(req.getRequestURI(), req.getContextPath()); |
There was a problem hiding this comment.
Didn't we want to get rid of commons.lang usage in core?
See JENKINS-76263.
This PR implements the
Content-Security-Policyheader in core. Previously, users had to install the Content Security Policy Plugin to get this protection.This feature is opt-in for now, given it is a major change and will break some plugins. Their compatibility is tracked in JENKINS-60865. Per this tracking spreadsheet, all popular maintained plugins are expected to be compatible. Some plugins (those with "avatar" icons) are being adapted to be compatible using the new APIs introduced in this PR, see below for details.
Additionally, this removes the
X-Webkit-CSPandX-Content-Security-Policyheaders fromDirectoryBrowserSupport. Those have been obsolete since about 2013, so even backward compatibility is no longer really a relevant concern.Closely related PRs:
CspRulein ATH: [JENKINS-76263] Adapt CspRule to core CSP change acceptance-test-harness#2269Loosely related PRs:
scripttag violations: Emit Jelly file path inside<script>tags to get better CSP samples stapler#612Open PR Tasks
Optionally fine-tune wording on the
CspRecommendation/indexpage, but for a weekly it seems fine as is IMO.Overview
Deciding which header to set
Details
There are four basic configurations/states controlling which header (CSP or CSP-Report-Only) is set. These are checked in order. See
CspHeaderDeciderfor details.jenkins.security.csp.CspHeader.headerNameis set to either of the two values. This hides/disables the configuration UI and sets the configured header.Main#isUnitTestorMain#isDevelopmentare true. This works similarly to the previous one, just a different message on the placeholder UI, and will always setCSP. Disable by settingjenkins.security.csp.impl.DevelopmentHeaderDecider.DISABLEDtotrue. The idea here is that developers should be the first to encounter compatibility problems, ideally fixing them rather than ignoring them / disabling this behavior.CSPheader (checked, "enforcing") orCSP-Report-Only(unchecked), if configuration is enabled.CSP-Report-Onlyand to show a placeholder UI that recommends enabling CSP configuration. This is what most users of Jenkins see first (besides the admin monitor).There are two different terms here: One is that configuration is enabled, the other that it is enforcing CSP. The latter just means we set the CSP header, not CSP-Report-Only. The former means that an admin chose to configure this. This two step process is needed to support the admin monitor showing while an admin has not made a choice yet; otherwise "blindly" submitting Manage Jenkins » Security would look like the admin opting out of protection.
Now the usual journey to enabling this option is as follows:
/configureSecurity/CspRecommendation/index.jellyview that explains the functionality and caveats./configureSecurity/.At each step, there's a way to opt out:
/configureSecurity//configureSecurity/submission is needed to persist the state from "enabling" the configuration.Interaction with Resource Root URL
Unfortunately, serving user-generated resource files from the same origin is a problem.
script-src 'self'is satisfied by/job/whatever/1/artifacts/lol.js. So users who want good protection definitely need to set up Resource Root URL, which is why this is called out in the form validation, and on https://github.com/jenkins-infra/jenkins.io/pull/8529/files#diff-2fdabed902fd1d2d7e1ee4cafa034e3c3733979b50b0cd949bc1f4d1589b84a1R42-R44.CasC support
See https://github.com/daniel-beck/csp-plugin/blob/v2/README.adoc#configuration-as-code for what it looks like with the CSP plugin installed (otherwise there's just
enforce).DirectoryBrowserSupportand Resource Root URLThe header is not being set for RRURL requests, same as CSP plugin 1.x.
Otherwise, DBS's more restrictive (e.g.,
sandbox) CSP takes precedence, and if disabled throughhudson.model.DirectoryBrowserSupport.CSPsystem property will also remove the regular header. SeeContentSecurityPolicyTestfor where all of this is being tested.Known potential problems
Besides the previously known problems of inline scripts etc. that need code restructuring to make work, the following more general features are inherently affected:
UserAvatarContributoris likely to result in broken image references unless the basic implementation in this PR takes care of everything.AvatarMetadataActioninscm-apidefines a similar feature for org folders, and either that will need to be adapted generally, or implementations need to add support for these new core APIs.These plugins need to implement
Contributor, or callAvatarContributor#allow(or locally cache avatar images). This is newly being documented by jenkins-infra/jenkins.io#8529.Current status of plugins known to implement these extension points:
UserAvatarResolver🚧 https://plugins.jenkins.io/oic-auth/ - jenkinsci/oic-auth-plugin#681
✅ https://plugins.jenkins.io/azure-ad/ - caches avatar images locally
🚧 https://plugins.jenkins.io/keycloak/ - jenkinsci/keycloak-plugin#27
🚧 https://plugins.jenkins.io/gravatar/ - jenkinsci/gravatar-plugin#166
✅ https://plugins.jenkins.io/avatar/ - only renders locally uploaded files
AvatarMetadataAction(scm-api)🚧 https://plugins.jenkins.io/github-branch-source/ - jenkinsci/github-branch-source-plugin#917
✅ https://plugins.jenkins.io/cloudbees-bitbucket-branch-source/ - appears to only render locally cached avatars
✅ https://plugins.jenkins.io/gitlab-branch-source/ - appears to only render locally cached avatars
✅ https://plugins.jenkins.io/gitea/ - appears to only render locally cached avatars
✅ https://plugins.jenkins.io/gerrit-code-review/ - renders bundled avatar icons
❓ https://plugins.jenkins.io/tuleap-git-branch-source/ - looks incomplete: this does not implement any of that.
Testing done
Autotests, and some manual tests:
DirectoryBrowserSupporthas custom CSP header if enforcedDelivery plan
Details
Step 1 – Opt in core feature (~Nov 2025)
Gate: Core PR done, CSP plugin PR (for customization) done.
Disabled by default even for new installations.
An admin monitor encourages admins to enable it (extra visibility). Can be disabled through the UI (for all users).
During development and testing, it’s always enabled to give an early warning to developers.
Planned for a weekly soon, so that it’s in the next LTS.
Set up new issue tracking (labels, Epic, separate reporting GH repo) perhaps to understand compatibility?
Step 2 – Advertise to developers (~Nov 2025)
Gate: Core PR is in weekly.
Encourage plugin maintainers to adapt their plugins. Doesn’t need new core dependency, can try with CSP plugin v1.
Maybe also publish a blog post to advertise further, including the plan?
Step 3 – Advertise to LTS users (late Jan 2026)
Gate: Core PR is in LTS.
Mention it in some detail in the upgrade guide.
Perhaps a blog post?
Step 4 – Opt out core feature (Q2/2026, maybe?)
Gate: Plugins are sufficiently adapted (TBD threshold etc.)
Switch the core feature to opt out (via UI).
At this point we’re basically “done” – removing the UI option to opt out can be done whenever we want.
Proposed changelog entries
csp) installed, update it to version 2.x.Proposed changelog category
/label major-rfe,developer
Proposed upgrade guidelines
Content Security Policy support in Jenkins core
Jenkins now allows enforcing Content Security Policy. This is a security mechanism that can reduce or eliminate the impact of web security vulnerabilities like cross-site-scripting (XSS). Jenkins core and most popular Jenkins plugins are compatible with Jenkins's default rule set, but for backwards compatibility, CSP enforcement is disabled by default.
See the Content Security Policy documentation for more information.
Users of Content Security Policy Plugin should disable it or update it to version 2.x.
Submitter checklist
@Restrictedor have@since TODOJavadocs, as appropriate.@Deprecated(since = "TODO")or@Deprecated(forRemoval = true, since = "TODO"), if applicable.evalto ease future introduction of Content Security Policy (CSP) directives (see documentation).Desired reviewers
@mention
Before the changes are marked as
ready-for-merge:Maintainer checklist
upgrade-guide-neededlabel is set and there is a Proposed upgrade guidelines section in the pull request title (see example).lts-candidateto be considered (see query).