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

RESOLVED FIXED in Firefox 48

Status

()

Core
Security: PSM
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: emk, Assigned: emk)

Tracking

unspecified
mozilla48
Points:
---

Firefox Tracking Flags

(firefox48 fixed)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

2 years ago
I haven't inspected the reason yet.
(Assignee)

Updated

2 years ago
(Assignee)

Comment 1

2 years ago
This is not limited to TLS 1.3.
No longer blocks: 1250568
Summary: RC4 inappropriately fallbacks if TLS 1.3 is enabled after bug 1253166 → RC4 inappropriately fallbacks if the fallback limit version is lower than the max enabled version
(Assignee)

Comment 2

2 years ago
Created attachment 8727788 [details] [diff] [review]
Do not check the fallback limit version for the RC4 fallback
Assignee: nobody → VYV03354
Status: NEW → ASSIGNED
Attachment #8727788 - Flags: review?(dkeeler)
(Assignee)

Comment 3

2 years ago
Created attachment 8727789 [details] [diff] [review]
Do not check the fallback limit version for the RC4 fallback
Attachment #8727788 - Attachment is obsolete: true
Attachment #8727788 - Flags: review?(dkeeler)
Attachment #8727789 - Flags: review?(dkeeler)
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).
Attachment #8727789 - Flags: review?(dkeeler) → review+
(Assignee)

Comment 5

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

Comment 6

2 years ago
(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

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/25d4e570f1d9
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.