search

sstephenson / sprockets

Rack-based asset packaging system
2.55k stars 24 forks source link

Yank integrity support #686

Closed josh closed 9 years ago

josh commented 9 years ago

Some nice Subresource Integrity support went into the 3.x beta. However, the URI ni:/// format itself still seems undecided. Its unclear if the spec is going to change or Chrome's current implementation.

I think I'm going to disable the feature until its sorted out.

@mastahyeti sound okay?

btoews commented 9 years ago

@freddyb Do you think we're safe to stick with the ni:///sha-256;bhHHL3z2vDgxUt0W3dWQOrprscmda2Y5pLsLg4GF-pI formatting, or is that likely to change in the spec?

mozfreddyb commented 9 years ago

The only thing we plan to change is that we want to add some sort of mandatory normalization so that content authors don't have to worry about algorithm names (sha256 vs. sha-256) and trailing equal signs in the digest.

I'm not sure what @josh thought might be undecided in the spec. I aim to bring SRI to Last Call in early 2015, see milestones.

josh commented 9 years ago

@mozfreddyb the algorithm name and url safe stuff was the 2 things I wasn't sure about.

The current implementation produces ni:///sha-256;bhHHL3z2vDgxUt0W3dWQOrprscmda2Y5pLsLg4GF-pI?ct=application/javascript. With the - in the name and using url safe base64.

I would like implement the preferred approach even if both are accepted. I'm just not sure what has been decided given implementation differences in Chrome and the spec.

https://github.com/sstephenson/sprockets/blob/46717091455ed026b0552a87a10cf961e39b75ed/test/test_digest_utils.rb#L79 https://github.com/sstephenson/sprockets/blob/9c01b7e6747904cbac9dc6a0e32b912dcf8f15c6/lib/sprockets/digest_utils.rb#L140-L144

mozfreddyb commented 9 years ago

There will be a mandatory normalization step to make sure that all implementations are alike. Actually, this is meant to avoid confusion for content authors.

On 12.12.2014 21:09, Joshua Peek wrote:

@mozfreddyb https://github.com/mozfreddyb the algorithm name and url safe stuff was the 2 things I wasn't sure about.

The current implementation produces |ni:///sha-256;bhHHL3z2vDgxUt0W3dWQOrprscmda2Y5pLsLg4GF-pI?ct=application/javascript|. With the |-| in the name and using url safe base64.

I would like implement the /preferred/ approach even if both are accepted. I'm just not sure what has been decided given implementation differences in Chrome and the spec.

https://github.com/sstephenson/sprockets/blob/46717091455ed026b0552a87a10cf961e39b75ed/test/test_digest_utils.rb#L79 https://github.com/sstephenson/sprockets/blob/9c01b7e6747904cbac9dc6a0e32b912dcf8f15c6/lib/sprockets/digest_utils.rb#L140-L144

— Reply to this email directly or view it on GitHub https://github.com/sstephenson/sprockets/issues/686#issuecomment-66827729.

josh commented 9 years ago

Taking the middle ground here 2b0bdf985a65086e74d8e0d8c1e93a956cb058fe.

Manifest writes are disabled since those are impossible to fix after people start using them. Asset#integrity will still work. I'd just need to bust some caches if that algorithm changed.

mozfreddyb commented 9 years ago

I was a little unavailable when this came up. Here's a link to the discussion for further reference: https://github.com/w3c/webappsec/pull/125

The spec is not going to change with regard to this. We intend to move it into Last Call this month and I'm looking forward to any kind of feedback about the spec and hash function naming in ni URIs in particular.

josh commented 9 years ago

Thanks @mozfreddyb.

I forgot to mention I reverted that commit, full support is coming back now that things are resolved in Chrome Canary. 777f1ebfcbb397479d28509bce2ac67d12d40320

We're going to ship a Subresource Integrity beta on github.com pretty soon!

mozfreddyb commented 9 years ago

A discussion just came up whether we want to change the syntax after all. Sorry about the inconvenience :/

Please read the whole thread. The pull request about this is at https://github.com/w3c/webappsec/pull/156