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

Status: RESOLVED FIXED

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

Description

[Vijairaj R]

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.

Comment 1

[Vijairaj R]

Attachment: Wireshark log during the scenario

Attached is a Wireshark log during the scenario.
Please note pkt#5 when Firefox initiates the close before a "Server hello"

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Not an NSS problem.

Comment 3

[Honza Bambas (:mayhemer)]

Confirming.

Comment 4

[Honza Bambas (:mayhemer)]

Attachment: v1 [Checkin comment 10]

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().

Comment 5

[Vijairaj R]

I can confirm that the patch v1 fixes this issue.

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

Kai, please review this patch, soon.

Comment 7

[Kai Engert [:KaiE:]]

> 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?

Comment 8

[Honza Bambas (:mayhemer)]

(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 9

[Kai Engert [:KaiE:]]

Comment on attachment 386841 [details] [diff] [review]
v1 [Checkin comment 10]

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

r=kaie

Comment 10

[Honza Bambas (:mayhemer)]

Comment on attachment 386841 [details] [diff] [review]
v1 [Checkin comment 10]

http://hg.mozilla.org/mozilla-central/rev/de4f82330aeb

Comment 11

[Honza Bambas (:mayhemer)]

Now I'm thinking of modifying ssltunnel to support tests for this...