Remove HTML caching since we are using nonces. #835

Merged
merged 1 commit into from Dec 22, 2016

Projects

None yet

3 participants

@XhmikosR
Member
XhmikosR commented Dec 21, 2016 edited

@ScottHelme: I tried finding any references about this without a lot of success. So, from what I understand, the HTML shouldn't be cached when nonce's are being used. Is this correct?

Thanks!

@jmervine jmervine temporarily deployed to bootstrapcdn-dev-pr-835 Dec 21, 2016 Inactive
@ScottHelme

Hmm I'm not sure on this one, good question. I use Nginx as a caching proxy and I cache the page prior to substitution of the nonce marker so even though the page is cached I write in a random nonce value each load. I hadn't though about client side though... I will ask around :-)

@XhmikosR
Member

Thanks!

@jmervine
Member

Removing this line should fix the tests.

@XhmikosR
Member

@jmervine: I didn't notice due to CI failing for other reasons. I will take care of it if we go with this solution (I'm waiting for ScottHelme's feedback).

@jmervine jmervine temporarily deployed to bootstrapcdn-dev-pr-835 Dec 22, 2016 Inactive
@XhmikosR
Member

@jmervine: FYI, that line is for the CDN assets and not for the website's HTML files. So, there are no failures. The previous one was due to the HTML validation error.

@ScottHelme

I spoke to @mikewest and he made the great point that hashes would be a better fit here. I guess whether or not that's a viable solution will depend on how many assets you'd need to hash and include in the policy. Nonces will not play nice with client side caching.

@XhmikosR
Member

Isn't the current patch a viable solution? Granted that we don't mind not caching the HTML files.

I don't know the difference between hashes and nonce so I don't know the pros and cons. I'll need to read up on that later.

@ScottHelme

2 quite different methods, at a high level:

Hashes: insert the hash of the script/style into the CSP like so 'sha256-[hash]'. If you have 10 scripts and/or styles though, you end up with that many hashes. The browser will hash each script and style looking for a match. If it matches one, it's allowed.
https://scotthelme.co.uk/csp-cheat-sheet/#hashes

Nonces: even if you have 100 scripts and styles they can all share the same nonce, so it's less of a burden in this regard, though if an attacker can get the nonce, they can fully bypass the CSP.
https://scotthelme.co.uk/csp-cheat-sheet/#nonces

@XhmikosR
Member

Thanks for the explanation! I had already your page in my bookmarks and forgot about it :p

Now, I believe the current approach is a good compromise for our case. It's less of a hassle to set up, less headers and I don't think we care a lot about HTML caching.

@jmervine: what are your thoughts on the matter?

@jmervine
Member

Hashing seems safer overall and we don't have that many scripts included for me to worry about their being a burden.

That said, I agree that we don't care much about HTML caching, so the currently implemented solutions seems acceptable.

@XhmikosR XhmikosR Remove HTML caching since we are using nonces.
b500274
@XhmikosR XhmikosR removed the investigating label Dec 22, 2016
@XhmikosR XhmikosR merged commit f8f5435 into develop Dec 22, 2016

5 checks passed

continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
security/snyk No new vulnerabilities
Details
@XhmikosR XhmikosR deleted the xmr-rm-cache-html branch Dec 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment