Implementation of subresource integrity attribute for secure origins.

Source/core/frame/SubresourceIntegrity.cpp

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "config.h"
+#include "core/frame/SubresourceIntegrity.h"
+
+#include "core/HTMLNames.h"
+#include "core/dom/Document.h"
+#include "core/dom/Element.h"
+#include "core/frame/UseCounter.h"
+#include "platform/Crypto.h"
+#include "platform/ParsingUtilities.h"
+#include "platform/RuntimeEnabledFeatures.h"
+#include "platform/weborigin/KURL.h"
+#include "platform/weborigin/SecurityOrigin.h"
+#include "public/platform/WebCrypto.h"
+#include "public/platform/WebCryptoAlgorithm.h"
+#include "wtf/ASCIICType.h"
+#include "wtf/text/Base64.h"
+#include "wtf/text/StringUTF8Adaptor.h"
+#include "wtf/text/WTFString.h"
+
+namespace blink {
+
+static bool isIntegrityCharacter(UChar c)
+{
+    // Check if it's a base64 encoded value.
+    return isASCIIAlphanumeric(c) || c == '+' || c == '/' || c == '=';

[Mike West, 2014/09/16 06:45:04]

Nit: It's probably worth sharing the base64 check with CSP (in a future CL).

[jww, 2014/09/16 22:34:49]

Acknowledged.

+}
+
+static bool DigestsEqual(const DigestValue& digest1, const DigestValue& digest2)
+{
+    if (digest1.size() != digest2.size())
+        return false;
+
+    for (size_t i = 0; i < digest1.size(); i++) {
+        if (digest1[i] != digest2[i])
+            return false;
+    }
+
+    return true;
+}
+
+// TODO(jww) If CheckSubresourceIntegrity fails, Blink should create a console
+// message to alert the developer of the failure.
+bool SubresourceIntegrity::CheckSubresourceIntegrity(const Element& element, const String& source, const KURL& resourceUrl)
+{
+    if (!RuntimeEnabledFeatures::subresourceIntegrityEnabled())
+        return true;
+
+    if (!element.fastHasAttribute(HTMLNames::integrityAttr))
+        return true;
+
+    // TODO(jww): If insecureOriginMsg is not empty after the check, Blink
+    // should send a console message.
+    //
+    // Instead of just checking SecurityOrigin::isSecure on resourceUrl, this
+    // checks canAccessFeatureRequiringSecureOrigin so that file:// protocols
+    // and localhost resources can be allowed. These may be useful for testing
+    // and are allowed for features requiring authenticated origins, so Chrome
+    // allows them here.

[Mike West, 2014/09/16 06:45:04]

I don't like that we've ended up with two levels of "secure", and I think we
should either consider renaming one or both methods, or merging them.

It's nothing you need to resolve in this CL, but I'd appreciate it if you filed
a bug about this so we can figure out whether it's a problem worth solving.

[jww, 2014/09/16 22:34:49]

Okay, I will do that, although there *is* a subtle difference. localhost and
file:// are not inherently authenticated. That is, there's nothing about the
host or protocol, respectively, that are authenticated, it's just that in
practice we're cool with them. So it does sort of make sense to me, but I think
it's worth a debate on a bug.

In any case, I filed https://crbug.com/414928.

+    String insecureOriginMsg = "";
+    RefPtr<SecurityOrigin> resourceSecurityOrigin = SecurityOrigin::create(resourceUrl);
+    if (!element.document().securityOrigin()->canAccessFeatureRequiringSecureOrigin(insecureOriginMsg) || !resourceSecurityOrigin->canAccessFeatureRequiringSecureOrigin(insecureOriginMsg)) {
+        UseCounter::count(element.document(), UseCounter::SRIElementWithIntegrityAttributeAndInsecureResource);

[Mike West, 2014/09/16 06:45:04]

This will also trigger if the document itself is insecure. Perhaps it's worth
separating those two cases so we know what we're measuring?

[jww, 2014/09/16 22:34:49]

Done.

+        return false;
+    }
+
+    String integrity;
+    HashAlgorithm algorithm;
+    if (!parseIntegrityAttribute(element.fastGetAttribute(HTMLNames::integrityAttr), integrity, algorithm)) {
+        UseCounter::count(element.document(), UseCounter::SRIElementWithUnparsableIntegrityAttribute);
+        return false;
+    }
+
+    Vector<char> hashVector;
+    base64Decode(integrity, hashVector);
+
+    StringUTF8Adaptor normalizedSource(source, StringUTF8Adaptor::Normalize, WTF::EntitiesForUnencodables);
+
+    DigestValue digest;
+    bool digestSuccess = computeDigest(algorithm, normalizedSource.data(), normalizedSource.length(), digest);
+
+    if (digestSuccess) {
+        DigestValue convertedHashVector;
+        convertedHashVector.append(reinterpret_cast<uint8_t*>(hashVector.data()), hashVector.size());
+        if (DigestsEqual(digest, convertedHashVector)) {
+            UseCounter::count(element.document(), UseCounter::SRIElementWithMatchingIntegrityAttribute);
+            return true;
+        }
+    }
+
+    UseCounter::count(element.document(), UseCounter::SRIElementWithNonMatchingIntegrityAttribute);
+    return false;
+}
+
+bool SubresourceIntegrity::parseIntegrityAttribute(const String& attribute, String& integrity, HashAlgorithm& algorithm)

[Mike West, 2014/09/16 06:45:03]

This parsing algorithm looks solid, but I'd really like to see some unit tests
with weird input to have a bit more confidence that it covers all the bases.

[jww, 2014/09/16 22:34:49]

Done.

+{
+    DEFINE_STATIC_LOCAL(const String, integrityPrefix, ("ni://"));
+    // Any additions or subtractions from this struct should also modify the
+    // respective entries in the kAlgorithmMap array in checkDigest().
+    static const struct {
+        const char* prefix;
+        HashAlgorithm algorithm;
+    } kSupportedPrefixes[] = {
+        { "sha256", HashAlgorithmSha256 },
+        { "sha384", HashAlgorithmSha384 },
+        { "sha512", HashAlgorithmSha512 }
+    };
+    Vector<UChar> characters;
+    attribute.appendTo(characters);
+    UChar* begin = characters.data();
+    UChar* end = characters.end();
+
+    if (!equalIgnoringCase(integrityPrefix.characters8(), begin, integrityPrefix.length()))

[Mike West, 2014/09/16 06:45:03]

I don't recall if 'fastGetAttribute' strips whitespace or not. I don't think it
does. If that's the case, you'll want to do something to support '  
ni://lefkjw'.

[jww, 2014/09/16 22:34:49]

Done.

+        return false;
+
+    const UChar* algorithmStart = begin + integrityPrefix.length();
+    const UChar* algorithmEnd = algorithmStart;
+
+    skipUntil<UChar>(algorithmEnd, end, ';');
+
+    // Instead of this sizeof() calculation to get the length of this array,
+    // it would be preferable to use WTF_ARRAY_LENGTH for simplicity and to
+    // guarantee a compile time calculation. Unfortunately, on some
+    // compliers, the call to WTF_ARRAY_LENGTH fails on arrays of anonymous
+    // stucts, so, for now, it is necessary to resort to this sizeof
+    // calculation.
+    size_t i = 0;
+    size_t kSupportedPrefixesLength = sizeof(kSupportedPrefixes) / sizeof(kSupportedPrefixes[0]);
+    for (; i < kSupportedPrefixesLength; i++) {
+        if (equalIgnoringCase(kSupportedPrefixes[i].prefix, algorithmStart, strlen(kSupportedPrefixes[i].prefix))) {
+            algorithm = kSupportedPrefixes[i].algorithm;
+            break;
+        }
+    }
+
+    if (i == kSupportedPrefixesLength)
+        return false;
+
+    const UChar* integrityStart = algorithmEnd;
+    if (!skipExactly<UChar>(integrityStart, end, ';'))
+        return false;
+
+    const UChar* integrityEnd = integrityStart;
+    skipWhile<UChar, isIntegrityCharacter>(integrityEnd, end);
+    if (integrityEnd != end)
+        return false;
+
+    integrity = String(integrityStart, integrityEnd - integrityStart);
+    return true;
+}
+
+} // namespace blink