WIP: feat: Enable CSP with nonce for Helix 5

[andreituicu]

1. Description

1. Allow customers to define a nonce based CSP using either a meta tag or header, with cached nonce, with the keyword 'nonce-aem'.

Meta:

    <meta http-equiv="content-security-policy" content="script-src 'nonce-aem' 'strict-dynamic'; style-src 'nonce-aem'; base-uri 'self'; object-src 'none';">

Header

content-security-policy: script-src 'nonce-aem' 'strict-dynamic'; style-src 'nonce-aem'; base-uri 'self'; object-src 'none';

3. Developers need to also add nonce="aem" on:

• each script from head.html
• each script from static html files in github

Example:

<script nonce="aem" src="/scripts/aem.js" type="module"></script>
<script nonce="aem" src="/scripts/scripts.js" type="module"></script>

4. Add support for a move-as-header attribute for the meta based CSP, where the Helix Pipeline will that tag as a header

Note: if this feature is premature, I can remove it and file it as a separate PR

Example:

<meta http-equiv="content-security-policy" content="script-src 'nonce-aem' 'strict-dynamic'; style-src 'nonce-aem'; base-uri 'self'; object-src 'none';" move-as-header="true">

5. The pipeline will generate a 16byte base64 encoded nonce, place it the aem part of the nonce-aem CSP and on each trusted script where it finds nonce="aem"

2. Implementation choice

After some back and forth between:

• having 'nonce' vs 'nonce-aem' as a keyword in the CSP
• requiring developers to add 'nonce="aem" or not to the scripts, to make it as friction-less and transparent as possible for devs.

I chose for the moment 'nonce-aem' and needing to add 'nonce="aem" to the scripts, because in this implementation if there's any reason to deactivate/remove or if the hlx pipeline fails to render the nonce, customers that have this enabled will still have a valid page that is rendered by the browser.
In the other variants, if for any reason 'nonce' is returned the browser will show just a white page.

3. Expected level of protection with a nonce cached on the CDN

1. Protected even if the nonce is cached

✅ .innerHTML for XSS
✅ .outerHTML for XSS
✅ .insertAdjecentHTML for XSS
✅ .setHTMLUnsafe for XSS
✅ eval / setTimeout / setInterval / Function for XSS
✅ javascript: protocol in the href/src attributes
✅ location.href / location.assign() / location.replace for XSS
✅ attribute event handlers (onclick, onblur, onload, onerror etc.) for XSS
✅ stored/reflected XSS

• as long as there is no server side manipulation of the HTML in the customer's CDN where user input is involved
• as long as the nonce is regenerated for requests that hit the origin
• for meta tag: as long as the XSS doesn't happen before the meta tag

2. not protected, because the nonce is cached

❌ document.write / document.writeln -> because you could get the nonce and use it in the exploit

3.not protected for other reasons

❌ document.createRange().createContextualFragment -> whether the nonce is cached/known is irrelevant, scripts are always executed with strict-dynamic. (needs ticket in https://github.com/w3c/webappsec-csp - awaiting OSS approval)
❌ src attribute of a <script> tag created by document.createElement('script') / text content of a <script> tag created by document.createElement('script') / import -> permitted by strict-dynamic

4. not protected by CSPs in general
Script-less attacks
❌ HTML injection
❌ DOM clobbering

5. unknown status
❓ element.style or cssText -> still researching, can’t find a working exploit even without CSP

4. Still TODO

• Backport to helix 4 in the 5.x branch.
• Validate that pages render correctly E2E in the helix 4 branch (helix5 doesn't support - hlx-pipeline-version)
• Add support in the aem cli for nonce on the live-reload script

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (d239b58) to head (2489e05).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##              main      #773    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files           45        46     +1     
  Lines         3651      3801   +150     
==========================================
+ Hits          3651      3801   +150     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[andreituicu]

Opening this PR in a WIP state, while I still work through the remaining TODOs to collect feedback and iterate in case there are changes needed.
Please let me know your thoughts! 🙂

[tripodsan]

LGTM in general.

• not sure about the extra parsing for all static html.
• remove the padding from the base64 nonce

[tripodsan]

not sure if we really should rewrite the provided HTML. and if so, with unified which might alter the html. maybe using a very simple text based parser that only understands minimal HTML and repaces the nonce tokens would less intrusive.

[andreituicu]

I used unified because that what was used in processing the head.html, which allowed me to keep the same codebase for processing both the document based and the static html and I like how it works with selectors.

Indeed, it does alter the resulting HTML, it makes it canonical (e.g. from the point of view of spaces, indentation, <SCRIPT> -> <script>, etc.).

very simple text based parser that only understands minimal HTML and repaces the nonce tokens would less intrusive.

Would you have any in mind that I could try out? Intuitively, I would expect that any parser when serialising back to canonicalise, since otherwise it would be very hard to store all the nuances of the original HTML.

If we want to keep the customer's HTML untouched except for the nonce generation, we could try a regex approach.

I usually avoid regexes for this kind of processing, because I'm not good at them, they become hard to maintain troubleshoot and I've seen Kodiak complain about ReDos, which means they could be slow for certain files.

Did a quick try with

res.body.replace(/(<script\b[^>]*\bnonce=")aem(")/ig, `$1${nonce}$2`)
      .replace(/(<style\b[^>]*\bnonce=")aem(")/ig, `$1${nonce}$2`)
      .replace(/(<link\b[^>]*\bnonce=")aem(")/ig, `$1${nonce}$2`)
      .replace(/(<meta\b[^>]*'nonce-)aem(')/ig, `$1${nonce}$2`)
      .replace(/(<meta\b[^>]*'nonce-)aem(')/ig, `$1${nonce}$2`) //can appear twice

I can't speak for all customers, but personally, as a developer, I think I would like that if I drop some messy HTML in github that I copied from somewhere I get a clean one when looking through the delivery service.

[tripodsan]

would it be possible that a header uses double quotes?

[andreituicu]

No, it is not possible. I tried it out both as meta tag and as header resulting the browser will show the following error and a blank page, because no script loads.

The source list for the Content Security Policy directive 'script-src' contains an invalid source: '"nonce-r4Nd0mmm"'. It will be ignored.

[github-actions]

This PR will trigger a minor release when merged.