Bug 143458 - Implement PBKDF2 in WebCrypto
Summary: Implement PBKDF2 in WebCrypto
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jiewen Tan
URL:
Keywords: InRadar
Depends on: 160880
Blocks: 166746
  Show dependency treegraph
 
Reported: 2015-04-06 16:18 PDT by ryan
Modified: 2017-03-12 14:55 PDT (History)
11 users (show)

See Also:


Attachments
Patch (5.36 MB, patch)
2017-03-09 04:41 PST, Jiewen Tan
bfulgham: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-elcapitan (904.31 KB, application/zip)
2017-03-09 05:46 PST, Build Bot
no flags Details
Archive of layout-test-results from ews105 for mac-elcapitan-wk2 (907.24 KB, application/zip)
2017-03-09 05:50 PST, Build Bot
no flags Details
Archive of layout-test-results from ews113 for mac-elcapitan (1.68 MB, application/zip)
2017-03-09 05:53 PST, Build Bot
no flags Details
Archive of layout-test-results from ews126 for ios-simulator-wk2 (860.29 KB, application/zip)
2017-03-09 06:03 PST, Build Bot
no flags Details
Patch for landing (5.36 MB, patch)
2017-03-09 13:04 PST, Jiewen Tan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description ryan 2015-04-06 16:18:39 PDT
The Password Based Key Derivation Function 2.0 (PBKDF2) is crucial for sites that wish to use a password to derive a key versus generating one for a user:


https://dvcs.w3.org/hg/webcrypto-api/raw-file/tip/spec/Overview.html#pbkdf2
Comment 1 Yingle 2016-04-16 13:29:53 PDT
WebKit is lagging on web API support, this is one example. Can someone work on this?
Comment 2 unsung_hero-97 2016-05-19 10:37:48 PDT
+1
Comment 3 Radar WebKit Bug Importer 2016-07-12 16:06:35 PDT
<rdar://problem/27311703>
Comment 4 Jiewen Tan 2017-03-09 04:41:16 PST
Created attachment 303906 [details]
Patch
Comment 5 Jiewen Tan 2017-03-09 04:42:06 PST
90% of the patch is w3c tests rebaseline.
Comment 6 WebKit Commit Bot 2017-03-09 04:43:26 PST
Attachment 303906 [details] did not pass style-queue:


ERROR: Source/WebCore/crypto/CryptoKey.h:48:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 1 in 49 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Build Bot 2017-03-09 05:46:10 PST
Comment on attachment 303906 [details]
Patch

Attachment 303906 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3276047

New failing tests:
crypto/subtle/derive-key-malformed-parameters.html
Comment 8 Build Bot 2017-03-09 05:46:13 PST
Created attachment 303910 [details]
Archive of layout-test-results from ews103 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 9 Build Bot 2017-03-09 05:50:46 PST
Comment on attachment 303906 [details]
Patch

Attachment 303906 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3276078

