Bug 171287 - [WebCrypto] Enhance ways to convert an ECDSA signature binary into DER format
Summary: [WebCrypto] Enhance ways to convert an ECDSA signature binary into DER format
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:
 
Reported: 2017-04-25 13:53 PDT by Jiewen Tan
Modified: 2017-04-26 00:23 PDT (History)
6 users (show)

See Also:

Attachments
Patch (5.12 KB, patch)
2017-04-25 14:00 PDT, Jiewen Tan 		no flags Details | Formatted Diff | Diff
Patch (3.49 KB, patch)
2017-04-25 14:17 PDT, Jiewen Tan 		bfulgham: review+
Details | Formatted Diff | Diff
Patch for landing (3.49 KB, patch)
2017-04-25 17:42 PDT, Jiewen Tan 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Jiewen Tan 2017-04-25 13:53:07 PDT 
Enhance ways to convert an ECDSA signature binary(r||s) into DER format.
Comment 1 Jiewen Tan 2017-04-25 13:53:39 PDT 
<rdar://problem/31735332>
Comment 2 Jiewen Tan 2017-04-25 14:00:08 PDT 
Created attachment 308138 [details]
Patch
Comment 3 Jiewen Tan 2017-04-25 14:17:13 PDT 
Created attachment 308143 [details]
Patch
Comment 4 Sam Weinig 2017-04-25 15:39:48 PDT 
What does it mean this covered by existing tests? Do those tests change their results?
Comment 5 Jiewen Tan 2017-04-25 15:55:11 PDT 
(In reply to Sam Weinig from comment #4)
> What does it mean this covered by existing tests? Do those tests change
> their results?

crypto/subtle/ecdsa-import-key-verify-p384.html fails in Internal Bots. After this change, it should pass. That's why this change is covered by existing tests. Please see radars for further information.
Comment 6 Brent Fulgham 2017-04-25 17:31:12 PDT 
Comment on attachment 308143 [details]
Patch

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

r=me.

> Source/WebCore/crypto/mac/CryptoAlgorithmECDSAMac.cpp:147
> +    // InitialOctet is needed when the first byte of r/s is larger or equal than 128.

"larger or equal than" -> "greater than or equal to"
Comment 7 Jiewen Tan 2017-04-25 17:40:45 PDT 
Comment on attachment 308143 [details]
Patch

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

Thanks Brent for r+ my patch.

>> Source/WebCore/crypto/mac/CryptoAlgorithmECDSAMac.cpp:147
>> +    // InitialOctet is needed when the first byte of r/s is larger or equal than 128.
> 
> "larger or equal than" -> "greater than or equal to"

Fixed.
Comment 8 Jiewen Tan 2017-04-25 17:42:41 PDT 
Created attachment 308176 [details]
Patch for landing
Comment 9 WebKit Commit Bot 2017-04-25 21:32:01 PDT 
Comment on attachment 308176 [details]
Patch for landing

Clearing flags on attachment: 308176

Committed r215791: <http://trac.webkit.org/changeset/215791>