[webkit-reviews] review granted: [Bug 124236] Implement key generation and JWK import for RSASSA-PKCS1-v1_5 : [Attachment 216736] proposed patch

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has granted Alexey Proskuryakov <ap at webkit.org>'s
request for review:
Bug 124236: Implement key generation and JWK import for RSASSA-PKCS1-v1_5
https://bugs.webkit.org/show_bug.cgi?id=124236

Attachment 216736: proposed patch
https://bugs.webkit.org/attachment.cgi?id=216736&action=review

------- Additional Comments from Sam Weinig <sam at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=216736&action=review


> Source/WebCore/bindings/js/JSCryptoAlgorithmDictionary.cpp:231
> +    RefPtr<Uint8Array> publicExponentArray =
toUint8Array(publicExponentValue);

Do you need to type check this?

> Source/WebCore/bindings/js/JSCryptoAlgorithmDictionary.cpp:241
> +    auto result = std::make_unique<CryptoAlgorithmRsaSsaKeyParams>();
> +    return std::move(result);

One line?

> Source/WebCore/bindings/js/JSCryptoKeySerializationJWK.cpp:331
> +	   return CryptoKeyDataRSAComponents::createPrivate(modulus, exponent,
privateExponent);

This part isn't right, as you told me.

> Source/WebCore/crypto/keys/CryptoKeyAES.h:66
> +    RELEASE_ASSERT(isCryptoKeyAES(key));

This should use ASSERT_WITH_SECURITY_IMPLICATION

> Source/WebCore/crypto/keys/CryptoKeyAES.h:72
> +    RELEASE_ASSERT(isCryptoKeyAES(key));

As should this.

> Source/WebCore/crypto/keys/CryptoKeyRSA.h:79
> +    RELEASE_ASSERT(isCryptoKeyRSA(key));

ASSERT_WITH_SECURITY_IMPLICATION

> Source/WebCore/crypto/keys/CryptoKeyRSA.h:85
> +    RELEASE_ASSERT(isCryptoKeyRSA(key));

ASSERT_WITH_SECURITY_IMPLICATION