lasso-js / lasso

Advanced JavaScript module bundler, asset pipeline and optimizer
581 stars 75 forks source link

Update lasso to add support for CSP nonce #93

Closed ppattanayak closed 8 years ago

ppattanayak commented 8 years ago

Need to update lasso to support the nonce feature of Content Security Policy 2, for the script tags in the HTML.

Example:

<script nonce="a613a14b8875acfa0fa40d2229d8786c9a5a2d260"> 
//JS Code 
</script>
patrick-steele-idem commented 8 years ago

Hey @ppattanayak, thanks for opening an issue. This will require some changes to Lasso.js. At a high-level:

NOTE: Nonce can be retrieved from out using following code:

var res = out.stream;
var req = res.req;
var csp = req.csp;
var nonce = csp.nonce;

As soon as I get a chance, I'll try to point you in the right direction so that you can make the appropriate changes to support these new hooks. In the meantime, please try to familiarize yourself with some of the Lasso.js code.

Thanks again, for opening the issue!

patrick-steele-idem commented 8 years ago

@ppattanayak can you please confirm if the nonce is needed on all of the following?:

<script type="text/javascript" src="/foo.js" nonce="abc123"></script>
<script type="text/javascript" nonce="abc123">console.log('foo')</script>
<link rel="stylesheet" type="text/css" href="foo.css" nonce="abc123">
<style type="text/css" nonce="abc123">.foo { }</style>

Or, is it only needed for inline scripts?

Thanks.

ppattanayak commented 8 years ago

Its needed in second and fourth. Not in first and third.

patrick-steele-idem commented 8 years ago

Great. Thank you for confirming. So only for inline scripts and inline styles.

ppattanayak commented 8 years ago

Yes, thats correct.

patrick-steele-idem commented 8 years ago

Hey @ppattanayak and @tropperstyle, please review the following Pull Request to provide CSP nonce support: https://github.com/lasso-js/lasso/pull/94

tropperstyle commented 8 years ago

Getting the following error after running npm install lasso-js/lasso#issue-93

ReferenceError: rmod is not defined
    at Object.render [as _] (/app/node_modules/lasso/body-116-0-12-31904-1qpgh44.marko.js:14:17)
    at Object.Template.renderSync (/app/node_modules/marko/runtime/marko-runtime.js:114:14)
    at Object.LassoPageResult.getHtmlForSlot (/app/node_modules/lasso/lib/LassoPageResult.js:101:25)
    at Object.LassoPageResult.getSlotHtml (/app/node_modules/lasso/lib/LassoPageResult.js:140:21)
    at renderSlot (/app/node_modules/lasso/taglib/slot-tag.js:21:36)
    at Object.<anonymous> (/app/node_modules/lasso/taglib/slot-tag.js:56:13)
    at notifyCallbacks (/app/node_modules/lasso/node_modules/raptor-async/AsyncValue.js:76:35)
    at Object.AsyncValue.resolve (/app/node_modules/lasso/node_modules/raptor-async/AsyncValue.js:262:9)
    at done (/app/node_modules/lasso/taglib/page-tag.js:67:39)
    at done (/app/node_modules/lasso/lib/Lasso.js:664:17)
    at CacheEntry.proto.readValue (/app/node_modules/lasso/node_modules/raptor-cache/lib/CacheEntry.js:54:16)
    at /app/node_modules/lasso/node_modules/raptor-cache/lib/Cache.js:251:28
    at /app/node_modules/lasso/node_modules/raptor-cache/lib/Cache.js:84:20
    at Object.MemoryStore.get (/app/node_modules/lasso/node_modules/raptor-cache/lib/MemoryStore.js:14:16)
    at getCacheEntry (/app/node_modules/lasso/node_modules/raptor-cache/lib/Cache.js:62:22)
    at /app/node_modules/lasso/node_modules/raptor-cache/lib/Cache.js:126:21
    at /app/node_modules/lasso/lib/Lasso.js:287:13
    at /app/node_modules/lasso/node_modules/raptor-async/series.js:30:33
    at /app/node_modules/lasso/node_modules/raptor-async/series.js:30:33
    at done (/app/node_modules/lasso/lib/writers/Writer.js:96:20)
    at /app/node_modules/lasso/node_modules/raptor-async/series.js:30:33
    at /app/node_modules/lasso/lib/writers/Writer.js:172:32
    at /app/node_modules/lasso/lib/writers/file-writer.js:453:17
    at handleSuccess (/app/node_modules/lasso/lib/writers/file-writer.js:129:13)
    at /app/node_modules/lasso/lib/writers/file-writer.js:167:37
    at FSReqWrap.oncomplete (fs.js:82:15)
