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

RESOLVED FIXED

Status

()

P1
normal
RESOLVED FIXED
10 years ago
2 years ago

People

(Reporter: vijairaj.r, Assigned: mayhemer)

Tracking

1.9.0 Branch
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(status2.0 ?, status1.9.2 ?, status1.9.1 ?)

Details

(Whiteboard: [psm-tcpip], URL)

Attachments

(2 attachments)

(Reporter)

Description

10 years ago
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.
(Reporter)

Comment 1

10 years ago
Created attachment 383239 [details]
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"
Not an NSS problem.
Assignee: nobody → kaie
Component: Libraries → Security: PSM
Product: NSS → Core
QA Contact: libraries → psm
Version: unspecified → 1.9.0 Branch
(Assignee)

Comment 3

10 years ago
Confirming.
Assignee: kaie → honzab.moz
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
(Assignee)

Comment 4

10 years ago
Created attachment 386841 [details] [diff] [review]
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().
Attachment #386841 - Flags: review?(kaie)
(Reporter)

Comment 5

10 years ago
I can confirm that the patch v1 fixes this issue.
Kai, please review this patch, soon.
Priority: -- → P1
(Assignee)

Updated

9 years ago
Blocks: 508291
(Assignee)

Updated

9 years ago
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?

Updated

9 years ago
Blocks: 450280

Updated

9 years ago
Whiteboard: [psm-tcpip]
(Assignee)

Comment 8

9 years ago
(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+

Updated

9 years ago
Keywords: checkin-needed
(Assignee)

Comment 10

9 years ago
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]
(Assignee)

Updated

9 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
status1.9.1: --- → ?
status1.9.2: --- → ?
status2.0: --- → ?
Resolution: --- → FIXED
(Assignee)

Comment 11

9 years ago
Now I'm thinking of modifying ssltunnel to support tests for this...
Flags: in-testsuite?
Keywords: checkin-needed
(Assignee)

Updated

2 years ago
Flags: in-testsuite? → in-testsuite-
You need to log in before you can comment on or make changes to this bug.