Bug 124236 - Implement key generation and JWK import for RSASSA-PKCS1-v1_5
Summary: Implement key generation and JWK import for RSASSA-PKCS1-v1_5
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords:
Depends on: 124253 124280
Blocks: 122679
  Show dependency treegraph
 
Reported: 2013-11-12 15:00 PST by Alexey Proskuryakov
Modified: 2013-11-13 09:45 PST (History)
5 users (show)

See Also:


Attachments
proposed patch (110.49 KB, patch)
2013-11-12 15:55 PST, Alexey Proskuryakov
sam: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
combined fixed patch for re-landing (109.70 KB, patch)
2013-11-13 00:49 PST, Alexey Proskuryakov
commit-queue: commit-queue-
Details | Formatted Diff | Diff
patch for landing (108.61 KB, patch)
2013-11-13 00:59 PST, Alexey Proskuryakov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2013-11-12 15:00:11 PST
Implement key generation and JWK import for RSASSA-PKCS1-v1_5.
Comment 1 Alexey Proskuryakov 2013-11-12 15:55:01 PST
Created attachment 216736 [details]
proposed patch
Comment 2 WebKit Commit Bot 2013-11-12 15:56:25 PST
Attachment 216736 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/crypto/subtle/resources/common.js', u'LayoutTests/crypto/subtle/rsassa-pkcs1-v1_5-generate-key-expected.txt', u'LayoutTests/crypto/subtle/rsassa-pkcs1-v1_5-generate-key.html', u'LayoutTests/crypto/subtle/rsassa-pkcs1-v1_5-import-jwk-expected.txt', u'LayoutTests/crypto/subtle/rsassa-pkcs1-v1_5-import-jwk.html', u'Source/WebCore/ChangeLog', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/bindings/js/JSCryptoAlgorithmBuilder.cpp', u'Source/WebCore/bindings/js/JSCryptoAlgorithmBuilder.h', u'Source/WebCore/bindings/js/JSCryptoAlgorithmDictionary.cpp', u'Source/WebCore/bindings/js/JSCryptoKeySerializationJWK.cpp', u'Source/WebCore/bindings/js/JSCryptoKeySerializationJWK.h', u'Source/WebCore/crypto/CryptoAlgorithmDescriptionBuilder.h', u'Source/WebCore/crypto/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp', u'Source/WebCore/crypto/CryptoKey.h', u'Source/WebCore/crypto/CryptoKeyData.h', u'Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CBC.cpp', u'Source/WebCore/crypto/algorithms/CryptoAlgorithmHMAC.cpp', u'Source/WebCore/crypto/algorithms/CryptoAlgorithmRSASSA_PKCS1_v1_5.cpp', u'Source/WebCore/crypto/algorithms/CryptoAlgorithmRSASSA_PKCS1_v1_5.h', u'Source/WebCore/crypto/keys/CryptoKeyAES.h', u'Source/WebCore/crypto/keys/CryptoKeyDataOctetSequence.h', u'Source/WebCore/crypto/keys/CryptoKeyDataRSAComponents.cpp', u'Source/WebCore/crypto/keys/CryptoKeyDataRSAComponents.h', u'Source/WebCore/crypto/keys/CryptoKeyHMAC.h', u'Source/WebCore/crypto/keys/CryptoKeyRSA.h', u'Source/WebCore/crypto/mac/CryptoAlgorithmAES_CBCMac.cpp', u'Source/WebCore/crypto/mac/CryptoAlgorithmHMACMac.cpp', u'Source/WebCore/crypto/mac/CryptoAlgorithmRegistryMac.cpp', u'Source/WebCore/crypto/mac/CryptoKeyRSAMac.cpp', u'Source/WebCore/crypto/parameters/CryptoAlgorithmHmacKeyParams.h', u'Source/WebCore/crypto/parameters/CryptoAlgorithmRsaKeyGenParams.h', u'Source/WebCore/crypto/parameters/CryptoAlgorithmRsaSsaKeyParams.h', u'Source/WebCore/crypto/parameters/CryptoAlgorithmRsaSsaParams.h']" exit_code: 1
Source/WebCore/crypto/keys/CryptoKeyRSA.h:50:  The parameter name "identifier" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/crypto/keys/CryptoKeyRSA.h:50:  The parameter name "keyData" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/crypto/keys/CryptoKeyRSA.h:50:  The parameter name "usage" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/crypto/algorithms/CryptoAlgorithmRSASSA_PKCS1_v1_5.h:45:  The parameter name "data" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/crypto/mac/CryptoKeyRSAMac.cpp:33:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/crypto/mac/CryptoKeyRSAMac.cpp:39:  Missing spaces around /  [whitespace/operators] [3]
Source/WebCore/crypto/mac/CryptoKeyRSAMac.cpp:46:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/crypto/mac/CryptoKeyRSAMac.cpp:47:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/crypto/mac/CryptoKeyRSAMac.cpp:170:  Missing space before {  [whitespace/braces] [5]
Source/WebCore/crypto/mac/CryptoKeyRSAMac.cpp:176:  Missing space before {  [whitespace/braces] [5]
Source/WebCore/crypto/mac/CryptoKeyRSAMac.cpp:185:  Missing space before {  [whitespace/braces] [5]
Source/WebCore/bindings/js/JSCryptoKeySerializationJWK.cpp:118:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/crypto/algorithms/CryptoAlgorithmRSASSA_PKCS1_v1_5.cpp:39:  CryptoAlgorithmRSASSA_PKCS1_v1_5::s_name is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WebCore/crypto/algorithms/CryptoAlgorithmRSASSA_PKCS1_v1_5.cpp:49:  CryptoAlgorithmRSASSA_PKCS1_v1_5::create is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WebCore/crypto/algorithms/CryptoAlgorithmRSASSA_PKCS1_v1_5.cpp:54:  CryptoAlgorithmRSASSA_PKCS1_v1_5::identifier is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WebCore/crypto/algorithms/CryptoAlgorithmRSASSA_PKCS1_v1_5.cpp:59:  CryptoAlgorithmRSASSA_PKCS1_v1_5::importKey is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WebCore/crypto/algorithms/CryptoAlgorithmRSASSA_PKCS1_v1_5.cpp:76:  CryptoAlgorithmRSASSA_PKCS1_v1_5::generateKey is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WebCore/crypto/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:35:  CryptoAlgorithmRSASSA_PKCS1_v1_5::sign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WebCore/crypto/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:40:  CryptoAlgorithmRSASSA_PKCS1_v1_5::verify is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WebCore/crypto/CryptoKey.h:44:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/crypto/CryptoKey.h:45:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 21 in 35 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Sam Weinig 2013-11-12 16:12:39 PST
Comment on attachment 216736 [details]
proposed patch

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
Comment 4 Build Bot 2013-11-12 16:25:47 PST
Comment on attachment 216736 [details]
proposed patch

Attachment 216736 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/22749645
Comment 5 Alexey Proskuryakov 2013-11-12 16:30:50 PST
> This part isn't right, as you told me.

Upon further inspection, it turned out that this is correct - having all or none additional parameters is a requirement for JWK producers, not for JWK consumers. 

Committed <http://trac.webkit.org/r159160>.
Comment 6 Alexey Proskuryakov 2013-11-12 16:38:59 PST
Release build fix in <http://trac.webkit.org/r159161>.

Looking into the other failure now.
Comment 7 Alexey Proskuryakov 2013-11-12 17:12:12 PST
Another build fix in <http://trac.webkit.org/r159164>.
Comment 8 WebKit Commit Bot 2013-11-12 17:29:05 PST
Re-opened since this is blocked by bug 124253
Comment 9 Alexey Proskuryakov 2013-11-13 00:49:29 PST
Created attachment 216778 [details]
combined fixed patch for re-landing
Comment 10 WebKit Commit Bot 2013-11-13 00:50:25 PST
Comment on attachment 216778 [details]
combined fixed patch for re-landing

Rejecting attachment 216778 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-01', 'apply-attachment', '--no-update', '--non-interactive', 216778, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
it_code: 29 cwd: /Volumes/Data/EWS/WebKit

Parsed 36 diffs from patch file(s).
cp: Source/WebCore/crypto/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp: No such file or directory
Failed to copy Source/WebCore/crypto/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp Source/WebCore/crypto/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp. at /Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply line 436.

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force']" exit_code: 29 cwd: /Volumes/Data/EWS/WebKit

Full output: http://webkit-queues.appspot.com/results/22719840
Comment 11 Alexey Proskuryakov 2013-11-13 00:59:21 PST
Created attachment 216780 [details]
patch for landing

Fixed format for svn-apply to work.
Comment 12 WebKit Commit Bot 2013-11-13 01:29:42 PST
Comment on attachment 216780 [details]
patch for landing

Clearing flags on attachment: 216780

Committed r159180: <http://trac.webkit.org/changeset/159180>
Comment 13 WebKit Commit Bot 2013-11-13 01:29:45 PST
All reviewed patches have been landed.  Closing bug.
Comment 15 WebKit Commit Bot 2013-11-13 07:19:56 PST
Re-opened since this is blocked by bug 124280
Comment 16 Alexey Proskuryakov 2013-11-13 09:44:06 PST
Committed <http://trac.webkit.org/changeset/159180>.
Comment 17 Alexey Proskuryakov 2013-11-13 09:44:48 PST
Oops, I thought that I forgot to close the bug yesterday, but it's opened again.
Comment 18 Alexey Proskuryakov 2013-11-13 09:45:47 PST
OK, so this wasn't rolled out, just a test was skipped. Marking resolved/fixed again.