1254306 - RC4 inappropriately fallbacks if the fallback limit version is lower than the max enabled version

Status: RESOLVED FIXED

(Core :: Security: PSM, defect)

Description

[Masatoshi Kimura [:emk]]

I haven't inspected the reason yet.

Comment 1

[Masatoshi Kimura [:emk]]

This is not limited to TLS 1.3.

Comment 2

[Masatoshi Kimura [:emk]]

Attachment: Do not check the fallback limit version for the RC4 fallback

Comment 3

[Masatoshi Kimura [:emk]]

Attachment: Do not check the fallback limit version for the RC4 fallback

Comment 4

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

Comment on attachment 8727789 [details] [diff] [review]
Do not check the fallback limit version for the RC4 fallback

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

Ok - check my understanding here: right now, if the fallback limit is less than the maximum enabled version, RC4 fallback will occur for no cipher overlap, connection reset, and end-of-file errors. I suppose that's not exactly what we want. This change will make it so we only do RC4 fallback if the unrestricted RC4 fallback pref is set or if the site in question is on the fallback list.

::: security/manager/ssl/nsNSSIOLayer.cpp
@@ +1798,5 @@
>    }
>  }
>  
> +bool
> +nsSSLIOLayerHelpers::isInsecureFallbackSite(const nsACString& hostname)

Might update nsSSLIOLayerHelpers::fallbackLimitReached to call this (I know it only saves one line of code, but still).

Comment 5

[Masatoshi Kimura [:emk]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/25d4e570f1d983ecc112e835d8a23eeafa8cd684
Bug 1254306 - Do not check the fallback limit version for the RC4 fallback. r=keeler

Comment 6

[Masatoshi Kimura [:emk]]

(In reply to David Keeler [:keeler] (use needinfo?) from comment #4)
> Ok - check my understanding here: right now, if the fallback limit is less
> than the maximum enabled version, RC4 fallback will occur for no cipher
> overlap, connection reset, and end-of-file errors. I suppose that's not
> exactly what we want. This change will make it so we only do RC4 fallback if
> the unrestricted RC4 fallback pref is set or if the site in question is on
> the fallback list.

Correct.

> > +bool
> > +nsSSLIOLayerHelpers::isInsecureFallbackSite(const nsACString& hostname)
> 
> Might update nsSSLIOLayerHelpers::fallbackLimitReached to call this (I know
> it only saves one line of code, but still).

Done.

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/25d4e570f1d9