Commit bcd2e485 authored by jww@chromium.org's avatar jww@chromium.org

Initial Fetch integration for Subresource Integrity

This adds support to the fetch() API for the integrity attribute, both
the setting of the attribute and the actual SRI enforcement.

For the regular integrity attribute on elements, this leaves in place
the checking of SRI in the Script element and Style element execution,
rather than integrating it more fully into the Resource Loader because
of technical complications. This may be worth doing in the future, but
will require some significant redesign of how Resource objects are
returned from the Memory Cache in ResourceFetcher::requestScript.

BUG=502361

Review URL: https://codereview.chromium.org/1279163005

git-svn-id: svn://svn.chromium.org/blink/trunk@200995 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 600e1412
......@@ -248,6 +248,29 @@ test(function() {
});
}, 'Request redirect test');
test(function() {
var request1 = {};
var request2 = {};
var init = {};
request1 = new Request(URL, init);
assert_equals(request1.integrity, '',
'Request.integrity should be empty on init');
init['integrity'] = 'sha256-deadbeef';
request1 = new Request(URL, init);
assert_equals(request1.integrity, 'sha256-deadbeef',
'Request.integrity match the integrity of init');
request2 = new Request(request1);
assert_equals(request2.integrity, 'sha256-deadbeef',
'Request.integrity should match');
init['mode'] = 'no-cors';
assert_throws(
{name: 'TypeError'},
function() {
var request = new Request(URL, init);
},
'new Request with a non-empty integrity and mode of \'no-cors\' should throw');
}, 'Request integrity test');
test(function() {
['same-origin', 'cors', 'no-cors'].forEach(function(mode) {
FORBIDDEN_METHODS.forEach(function(method) {
......
<!DOCTYPE html>
<html>
<head>
<title>Tests integrity enforcement on fetch()</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
</head>
<body>
<script>
var SRITest = function(pass, name, src, integrity, expectedValue) {
this.pass = pass;
this.name = name;
this.src = src;
this.integrity = integrity;
this.expectedValue = expectedValue;
}
SRITest.prototype.execute = function() {
var pass = this.pass;
var src = this.src;
var integrity = this.integrity;
var expectedValue = this.expectedValue;
var options = {};
if (integrity !== '') {
options.integrity = integrity;
}
promise_test(function() {
return fetch(src, options)
.then(function(response) {
assert_true(pass, "Response should resolve");
if (expectedValue) {
return response.text().then(function(actualValue) {
assert_equals(actualValue, expectedValue, "Value consumed must match hashed value.");
});
}
}, function() {
assert_false(pass, "Response should be rejected");
})
}, this.name);
}
new SRITest(true, 'No integrity', 'call-success.js', '', 'success();\n').execute();
new SRITest(true, 'Good integrity', 'call-success.js', 'sha256-B0/62fJSJFrdjEFR9ba04m/D+LHQ+zG6PGcaR0Trpxg=', 'success();\n').execute();
new SRITest(false, 'Bad integrity', 'call-success.js', 'sha256-deadbeef').execute();
new SRITest(false, 'Bad integrity and an img', '/resources/square100.png', 'sha256-B0/62fJSJFrdjEFR9ba04m/D+LHQ+zG6PGcaR0Trpxg=').execute();
new SRITest(true, 'Good integrity and an img', '/resources/square100.png', 'sha256-xZjdcorjj+TiKGteFFcrNbdqrDns2iVURBpGpAwp12k=').execute();
</script>
</body>
</html>
......@@ -497,6 +497,7 @@ interface Request
getter bodyUsed
getter credentials
getter headers
getter integrity
getter method
getter mode
getter redirect
......
......@@ -424,6 +424,7 @@ interface Request
getter bodyUsed
getter credentials
getter headers
getter integrity
getter method
getter mode
getter redirect
......
......@@ -410,6 +410,7 @@ Starting worker: resources/global-interface-listing.js
[Worker] getter bodyUsed
[Worker] getter credentials
[Worker] getter headers
[Worker] getter integrity
[Worker] getter method
[Worker] getter mode
[Worker] getter redirect
......
......@@ -3368,6 +3368,7 @@ interface Request
getter bodyUsed
getter credentials
getter headers
getter integrity
getter method
getter mode
getter redirect
......
......@@ -397,6 +397,7 @@ Starting worker: resources/global-interface-listing.js
[Worker] getter bodyUsed
[Worker] getter credentials
[Worker] getter headers
[Worker] getter integrity
[Worker] getter method
[Worker] getter mode
[Worker] getter redirect
......
......@@ -448,6 +448,7 @@ Starting worker: resources/global-interface-listing.js
[Worker] getter bodyUsed
[Worker] getter credentials
[Worker] getter headers
[Worker] getter integrity
[Worker] getter method
[Worker] getter mode
[Worker] getter redirect
......
......@@ -3851,6 +3851,7 @@ interface Request
getter bodyUsed
getter credentials
getter headers
getter integrity
getter method
getter mode
getter redirect
......
......@@ -435,6 +435,7 @@ Starting worker: resources/global-interface-listing.js
[Worker] getter bodyUsed
[Worker] getter credentials
[Worker] getter headers
[Worker] getter integrity
[Worker] getter method
[Worker] getter mode
[Worker] getter redirect
......
......@@ -378,6 +378,24 @@ bool ScriptLoader::executeScript(const ScriptSourceCode& sourceCode, double* com
}
}
// The following SRI checks need to be here because, unfortunately, fetches
// are not done purely according to the Fetch spec. In particular,
// different requests for the same resource do not have different
// responses; the memory cache can (and will) return the exact same
// Resource object. For different requests, the same Resource object will
// be returned and will not be associated with the particular request.
// Therefore, when the body of the response comes in, there's no way to
// validate the integrity of the Resource object against a particular
// request (since there may be several pending requests all tied to the
// identical object, and the actual requests are not stored).
//
// In order to simulate the correct behavior, Blink explicitly does the SRI
// checks at execution here (similar to the AccessControlStatus check done
// above), while having proper Fetch checks in the fetch module for use in
// the fetch JavaScript API. In a future world where the ResourceFetcher
// uses the Fetch algorithm, this should be fixed by having separate
// Response objects (perhaps attached to identical Resource objects) per
// request. See https://crbug.com/500701 for more information.
if (m_isExternalScript) {
const KURL resourceUrl = sourceCode.resource()->resourceRequest().url();
if (!SubresourceIntegrity::CheckSubresourceIntegrity(*m_element, sourceCode.source(), sourceCode.resource()->url(), *sourceCode.resource())) {
......
......@@ -114,8 +114,17 @@ bool SubresourceIntegrity::CheckSubresourceIntegrity(const Element& element, con
return false;
}
String errorMessage;
bool result = CheckSubresourceIntegrity(attribute, source, resourceUrl, document, errorMessage);
if (!result)
logErrorToConsole(errorMessage, document);
return result;
}
bool SubresourceIntegrity::CheckSubresourceIntegrity(const String& integrityMetadata, const WTF::String& source, const KURL& resourceUrl, Document& document, String& errorMessage)
{
WTF::Vector<IntegrityMetadata> metadataList;
IntegrityParseResult integrityParseResult = parseIntegrityAttribute(attribute, metadataList, document);
IntegrityParseResult integrityParseResult = parseIntegrityAttribute(integrityMetadata, metadataList, document);
// On failed parsing, there's no need to log an error here, as
// parseIntegrityAttribute() will output an appropriate console message.
if (integrityParseResult != IntegrityParseValidResult)
......@@ -158,9 +167,9 @@ bool SubresourceIntegrity::CheckSubresourceIntegrity(const Element& element, con
// need to be very careful not to expose this in exceptions or
// JavaScript, otherwise it risks exposing information about the
// resource cross-origin.
logErrorToConsole("Failed to find a valid digest in the 'integrity' attribute for resource '" + resourceUrl.elidedString() + "' with computed SHA-256 integrity '" + digestToString(digest) + "'. The resource has been blocked.", document);
errorMessage = "Failed to find a valid digest in the 'integrity' attribute for resource '" + resourceUrl.elidedString() + "' with computed SHA-256 integrity '" + digestToString(digest) + "'. The resource has been blocked.";
} else {
logErrorToConsole("There was an error computing an integrity value for resource '" + resourceUrl.elidedString() + "'. The resource has been blocked.", document);
errorMessage = "There was an error computing an integrity value for resource '" + resourceUrl.elidedString() + "'. The resource has been blocked.";
}
UseCounter::count(document, UseCounter::SRIElementWithNonMatchingIntegrityAttribute);
return false;
......
......@@ -27,6 +27,7 @@ public:
};
static bool CheckSubresourceIntegrity(const Element&, const WTF::String& content, const KURL& resourceUrl, const Resource&);
static bool CheckSubresourceIntegrity(const String&, const WTF::String& content, const KURL& resourceUrl, Document&, WTF::String&);
private:
// FIXME: After the merge with the Chromium repo, this should be refactored
......
......@@ -510,6 +510,8 @@ void LinkStyle::setCSSStyleSheet(const String& href, const KURL& baseURL, const
return;
}
// See the comment in ScriptLoader.cpp about why this check is necessary
// here. https://crbug.com/500701.
if (!cachedStyleSheet->errorOccurred() && !SubresourceIntegrity::CheckSubresourceIntegrity(*m_owner, cachedStyleSheet->sheetText(), KURL(baseURL, href), *cachedStyleSheet)) {
m_loading = false;
removePendingSheet();
......
......@@ -15,6 +15,7 @@
#include "core/fetch/FetchUtils.h"
#include "core/fileapi/Blob.h"
#include "core/frame/Frame.h"
#include "core/frame/SubresourceIntegrity.h"
#include "core/frame/csp/ContentSecurityPolicy.h"
#include "core/inspector/ConsoleMessage.h"
#include "core/inspector/InspectorInstrumentation.h"
......@@ -24,7 +25,9 @@
#include "core/page/Page.h"
#include "modules/fetch/Body.h"
#include "modules/fetch/BodyStreamBuffer.h"
#include "modules/fetch/CompositeDataConsumerHandle.h"
#include "modules/fetch/DataConsumerHandleUtil.h"
#include "modules/fetch/FetchFormDataConsumerHandle.h"
#include "modules/fetch/FetchRequestData.h"
#include "modules/fetch/Response.h"
#include "modules/fetch/ResponseInit.h"
......@@ -34,6 +37,8 @@
#include "platform/weborigin/SecurityOrigin.h"
#include "public/platform/WebURLRequest.h"
#include "wtf/HashSet.h"
#include "wtf/Vector.h"
#include "wtf/text/WTFString.h"
namespace blink {
......@@ -66,6 +71,83 @@ public:
void start();
void dispose();
class SRIVerifier final : public GarbageCollectedFinalized<SRIVerifier>, public WebDataConsumerHandle::Client {
public:
// SRIVerifier takes ownership of |handle| and |response|.
// |updater| must be garbage collected. The other arguments
// all must have the lifetime of the give loader.
SRIVerifier(PassOwnPtr<WebDataConsumerHandle> handle, CompositeDataConsumerHandle::Updater* updater, Response* response, FetchManager::Loader* loader, String integrityMetadata, const KURL& url)
: m_handle(handle)
, m_updater(updater)
, m_response(response)
, m_loader(loader)
, m_integrityMetadata(integrityMetadata)
, m_url(url)
, m_finished(false)
{
m_reader = m_handle->obtainReader(this);
}
void didGetReadable() override
{
ASSERT(m_reader);
ASSERT(m_loader);
ASSERT(m_response);
WebDataConsumerHandle::Result r = WebDataConsumerHandle::Ok;
while (r == WebDataConsumerHandle::Ok) {
const void* buffer;
size_t size;
r = m_reader->beginRead(&buffer, WebDataConsumerHandle::FlagNone, &size);
if (r == WebDataConsumerHandle::Ok) {
m_buffer.append(static_cast<const char*>(buffer), size);
m_reader->endRead(size);
}
}
if (r == WebDataConsumerHandle::ShouldWait)
return;
String errorMessage = "Unknown error occurred while trying to verify integrity.";
m_finished = true;
if (r == WebDataConsumerHandle::Done) {
if (SubresourceIntegrity::CheckSubresourceIntegrity(m_integrityMetadata, String(m_buffer.data(), m_buffer.size()), m_url, *m_loader->document(), errorMessage)) {
m_updater->update(FetchFormDataConsumerHandle::create(m_buffer.data(), m_buffer.size()));
m_loader->m_resolver->resolve(m_response);
m_loader->m_resolver.clear();
// FetchManager::Loader::didFinishLoading() can
// be called before didGetReadable() is called
// when the data is ready. In that case,
// didFinishLoading() doesn't clean up and call
// notifyFinished(), so it is necessary to
// explicitly finish the loader here.
if (m_loader->m_didFinishLoading)
m_loader->loadSucceeded();
return;
}
}
m_updater->update(createUnexpectedErrorDataConsumerHandle());
m_loader->performNetworkError(errorMessage);
}
bool isFinished() const { return m_finished; }
DEFINE_INLINE_TRACE()
{
visitor->trace(m_updater);
visitor->trace(m_response);
visitor->trace(m_loader);
}
private:
OwnPtr<WebDataConsumerHandle> m_handle;
Member<CompositeDataConsumerHandle::Updater> m_updater;
Member<Response> m_response;
RawPtrWillBeMember<FetchManager::Loader> m_loader;
String m_integrityMetadata;
KURL m_url;
OwnPtr<WebDataConsumerHandle::Reader> m_reader;
Vector<char> m_buffer;
bool m_finished;
};
private:
Loader(ExecutionContext*, FetchManager*, ScriptPromiseResolver*, FetchRequestData*);
......@@ -75,6 +157,7 @@ private:
void failed(const String& message);
void notifyFinished();
Document* document() const;
void loadSucceeded();
RawPtrWillBeMember<FetchManager> m_fetchManager;
PersistentWillBeMember<ScriptPromiseResolver> m_resolver;
......@@ -83,6 +166,8 @@ private:
bool m_failed;
bool m_finished;
int m_responseHttpStatusCode;
PersistentWillBeMember<SRIVerifier> m_integrityVerifier;
bool m_didFinishLoading;
};
FetchManager::Loader::Loader(ExecutionContext* executionContext, FetchManager* fetchManager, ScriptPromiseResolver* resolver, FetchRequestData* request)
......@@ -93,6 +178,8 @@ FetchManager::Loader::Loader(ExecutionContext* executionContext, FetchManager* f
, m_failed(false)
, m_finished(false)
, m_responseHttpStatusCode(0)
, m_integrityVerifier(nullptr)
, m_didFinishLoading(false)
{
}
......@@ -106,6 +193,7 @@ DEFINE_TRACE(FetchManager::Loader)
visitor->trace(m_fetchManager);
visitor->trace(m_resolver);
visitor->trace(m_request);
visitor->trace(m_integrityVerifier);
ContextLifecycleObserver::trace(visitor);
}
......@@ -131,7 +219,13 @@ void FetchManager::Loader::didReceiveResponse(unsigned long, const ResourceRespo
break;
}
}
FetchResponseData* responseData = FetchResponseData::createWithBuffer(new BodyStreamBuffer(createFetchDataConsumerHandleFromWebHandle(handle)));
FetchResponseData* responseData = nullptr;
CompositeDataConsumerHandle::Updater* updater = nullptr;
if (m_request->integrity().isEmpty())
responseData = FetchResponseData::createWithBuffer(new BodyStreamBuffer(createFetchDataConsumerHandleFromWebHandle(handle)));
else
responseData = FetchResponseData::createWithBuffer(new BodyStreamBuffer(createFetchDataConsumerHandleFromWebHandle(CompositeDataConsumerHandle::create(createWaitingDataConsumerHandle(), &updater))));
responseData->setStatus(response.httpStatusCode());
responseData->setStatusMessage(response.httpStatusText());
for (auto& it : response.httpHeaderFields())
......@@ -173,23 +267,29 @@ void FetchManager::Loader::didReceiveResponse(unsigned long, const ResourceRespo
break;
}
}
Response* r = Response::create(m_resolver->executionContext(), taintedResponse);
r->headers()->setGuard(Headers::ImmutableGuard);
if (m_request->integrity().isEmpty()) {
m_resolver->resolve(r);
m_resolver.clear();
} else {
ASSERT(!m_integrityVerifier);
m_integrityVerifier = new SRIVerifier(handle, updater, r, this, m_request->integrity(), response.url());
}
}
void FetchManager::Loader::didFinishLoading(unsigned long, double)
{
ASSERT(!m_failed);
m_finished = true;
m_didFinishLoading = true;
// If there is an integrity verifier, and it has not already finished, it
// will take care of finishing the load or performing a network error when
// verification is complete.
if (m_integrityVerifier && !m_integrityVerifier->isFinished())
return;
if (document() && document()->frame() && document()->frame()->page()
&& m_responseHttpStatusCode >= 200 && m_responseHttpStatusCode < 300) {
document()->frame()->page()->chromeClient().ajaxSucceeded(document()->frame());
}
InspectorInstrumentation::didFinishFetch(executionContext(), this, m_request->method(), m_request->url().string());
notifyFinished();
loadSucceeded();
}
void FetchManager::Loader::didFail(const ResourceError& error)
......@@ -221,6 +321,20 @@ Document* FetchManager::Loader::document() const
return nullptr;
}
void FetchManager::Loader::loadSucceeded()
{
ASSERT(!m_failed);
m_finished = true;
if (document() && document()->frame() && document()->frame()->page()
&& m_responseHttpStatusCode >= 200 && m_responseHttpStatusCode < 300) {
document()->frame()->page()->chromeClient().ajaxSucceeded(document()->frame());
}
InspectorInstrumentation::didFinishFetch(executionContext(), this, m_request->method(), m_request->url().string());
notifyFinished();
}
void FetchManager::Loader::start()
{
// "1. If |request|'s url contains a Known HSTS Host, modify it per the
......
......@@ -57,6 +57,7 @@ FetchRequestData* FetchRequestData::cloneExceptBody()
request->m_redirect = m_redirect;
request->m_responseTainting = m_responseTainting;
request->m_mimeType = m_mimeType;
request->m_integrity = m_integrity;
return request;
}
......
......@@ -92,6 +92,8 @@ public:
void setBuffer(BodyStreamBuffer* buffer) { m_buffer = buffer; }
String mimeType() const { return m_mimeType; }
void setMIMEType(const String& type) { m_mimeType = type; }
String integrity() const { return m_integrity; }
void setIntegrity(const String& integrity) { m_integrity = integrity; }
DECLARE_TRACE();
......@@ -120,6 +122,7 @@ private:
Tainting m_responseTainting;
Member<BodyStreamBuffer> m_buffer;
String m_mimeType;
String m_integrity;
};
} // namespace blink
......
......@@ -42,6 +42,7 @@ FetchRequestData* createCopyOfFetchRequestDataForFetch(ScriptState* scriptState,
request->setMode(original->mode());
request->setCredentials(original->credentials());
request->setRedirect(original->redirect());
request->setIntegrity(original->integrity());
// FIXME: Set cache mode.
// TODO(yhirano): Set redirect mode.
return request;
......@@ -166,8 +167,10 @@ Request* Request::createRequestWithRequestOrString(ScriptState* scriptState, Req
request->setRedirect(WebURLRequest::FetchRedirectModeManual);
}
// TODO(jww): "23. If |init|'s integrity member is present, set |request|'s
// "If |init|'s integrity member is present, set |request|'s
// integrity metadata to it."
if (!init.integrity.isNull())
request->setIntegrity(init.integrity);
// "24. If |init|'s method member is present, let |method| be it and run
// these substeps:"
......@@ -208,7 +211,13 @@ Request* Request::createRequestWithRequestOrString(ScriptState* scriptState, Req
exceptionState.throwTypeError("'" + r->request()->method() + "' is unsupported in no-cors mode.");
return nullptr;
}
// "Set |r|'s Headers object's guard to |request-no-CORS|.
// "2. If |request|'s integrity metadata is not the empty string, throw
// a TypeError."
if (!request->integrity().isEmpty()) {
exceptionState.throwTypeError("The integrity attribute is unsupported in no-cors mode.");
return nullptr;
}
// "3. Set |r|'s Headers object's guard to |request-no-CORS|.
r->headers()->setGuard(Headers::RequestNoCORSGuard);
}
// "30. Fill |r|'s Headers object with |headers|. Rethrow any exceptions."
......@@ -486,6 +495,11 @@ String Request::redirect() const
return "";
}
String Request::integrity() const
{
return m_request->integrity();
}
Request* Request::clone(ExceptionState& exceptionState)
{
if (bodyUsed()) {
......
......@@ -50,6 +50,7 @@ public:
String mode() const;
String credentials() const;
String redirect() const;
String integrity() const;
// From Request.idl:
Request* clone(ExceptionState&);
......
......@@ -33,6 +33,7 @@ enum RequestRedirect { "follow", "error", "manual" };
readonly attribute RequestMode mode;
readonly attribute RequestCredentials credentials;
readonly attribute RequestRedirect redirect;
readonly attribute DOMString integrity;
[RaisesException] Request clone();
};
......
......@@ -34,6 +34,7 @@ RequestInit::RequestInit(ExecutionContext* context, const Dictionary& options, E
DictionaryHelper::get(options, "mode", mode);
DictionaryHelper::get(options, "credentials", credentials);
DictionaryHelper::get(options, "redirect", redirect);
DictionaryHelper::get(options, "integrity", integrity);
v8::Local<v8::Value> v8Body;
if (!DictionaryHelper::get(options, "body", v8Body) || v8Body->IsUndefined() || v8Body->IsNull())
......
......@@ -31,6 +31,7 @@ public:
String mode;
String credentials;
String redirect;
String integrity;
};
}
......
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