Allow secp521r1 as named curve in TLS 1.3 by default

RESOLVED FIXED in Firefox 52

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: darkspirit, Unassigned)

Tracking

Trunk
mozilla52
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox50 unaffected, firefox51-, firefox52+ fixed)

Details

Attachments

(1 attachment)

58 bytes, text/x-review-board-request
ekr
: review+
rbarnes
: review+
Details
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0
Build ID: 20160928030201

Steps to reproduce:

about:buildconfig -> https://hg.mozilla.org/mozilla-central/rev/66a77b9bfe5dcacd50eccf85de7c0e7e15ce0ffd


Actual results:

Missing P-521 here

https://reviewboard.mozilla.org/r/80636/diff/3#1
// These are the named groups that we will allow.
static const SSLNamedGroup NamedGroupPreferences[] = {
  ssl_grp_ec_curve25519,
  ssl_grp_ec_secp256r1,
  ssl_grp_ec_secp384r1,
  ssl_grp_ffdhe_2048,
  ssl_grp_ffdhe_3072
};


Expected results:

Please value this comment https://bugzilla.mozilla.org/show_bug.cgi?id=1128792#c15
You are weakening security (defaults) with no need.
Blocks: 1304919
OS: Unspecified → All
Hardware: Unspecified → All
You can stop announce P-521 be default, only if https://bugzilla.mozilla.org/show_bug.cgi?id=1305243 Curve 448 has been implemented and if Let's Encrypt signs Curve 448 Certificates and if other browsers speak that curve too.
Otherwise you would force people to lower their security on the move from TLS 1.2 to TLS 1.3.
https://www.keylength.com/en/3/ Level 8 is desired.
Summary: Allow P521 as named curve in TLS 1.3 by default → Allow secp521r1 as named curve in TLS 1.3 by default
:mt - thoughts?
Flags: needinfo?(martin.thomson)
Comment hidden (mozreview-request)

Comment 4

3 years ago
There's also bug 1129077 about removing P-521, and Chrome has already dropped support for it.
See https://bugzilla.mozilla.org/show_bug.cgi?id=1306085#c1 for the problems dropping P-521 causes, when combined with our preference for forward secrecy (which Chrome either doesn't have, or implements differently).
Duplicate of this bug: 1306085
[Tracking Requested - why for this release]: Please transfer tracking flags when marking duplicates...
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 8

3 years ago
mozreview-review
Comment on attachment 8795952 [details]
Bug 1306003 - Enable P-521,

https://reviewboard.mozilla.org/r/81928/#review80522

::: security/manager/ssl/nsNSSIOLayer.cpp:2501
(Diff revision 1)
>    const SSLNamedGroup namedGroups[] = {
>      ssl_grp_ec_curve25519, ssl_grp_ec_secp256r1, ssl_grp_ec_secp384r1,
> -    ssl_grp_ffdhe_2048, ssl_grp_ffdhe_3072
> +    ssl_grp_ec_secp521r1, ssl_grp_ffdhe_2048, ssl_grp_ffdhe_3072
>    };
>    if (SECSuccess != SSL_NamedGroupConfig(fd, namedGroups,

LGTM
Attachment #8795952 - Flags: review+
Thank you. But this patch isn't including secp521r1 for the use with DTLS (TLS 1.3) in /media/mtransport/transportlayerdtls.cpp as introduced with https://reviewboard.mozilla.org/r/80636/diff/3#1 and mentioned in my #c0. Would be great to see it there too.
The web-compat profile is different for WebRTC.
Flags: needinfo?(martin.thomson)
Comment on attachment 8795952 [details]
Bug 1306003 - Enable P-521,

Two reviews will do, thanks.
Attachment #8795952 - Flags: review?(dkeeler)
(In reply to Martin Thomson [:mt:] from comment #11)
> The web-compat profile is different for WebRTC.

Are you saying that secp521r1 was not avaiable in WebRTC/DTLS 1.2? A compatible (or server preferred) curve should be selected through negotiation/handshake. Would there be any reason for a limitation here? If incompatibilites expected: Why?

Comment 15

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/51cbff25e017
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.