Bug 164740 - Update SubtleCrypto::sign to match the latest spec
Summary: Update SubtleCrypto::sign to match the latest spec
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jiewen Tan
URL:
Keywords: InRadar
Depends on:
Blocks: 160883
  Show dependency treegraph
 
Reported: 2016-11-14 15:52 PST by Jiewen Tan
Modified: 2016-11-29 14:52 PST (History)
8 users (show)

See Also:


Attachments
Patch (73.17 KB, patch)
2016-11-22 15:36 PST, Jiewen Tan
no flags Details | Formatted Diff | Diff
Patch (75.49 KB, patch)
2016-11-27 13:45 PST, Jiewen Tan
no flags Details | Formatted Diff | Diff
Patch (75.26 KB, patch)
2016-11-28 12:15 PST, Jiewen Tan
no flags Details | Formatted Diff | Diff
Patch (76.25 KB, patch)
2016-11-28 12:29 PST, Jiewen Tan
no flags Details | Formatted Diff | Diff
Patch (76.71 KB, patch)
2016-11-28 13:02 PST, Jiewen Tan
no flags Details | Formatted Diff | Diff
Patch (77.28 KB, patch)
2016-11-28 20:17 PST, Jiewen Tan
bfulgham: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews125 for ios-simulator-wk2 (17.74 MB, application/zip)
2016-11-28 22:07 PST, Build Bot
no flags Details
Patch for landing (77.25 KB, patch)
2016-11-29 13:21 PST, Jiewen Tan
no flags Details | Formatted Diff | Diff
Patch for landing (77.35 KB, patch)
2016-11-29 14:47 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 Jiewen Tan 2016-11-14 15:52:16 PST
Update SubtleCrypto::sign to match the latest spec.
Comment 1 Radar WebKit Bug Importer 2016-11-14 16:40:51 PST
<rdar://problem/29257864>
Comment 2 Jiewen Tan 2016-11-22 15:36:51 PST
Created attachment 295347 [details]
Patch
Comment 3 WebKit Commit Bot 2016-11-22 15:39:26 PST
Attachment 295347 [details] did not pass style-queue:


ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmRSASSA_PKCS1_v1_5.cpp:71:  CryptoAlgorithmRSASSA_PKCS1_v1_5::sign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:60:  signRSASSA_PKCS1_v1_5 is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:85:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:105:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSASSA_PKCS1_v1_5GnuTLS.cpp:38:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 5 in 44 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 youenn fablet 2016-11-23 01:43:48 PST
Comment on attachment 295347 [details]
Patch

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

