Add Subresource Integrity (SRI) on Highlight.js styles and scripts #440

ggrossetie · 2021-09-11T12:43:57Z

Register a syntax highlighter based on Highlight.js that adds integrity, crossorigin and referrerpolicy on <link> and <script> elements (as suggested in: asciidoctor/asciidoctor#3728 (comment))

= Code
:source-highlighter: highlightjs
:highlightjs-languages: haskell
:highlightjs-theme: darcula

[source,js]
----
console.log('hello world')
----

[source,haskell]
----
main = putStrLn "Hello, World!"
----

<!-- ... -->
<link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/highlight.js/9.18.3/styles/darcula.min.css" integrity="sha512-mDkfs1JGo+aBCXtuQy9g47Dssbnh99VbMhso4+2fiCgonurHnBuOZadZ9kb7URAc/L4enWNzAth0ghWVnH2xyQ==" crossorigin="anonymous" referrerpolicy="no-referrer">
<!-- ... -->
<script src="https://cdnjs.cloudflare.com/ajax/libs/highlight.js/9.18.3/highlight.min.js" integrity="sha512-tHQeqtcNWlZtEh8As/4MmZ5qpy0wj04svWFK7MIzLmUVIzaHXS8eod9OmHxyBL1UET5Rchvw7Ih4ZDv5JojZww==" crossorigin="anonymous" referrerpolicy="no-referrer"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/highlight.js/9.18.3/languages/haskell.min.js" integrity="sha512-PbhmxFKkjbXy/b1stHLnqO8IisGOhq/xmkO1G/pRS4Ng3iODhUZp0t/HxwXf50Bt9sz6muiV/3wRD4gOwvefFw==" crossorigin="anonymous" referrerpolicy="no-referrer"></script>

In theory, users won't need to disable security policies when enabling syntax highlighting (with Highlight.js).

ggrossetie · 2021-09-11T12:49:33Z

src/highlightjs-adapter.ts

@@ -0,0 +1,36 @@
+// from https://github.com/cdnjs/SRIs/blob/master/highlight.js/9.18.3.json
+import * as sri from './highlightjs-sri-9.18.3.json'
+const {Opal} = require('asciidoctor-opal-runtime')


Asciidoctor.js does not provide an API to extend a class, that's why we are using Opal directly.

See: asciidoctor/asciidoctor.js#1418

I am so glad you are here to help 😺

danyill · 2021-09-12T02:53:15Z

Thanks for this opportunity.

I'll take a look at it this afternoon and hope to learn something.

On the face of it without disabling preview security highlight.js is still not running, so I must investigate.

danyill · 2021-09-12T04:12:33Z

There are two problems with the existing code AFAIU:

The integrity of the highlight.min.js must be included in a meta element as part of a CSP declaration:

So we need to include the integrity definition in the following:

<script src="https://cdnjs.cloudflare.com/ajax/libs/highlight.js/9.18.3/highlight.min.js" integrity="sha512-tHQeqtcNWlZtEh8As/4MmZ5qpy0wj04svWFK7MIzLmUVIzaHXS8eod9OmHxyBL1UET5Rchvw7Ih4ZDv5JojZww==" crossorigin="anonymous" referrerpolicy="no-referrer"></script>

In the CSP definition here:

asciidoctor-vscode/src/features/previewContentProvider.ts

Lines 196 to 198 in 05e2413

    
           case AsciidocPreviewSecurityLevel.Strict: 
        
           default: 
        
             return `<meta http-equiv="Content-Security-Policy" content="default-src 'none'; img-src vscode-resource: https: data:; media-src vscode-resource: https: data:; script-src 'nonce-${nonce}'; style-src vscode-resource: 'unsafe-inline' https: data:; font-src vscode-resource: https: data:;">`;

So that we end up with something like:

<meta http-equiv="Content-Security-Policy" 

content="default-src 'none'; img-src https://*.vscode-webview.net https: data:; media-src https://*.vscode-webview.net https: data:; 

script-src 'nonce-163141872707272' 'sha512-tHQeqtcNWlZtEh8As/4MmZ5qpy0wj04svWFK7MIzLmUVIzaHXS8eod9OmHxyBL1UET5Rchvw7Ih4ZDv5JojZww=='; 

style-src https://*.vscode-webview.net 'unsafe-inline' https: data:; font-src https://*.vscode-webview.net https: data:;

">

That will then allow highlight.min.js to run correctly.
At least that's what I thought would happen, but I still noticed no highlighting. 😕
Then 💡 I realised we had not considered CSP when we have injected a further script into the web page in the following:

https://github.com/Mogztter/asciidoctor-vscode/blob/676fad710f7287af31e83b2856c33617c33733bf/src/highlightjs-adapter.ts#L29-L34

<script>
if (!hljs.initHighlighting.called) {
  hljs.initHighlighting.called = true
  ;[].slice.call(document.querySelectorAll("pre.highlight > code")).forEach(function (el) { hljs.highlightBlock(el) })
}
</script>`

This script needs to have a nonce included in it.

The easiest solution would be to move this code into the return value of providePreviewHTML where the nonce is readily available:

asciidoctor-vscode/src/features/previewContentProvider.ts

Lines 73 to 92 in 05e2413

    
               return `<!DOCTYPE html> 
        
           			<html> 
        
           			<head> 
        
           				<meta http-equiv="Content-type" content="text/html;charset=UTF-8"> 
        
           				${csp} 
        
           				<meta id="vscode-asciidoc-preview-data" 
        
           					data-settings="${JSON.stringify(initialData).replace(/"/g, '&quot;')}" 
        
           					data-strings="${JSON.stringify(previewStrings).replace(/"/g, '&quot;')}" 
        
           					data-state="${JSON.stringify(state || {}).replace(/"/g, '&quot;')}"> 
        
           				<script src="${this.extensionScriptPath('pre.js')}" nonce="${nonce}"></script> 
        
           				${this.getStyles(sourceUri, nonce, config, state)} 
        
           				<base href="${asciidocDocument.uri.with({ scheme: 'vscode-resource' }).toString(true)}"> 
        
           			</head> 
        
           			<body class="${bodyClassesVal} vscode-body ${config.scrollBeyondLastLine ? 'scrollBeyondLastLine' : ''} ${config.wordWrap ? 'wordWrap' : ''} ${config.markEditorSelection ? 'showEditorSelection' : ''}"> 
        
           				${body} 
        
           				<div class="code-line" data-line="${asciidocDocument.lineCount}"></div> 
        
           				<script async src="${this.extensionScriptPath('index.js')}" nonce="${nonce}" charset="UTF-8"></script> 
        
           			</body> 
        
           			</html>`; 
        
             }

Something like:

    return `<!DOCTYPE html>
			<html>
			<head>
				<meta http-equiv="Content-type" content="text/html;charset=UTF-8">
				${csp}
				<meta id="vscode-asciidoc-preview-data"
					data-settings="${JSON.stringify(initialData).replace(/"/g, '&quot;')}"
					data-strings="${JSON.stringify(previewStrings).replace(/"/g, '&quot;')}"
					data-state="${JSON.stringify(state || {}).replace(/"/g, '&quot;')}">
				<script src="${this.extensionScriptPath('pre.js')}" nonce="${nonce}"></script>
				${this.getStyles(sourceUri, nonce, config, state)}
				<base href="${asciidocDocument.uri.with({ scheme: 'vscode-resource' }).toString(true)}">
			</head>
			<body class="${bodyClassesVal} vscode-body ${config.scrollBeyondLastLine ? 'scrollBeyondLastLine' : ''} ${config.wordWrap ? 'wordWrap' : ''} ${config.markEditorSelection ? 'showEditorSelection' : ''}">
				${body}
				<div class="code-line" data-line="${asciidocDocument.lineCount}"></div>
				<script async src="${this.extensionScriptPath('index.js')}" nonce="${nonce}" charset="UTF-8"></script>
        <script nonce="${nonce}">
        if (!hljs.initHighlighting.called) {
          hljs.initHighlighting.called = true
          ;[].slice.call(document.querySelectorAll("pre.highlight > code")).forEach(function (el) { hljs.highlightBlock(el) })
        }
        </script>
			</body>
			</html>`;
  }

When I have locally hacked this, I find that this works correctly. But we should probably avoid calling hljs if there is no source highlighting and probably all nonces and integrity references should be passed as an array.

Hack here:

https://github.com/Mogztter/asciidoctor-vscode/compare/highlightjs-integrity...danyill:highlightjs-integrity-hack?expand=1

Now that I've written all this I realise I should probably have scoped this more tightly against specific lines of code. I'll put a little comment in a few places.

danyill

Thank you so much for helping out here. I think I understand why this isn't quite working yet. I have made many comments and am happy to help out in any way.

danyill · 2021-09-12T04:13:41Z

src/highlightjs-adapter.ts

+    }
+    return `<script src="${baseUrl}/highlight.min.js" integrity="${sri['highlight.min.js']}" crossorigin="anonymous" referrerpolicy="no-referrer"></script>
+${languageScripts}
+<script>


