Closed Bug 498311 Opened 12 years ago Closed 11 years ago

HTTPS site is marked TLS intolerant and SSLv2 handshake is used when the stop loading button is clicked

Categories

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

1.9.0 Branch
defect

Tracking

()

RESOLVED FIXED
Tracking Status
status2.0 --- ?
status1.9.2 --- ?
status1.9.1 --- ?

People

(Reporter: vijairaj.r, Assigned: mayhemer)

References

()

Details

(Whiteboard: [psm-tcpip])

Attachments

(2 files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.11) Gecko/2009060215 Firefox/3.0.11 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.11) Gecko/2009060215 Firefox/3.0.11 (.NET CLR 3.5.30729)

When a https site is being loaded and the "stop loading" button is clicked, then SSLv2 is used for next connection even though the site supports TLS1.0

Reproducible: Always

Steps to Reproduce:
1.Visit a https site which supports TLS1.0
2.Click on the "Stop loading" button when the url is still being loaded. (The best time would be after the Client hello and before a Server hello)
3.Visit the same site again
4.Check the Wireshark logs
Actual Results:  
Firefox started using SSLv2 handshake for the second connection instead of TLSv1.
If the server supports only TLSv1 then it's not possible to use FF at all.

Expected Results:  
Firefox should should not switch to SSLv2 if server supports TLSv1.

https sites should be marked TLS intolerant only if the server terminated the connection and not if the client closes the connection.
Attached is a Wireshark log during the scenario.
Please note pkt#5 when Firefox initiates the close before a "Server hello"
Not an NSS problem.
Assignee: nobody → kaie
Component: Libraries → Security: PSM
Product: NSS → Core
QA Contact: libraries → psm
Version: unspecified → 1.9.0 Branch
Confirming.
Assignee: kaie → honzab.moz
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
No need to remember the site as intolerant when we are originators of the connection break out. This code is called only from PR_Close().
Attachment #386841 - Flags: review?(kaie)
I can confirm that the patch v1 fixes this issue.
Kai, please review this patch, soon.
Priority: -- → P1
Blocks: 508291
Blocks: 412834
> No need to remember the site as intolerant when we are originators of the
> connection break out. This code is called only from PR_Close().

But, won't we always go through PR_Close, even when the other side closed it first?
Blocks: 450280
Whiteboard: [psm-tcpip]
(In reply to comment #7)
> > No need to remember the site as intolerant when we are originators of the
> > connection break out. This code is called only from PR_Close().
> 
> But, won't we always go through PR_Close, even when the other side closed it
> first?

We always get PR_Close when socket is about to vanish from any reason.  However it doesn't directly implies an error (like tls intolerance implication) even there is failure condition set on the socket.

Errors that show for a tls intolerant server and happens during handshake are all detected in checkHandshake - called after each read or write op, what is the correct and only place to catch such errors.

If PR_Close gets called before we even try to write or read on the socket, then it is because the server is e.g. completely down, there were a name resolution error or such.  That cannot tell us if the server is TLS intolerant as we didn't talk to it yet.

If the server closes the connection after we try to send something to it, or before we read (what would imply TLS intolerance), then checkHandshake catches it.  Calling rememberPossibleTLSProblemSite from PR_Close is redundant.
Comment on attachment 386841 [details] [diff] [review]
v1 [Checkin comment 10]

ok, you've convinced me, thanks for your thoughts

r=kaie
Attachment #386841 - Flags: review?(kaie) → review+
Keywords: checkin-needed
Comment on attachment 386841 [details] [diff] [review]
v1 [Checkin comment 10]

http://hg.mozilla.org/mozilla-central/rev/de4f82330aeb
Attachment #386841 - Attachment description: v1 → v1 [Checkin comment 10]
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
status1.9.1: --- → ?
status1.9.2: --- → ?
status2.0: --- → ?
Resolution: --- → FIXED
Now I'm thinking of modifying ssltunnel to support tests for this...
Flags: in-testsuite?
Keywords: checkin-needed
Flags: in-testsuite? → in-testsuite-
You need to log in before you can comment on or make changes to this bug.