1306003 - Allow secp521r1 as named curve in TLS 1.3 by default

Status: RESOLVED FIXED

(Core :: Security: PSM, defect)

Description

[Darkspirit]

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.

Comment 1

[Darkspirit]

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.

Comment 2

[Dana Keeler (she/her) [:keeler]]

:mt - thoughts?

Comment 3

[Martin Thomson [:mt:]]

Attachment: Bug 1306003 - Enable P-521,

Review commit: https://reviewboard.mozilla.org/r/81928/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/81928/

Comment 4

[Alex]

There's also bug 1129077 about removing P-521, and Chrome has already dropped support for it.

Comment 5

[Boris Zbarsky [:bzbarsky]]

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).

Comment 7

[Boris Zbarsky [:bzbarsky]]

[Tracking Requested - why for this release]: Please transfer tracking flags when marking duplicates...

Comment 8

[Eric Rescorla (:ekr)]

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

Comment 9

[Darkspirit]

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.

Comment 10

[Richard Barnes [:rbarnes]]

Comment on attachment 8795952 [details]
Bug 1306003 - Enable P-521,

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

Comment 11

[Martin Thomson [:mt:]]

The web-compat profile is different for WebRTC.

Comment 12

[Martin Thomson [:mt:]]

Comment on attachment 8795952 [details]
Bug 1306003 - Enable P-521,

Two reviews will do, thanks.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/51cbff25e017
Enable P-521, r=ekr,rbarnes

Comment 14

[Darkspirit]

(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

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/51cbff25e017

Comment 16

[Marcia Knous [:marcia]]

Tracking 52+.