[webcrypto] Remove RSA-ES support (3 of 3)

Source/bindings/v8/SerializedScriptValue.cpp

Index: Source/bindings/v8/SerializedScriptValue.cpp
diff --git a/Source/bindings/v8/SerializedScriptValue.cpp b/Source/bindings/v8/SerializedScriptValue.cpp
index e2e4348a71d62be60e81f3ea96084707819292ca..863dd19f751cbee86988fba20329f80baf363c83 100644
--- a/Source/bindings/v8/SerializedScriptValue.cpp
+++ b/Source/bindings/v8/SerializedScriptValue.cpp
@@ -221,10 +221,8 @@ enum SerializationTag {
                         //       props = keyLengthBytes:uint32_t, algorithmId:uint32_t
                         //   If subtag=HmacKeyTag:
                         //       props = keyLengthBytes:uint32_t, hashId:uint32_t
-                        //   If subtag=RsaKeyTag:
-                        //       props = algorithmId:uint32_t, type:uint32_t, modulusLengthBits:uint32_t, publicExponentLength:uint32_t, publicExponent:byte[publicExponentLength]
                         //   If subtag=RsaHashedKeyTag:
-                        //       props = <same as for RsaKeyTag>, hashId:uint32_t
+                        //       props = algorithmId:uint32_t, type:uint32_t, modulusLengthBits:uint32_t, publicExponentLength:uint32_t, publicExponent:byte[publicExponentLength], hashId:uint32_t
     ObjectReferenceTag = '^', // ref:uint32_t -> reference table[ref]
     GenerateFreshObjectTag = 'o', // -> empty object allocated an object ID and pushed onto the open stack (ref)
     GenerateFreshSparseArrayTag = 'a', // length:uint32_t -> empty array[length] allocated an object ID and pushed onto the open stack (ref)
@@ -253,7 +251,7 @@ enum ArrayBufferViewSubTag {
 enum CryptoKeySubTag {
     AesKeyTag = 1,
     HmacKeyTag = 2,
-    RsaKeyTag = 3,
+    Deprecated_RsaKeyTag = 3,

[jww, 2014/05/21 17:19:23]

I'm unclear as to why you leave this (and RsaEsPkcs1v1_5Tag) in the enum as
deprecated. It seems like they're used in a very limited number of places, and,
as far as I can tell, aren't persisted anywhere.

And even if it does break something, aren't we not too sad since WebCrypto is
behind an experimental flag?

[eroman, 2014/05/21 18:17:24]

Done, I removed those.

I thought it would be confusing why a number was missing, I have added a comment
instead. And agree it should be safe to re-use it in the future.

     RsaHashedKeyTag = 4,
     // Maximum allowed value is 255
 };
@@ -268,7 +266,7 @@ enum CryptoKeyAlgorithmTag {
     AesCbcTag = 1,
     HmacTag = 2,
     RsaSsaPkcs1v1_5Tag = 3,
-    RsaEsPkcs1v1_5Tag = 4,
+    Deprecated_RsaEsPkcs1v1_5Tag = 4,
     Sha1Tag = 5,
     Sha256Tag = 6,
     Sha384Tag = 7,
@@ -517,9 +515,8 @@ public:
         case blink::WebCryptoKeyAlgorithmParamsTypeHmac:
             doWriteHmacKey(key);
             break;
-        case blink::WebCryptoKeyAlgorithmParamsTypeRsa:
         case blink::WebCryptoKeyAlgorithmParamsTypeRsaHashed:
-            doWriteRsaKey(key);
+            doWriteRsaHashedKey(key);
             break;
         case blink::WebCryptoKeyAlgorithmParamsTypeNone:
             ASSERT_NOT_REACHED();
@@ -730,12 +727,10 @@ private:
         doWriteUint32(key.algorithm().aesParams()->lengthBits() / 8);
     }
 
-    void doWriteRsaKey(const blink::WebCryptoKey& key)
+    void doWriteRsaHashedKey(const blink::WebCryptoKey& key)
     {
-        if (key.algorithm().rsaHashedParams())
-            append(static_cast<uint8_t>(RsaHashedKeyTag));
-        else
-            append(static_cast<uint8_t>(RsaKeyTag));
+        ASSERT(key.algorithm().rsaHashedParams());
+        append(static_cast<uint8_t>(RsaHashedKeyTag));
 
         doWriteAlgorithmId(key.algorithm().id());
 
@@ -750,13 +745,11 @@ private:
             ASSERT_NOT_REACHED();
         }
 
-        const blink::WebCryptoRsaKeyAlgorithmParams* params = key.algorithm().rsaParams();
+        const blink::WebCryptoRsaHashedKeyAlgorithmParams* params = key.algorithm().rsaHashedParams();
         doWriteUint32(params->modulusLengthBits());
         doWriteUint32(params->publicExponent().size());
         append(params->publicExponent().data(), params->publicExponent().size());
-
-        if (key.algorithm().rsaHashedParams())
-            doWriteAlgorithmId(key.algorithm().rsaHashedParams()->hash().id());
+        doWriteAlgorithmId(key.algorithm().rsaHashedParams()->hash().id());
     }
 
     void doWriteAlgorithmId(blink::WebCryptoAlgorithmId id)
@@ -768,8 +761,6 @@ private:
             return doWriteUint32(HmacTag);
         case blink::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5:
             return doWriteUint32(RsaSsaPkcs1v1_5Tag);
-        case blink::WebCryptoAlgorithmIdRsaEsPkcs1v1_5:
-            return doWriteUint32(RsaEsPkcs1v1_5Tag);
         case blink::WebCryptoAlgorithmIdSha1:
             return doWriteUint32(Sha1Tag);
         case blink::WebCryptoAlgorithmIdSha256:
@@ -2247,12 +2238,10 @@ private:
             if (!doReadHmacKey(algorithm, type))
                 return false;
             break;
-        case RsaKeyTag:
-            if (!doReadRsaKey(false, algorithm, type))
-                return false;
-            break;
+        case Deprecated_RsaKeyTag:
+            return false;

[jww, 2014/05/21 17:19:23]

If you do leave in Deprecated_RsaKeyTag, shouldn't this be an
ASSERT_NOT_REACHED?

[eroman, 2014/05/21 18:17:24]

No longer applicable.

         case RsaHashedKeyTag:
-            if (!doReadRsaKey(true, algorithm, type))
+            if (!doReadRsaHashedKey(algorithm, type))
                 return false;
             break;
         default:
@@ -2412,7 +2401,7 @@ private:
         return !algorithm.isNull();
     }
 
