Implement hash function prioritization for SRI.

Source/core/frame/SubresourceIntegrity.cpp

Index: Source/core/frame/SubresourceIntegrity.cpp
diff --git a/Source/core/frame/SubresourceIntegrity.cpp b/Source/core/frame/SubresourceIntegrity.cpp
index e65193dec20431aaa7c12ea52ee20b60d6ecaaa7..73092a88ceb94bc5e10c2723502115a48390fb41 100644
--- a/Source/core/frame/SubresourceIntegrity.cpp
+++ b/Source/core/frame/SubresourceIntegrity.cpp
@@ -21,6 +21,7 @@
 #include "public/platform/WebCryptoAlgorithm.h"
 #include "wtf/ASCIICType.h"
 #include "wtf/Vector.h"
+#include "wtf/dtoa/utils.h"
 #include "wtf/text/Base64.h"
 #include "wtf/text/StringUTF8Adaptor.h"
 #include "wtf/text/WTFString.h"
@@ -65,6 +66,45 @@ static String digestToString(const DigestValue& digest)
     return base64URLEncode(reinterpret_cast<const char*>(digest.data()), digest.size(), Base64DoNotInsertLFs);
 }
 
+
+SubresourceIntegrity::PrioritizationResult SubresourceIntegrity::getPrioritizedHashFunction(HashAlgorithm algorithm1, HashAlgorithm algorithm2)
+{
+    const HashAlgorithm weakerThanSha256[] = { HashAlgorithmSha1 };

[Mike West, 2015/06/03 09:13:33]

Why do we support SHA-1 at all? I don't remember it being in the spec. Can we
ASSERT that we never get to this check for that known-bad algorithm?

[jww, 2015/06/03 19:26:56]

SHA1 is not supported, so, yes, we can ASSERT that. It's not that we support
SHA-1 (in fact, there's a list of supported hash functions later in
SubresourceIntegrity.cpp that doesn't include SHA-1), but it does exist as an
algorithm listed in HashAlgorithm.

I'll take it out of the lists and add asserts that neither algorithm is ever
SHA1.

+    const HashAlgorithm weakerThanSha384[] = { HashAlgorithmSha1, HashAlgorithmSha256 };
+    const HashAlgorithm weakerThanSha512[] = { HashAlgorithmSha1, HashAlgorithmSha256, HashAlgorithmSha384 };
+
+    if (algorithm1 == algorithm2)
+        return SameStrength;
+
+    const HashAlgorithm* weakerAlgorithms;
+    size_t length;
+    switch (algorithm1) {
+    case HashAlgorithmSha1:
+        return SecondAlgorithmIsStronger;
+    case HashAlgorithmSha256:
+        weakerAlgorithms = weakerThanSha256;
+        length = ARRAY_SIZE(weakerThanSha256);
+        break;
+    case HashAlgorithmSha384:
+        weakerAlgorithms = weakerThanSha384;
+        length = ARRAY_SIZE(weakerThanSha384);
+        break;
+    case HashAlgorithmSha512:
+        weakerAlgorithms = weakerThanSha512;
+        length = ARRAY_SIZE(weakerThanSha512);
+        break;
+    default:
+        ASSERT_NOT_REACHED();
+    };
+
+    for (size_t i = 0; i < length; i++) {
+        if (weakerAlgorithms[i] == algorithm2)
+            return FirstAlgorithmIsStronger;
+    }
+
+    return SecondAlgorithmIsStronger;
+}
+
 bool SubresourceIntegrity::CheckSubresourceIntegrity(const Element& element, const String& source, const KURL& resourceUrl, const Resource& resource)
 {
     if (!RuntimeEnabledFeatures::subresourceIntegrityEnabled())
@@ -90,8 +130,18 @@ bool SubresourceIntegrity::CheckSubresourceIntegrity(const Element& element, con
     if (!metadataList.size())
         return true;
 
+    HashAlgorithm strongestAlgorithm = HashAlgorithmSha256;
+    for (IntegrityMetadata& metadata : metadataList) {

[Mike West, 2015/06/03 09:13:33]

Nit: `const IntegrityMetadata&`

[jww, 2015/06/03 19:26:56]

Done.

+        if (FirstAlgorithmIsStronger == getPrioritizedHashFunction(metadata.algorithm, strongestAlgorithm))
+            strongestAlgorithm = metadata.algorithm;

[Mike West, 2015/06/03 09:13:33]

If you just return the strongest algorithm, you can drop the `if` entirely.

[jww, 2015/06/03 19:26:56]

Done.

+    }
+
     DigestValue digest;
     for (IntegrityMetadata& metadata : metadataList) {
+        PrioritizationResult result = getPrioritizedHashFunction(metadata.algorithm, strongestAlgorithm);
+        if (result != SameStrength)

[Mike West, 2015/06/03 09:13:33]

Nit: It seems like this could be simplified to `if (result !=
strongestAlgorithm)`

[jww, 2015/06/03 19:26:56]

Long term, that's not strictly true because in theory two different hash
functions can be equally strong (and the spec's definition of
getPrioritizedHashFunction is supposed to account for that). However, given
that's not currently the case, I'll modify getPrioritizedHashFunction to just
return the strongest of the two.

+            continue;
+
         digest.clear();
         bool digestSuccess = computeDigest(metadata.algorithm, normalizedSource.data(), normalizedSource.length(), digest);