Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(390)

Issue 1360693002: Apply CSP default-src hash values to script-src and style src. (Closed)

Created:
3 years, 11 months ago by jww
Modified:
3 years, 11 months ago
Reviewers:
Mike West
CC:
blink-reviews, mkwst+watchlist-csp_chromium.org
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Apply CSP default-src hash values to script-src and style-src. This fixes a minor bug where we forgot to add hash values in the default-src CSP directive to the list of hash algorithms seen. Thus, when the hash whitelist was checked for inline styles and scripts, the CSP potentially might believe that no algorithms have been seen, so the whitelist check would skip all of the stored hash values. This fixes the bug by adding the algorithms to the list of algorithms seen when a default-src directive is reached. BUG=534568 R=mkwst@chromium.org Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=202656

Patch Set 1 #

Total comments: 6

Patch Set 2 : Nits from mkwst #

Patch Set 3 : Test fix #

Unified diffs Side-by-side diffs Delta from patch set Stats (+37 lines, -0 lines) Patch
A LayoutTests/http/tests/security/contentSecurityPolicy/1.1/scripthash-default-src.html View 1 1 chunk +13 lines, -0 lines 0 comments Download
A LayoutTests/http/tests/security/contentSecurityPolicy/1.1/stylehash-default-src.html View 1 2 1 chunk +19 lines, -0 lines 0 comments Download
M Source/core/frame/csp/CSPDirectiveList.cpp View 1 1 chunk +5 lines, -0 lines 0 comments Download

Messages

Total messages: 19 (9 generated)
jww
Mike, can you take a look? Thanks!
3 years, 11 months ago (2015-09-21 23:53:17 UTC) #1
Mike West
LGTM % nits. https://codereview.chromium.org/1360693002/diff/1/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/scripthash-default-src.html File LayoutTests/http/tests/security/contentSecurityPolicy/1.1/scripthash-default-src.html (right): https://codereview.chromium.org/1360693002/diff/1/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/scripthash-default-src.html#newcode8 LayoutTests/http/tests/security/contentSecurityPolicy/1.1/scripthash-default-src.html:8: alert('PASS'); Nit: See below. https://codereview.chromium.org/1360693002/diff/1/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/stylehash-default-src.html File ...
3 years, 11 months ago (2015-09-22 04:37:38 UTC) #2
jww
https://codereview.chromium.org/1360693002/diff/1/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/scripthash-default-src.html File LayoutTests/http/tests/security/contentSecurityPolicy/1.1/scripthash-default-src.html (right): https://codereview.chromium.org/1360693002/diff/1/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/scripthash-default-src.html#newcode8 LayoutTests/http/tests/security/contentSecurityPolicy/1.1/scripthash-default-src.html:8: alert('PASS'); On 2015/09/22 04:37:38, Mike West (slow) wrote: > ...
3 years, 11 months ago (2015-09-22 16:55:38 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1360693002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1360693002/20001
3 years, 11 months ago (2015-09-22 16:55:58 UTC) #6
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/116758)
3 years, 11 months ago (2015-09-22 17:43:45 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1360693002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1360693002/20001
3 years, 11 months ago (2015-09-22 17:45:34 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1360693002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1360693002/20001
3 years, 11 months ago (2015-09-22 18:36:48 UTC) #13
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/116826)
3 years, 11 months ago (2015-09-22 19:52:46 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1360693002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1360693002/40001
3 years, 11 months ago (2015-09-22 21:34:19 UTC) #18
commit-bot: I haz the power
3 years, 11 months ago (2015-09-23 01:36:40 UTC) #19
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=202656

Powered by Google App Engine
This is Rietveld 408576698