patrick-steele-idem commented 8 years ago

Hey @tropperstyle, can you try the following?:

rm -rf node_modules/
npm install
npm install lasso-js/lasso#issue-93
rm -rf .cache/ static/
MARKO_CLEAN=true node .

That should wipe out all caches which can cause problems when switching to a new version of a modules. Please let me konw if that solves your problem.

patrick-steele-idem commented 8 years ago

@tropperstyle Actually, I am able to reproduce. Let me figure out what went wrong!

tropperstyle commented 8 years ago

I tried using rm -rf .cache and find . -name *.marko.js -exec rm {} \; - Would that suffice to wipe out the cache?

patrick-steele-idem commented 8 years ago

It's not a caching issue. We converted the HTML for the head and body slots to templates and the Marko compiler is not liking the $ that appears in an inline script. I am working on a fix.

patrick-steele-idem commented 8 years ago

Hey @tropperstyle, should be fixed now. Please try again:

npm install lasso-js/lasso#issue-93
rm -rf .cache/ static/
MARKO_CLEAN=true node .

Thanks for your help!

tropperstyle commented 8 years ago

It doesn't seem to be picking up the lasso-nonce attribute for templates in node_modules/*/marko-taglib.json

patrick-steele-idem commented 8 years ago

@tropperstyle can you give me a more specific path to try?

patrick-steele-idem commented 8 years ago

@tropperstyle I tried to reproduce your problem by doing the following:

Created a new template:

_nodemodules/foo/foo-template.marko

<script lasso-nonce>
    Hello foo!
</script>

Compiled it using markoc:

markoc node_modules/foo/foo-template.marko

Compiled output:

function create(__helpers) {
  var str = __helpers.s,
      empty = __helpers.e,
      notEmpty = __helpers.ne,
      __getNonce = require("lasso/taglib/helper-getNonce"),
      attr = __helpers.a;

  return function render(data, out) {
    out.w('<script' +
      attr("nonce", __getNonce(out)) +
      '>\n    Hello foo!\n</script>');
  };
}
(module.exports = require("marko").c(__filename)).c(create);

You can see from the following line that everything worked as expected:

attr("nonce", __getNonce(out))

Please let me know if there is another way I should try to reproduce. Thanks.

tropperstyle commented 8 years ago

Doh it was on my end. I was editing a file in node_modules/foo/template.marko but the actual file being loaded was nested under node_modules/another-page/node_modules/foo/template.marko - Which now has me paying closer attention to my dependency graph. Sorry about that. Looks like everything is working great now. Thanks!

tropperstyle commented 8 years ago

Looks like this output will also require the nonce attribute: <span id="markoWidgets" data-ids="w0" style="display:none;"></span>

patrick-steele-idem commented 8 years ago

Why do you say that @tropperstyle? There is no script on that line.

However, we will need to update Marko Widgets to not use eval() in CSP mode (see marko-widgets/lib/init-widgets-browser.js)

We use eval() for performance reasons, but there is no reason we can't use JSON.parse() (with a few minor changes).

tropperstyle commented 8 years ago

Inline styles are banned the same way as inline scripts and inline event handlers. Safari is throwing a CSP report on this line unless I add a nonce attribute or add unsafe-inline to style-src

patrick-steele-idem commented 8 years ago

What world are we living in where inline styles are considered dangerous? :)

Ok, I propose we switch to the following:

<noscript id="markoWidgets" data-ids="w0"></noscript>

The <noscript> has no impacted on the view and is supported by all browsers so I think that will be a good change. Thoughts or concerns @tropperstyle?

patrick-steele-idem commented 8 years ago

@tropperstyle @ppattanayak I opened a related issue on the Marko Widgets project to support CSP: https://github.com/marko-js/marko-widgets/issues/115

patrick-steele-idem commented 8 years ago

New version published: lasso@1.15.0