Closed Bug 1242926 Opened 4 years ago Closed 4 years ago

TLS insecure fallback fails intermittently

Categories

(Core :: Networking: HTTP, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox46 --- fixed
firefox47 --- fixed

People

(Reporter: emk, Assigned: emk)

References

Details

Attachments

(2 files, 1 obsolete file)

Attached patch update_whitelistSplinter Review
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
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-
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+
https://hg.mozilla.org/mozilla-central/rev/b9ca7f65cdb7
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
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+
You need to log in before you can comment on or make changes to this bug.