Commit 3305344c authored by Hiroshige Hayashizaki's avatar Hiroshige Hayashizaki Committed by Commit Bot

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/576950Reviewed-by: default avatarKouhei Ueno <kouhei@chromium.org>
Reviewed-by: default avatarMike West <mkwst@chromium.org>
Reviewed-by: default avatarDaniel Vogelheim <vogelheim@chromium.org>
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491526}
parent 5bc690a0
CONSOLE ERROR: Failed to find a valid digest in the 'integrity' attribute for resource 'http://127.0.0.1:8000/security/subresourceIntegrity/resources/clear-integrity-attribute.js' with computed SHA-256 integrity 'yM5ZyzNsyKfaXRY78zSGapeQKtl0oGdpPpYxgwl8XW8='. The resource has been blocked. CONSOLE ERROR: Failed to find a valid digest in the 'integrity' attribute for resource 'http://127.0.0.1:8000/security/subresourceIntegrity/resources/clear-integrity-attribute.js' with computed SHA-256 integrity 'yM5ZyzNsyKfaXRY78zSGapeQKtl0oGdpPpYxgwl8XW8='. The resource has been blocked.
ALERT: FAIL ALERT: FAIL
CONSOLE ERROR: line 15: Failed to find a valid digest in the 'integrity' attribute for resource 'http://127.0.0.1:8000/security/subresourceIntegrity/resources/clear-integrity-attribute.js' with computed SHA-256 integrity 'yM5ZyzNsyKfaXRY78zSGapeQKtl0oGdpPpYxgwl8XW8='. The resource has been blocked.
This test passes if only one 'FAIL' alert appears. This test passes if only one 'FAIL' alert appears.
CONSOLE ERROR: Failed to find a valid digest in the 'integrity' attribute for resource 'http://127.0.0.1:8000/security/subresourceIntegrity/resources/shared-with-xhtml.js' with computed SHA-256 integrity 'yM5ZyzNsyKfaXRY78zSGapeQKtl0oGdpPpYxgwl8XW8='. The resource has been blocked. CONSOLE ERROR: Failed to find a valid digest in the 'integrity' attribute for resource 'http://127.0.0.1:8000/security/subresourceIntegrity/resources/shared-with-xhtml.js' with computed SHA-256 integrity 'yM5ZyzNsyKfaXRY78zSGapeQKtl0oGdpPpYxgwl8XW8='. The resource has been blocked.
CONSOLE ERROR: Failed to find a valid digest in the 'integrity' attribute for resource 'http://127.0.0.1:8000/security/subresourceIntegrity/resources/shared-with-xhtml.js' with computed SHA-256 integrity 'yM5ZyzNsyKfaXRY78zSGapeQKtl0oGdpPpYxgwl8XW8='. The resource has been blocked.
This test passes if no 'FAIL' alert appears. This test passes if no 'FAIL' alert appears.
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "core/dom/Document.h" #include "core/dom/Document.h"
#include "core/dom/TaskRunnerHelper.h" #include "core/dom/TaskRunnerHelper.h"
#include "core/frame/LocalFrame.h" #include "core/frame/LocalFrame.h"
#include "core/loader/SubresourceIntegrityHelper.h"
#include "platform/bindings/ScriptState.h" #include "platform/bindings/ScriptState.h"
#include "platform/loader/fetch/MemoryCache.h" #include "platform/loader/fetch/MemoryCache.h"
...@@ -124,7 +125,8 @@ void ClassicPendingScript::NotifyFinished(Resource* resource) { ...@@ -124,7 +125,8 @@ void ClassicPendingScript::NotifyFinished(Resource* resource) {
CheckState(); CheckState();
ScriptElementBase* element = GetElement(); ScriptElementBase* element = GetElement();
if (element) { if (element) {
GetResource()->CheckResourceIntegrity(element->GetDocument()); SubresourceIntegrityHelper::DoReport(element->GetDocument(),
GetResource()->IntegrityReportInfo());
// It is possible to get back a script resource with integrity metadata // It is possible to get back a script resource with integrity metadata
// for a request with an empty integrity attribute. In that case, the // for a request with an empty integrity attribute. In that case, the
......
...@@ -83,30 +83,8 @@ void LinkStyle::SetCSSStyleSheet( ...@@ -83,30 +83,8 @@ void LinkStyle::SetCSSStyleSheet(
ResourceIntegrityDisposition disposition = ResourceIntegrityDisposition disposition =
cached_style_sheet->IntegrityDisposition(); cached_style_sheet->IntegrityDisposition();
if (disposition == ResourceIntegrityDisposition::kNotChecked && SubresourceIntegrityHelper::DoReport(
!cached_style_sheet->LoadFailedOrCanceled()) { GetDocument(), cached_style_sheet->IntegrityReportInfo());
bool check_result;
// cachedStyleSheet->resourceBuffer() can be nullptr on load success.
// If response size == 0.
const char* data = nullptr;
size_t size = 0;
if (cached_style_sheet->ResourceBuffer()) {
data = cached_style_sheet->ResourceBuffer()->Data();
size = cached_style_sheet->ResourceBuffer()->size();
}
SubresourceIntegrity::ReportInfo report_info;
check_result = SubresourceIntegrity::CheckSubresourceIntegrity(
cached_style_sheet->IntegrityMetadata(), data, size,
cached_style_sheet->Url(), *cached_style_sheet, report_info);
SubresourceIntegrityHelper::DoReport(GetDocument(), report_info);
disposition = check_result ? ResourceIntegrityDisposition::kPassed
: ResourceIntegrityDisposition::kFailed;
// TODO(kouhei): Remove this const_cast crbug.com/653502
const_cast<CSSStyleSheetResource*>(cached_style_sheet)
->SetIntegrityDisposition(disposition);
}
if (disposition == ResourceIntegrityDisposition::kFailed) { if (disposition == ResourceIntegrityDisposition::kFailed) {
loading_ = false; loading_ = false;
......
...@@ -120,46 +120,4 @@ AccessControlStatus ScriptResource::CalculateAccessControlStatus() const { ...@@ -120,46 +120,4 @@ AccessControlStatus ScriptResource::CalculateAccessControlStatus() const {
return kNotSharableCrossOrigin; return kNotSharableCrossOrigin;
} }
void ScriptResource::CheckResourceIntegrity(Document& document) {
// Already checked? Retain existing result.
//
// TODO(vogelheim): If IntegrityDisposition() is kFailed, this should
// probably also generate a console message identical to the one produced
// by the CheckSubresourceIntegrity call below. See crbug.com/585267.
if (IntegrityDisposition() != ResourceIntegrityDisposition::kNotChecked)
return;
CHECK(source_text_.IsNull());
// Loading error occurred? Then result is uncheckable.
if (ErrorOccurred())
return;
// No integrity attributes to check? Then we're passing.
if (IntegrityMetadata().IsEmpty()) {
SetIntegrityDisposition(ResourceIntegrityDisposition::kPassed);
return;
}
const char* data = nullptr;
size_t data_length = 0;
// Edge case: If a resource actually has zero bytes then it will not
// typically have a resource buffer, but we still need to check integrity
// because people might want to assert a zero-length resource.
CHECK(EncodedSize() + DecodedSize() == 0 || ResourceBuffer());
if (ResourceBuffer()) {
data = ResourceBuffer()->Data();
data_length = ResourceBuffer()->size();
}
SubresourceIntegrity::ReportInfo report_info;
bool passed = SubresourceIntegrity::CheckSubresourceIntegrity(
IntegrityMetadata(), data, data_length, Url(), *this, report_info);
SubresourceIntegrityHelper::DoReport(document, report_info);
SetIntegrityDisposition(passed ? ResourceIntegrityDisposition::kPassed
: ResourceIntegrityDisposition::kFailed);
DCHECK_NE(IntegrityDisposition(), ResourceIntegrityDisposition::kNotChecked);
}
} // namespace blink } // namespace blink
...@@ -36,7 +36,6 @@ ...@@ -36,7 +36,6 @@
namespace blink { namespace blink {
class Document;
class FetchParameters; class FetchParameters;
class KURL; class KURL;
class ResourceFetcher; class ResourceFetcher;
...@@ -85,8 +84,6 @@ class CORE_EXPORT ScriptResource final : public TextResource { ...@@ -85,8 +84,6 @@ class CORE_EXPORT ScriptResource final : public TextResource {
AccessControlStatus CalculateAccessControlStatus() const; AccessControlStatus CalculateAccessControlStatus() const;
void CheckResourceIntegrity(Document&);
private: private:
class ScriptResourceFactory : public ResourceFactory { class ScriptResourceFactory : public ResourceFactory {
public: public:
......
...@@ -60,6 +60,11 @@ void SubresourceIntegrity::ReportInfo::AddConsoleErrorMessage( ...@@ -60,6 +60,11 @@ void SubresourceIntegrity::ReportInfo::AddConsoleErrorMessage(
console_error_messages_.push_back(message); console_error_messages_.push_back(message);
} }
void SubresourceIntegrity::ReportInfo::Clear() {
use_counts_.clear();
console_error_messages_.clear();
}
HashAlgorithm SubresourceIntegrity::GetPrioritizedHashFunction( HashAlgorithm SubresourceIntegrity::GetPrioritizedHashFunction(
HashAlgorithm algorithm1, HashAlgorithm algorithm1,
HashAlgorithm algorithm2) { HashAlgorithm algorithm2) {
......
...@@ -33,6 +33,7 @@ class PLATFORM_EXPORT SubresourceIntegrity final { ...@@ -33,6 +33,7 @@ class PLATFORM_EXPORT SubresourceIntegrity final {
void AddUseCount(UseCounterFeature); void AddUseCount(UseCounterFeature);
void AddConsoleErrorMessage(const String&); void AddConsoleErrorMessage(const String&);
void Clear();
const Vector<UseCounterFeature>& UseCounts() const { return use_counts_; } const Vector<UseCounterFeature>& UseCounts() const { return use_counts_; }
const Vector<String>& ConsoleErrorMessages() const { const Vector<String>& ConsoleErrorMessages() const {
......
...@@ -324,6 +324,42 @@ void Resource::SetLoader(ResourceLoader* loader) { ...@@ -324,6 +324,42 @@ void Resource::SetLoader(ResourceLoader* loader) {
status_ = ResourceStatus::kPending; status_ = ResourceStatus::kPending;
} }
void Resource::CheckResourceIntegrity() {
// Loading error occurred? Then result is uncheckable.
integrity_report_info_.Clear();
if (ErrorOccurred()) {
CHECK(!Data());
integrity_disposition_ = ResourceIntegrityDisposition::kFailed;
return;
}
// No integrity attributes to check? Then we're passing.
if (IntegrityMetadata().IsEmpty()) {
integrity_disposition_ = ResourceIntegrityDisposition::kPassed;
return;
}
const char* data = nullptr;
size_t data_length = 0;
// Edge case: If a resource actually has zero bytes then it will not
// typically have a resource buffer, but we still need to check integrity
// because people might want to assert a zero-length resource.
CHECK(EncodedSize() + DecodedSize() == 0 || Data());
if (Data()) {
data = Data()->Data();
data_length = Data()->size();
}
if (SubresourceIntegrity::CheckSubresourceIntegrity(IntegrityMetadata(), data,
data_length, Url(), *this,
integrity_report_info_))
integrity_disposition_ = ResourceIntegrityDisposition::kPassed;
else
integrity_disposition_ = ResourceIntegrityDisposition::kFailed;
DCHECK_NE(IntegrityDisposition(), ResourceIntegrityDisposition::kNotChecked);
}
void Resource::CheckNotify() { void Resource::CheckNotify() {
if (IsLoading()) if (IsLoading())
return; return;
...@@ -408,6 +444,7 @@ void Resource::FinishAsError(const ResourceError& error) { ...@@ -408,6 +444,7 @@ void Resource::FinishAsError(const ResourceError& error) {
DCHECK(ErrorOccurred()); DCHECK(ErrorOccurred());
ClearData(); ClearData();
loader_ = nullptr; loader_ = nullptr;
CheckResourceIntegrity();
CheckNotify(); CheckNotify();
} }
...@@ -417,6 +454,7 @@ void Resource::Finish(double load_finish_time) { ...@@ -417,6 +454,7 @@ void Resource::Finish(double load_finish_time) {
if (!ErrorOccurred()) if (!ErrorOccurred())
status_ = ResourceStatus::kCached; status_ = ResourceStatus::kCached;
loader_ = nullptr; loader_ = nullptr;
CheckResourceIntegrity();
CheckNotify(); CheckNotify();
} }
...@@ -424,13 +462,6 @@ AtomicString Resource::HttpContentType() const { ...@@ -424,13 +462,6 @@ AtomicString Resource::HttpContentType() const {
return GetResponse().HttpContentType(); return GetResponse().HttpContentType();
} }
void Resource::SetIntegrityDisposition(
ResourceIntegrityDisposition disposition) {
DCHECK_NE(disposition, ResourceIntegrityDisposition::kNotChecked);
DCHECK(type_ == Resource::kScript || type_ == Resource::kCSSStyleSheet);
integrity_disposition_ = disposition;
}
bool Resource::MustRefetchDueToIntegrityMetadata( bool Resource::MustRefetchDueToIntegrityMetadata(
const FetchParameters& params) const { const FetchParameters& params) const {
if (params.IntegrityMetadata().IsEmpty()) if (params.IntegrityMetadata().IsEmpty())
......
...@@ -31,6 +31,7 @@ ...@@ -31,6 +31,7 @@
#include "platform/Timer.h" #include "platform/Timer.h"
#include "platform/WebTaskRunner.h" #include "platform/WebTaskRunner.h"
#include "platform/instrumentation/tracing/web_process_memory_dump.h" #include "platform/instrumentation/tracing/web_process_memory_dump.h"
#include "platform/loader/SubresourceIntegrity.h"
#include "platform/loader/fetch/CachedMetadataHandler.h" #include "platform/loader/fetch/CachedMetadataHandler.h"
#include "platform/loader/fetch/IntegrityMetadata.h" #include "platform/loader/fetch/IntegrityMetadata.h"
#include "platform/loader/fetch/ResourceError.h" #include "platform/loader/fetch/ResourceError.h"
...@@ -248,11 +249,12 @@ class PLATFORM_EXPORT Resource : public GarbageCollectedFinalized<Resource>, ...@@ -248,11 +249,12 @@ class PLATFORM_EXPORT Resource : public GarbageCollectedFinalized<Resource>,
const IntegrityMetadataSet& IntegrityMetadata() const { const IntegrityMetadataSet& IntegrityMetadata() const {
return integrity_metadata_; return integrity_metadata_;
} }
// The argument must never be |NotChecked|.
void SetIntegrityDisposition(ResourceIntegrityDisposition);
ResourceIntegrityDisposition IntegrityDisposition() const { ResourceIntegrityDisposition IntegrityDisposition() const {
return integrity_disposition_; return integrity_disposition_;
} }
const SubresourceIntegrity::ReportInfo& IntegrityReportInfo() const {
return integrity_report_info_;
}
bool MustRefetchDueToIntegrityMetadata(const FetchParameters&) const; bool MustRefetchDueToIntegrityMetadata(const FetchParameters&) const;
bool IsAlive() const { return is_alive_; } bool IsAlive() const { return is_alive_; }
...@@ -421,6 +423,8 @@ class PLATFORM_EXPORT Resource : public GarbageCollectedFinalized<Resource>, ...@@ -421,6 +423,8 @@ class PLATFORM_EXPORT Resource : public GarbageCollectedFinalized<Resource>,
// MemoryCoordinatorClient overrides: // MemoryCoordinatorClient overrides:
void OnPurgeMemory() override; void OnPurgeMemory() override;
void CheckResourceIntegrity();
Type type_; Type type_;
ResourceStatus status_; ResourceStatus status_;
CORSStatus cors_status_; CORSStatus cors_status_;
...@@ -458,6 +462,7 @@ class PLATFORM_EXPORT Resource : public GarbageCollectedFinalized<Resource>, ...@@ -458,6 +462,7 @@ class PLATFORM_EXPORT Resource : public GarbageCollectedFinalized<Resource>,
ResourceIntegrityDisposition integrity_disposition_; ResourceIntegrityDisposition integrity_disposition_;
IntegrityMetadataSet integrity_metadata_; IntegrityMetadataSet integrity_metadata_;
SubresourceIntegrity::ReportInfo integrity_report_info_;
// Ordered list of all redirects followed while fetching this resource. // Ordered list of all redirects followed while fetching this resource.
Vector<RedirectPair> redirect_chain_; Vector<RedirectPair> redirect_chain_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment