dfreeman / ember-ace

An Ember component for the Ace code editor
MIT License
29 stars 19 forks source link

Add Content Security Policy support #3

Open gschorkopf opened 7 years ago

gschorkopf commented 7 years ago

I've really enjoyed using your add-on so far!

One thing I've noticed is I get numerous report-only warnings, along the lines of this:

screen shot 2017-04-05 at 12 00 54 pm

Could you add a hash/nonce to avoid these errors?

jamesarosen commented 7 years ago

rwjblue/ember-cli-content-security-policy#67 and ember-cli/ember-cli#3350 suggest that the community has not really figured out how to address this problem. The ember-cli team's answer so far is "turn CSP off."

jamesarosen commented 7 years ago

It might be challenging for ember-ace to do this because every time the core ACE library changes any of its CSS, this library will have to change the hashes. I've opened https://github.com/ajaxorg/ace/issues/3260 to ask the core ACE team to help us out.

(ACE has the CSS as individual files in the project, but they're not included in the ace-builds node module. By that point, they've been turned into JavaScript strings. For example, editorCss in node_modules/ace-builds/src/ace.js. We could get at the source CSS if we depended on ace rather than ace-builds here.)

jamesarosen commented 7 years ago

ACE also calculates lots of styles on the fly and then modifies the DOM with .innerHTML = '...'. For example, <div class='ace_selection ace_br12' style='height:16px;width:208.83625px;top:64px;left:4px;'>. There's no way for this library to calculate all the possible hashes and add them to the CSP.

At least this part has to be solved by ACE core using HTMLElement manipulation instead of innerHTML.

dfreeman commented 7 years ago

@jamesarosen Really appreciate all the thought and effort you've put into this so far! I'll keep a close eye on the Ace issue you linked.

The inline style attributes look like they're going to be problematic — at a glance, it seems like it could be a fair amount of work to refactor the innerHTML usage into working with actual DOM objects, and activity in the repo seems to have been fairly low in recent months.

dfreeman commented 7 years ago

Chrome, as of version 46, no longer supports the unsafe-inline option because it's too broad an attack vector, so that isn't an option.

It actually looks like Chrome only ignores unsafe-inline if at least one hash or nonce is supplied, so that's not completely off the table.

Opening that attack vector obviously isn't ideal, but it's less extreme than disabling CSP completely, and using it for style-src eliminates most of the violations.

jamesarosen commented 7 years ago

It actually looks like Chrome only ignores unsafe-inline if at least one hash or nonce is supplied, so that's not completely off the table. Opening that attack vector obviously isn't ideal, but it's less extreme than disabling CSP completely, and using it for style-src eliminates most of the violations.

Excellent find! It's a compromise, but it'll do for now.

gschorkopf commented 7 years ago

🌈

dfreeman commented 7 years ago

One interesting thing I came across when experimenting with this — it looks like ember-cli-content-security-policy doesn't believe in worker-src, so you have to set a default-src to get the language workers to load.

I'm guessing that would be a trivial fix on that end, but I haven't actually checked to be sure.