This will not work at all 😉 because it will not be run due to CSP which is quite funny 😜

danyill · 2021-09-12T04:15:15Z

src/highlightjs-adapter.ts

+      languageScripts = doc.$attr('highlightjs-languages').split(',').map((lang) => {
+        const key = `languages/${lang.trim()}.min.js`
+        const integrityValue = sri[key]
+        return `<script src="${baseUrl}/${key}" ${integrityValue ? `integrity="${integrityValue}" ` : ''}crossorigin="anonymous" referrerpolicy="no-referrer"></script>`


The integrity of this script also needs to be included in the <meta> tag of the document, see

asciidoctor-vscode/src/features/previewContentProvider.ts

Line 198 in 7b7d04d

return `<meta http-equiv="Content-Security-Policy" content="default-src 'none'; img-src vscode-resource: https: data:; media-src vscode-resource: https: data:; script-src 'nonce-${nonce}'; style-src vscode-resource: 'unsafe-inline' https: data:; font-src vscode-resource: https: data:;">`;

The integrity needs to be included as a separate quoted value (in the same form as in the script) adjacent to the nonce.

danyill · 2021-09-12T04:17:47Z

src/highlightjs-adapter.ts

@@ -0,0 +1,36 @@
+// from https://github.com/cdnjs/SRIs/blob/master/highlight.js/9.18.3.json
+import * as sri from './highlightjs-sri-9.18.3.json'
+const {Opal} = require('asciidoctor-opal-runtime')


I am so glad you are here to help 😺

ggrossetie · 2021-09-12T09:12:10Z

@joaompinto You were too eager to merge 😉
As mentioned by @danyill, my changes are not enough, I will open a new pull request to address your comments.

By the way, how do you re-enable security? I wasn't able to test since I already disabled security on VS Code 😞

danyill · 2021-09-12T09:22:34Z

If you open the command palette (on my system Ctrl+Shift P) and write some keywords, e.g. 'AsciiDoc security' you should see the extension security options. Typically you must force a preview refresh by writing some more text before the preview updates to see the outcome. Also in the command palette you can open the web developer tools and inspect the console. Thanks!

…

On Sun, 12 Sep 2021, 21:12 Guillaume Grossetie, ***@***.***> wrote: @joaompinto <https://github.com/joaompinto> You were too eager to merge 😉 As mentioned by @danyill <https://github.com/danyill>, my changes are not enough, I will open a new pull request to address your comments. By the way, how do you re-enable security? I wasn't able to test since I already disabled security on VS Code 😞 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#440 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFEXXY4LJFGYQTJDNYFCG3UBRVHLANCNFSM5D25VQRQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

ggrossetie · 2021-09-12T12:27:49Z

If you open the command palette (on my system Ctrl+Shift P) and write some keywords, e.g. 'AsciiDoc security' you should see the extension security options

Thanks for the tip 👍

I just noticed that it won't work when using the asciidoctor CLI because we are only registering the custom syntax highlighter on Asciidoctor.js 😞

joaompinto · 2021-09-12T14:13:22Z

@joaompinto You were too eager to merge 😉
As mentioned by @danyill, my changes are not enough, I will open a new pull request to address your comments.

By the way, how do you re-enable security? I wasn't able to test since I already disabled security on VS Code 😞

Sorry, was trying to help :P

ggrossetie · 2021-09-12T14:15:09Z

Sorry, was trying to help :P

No worries, I have push permission on the main branch so I can merge pull request when it's ready 😉

Add Subresource Integrity (SRI) on Highlight.js styles and scripts

676fad7

ggrossetie requested a review from danyill September 11, 2021 12:43

ggrossetie commented Sep 11, 2021

View reviewed changes

ggrossetie mentioned this pull request Sep 11, 2021

No code highlight in preview #437

Closed

danyill requested changes Sep 12, 2021

View reviewed changes

joaompinto merged commit 74bce1d into asciidoctor:master Sep 12, 2021

danyill added this to the 2.8.10 milestone Oct 21, 2021

ggrossetie deleted the highlightjs-integrity branch March 6, 2022 13:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Subresource Integrity (SRI) on Highlight.js styles and scripts #440

Add Subresource Integrity (SRI) on Highlight.js styles and scripts #440

ggrossetie commented Sep 11, 2021

ggrossetie Sep 11, 2021 •

edited

Loading

danyill Sep 12, 2021

danyill commented Sep 12, 2021

danyill commented Sep 12, 2021

danyill left a comment

danyill Sep 12, 2021

danyill Sep 12, 2021

danyill Sep 12, 2021

ggrossetie commented Sep 12, 2021

danyill commented Sep 12, 2021 via email

ggrossetie commented Sep 12, 2021

joaompinto commented Sep 12, 2021

ggrossetie commented Sep 12, 2021

Add Subresource Integrity (SRI) on Highlight.js styles and scripts #440

Add Subresource Integrity (SRI) on Highlight.js styles and scripts #440

Conversation

ggrossetie commented Sep 11, 2021

ggrossetie Sep 11, 2021 • edited Loading

Choose a reason for hiding this comment

danyill Sep 12, 2021

Choose a reason for hiding this comment

danyill commented Sep 12, 2021

danyill commented Sep 12, 2021

danyill left a comment

Choose a reason for hiding this comment

danyill Sep 12, 2021

Choose a reason for hiding this comment

danyill Sep 12, 2021

Choose a reason for hiding this comment

danyill Sep 12, 2021

Choose a reason for hiding this comment

ggrossetie commented Sep 12, 2021

danyill commented Sep 12, 2021 via email

ggrossetie commented Sep 12, 2021

joaompinto commented Sep 12, 2021

ggrossetie commented Sep 12, 2021

ggrossetie Sep 11, 2021 •

edited

Loading