Bug 164740

Summary:

Update SubtleCrypto::sign to match the latest spec

Product:

WebKit

Reporter:

Jiewen Tan <jiewen_tan>

Component:

WebCore Misc.

Assignee:

Jiewen Tan <jiewen_tan>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bfulgham, cdumez, commit-queue, esprehn+autocc, jiewen_tan, kondapallykalyan, rob, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

160883

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	bfulgham: review+, buildbot: commit-queue-
Archive of layout-test-results from ews125 for ios-simulator-wk2	none
Patch for landing	none
Patch for landing	none

Jiewen Tan

Reported 2016-11-14 15:52:16 PST

Update SubtleCrypto::sign to match the latest spec.

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
Show Obsolete (7) View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2016-11-14 16:40:51 PST

<rdar://problem/29257864>

Jiewen Tan

Comment 2 2016-11-22 15:36:51 PST

Created attachment 295347 [details] Patch

WebKit Commit Bot

Comment 3 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.

youenn fablet

Comment 4 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.

Brent Fulgham

Comment 5 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?

Jiewen Tan

Comment 6 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.

Jiewen Tan

Comment 7 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?

Jiewen Tan

Comment 8 2016-11-27 13:45:59 PST

Created attachment 295462 [details] Patch

Brent Fulgham

Comment 9 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!

Jiewen Tan

Comment 10 2016-11-28 12:15:08 PST

Created attachment 295511 [details] Patch

WebKit Commit Bot

Comment 11 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.

Jiewen Tan

Comment 12 2016-11-28 12:29:33 PST

Created attachment 295515 [details] Patch

WebKit Commit Bot

Comment 13 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.

Jiewen Tan

Comment 14 2016-11-28 13:02:55 PST

Created attachment 295519 [details] Patch

WebKit Commit Bot

Comment 15 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.

Jiewen Tan

Comment 16 2016-11-28 20:17:06 PST

Created attachment 295575 [details] Patch

WebKit Commit Bot

Comment 17 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.

Build Bot

Comment 18 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

Build Bot

Comment 19 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

Brent Fulgham

Comment 20 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.

Jiewen Tan

Comment 21 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.

Jiewen Tan

Comment 22 2016-11-29 10:50:52 PST

Jiewen Tan

Comment 23 2016-11-29 13:21:17 PST

Created attachment 295629 [details] Patch for landing

Jiewen Tan

Comment 24 2016-11-29 14:47:57 PST

Created attachment 295645 [details] Patch for landing

WebKit Commit Bot

Comment 25 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.

Jiewen Tan

Comment 26 2016-11-29 14:52:36 PST

Committed r209092: <http://trac.webkit.org/changeset/209092>

Note You need to log in before you can comment on or make changes to this bug.