1224875 - Enable TLS extended master secret

Status: RESOLVED FIXED

(Core :: Security: PSM, defect)

Description

[Martin Thomson [:mt:]]

SSL_ENABLE_EXTENDED_MASTER_SECRET is now available.  We should turn it on.

Comment 1

[Masatoshi Kimura [:emk]]

Attachment: Enable extended master secret

Comment 2

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Comment on attachment 8697414 [details] [diff] [review]
Enable extended master secret

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

r=me

::: security/manager/ssl/nsNSSComponent.cpp
@@ +1078,5 @@
>    SSL_OptionSetDefault(SSL_ENABLE_ALPN,
>                         Preferences::GetBool("security.ssl.enable_alpn",
>                                              ALPN_ENABLED_DEFAULT));
>  
> +  SSL_OptionSetDefault(SSL_ENABLE_EXTENDED_MASTER_SECRET, true);

Let's actually put this with the safe negotiation/renegotiation options a few lines up.

Comment 3

[Masatoshi Kimura [:emk]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2767f381c592989277a68c7670eef919722b9f34
Bug 1224875 - Enable TLS extended master secret. r=keeler

Comment 4

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/2767f381c592

Comment 5

[Matt Wobensmith [:mwobensmith][:matt:]]

This change appears to break a few sites with the error SSL_ERROR_HANDSHAKE_UNEXPECTED_ALERT.

apps.reg.uga.edu
appserver.lasalle.edu.co
correo.uchile.cl

Comment 6

[Martin Thomson [:mt:]]

Is that enough to justify putting this behind a pref?

Comment 7

[Masatoshi Kimura [:emk]]

Why are other browsers unaffected? I thought Firefox was the last browser that implemented Extended Master Secret.

Comment 8

[Matt Wobensmith [:mwobensmith][:matt:]]

FWIW, the three sites in comment 5 all work fine in latest Chrome.

Comment 9

[Masatoshi Kimura [:emk]]

Also works with IE11/Edge, despite that MS15-121 introduced Extended Master Secret. It's the reason I'm asking the question.

Comment 10

[Martin Thomson [:mt:]]

I cant see why Chrome would succeed unless it was an extension ordering problem.  Chrome includes two more extensions than we do and more cipher suites.

Comment 11

[Masatoshi Kimura [:emk]]

AAR, we should not continue discussion in a fixed bug. Filed bug 1243641.

Comment 12

[Masatoshi Kimura [:emk]]

Attachment: backout patch

Approval Request Comment
[Feature/regressing bug #]: this bug
[User impact if declined]: Users cannot connect some secure servers.
[Describe test coverage new/current, TreeHerder]: no, backout
[Risks and why]: extremely low. only revert changes to enables an options that had not been enabled.
[String/UUID change made/needed]: none

Comment 13

[Eric Rescorla (:ekr)]

Rather than backing this out, I would prefer to reorder the extensions as 1243641

Comment 14

[Masatoshi Kimura [:emk]]

Then bug 1243641 should be backported to NSS 3.22 (for Firefox 46) and 3.21 (for Firefox 45).

Comment 15

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8722936 [details] [diff] [review]
backout patch

We (nss ml) agreed to take that for beta, the feature should be enabled for 46.
Should be in 45 beta 10

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out from beta (45) in https://hg.mozilla.org/releases/mozilla-beta/rev/86b07e82b741 

Unsure if the status flag should go to 'affected' or 'wontfix'...

Comment 17

[Byron Campen [:bwc]]

Looks like this one fell through the cracks. Are we ready to re-land this, now that bug 1243641 is fixed?

Comment 18

[Byron Campen [:bwc]]

Hmm, I see the flag enabled here: https://searchfox.org/mozilla-central/rev/3a61fb322f74a0396878468e50e4f4e97e369825/security/manager/ssl/nsNSSComponent.cpp#1744

Was this not backed out after all?

Comment 19

[J.C. Jones [:jcj] (he/him)]

It was only backed out for Firefox 45 - there wasn't a backout on mozilla-central that I see, so 46 had it.