-    bool doReadRsaKey(bool hasHash, blink::WebCryptoKeyAlgorithm& algorithm, blink::WebCryptoKeyType& type)
+    bool doReadRsaHashedKey(blink::WebCryptoKeyAlgorithm& algorithm, blink::WebCryptoKeyType& type)
     {
         blink::WebCryptoAlgorithmId id;
         if (!doReadAlgorithmId(id))
@@ -2447,14 +2436,10 @@ private:
         const uint8_t* publicExponent = m_buffer + m_position;
         m_position += publicExponentSize;
 
-        if (hasHash) {
-            blink::WebCryptoAlgorithmId hash;
-            if (!doReadAlgorithmId(hash))
-                return false;
-            algorithm = blink::WebCryptoKeyAlgorithm::createRsaHashed(id, modulusLengthBits, publicExponent, publicExponentSize, hash);
-        } else {
-            algorithm = blink::WebCryptoKeyAlgorithm::createRsa(id, modulusLengthBits, publicExponent, publicExponentSize);
-        }
+        blink::WebCryptoAlgorithmId hash;
+        if (!doReadAlgorithmId(hash))
+            return false;
+        algorithm = blink::WebCryptoKeyAlgorithm::createRsaHashed(id, modulusLengthBits, publicExponent, publicExponentSize, hash);
 
         return !algorithm.isNull();
     }
@@ -2475,9 +2460,8 @@ private:
         case RsaSsaPkcs1v1_5Tag:
             id = blink::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5;
             return true;
-        case RsaEsPkcs1v1_5Tag:
-            id = blink::WebCryptoAlgorithmIdRsaEsPkcs1v1_5;
-            return true;
+        case Deprecated_RsaEsPkcs1v1_5Tag:
+            return false;

[jww, 2014/05/21 17:19:23]

Same question about ASSERT_NOT_REACHED as above.

[eroman, 2014/05/21 18:17:24]

No longer applicable.

         case Sha1Tag:
             id = blink::WebCryptoAlgorithmIdSha1;
             return true;