> Source/WebCore/bindings/js/JSSubtleCryptoCustom.cpp:557
> +    auto exceptionCallback = [capturedPromise = promise.copyRef()](ExceptionCode ec) mutable {

You can probably move the promise here.

> Source/WebCore/crypto/SubtleCrypto.idl:36
> +    [Custom] Promise<any> sign(AlgorithmIdentifier algorithm, CryptoKey key, BufferSource data);

Can we try making it non custom?
This would probably require adding converters for AlgorithmIdentifier and CryptoKey but the code is probably already in JSSubtleCryptoCustom.
Comment 5 Brent Fulgham 2016-11-23 09:16:39 PST
EFL/GTK seems to be failing due to "WebCore::CryptoAlgorithmHMAC::platformSign". Are they not getting built? Or do we need stubs for these platforms?
Comment 6 Jiewen Tan 2016-11-26 16:12:45 PST
(In reply to comment #5)
> EFL/GTK seems to be failing due to
> "WebCore::CryptoAlgorithmHMAC::platformSign". Are they not getting built? Or
> do we need stubs for these platforms?

I do add WebCore::CryptoAlgorithmHMAC::platformSign into the gnutls implementations. Not sure why GTK/EFL fail to build.
Comment 7 Jiewen Tan 2016-11-26 16:23:39 PST
Comment on attachment 295347 [details]
Patch

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

Thanks, Youenn.

>> Source/WebCore/bindings/js/JSSubtleCryptoCustom.cpp:557
>> +    auto exceptionCallback = [capturedPromise = promise.copyRef()](ExceptionCode ec) mutable {
> 
> You can probably move the promise here.

Hmmm, you are right.

>> Source/WebCore/crypto/SubtleCrypto.idl:36
>> +    [Custom] Promise<any> sign(AlgorithmIdentifier algorithm, CryptoKey key, BufferSource data);
> 
> Can we try making it non custom?
> This would probably require adding converters for AlgorithmIdentifier and CryptoKey but the code is probably already in JSSubtleCryptoCustom.

CryptoKey is an interface, it should be fine. However, AlgorithmIdentifier equals any here. I don't think it is good to make it an interface. Or if I miss some ways that we could add a customized type?
Comment 8 Jiewen Tan 2016-11-27 13:45:59 PST
Created attachment 295462 [details]
Patch
Comment 9 Brent Fulgham 2016-11-28 10:00:33 PST
Comment on attachment 295462 [details]
Patch

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

This patch doesn't seem to apply on EWS. Can you upload a rebaselined version?

> LayoutTests/imported/w3c/WebCryptoAPI/idlharness-expected.txt:51
> +PASS SubtleCrypto interface: calling sign(AlgorithmIdentifier,CryptoKey,BufferSource) on crypto.subtle with too few arguments must throw TypeError 

Yay!
Comment 10 Jiewen Tan 2016-11-28 12:15:08 PST
Created attachment 295511 [details]
Patch
Comment 11 WebKit Commit Bot 2016-11-28 12:17:58 PST
Attachment 295511 [details] did not pass style-queue:


ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmRSASSA_PKCS1_v1_5.cpp:71:  CryptoAlgorithmRSASSA_PKCS1_v1_5::sign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:60:  signRSASSA_PKCS1_v1_5 is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:85:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:105:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSASSA_PKCS1_v1_5GnuTLS.cpp:38:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 5 in 44 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 12 Jiewen Tan 2016-11-28 12:29:33 PST
Created attachment 295515 [details]
Patch
Comment 13 WebKit Commit Bot 2016-11-28 12:32:28 PST
Attachment 295515 [details] did not pass style-queue:


ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmRSASSA_PKCS1_v1_5.cpp:71:  CryptoAlgorithmRSASSA_PKCS1_v1_5::sign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:60:  signRSASSA_PKCS1_v1_5 is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:85:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:105:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSASSA_PKCS1_v1_5GnuTLS.cpp:38:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 5 in 45 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 14 Jiewen Tan 2016-11-28 13:02:55 PST
Created attachment 295519 [details]
Patch
Comment 15 WebKit Commit Bot 2016-11-28 13:06:48 PST
Attachment 295519 [details] did not pass style-queue:


ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmRSASSA_PKCS1_v1_5.cpp:71:  CryptoAlgorithmRSASSA_PKCS1_v1_5::sign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:60:  signRSASSA_PKCS1_v1_5 is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:85:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:105:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSASSA_PKCS1_v1_5GnuTLS.cpp:38:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 5 in 45 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 16 Jiewen Tan 2016-11-28 20:17:06 PST
Created attachment 295575 [details]
Patch
Comment 17 WebKit Commit Bot 2016-11-28 20:19:08 PST
Attachment 295575 [details] did not pass style-queue:


ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmRSASSA_PKCS1_v1_5.cpp:71:  CryptoAlgorithmRSASSA_PKCS1_v1_5::sign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:60:  signRSASSA_PKCS1_v1_5 is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:85:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:107:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSASSA_PKCS1_v1_5GnuTLS.cpp:38:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 5 in 45 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 18 Build Bot 2016-11-28 22:07:53 PST
Comment on attachment 295575 [details]
Patch

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

New failing tests:
intersection-observer/intersection-observer-entry-interface.html
Comment 19 Build Bot 2016-11-28 22:07:57 PST
Created attachment 295583 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6
Comment 20 Brent Fulgham 2016-11-29 09:26:25 PST
Comment on attachment 295575 [details]
Patch

I think this looks good. Can you investigate the ios-sim failure and see whether that's a real bug? Otherwise, r=me.
Comment 21 Jiewen Tan 2016-11-29 10:50:19 PST
(In reply to comment #20)
> Comment on attachment 295575 [details]
> Patch
> 
> I think this looks good. Can you investigate the ios-sim failure and see
> whether that's a real bug? Otherwise, r=me.
According to the test history:
https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=intersection-observer%2Fintersection-observer-entry-interface.html

I think the test just somehow fails all the time regardless of my patch.
Comment 22 Jiewen Tan 2016-11-29 10:50:52 PST
(In reply to comment #20)
> Comment on attachment 295575 [details]
> Patch
> 
> I think this looks good. Can you investigate the ios-sim failure and see
> whether that's a real bug? Otherwise, r=me.

Thanks Brent for r+ my patch.
Comment 23 Jiewen Tan 2016-11-29 13:21:17 PST
Created attachment 295629 [details]
Patch for landing
Comment 24 Jiewen Tan 2016-11-29 14:47:57 PST
Created attachment 295645 [details]
Patch for landing
Comment 25 WebKit Commit Bot 2016-11-29 14:50:48 PST
Attachment 295645 [details] did not pass style-queue:


ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmRSASSA_PKCS1_v1_5.cpp:71:  CryptoAlgorithmRSASSA_PKCS1_v1_5::sign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:60:  signRSASSA_PKCS1_v1_5 is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:85:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:107:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSASSA_PKCS1_v1_5GnuTLS.cpp:38:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 5 in 45 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 26 Jiewen Tan 2016-11-29 14:52:36 PST
Committed r209092: <http://trac.webkit.org/changeset/209092>