1305561 - CloudFlare triggers unsafe negotiation warning with TLS 1.3

Status: RESOLVED FIXED

(Core :: Security: PSM, defect, P1)

Description

[sjw]

STR:
Set the following values in about:config:
security.tls.version.max = 4
security.ssl.require_safe_negotiation = true
security.ssl.treat_unsafe_negotiation_as_broken = true

Browse to a site that is served by CloudFlare and has TLS 1.3 enabled, e.g. https://blog.cloudflare.com/


Expected results:
If CloudFlare supports secure renegotiation in TLS 1.3 the warning should not be shown.

If CloudFlare does not support secure renegotiation in TLS 1.3 the site should not load at all, because of require_safe_negotiation.


Actual results:
The lock icon will get a warning triangle and the site info panel says, the site uses weak encryption. But the site loads fine.
TLS 1.2 works fine without any warning.

Comment 1

[sjw]

Note: sites that are using mod_nss also work fine for me (e.g. https://tls13.crypto.mozilla.org/)

Comment 2

[Masatoshi Kimura [:emk]]

TLS 1.3 killed renegotiation. We should rather suppress warning if the negotiated TLS version is >=1.3.

Comment 3

[Masatoshi Kimura [:emk]]

Attachment: Bug 1305561 - Do not check renego extension when TLS version is 1.3.

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

Comment 4

[Masatoshi Kimura [:emk]]

Comment on attachment 8795242 [details]
Bug 1305561 - Do not check renego extension when TLS version is 1.3.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81366/diff/1-2/

Comment 5

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

Comment on attachment 8795242 [details]
Bug 1305561 - Do not check renego extension when TLS version is 1.3.

https://reviewboard.mozilla.org/r/81366/#review80072

Looks good. Just the one comment.

::: security/manager/ssl/nsNSSCallbacks.cpp:1192
(Diff revision 2)
>            cipherInfo.symCipher);
>      }
>    }
>  
>    PRBool siteSupportsSafeRenego;
> +  if (channelInfo.protocolVersion <= SSL_LIBRARY_VERSION_TLS_1_2) {

I think it might be safer to make this `if (channelInfo.protocolVersion != SSL_LIBRARY_VERSION_TLS_1_3) {`

Comment 6

[Masatoshi Kimura [:emk]]

Comment on attachment 8795242 [details]
Bug 1305561 - Do not check renego extension when TLS version is 1.3.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81366/diff/2-3/

Comment 7

[Masatoshi Kimura [:emk]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ab597e6f1483

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bf025a49293e
Do not check renego extension when TLS version is 1.3. r=keeler

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/bf025a49293e