Move SubresourceIntegrity calls to Resource, SubresourceIntegrity to platform |
||||
Issue descriptionCurrently, ScriptLoader and LinkStyle calls SubresourceIntegrity functions, because it requires Document. However, - Resource::data_ is required to be available at the time of integrity check, and this causes race conditions between the integrity check and ClearData() in ScriptResource::SourceText() (Issue 735719 etc.) and CSSStyleSheetResource::CheckNotify() (blocking https://chromium-review.googlesource.com/c/562859/). - Correctness of the integrity check depends on the ResourceClient layer. (e.g. SRI can be broken when a new ScriptResourceClient class is added) - This is one of blockers that prevent data_ from being cleared when loading is finished. This issue aims to move SubresourceIntegrity calls to Resource (around CheckNotify()). In order to this, SubresourceIntegrity is moved to platform, by removing Document/ExecutionContext dependencies.
,
Jul 27 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d001e0d5d89d2ecdee30c3fe951f5088fc4f3262 commit d001e0d5d89d2ecdee30c3fe951f5088fc4f3262 Author: Hiroshige Hayashizaki <hiroshige@chromium.org> Date: Thu Jul 27 03:48:06 2017 Move SubresourceIntegrity to platform/loader This CL - Introduces SubresourceIntegrity::ReportInfo that represents the things that don't affect the check results themselves but require ExecutionContext, i.e. console error messages and UseCounter counts, - Factors out dependencies to Document/ExecutionContext from SubresourceIntegrity.h/cpp by making ReportInfo filled in SubresourceIntegrity and processed by the callers of SubresourceIntegrity that have ExecutionContext, and thereby - Moves SubresourceIntegrity to platform/loader. This is preparation to move SubresourceIntegrity calls to Resource in https://chromium-review.googlesource.com/c/576950/. Bug: 746115 , 701943 Change-Id: I3c1a4650291d01ab7162c72c6e682c31de45cbb6 Reviewed-on: https://chromium-review.googlesource.com/576968 Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org> Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Reviewed-by: Mike West <mkwst@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Reviewed-by: Daniel Vogelheim <vogelheim@chromium.org> Cr-Commit-Position: refs/heads/master@{#489851} [modify] https://crrev.com/d001e0d5d89d2ecdee30c3fe951f5088fc4f3262/third_party/WebKit/Source/core/BUILD.gn [modify] https://crrev.com/d001e0d5d89d2ecdee30c3fe951f5088fc4f3262/third_party/WebKit/Source/core/dom/ClassicPendingScript.cpp [modify] https://crrev.com/d001e0d5d89d2ecdee30c3fe951f5088fc4f3262/third_party/WebKit/Source/core/dom/ScriptLoader.cpp [modify] https://crrev.com/d001e0d5d89d2ecdee30c3fe951f5088fc4f3262/third_party/WebKit/Source/core/frame/BUILD.gn [modify] https://crrev.com/d001e0d5d89d2ecdee30c3fe951f5088fc4f3262/third_party/WebKit/Source/core/frame/csp/CSPDirectiveListTest.cpp [modify] https://crrev.com/d001e0d5d89d2ecdee30c3fe951f5088fc4f3262/third_party/WebKit/Source/core/html/LinkStyle.cpp [modify] https://crrev.com/d001e0d5d89d2ecdee30c3fe951f5088fc4f3262/third_party/WebKit/Source/core/html/parser/HTMLPreloadScanner.cpp [modify] https://crrev.com/d001e0d5d89d2ecdee30c3fe951f5088fc4f3262/third_party/WebKit/Source/core/loader/BUILD.gn [add] https://crrev.com/d001e0d5d89d2ecdee30c3fe951f5088fc4f3262/third_party/WebKit/Source/core/loader/SubresourceIntegrityHelper.cpp [add] https://crrev.com/d001e0d5d89d2ecdee30c3fe951f5088fc4f3262/third_party/WebKit/Source/core/loader/SubresourceIntegrityHelper.h [modify] https://crrev.com/d001e0d5d89d2ecdee30c3fe951f5088fc4f3262/third_party/WebKit/Source/core/loader/resource/ScriptResource.cpp [modify] https://crrev.com/d001e0d5d89d2ecdee30c3fe951f5088fc4f3262/third_party/WebKit/Source/modules/fetch/FetchManager.cpp [modify] https://crrev.com/d001e0d5d89d2ecdee30c3fe951f5088fc4f3262/third_party/WebKit/Source/platform/loader/BUILD.gn [rename] https://crrev.com/d001e0d5d89d2ecdee30c3fe951f5088fc4f3262/third_party/WebKit/Source/platform/loader/SubresourceIntegrity.cpp [rename] https://crrev.com/d001e0d5d89d2ecdee30c3fe951f5088fc4f3262/third_party/WebKit/Source/platform/loader/SubresourceIntegrity.h [rename] https://crrev.com/d001e0d5d89d2ecdee30c3fe951f5088fc4f3262/third_party/WebKit/Source/platform/loader/SubresourceIntegrityTest.cpp [modify] https://crrev.com/d001e0d5d89d2ecdee30c3fe951f5088fc4f3262/third_party/WebKit/Source/platform/testing/TestingPlatformSupport.h
,
Jul 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/55ebf5a43475f5addddf3fb9cd909b6ea70c9e5c commit 55ebf5a43475f5addddf3fb9cd909b6ea70c9e5c Author: Hiroshige Hayashizaki <hiroshige@chromium.org> Date: Fri Jul 28 05:06:50 2017 Use Resource's IntegrityMetadata() for LinkStyle SRI check SubresourceIntegrity check result is cached in Resource::IntegrityDisposition(), and thus reused for requests with the same integrity metadata at the time of requests. However, SubresourceIntegrity check is currently done against the integrity attribute value at the time of stylesheet load completion, which can be different from Resource::IntegrityDisposition(), causing inconsistent check results to be cached and SRI checks to be bypassed. This CL fixes this by checking SubresourceIntegrity against Resource::IntegrityDisposition(). Behavior change: SRI for <link rel=stylesheet> is checked against the integrity attribute at the time of load start, not at the time of load completion (subresource-integrity-style-attribute-changed.html). This is consistent with <script>'s SRI checks, and this CL prepares for merging <script> and <link rel=stylesheet> SRI checks into a single implementation in Resource in https://chromium-review.googlesource.com/c/576950/. Bug: 749281, 746115 , 701943 Change-Id: I521e49966ee8881ca7915da997c3b89238e5b700 Reviewed-on: https://chromium-review.googlesource.com/587227 Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org> Reviewed-by: Mike West <mkwst@chromium.org> Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Cr-Commit-Position: refs/heads/master@{#490274} [add] https://crrev.com/55ebf5a43475f5addddf3fb9cd909b6ea70c9e5c/third_party/WebKit/LayoutTests/http/tests/security/subresourceIntegrity/subresource-integrity-style-attribute-changed.html [add] https://crrev.com/55ebf5a43475f5addddf3fb9cd909b6ea70c9e5c/third_party/WebKit/LayoutTests/http/tests/security/subresourceIntegrity/subresource-integrity-style-bypass-by-attribute-change.html [modify] https://crrev.com/55ebf5a43475f5addddf3fb9cd909b6ea70c9e5c/third_party/WebKit/Source/core/html/LinkStyle.cpp
,
Jul 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/84515b96a245a237de89424989b385ec6a72d7f7 commit 84515b96a245a237de89424989b385ec6a72d7f7 Author: Hiroshige Hayashizaki <hiroshige@chromium.org> Date: Fri Jul 28 18:36:38 2017 Use Resource::Url() in LinkStyle SRI To prepare for merging SRI checks in LinkStyle and ScriptResource into a single implementation in Resource [1], this CL makes LinkStyle's SRI check to use Resource::Url(), which is consistent with ScriptResource::CheckResourceIntegrity(). This CL doesn't change the behavior, because SetCSSStyleSheet()'s |href| argument is String-converted Resource::Url(). [1] https://chromium-review.googlesource.com/c/576950/ Bug: 746115 , 701943 Change-Id: I55f0d36ef5ba791b7252b7f8a2460636bdfe680b Reviewed-on: https://chromium-review.googlesource.com/587247 Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org> Reviewed-by: Mike West <mkwst@chromium.org> Cr-Commit-Position: refs/heads/master@{#490474} [modify] https://crrev.com/84515b96a245a237de89424989b385ec6a72d7f7/third_party/WebKit/Source/core/html/LinkStyle.cpp
,
Aug 2 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3305344cb14705b1c8cb802cfd65e49d375aff85 commit 3305344cb14705b1c8cb802cfd65e49d375aff85 Author: Hiroshige Hayashizaki <hiroshige@chromium.org> Date: Wed Aug 02 22:14:28 2017 Move SubresourceIntegrity calls to Resource This CL moves SubresourceIntegrity calls to Resource, just before CheckNotify(), where Resource::Data() is always available. This makes - SubresourceIntegrity always checked if integrity metadata is set, and - SubresourceIntegrity and ResourceIntegrityDisposition control not depend on ResourceClients (ClassicPendingScript and LinkStyle). This removes Resource::SetIntegrityDisposition(), and and unblocks https://chromium-review.googlesource.com/c/562859/. This CL also makes Resource cache SubresourceIntegrity::ReportInfo in Resource, and thus outputs SRI console error messages and update UseCounters for every classic script and stylesheet that shares a Resource, not only for the first use ( Issue 585267 ). Bug: 746115 , 701943 , 585267 , 653502 Change-Id: I03e7c22319980bd297cb5d9fb58589966a0b2f71 Reviewed-on: https://chromium-review.googlesource.com/576950 Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Reviewed-by: Mike West <mkwst@chromium.org> Reviewed-by: Daniel Vogelheim <vogelheim@chromium.org> Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org> Cr-Commit-Position: refs/heads/master@{#491526} [modify] https://crrev.com/3305344cb14705b1c8cb802cfd65e49d375aff85/third_party/WebKit/LayoutTests/http/tests/security/subresourceIntegrity/clear-integrity-attribute-expected.txt [modify] https://crrev.com/3305344cb14705b1c8cb802cfd65e49d375aff85/third_party/WebKit/LayoutTests/http/tests/security/subresourceIntegrity/shared-with-xhtml-expected.txt [modify] https://crrev.com/3305344cb14705b1c8cb802cfd65e49d375aff85/third_party/WebKit/Source/core/dom/ClassicPendingScript.cpp [modify] https://crrev.com/3305344cb14705b1c8cb802cfd65e49d375aff85/third_party/WebKit/Source/core/html/LinkStyle.cpp [modify] https://crrev.com/3305344cb14705b1c8cb802cfd65e49d375aff85/third_party/WebKit/Source/core/loader/resource/ScriptResource.cpp [modify] https://crrev.com/3305344cb14705b1c8cb802cfd65e49d375aff85/third_party/WebKit/Source/core/loader/resource/ScriptResource.h [modify] https://crrev.com/3305344cb14705b1c8cb802cfd65e49d375aff85/third_party/WebKit/Source/platform/loader/SubresourceIntegrity.cpp [modify] https://crrev.com/3305344cb14705b1c8cb802cfd65e49d375aff85/third_party/WebKit/Source/platform/loader/SubresourceIntegrity.h [modify] https://crrev.com/3305344cb14705b1c8cb802cfd65e49d375aff85/third_party/WebKit/Source/platform/loader/fetch/Resource.cpp [modify] https://crrev.com/3305344cb14705b1c8cb802cfd65e49d375aff85/third_party/WebKit/Source/platform/loader/fetch/Resource.h
,
Aug 21 2017
I did sufficient part of this issue before M-62. Still some cleanups are needed, but not urgent. Making this Pri-2.
,
Nov 10 2017
,
Feb 13 2018
|
||||
►
Sign in to add a comment |
||||
Comment 1 by bugdroid1@chromium.org
, Jul 26 2017