1290037 - Update minimum keybits in H2

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[Franziskus Kiefer [:franziskus]]

Attachment: curve-ff.patch

H2 checks the ECDHE curve size in Http2Session::ConfirmTLSProfile. When enabling curve25519 this has to lowered to 255 bits.
Note that this curve offers 128 bit security as well. (Using keybits for checking the security isn't ideal anyway, but the best we can do at the moment.)

Comment 1

[Franziskus Kiefer [:franziskus]]

Attachment: curve-ff.patch

the comment should be updated as well ...

Comment 2

[u408661]

You should probably update the logging below the if, too :)

Comment 3

[Franziskus Kiefer [:franziskus]]

Attachment: curve-ff.patch

> You should probably update the logging below the if, too :)

Comment 4

[Patrick McManus [:mcmanus]]

Comment on attachment 8775588 [details] [diff] [review]
curve-ff.patch

Review of attachment 8775588 [details] [diff] [review]:
-----------------------------------------------------------------

thats not a detail I can confirm - but I know who can :)

Comment 5

[Martin Thomson [:mt:]]

Comment on attachment 8775588 [details] [diff] [review]
curve-ff.patch

Review of attachment 8775588 [details] [diff] [review]:
-----------------------------------------------------------------

This is fine.  However, if we go with 4Q I understand that this is further reduced, which would fail again.  RFC 7540 sets the limit to 224 bits, which seems like a limit we could get behind.

Comment 6

[Franziskus Kiefer [:franziskus]]

Attachment: curve-ff.patch

ok, then let's use 224

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/361ac226da2a
Update keybits in H2, r=mt

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/361ac226da2a

Comment 9

[Jan Beich]

Comment on attachment 8778661 [details] [diff] [review]
curve-ff.patch

Franziskus, are there (runtime) risks associated with backporting the patch to older Firefox releases?

[Approval Request Comment]
ESR consideration: Help downstream ubnundle NSS in case some secfixes don't or cannot trickle in ESR45 or its consumers or they have a lot of patches for Tier3 support.
User impact if declined: Broken HTTP/2 for --with-system-nss + NSS 3.28 builds
Fix Landed on Version: Firefox 51
Risk to taking this patch (and alternatives if risky): None build-wise and *probably* not for old bundled NSS, to be confirmed by :fkiefer
String or UUID changes made by this patch: None

Comment 11

[Franziskus Kiefer [:franziskus]]

There are no risks in uplifting this. The patch only allows smaller curves to be used in H2. By default NSS only provides one curve (25519) that is smaller. Because this is the default curve since NSS 3.28 we should uplift this fix to all branches to unbreak H2 with system NSS.

Comment 12

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8778661 [details] [diff] [review]
curve-ff.patch

It makes sense to take this in ESR45 (got a second opinion from Al/Dan), ESR45.7+

Comment 13

[Patrick McManus [:mcmanus]]

mt, does this also close bug 1224948 ?

Comment 14

[Martin Thomson [:mt:]]

No, this only affects ECDHE.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr45/rev/bf0dd9ae6807