New failing tests:
crypto/subtle/derive-key-malformed-parameters.html
Comment 10 Build Bot 2017-03-09 05:50:49 PST
Created attachment 303912 [details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 11 Build Bot 2017-03-09 05:53:23 PST
Comment on attachment 303906 [details]
Patch

Attachment 303906 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3276051

New failing tests:
crypto/subtle/derive-key-malformed-parameters.html
Comment 12 Build Bot 2017-03-09 05:53:26 PST
Created attachment 303913 [details]
Archive of layout-test-results from ews113 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 13 Build Bot 2017-03-09 06:03:03 PST
Comment on attachment 303906 [details]
Patch

Attachment 303906 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3276084

New failing tests:
crypto/subtle/derive-key-malformed-parameters.html
Comment 14 Build Bot 2017-03-09 06:03:06 PST
Created attachment 303915 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6
Comment 15 Brent Fulgham 2017-03-09 10:40:05 PST
Test failure on Mac (64-bit):

crypto/subtle/derive-key-malformed-parameters.html
Comment 16 Brent Fulgham 2017-03-09 10:41:22 PST
Some kind of whitespace issue:

--- /Volumes/Data/EWS/WebKit/WebKitBuild/Release/layout-test-results/crypto/subtle/derive-key-malformed-parameters-expected.txt
+++ /Volumes/Data/EWS/WebKit/WebKitBuild/Release/layout-test-results/crypto/subtle/derive-key-malformed-parameters-actual.txt
@@ -17,7 +17,7 @@
 PASS crypto.subtle.deriveKey({ name:"ECDH", public:publicKey }, privateKey, {name: "aes-cbc", length: 1}, extractable, ["encrypt"]) rejected promise  with OperationError (DOM Exception 34): Cannot get key length from derivedKeyType.
 PASS crypto.subtle.deriveKey({ name:"ECDH", public:publicKey }, privateKey, {name: "hmac", hash: "hmac"}, extractable, ["sign"]) rejected promise  with NotSupportedError (DOM Exception 9): The operation is not supported..
 PASS crypto.subtle.deriveKey({ name:"ECDH", public:publicKey }, privateKey, {name: "hmac", hash: "sha-1", length: 0}, extractable, ["sign"]) rejected promise  with TypeError: Cannot get key length from derivedKeyType.
-PASS crypto.subtle.deriveKey({ name:"ECDH", public:publicKey }, privateKey, {name: "aes-cbc", length: 128}, extractable, []) rejected promise  with SyntaxError (DOM Exception 12): A required parameter was missing or out-of-range.
+PASS crypto.subtle.deriveKey({ name:"ECDH", public:publicKey }, privateKey, {name: "aes-cbc", length: 128}, extractable, [ ]) rejected promise  with SyntaxError (DOM Exception 12): A required parameter was missing or out-of-range.
 PASS successfullyParsed is true
 
 TEST COMPLETE
Comment 17 Brent Fulgham 2017-03-09 10:54:21 PST
Comment on attachment 303906 [details]
Patch

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

Looks great! You have a whitespace error in your tests (which I think I noted below). r=me, but please fix the test.

> LayoutTests/crypto/subtle/derive-key-malformed-parameters.html:71
> +    return shouldReject('crypto.subtle.deriveKey({ name:"ECDH", public:publicKey }, privateKey, {name: "aes-cbc", length: 128}, extractable, [ ])');

This is either causing your test failure, or you need to add this space somewhere else to match the test output.

> LayoutTests/imported/w3c/web-platform-tests/WebCryptoAPI/derive_bits_keys/pbkdf2.worker-expected.txt:3
> +PASS Derived key of type name: AES-CBC length: 128  using short password, short salt, SHA-384, with 1 iterations 

Yay!

> LayoutTests/imported/w3c/web-platform-tests/WebCryptoAPI/derive_bits_keys/pbkdf2.worker-expected.txt:35
> +PASS Derived key of type name: AES-GCM length: 256  using short password, short salt, SHA-384, with 1 iterations 

Hooray! So many new passes!

> LayoutTests/imported/w3c/web-platform-tests/WebCryptoAPI/derive_bits_keys/pbkdf2.worker-expected.txt:8689
> +PASS Derived key of type name: HMAC hash: SHA-512 length: 256  using empty password, empty salt, SHA-256, with 0 iterations 

That's a lot of new passes!

> LayoutTests/imported/w3c/web-platform-tests/WebCryptoAPI/derive_bits_keys/test_pbkdf2-expected.txt:8691
> +PASS Derived key of type name: HMAC hash: SHA-512 length: 256  using empty password, empty salt, SHA-256, with 0 iterations 

I'm tired after scanning all of that!
Comment 18 Alexey Proskuryakov 2017-03-09 12:06:24 PST
One general comment.

> +        * crypto/mac/CryptoAlgorithmPBKDF2Mac.cpp: Added.

Mac is not a good directory or name for these, as they are in use on iOS too. I think that CoreCrypto would be best for most, and maybe Darwin for others?
Comment 19 Brent Fulgham 2017-03-09 12:28:14 PST
(In reply to comment #18)
> One general comment.
> 
> > +        * crypto/mac/CryptoAlgorithmPBKDF2Mac.cpp: Added.
> 
> Mac is not a good directory or name for these, as they are in use on iOS
> too. I think that CoreCrypto would be best for most, and maybe Darwin for
> others?

That's a good point. Jiewen, can you file a radar to do that reorganization? We should tackle that as clean-up in a few weeks.
Comment 20 Jiewen Tan 2017-03-09 12:57:15 PST
(In reply to comment #18)
> One general comment.
> 
> > +        * crypto/mac/CryptoAlgorithmPBKDF2Mac.cpp: Added.
> 
> Mac is not a good directory or name for these, as they are in use on iOS
> too. I think that CoreCrypto would be best for most, and maybe Darwin for
> others?

Filed: Bug 169432.
Comment 21 Jiewen Tan 2017-03-09 13:04:13 PST
Created attachment 303965 [details]
Patch for landing
Comment 22 WebKit Commit Bot 2017-03-09 13:12:48 PST
Attachment 303965 [details] did not pass style-queue:


ERROR: Source/WebCore/crypto/CryptoKey.h:48:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 1 in 50 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 23 Jiewen Tan 2017-03-09 14:24:38 PST
Committed r213671: <http://trac.webkit.org/changeset/213671>
Comment 24 Javier Fernandez 2017-03-10 08:10:26 PST
This patch caused several tests to fail in GTK. 
see bug #169468 for details.