Bug 124059 – Implement JWK key import for HMAC and AES-CBC

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Alexey Proskuryakov]

Starting with a simple case - JWK or another complex format will be needed to import RSA and EC keys.

[Alexey Proskuryakov]

Note that this is governed by multiple unfinished specs, those being WebCrypto, JWK/JWA, and Netflix key wrap proposal. Those don't agree about things yet.

In particular,
- Netflix proposes a JWK "use":"wrap" value, which is explicitly unnecessary per JWK,
- Netflix proposes an "extractable" JWK key, which is not in any formally published spec yet,
- we need algorithms that are not specified anywhere yet,
- WebCrypto and JWA appear to have vague but different requirements on key length for each algorithm (they are sometimes permissive).

It will get worse once we get to key unwrapping.

[Alexey Proskuryakov]

Created attachment 216421 [details]
proposed patch

[WebKit Commit Bot]

Attachment 216421 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/crypto/subtle/aes-cbc-import-jwk-expected.txt', u'LayoutTests/crypto/subtle/aes-cbc-import-jwk.html', u'LayoutTests/crypto/subtle/hmac-import-jwk-expected.txt', u'LayoutTests/crypto/subtle/hmac-import-jwk.html', u'LayoutTests/crypto/subtle/import-jwk-expected.txt', u'LayoutTests/crypto/subtle/import-jwk.html', u'Source/WebCore/ChangeLog', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/bindings/js/JSCryptoKeySerializationJWK.cpp', u'Source/WebCore/bindings/js/JSCryptoKeySerializationJWK.h', u'Source/WebCore/bindings/js/JSSubtleCryptoCustom.cpp', u'Source/WebCore/crypto/CryptoAlgorithm.cpp', u'Source/WebCore/crypto/CryptoAlgorithm.h', u'Source/WebCore/crypto/CryptoKeyData.h', u'Source/WebCore/crypto/CryptoKeyFormat.h', u'Source/WebCore/crypto/CryptoKeySerialization.h', u'Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CBC.cpp', u'Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CBC.h', u'Source/WebCore/crypto/algorithms/CryptoAlgorithmHMAC.cpp', u'Source/WebCore/crypto/algorithms/CryptoAlgorithmHMAC.h', u'Source/WebCore/crypto/keys/CryptoKeyDataOctetSequence.cpp', u'Source/WebCore/crypto/keys/CryptoKeyDataOctetSequence.h', u'Source/WebCore/crypto/keys/CryptoKeySerializationRaw.cpp', u'Source/WebCore/crypto/keys/CryptoKeySerializationRaw.h']" exit_code: 1
Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CBC.cpp:72:  CryptoAlgorithmAES_CBC::importKey is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WebCore/bindings/js/JSCryptoKeySerializationJWK.cpp:195:  An else if statement should be written as an if statement when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
Source/WebCore/bindings/js/JSSubtleCryptoCustom.cpp:53:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/bindings/js/JSSubtleCryptoCustom.cpp:56:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/bindings/js/JSSubtleCryptoCustom.cpp:59:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/bindings/js/JSSubtleCryptoCustom.cpp:435:  Missing space before ( in switch(  [whitespace/parens] [5]
Total errors found: 6 in 24 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alexey Proskuryakov]

Committed <http://trac.webkit.org/changeset/158943>.

[Darin Adler]

Comment on attachment 216421 [details]
proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=216421&action=review

> Source/WebCore/bindings/js/JSCryptoKeySerializationJWK.h:46
> +WTF_MAKE_NONCOPYABLE(JSCryptoKeySerializationJWK);

We normally indent these.

> Source/WebCore/bindings/js/JSCryptoKeySerializationJWK.h:60
> +    virtual bool reconcileAlgorithm(std::unique_ptr<CryptoAlgorithm>&, std::unique_ptr<CryptoAlgorithmParameters>&) const OVERRIDE;
> +
> +    virtual void reconcileUsages(CryptoKeyUsage&) const OVERRIDE;
> +    virtual void reconcileExtractable(bool&) const OVERRIDE;
> +
> +    virtual std::unique_ptr<CryptoKeyData> keyData() const OVERRIDE;

Can we make these private?

>> Source/WebCore/bindings/js/JSSubtleCryptoCustom.cpp:435
>> +    switch(keyFormat) {
> 
> Missing space before ( in switch(  [whitespace/parens] [5]

Yes, not our usual format.

> Source/WebCore/crypto/algorithms/CryptoAlgorithmHMAC.cpp:84
> +    RefPtr<CryptoKeyHMAC> result = CryptoKeyHMAC::create(keyDataOctetSequence.octetSequence(), hmacParameters.hash, extractable, usage);
> +    promise->fulfill(result.release());

Would read better without the local variable, I think.