Bug 216755 - [WebCrypto] Support Elliptic Curve P-521 on Mac
Summary: [WebCrypto] Support Elliptic Curve P-521 on Mac
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: Safari 13
Hardware: Mac macOS 10.15
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-09-20 15:44 PDT by Jonas Finnemann Jensen
Modified: 2021-07-07 17:25 PDT (History)
10 users (show)

See Also:


Attachments
WIP Patch (68.75 KB, patch)
2021-07-06 19:56 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
WIP Patch (69.67 KB, patch)
2021-07-06 20:02 PDT, Chris Dumez
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
WIP Patch (123.48 KB, patch)
2021-07-07 09:30 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (128.76 KB, patch)
2021-07-07 09:35 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (138.45 KB, patch)
2021-07-07 10:27 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (139.06 KB, patch)
2021-07-07 13:31 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (141.64 KB, patch)
2021-07-07 16:20 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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].