TLS insecure fallback fails intermittently

RESOLVED FIXED in Firefox 46

Status

()

Core
Networking: HTTP
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: emk, Assigned: emk)

Tracking

unspecified
mozilla47
Points:
---

Firefox Tracking Flags

(firefox46 fixed, firefox47 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

3 years ago
Created attachment 8712126 [details] [diff] [review]
update_whitelist

Steps to reproduce:
1. Apply the attachment.
2. Run the following command:
   <path/to/xpcshell> genIntolerantFallbackList.js IntolerantFallbackList.h
3. Search the log file (IntolerantFallbackList.errors) for NS_ERROR_NET_RESET.

Actual result:
Many NS_ERROR_NET_RESET will be found.

Expected result:
No NS_ERROR_NET_RESET should be found. The HTTP handler should handle net reset errors gracefully.

Regression range:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=da1d2c63b068&tochange=0ee0565977e1
(Assignee)

Comment 1

3 years ago
Created attachment 8712128 [details] [diff] [review]
revert the network.http.request.max-attempts change

If the server is RC4-only and TLS 1.1 intolerant, PSM will generate the fake net reset error three times:
1. for TLS 1.2 without RC4,
2. for TLS 1.2 with RC4, and
3. for TLS 1.1 with RC4.

Moreover, if the 4th request fails due to network glitch, PSM will forget the intolerance info and retry the fallback sequence. That is, three more fake net reset errors.

So network.http.request.max-attempts=3 is a bit too low.
Assignee: nobody → VYV03354
Status: NEW → ASSIGNED
Attachment #8712128 - Flags: review?(mcmanus)
10 is really a ridiculous number for the http logic. I would take a patch at a compromise of 6(?)

even better maybe we ought to change the code to something like ERROR_NET_INTERNAL_RESET in both necko and psm and have that not count against the limit of 3
Attachment #8712128 - Flags: review?(mcmanus) → review-
(Assignee)

Comment 3

3 years ago
Created attachment 8712625 [details] [diff] [review]
Increase the max retry attempt number

OK, 6 seems to be enough for https.
I made the change minimal because I would like to uplift this to 46.
Attachment #8712128 - Attachment is obsolete: true
Attachment #8712625 - Flags: review?(mcmanus)
Attachment #8712625 - Flags: review?(mcmanus) → review+

Comment 5

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b9ca7f65cdb7
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox47: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
(Assignee)

Comment 7

3 years ago
Comment on attachment 8712625 [details] [diff] [review]
Increase the max retry attempt number

Approval Request Comment
[Feature/regressing bug #]: bug 1236277
[User impact if declined]: Sometimes users will see error page when connecting to outdated https server.
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: Very low, pref change only. The new value is still lower than the pre-bug 1236277 value.
[String/UUID change made/needed]: none
Attachment #8712625 - Flags: approval-mozilla-aurora?
Comment on attachment 8712625 [details] [diff] [review]
Increase the max retry attempt number

Minor tweak to number of retries for fallback, please uplift to aurora.
Attachment #8712625 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Comment 9

3 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/c68f5e9a431a
status-firefox46: --- → fixed
You need to log in before you can comment on or make changes to this bug.