Skip to content

[Maps] Use static worker with mapbox-gl#40667

Closed
thomasneirynck wants to merge 1 commit intoelastic:masterfrom
thomasneirynck:maps/load_mapboxgl_with_worker
Closed

[Maps] Use static worker with mapbox-gl#40667
thomasneirynck wants to merge 1 commit intoelastic:masterfrom
thomasneirynck:maps/load_mapboxgl_with_worker

Conversation

@thomasneirynck
Copy link
Copy Markdown
Contributor

@thomasneirynck thomasneirynck commented Jul 9, 2019

DO NOT MERGE

Closes #33651.

This enables us to remove the worker-src and child-src directives.

The existing script-src directive no longer works, since the mapbox-gl code is now loading an external worker.

Not sure what the right way forward is. See inline comment here for details https://github.com/elastic/kibana/pull/40667/files#r301719668

@thomasneirynck thomasneirynck requested a review from a team as a code owner July 9, 2019 17:58
const randomBytesAsync = promisify(randomBytes);

export const DEFAULT_CSP_RULES = Object.freeze([
`script-src 'unsafe-eval' 'nonce-{nonce}'`,
Copy link
Copy Markdown
Contributor Author

@thomasneirynck thomasneirynck Jul 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove the worker-src blob: and child-src blob: directives because of this change.

The script-src directive, as is, will no longer work as the mapbox-gl code is now trying to load an external script. We could modify it and add strict-dynamic (?)

e.g.:

export const DEFAULT_CSP_RULES = Object.freeze([
  `script-src 'unsafe-eval' 'nonce-{nonce}' 'strict-dynamic`
]);

This is the suggested work-around as discussed here for workers and nonce-based policies w3c/webappsec-csp#375.

Maybe we should load mapbox-gl in a separate script tag (and not as a vendors-bundle), and only enable strict-dynamic for mapbox-gl?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume we don't have any options for specifying the nonce attribute on the script tag which is added to the DOM?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume we don't have any options for specifying the nonce attribute on the script tag which is added to the DOM?

Correct. We don't have control over how the worker-script loaded by mapbox-gl-csp.js, which in turn loads content with importScripts.

If we want to preserve a custom script-src directive with nonce based security (do we want this in the long run (?)), maybe we could modify how we load scripts and provided nonces on all <script> tags (same as now), but load mapbox-gl-csp.js in a separate script-tag (different as now, as it is packaged with vendors.dll.js now). Only for that mapbox-gl-csp.js script, we could enable strict-dynamic, because it's I think the only script which loads child-scripts (ie. the worker).

^ just a thought. Not 100% sure about the impact of adding strict-dynamic (or the removal of script-src directive completely for that matter).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want to preserve a custom script-src directive with nonce based security (do we want this in the long run (?))

At this time, that's the intent. There's potential for us to switch to script-src 'unsafe-inline' 'self' but it's not something we have a plan to do.

Only for that mapbox-gl-csp.js script, we could enable strict-dynamic, because it's I think the only script which loads child-scripts (ie. the worker).

I believe that we have to add strict-dynamic as the CSP policy which applies to all scripts, I don't believe there's a way to scope this to only a single script tag.

@thomasneirynck thomasneirynck added chore Team:Geo Former Team Label for Geo Team. Now use Team:Presentation labels Jul 9, 2019
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/kibana-gis

@thomasneirynck thomasneirynck requested review from nreese and spalger July 9, 2019 18:22
@thomasneirynck thomasneirynck added the release_note:skip Skip the PR/issue when compiling release notes label Jul 9, 2019
@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@thomasneirynck
Copy link
Copy Markdown
Contributor Author

replacing with #48449

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore release_note:skip Skip the PR/issue when compiling release notes Team:Geo Former Team Label for Geo Team. Now use Team:Presentation v8.0.0 WIP Work in progress

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Maps] Load mapbox-gl with static worker script

4 participants