Bug 216755

Summary: [WebCrypto] Support Elliptic Curve P-521 on Mac
Product: WebKit Reporter: Jonas Finnemann Jensen <jopsen>
Component: WebCore Misc.Assignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, bfulgham, cdumez, darin, ggaren, jiewen_tan, katherine_cheney, sam, webkit-bug-importer, yoshiaki.jitsukawa
Priority: P2 Keywords: InRadar
Version: Safari 13   
Hardware: Mac   
OS: macOS 10.15   
Attachments:
Description Flags
WIP Patch
none
WIP Patch
ews-feeder: commit-queue-
WIP Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Jonas Finnemann Jensen 2020-09-20 15:44:02 PDT 
WebKit on Mac built with CommonCrypto does not support Elliptic Curve P-521 in WebCrypto for ECDSA and ECDH.
The code [2] only allows P-256 or P-384.

Lack of support for P-521 is noted in the "Update on Web Cryptography" blog post [1], but the bug 169231 was resolved without
addressing support for P-521 on Mac with according to [Bug 169231 Comment 4][3] because of missing support in CommonCrypto.

Leaving out P-521 support might be reasonable, I'm not an expert on the strength of these primitives.
But I imagine that supporting it would be nice for compatibility with Firefox and Chrome, both of which support P-521.

Feel free to reopen bug 169231, and close this as a duplicate :)
(sorry, for the noise, but this seems like a compatibility issue worth tracking)

[1]: https://webkit.org/blog/7790/update-on-web-cryptography/
[2]: https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/crypto/mac/CryptoKeyECMac.cpp?rev=239692#L105
[3]: https://bugs.webkit.org/show_bug.cgi?id=169231#c4
Comment 1 Radar WebKit Bug Importer 2020-09-27 15:44:14 PDT 
<rdar://problem/69667220>
Comment 2 Chris Dumez 2021-07-06 19:56:08 PDT 
Created attachment 432998 [details]
WIP Patch
Comment 3 Chris Dumez 2021-07-06 20:02:14 PDT 
Created attachment 432999 [details]
WIP Patch
Comment 4 Chris Dumez 2021-07-06 20:18:57 PDT 
A couple of tests are crashing in signECDSA(), I will need to figure out why.
Comment 5 Chris Dumez 2021-07-07 09:30:36 PDT 
Created attachment 433036 [details]
WIP Patch
Comment 6 Chris Dumez 2021-07-07 09:35:11 PDT 
Created attachment 433037 [details]
Patch
Comment 7 Chris Dumez 2021-07-07 10:27:44 PDT 
Created attachment 433043 [details]
Patch
Comment 8 Chris Dumez 2021-07-07 13:31:55 PDT 
Created attachment 433065 [details]
Patch
Comment 9 Alex Christensen 2021-07-07 15:14:12 PDT 
Comment on attachment 433065 [details]
Patch

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

> Source/WebCore/crypto/mac/CryptoKeyECMac.cpp:62
> +        return size == 133;

It would be nice if these were given names instead of just magic numbers.  Or they could be calculated from constexpr expressions.

> Source/WebCore/crypto/mac/CryptoKeyECMac.cpp:78
> +        return size == 66;

ditto.
Comment 10 Chris Dumez 2021-07-07 16:20:14 PDT 
Created attachment 433090 [details]
Patch
Comment 11 Chris Dumez 2021-07-07 16:21:01 PDT 
(In reply to Alex Christensen from comment #9)
> Comment on attachment 433065 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=433065&action=review
> 
> > Source/WebCore/crypto/mac/CryptoKeyECMac.cpp:62
> > +        return size == 133;
> 
> It would be nice if these were given names instead of just magic numbers. 
> Or they could be calculated from constexpr expressions.
> 
> > Source/WebCore/crypto/mac/CryptoKeyECMac.cpp:78
> > +        return size == 66;
> 
> ditto.

Fixed. Now computed using constexpr functions instead of hardcoded values.
Comment 12 EWS 2021-07-07 17:25:53 PDT 
Committed r279688 (239493@main): <https://commits.webkit.org/239493@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 433090 [details].