Closed Bug 87902 Opened 23 years ago Closed 22 years ago

Cannot reach TLS intolerant servers through SSL proxy.

Categories

(Core Graveyard :: Security: UI, defect, P1)

1.0 Branch
x86
Windows NT

Tracking

(Not tracked)

VERIFIED FIXED
psm2.2

People

(Reporter: junruh, Assigned: KaiE)

References

()

Details

(Keywords: relnote, Whiteboard: [adt2 RTM] [ETA 06/26])

Attachments

(1 file, 3 obsolete files)

1.) Set SSL proxy to junruh.mcom.com:8080 (AOL in-house)
2.) Visit a TLS intolerant site like https://comhome.comdirect.de/ or 
https://www.gwla.com or https://www.macys.com/

What is expected: A successful connection.
What I get: <html><body></body></html>
setting:
Target -> 2.0
-> javi
P2
Assignee: ssaux → javi
Severity: normal → major
Priority: -- → P2
Target Milestone: --- → 2.0
-> nsbranch

Many users surf via a proxy.  It's important to fix this bug. 

Keywords: nsBranch
Whiteboard: PDT; want plus
-> ddrinan
Assignee: javi → ddrinan
Whiteboard: PDT; want plus → PDT+ want plus
At the PSM layer we cannot re-establish a proxied connection because we do not
know how to build up a proxied connection from scratch (which is what the
fallback method tries to do).  What we really need is to set some error code,
then have necko recognize that error and re-try the connection from scratch
telling us to not use TLS.

Since that would require major surgery, the proposed patch would be preferable
IMO.  When creating an SSL socket, it will check to see if we are going to
connect through a proxy, and if so it will turn off TLS for that connection. 
This is much lower risk and allows us to ship TLS as the default for non-proxied
users.
marek moved this from PDT to PDT+
Whiteboard: PDT+ want plus → PDT+
adding relnote keyword in case we need to release note it (likely I think).
Keywords: relnote
The release notes should say something like this, though someone should feel
free to re-write it.  Maybe it should be joined with the "TLS intolerant server"
section.

When you have selected the "Enable TLS" option from the SSL preferences panel,
the browser will attempt to use the TLS protocol when making secure connections
with a server. If that connection fails because the server is "TLS intolerent",
 the client will fall back to using SSL3.   There is an important exception:  If
you have selected to use a proxy for secure connections, the browser will not
attempt to use TLS.  It will attempt to use SSL3. 
r=ddrinan.
Adding blizzard to cc-list.

Chris,
Please super-review.
sr=brendan@mozilla.org, but please file a sequel bug (or leave this bug open and
retarget-milestone it) for the fix javi described: propagate a distinct result
code to necko and redo the proxied connection without TLS.

/be
New bug filed.
Whiteboard: PDT+ → PDT+, need a=
a= asa@mozilla.org for checkin to 0.9.2 branch.
(on behalf of drivers)
Checked into branch and trunk. Marking FIXED.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Whiteboard: PDT+, need a= → PDT+
Verified.
Status: RESOLVED → VERIFIED
Reopening. 11/30 trunk Win2000 build. Use in-house proxy server 
lab212sun.mcom.com:8080.
I cannot reach TLS intolerant server https://www.delphion.com at all.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
->rangnasen for investigation.
Assignee: ddrinan → rangansen
Status: REOPENED → NEW
Priority: P2 → P1
Target Milestone: 2.0 → 2.2
For me, both https://www.delphion.com and https://comhome.comdirect.de/ seems to
work ok - Trunk Build, Dec 1, evening, WinNT. One of the cases when this
infamous blank page shows up is when the server times out and disconects, which,
in my openion, might be the reason why it showed up in this case.
John - do you still see this problem?
I cannot reach the TLS intolerant server https://www.delphion.com through a 
proxy with Win2000, Win98 or Mac OSX. https://comhome.comdirect.de/ does not 
appear to be TLS intolerant, and thus can be reached through a proxy.
Relnote: When you have selected the "Enable TLS" option from the SSL preferences 
panel, the browser will attempt to use the TLS protocol when making secure 
connections with a server. If that connection fails because the server is "TLS 
intolerent", the client will fall back to using SSL3.   There is an important 
exception:  If you have selected to use a proxy for secure connections, the 
browser will not be able to reach a "TLS intolerent" web site.
*** Bug 146270 has been marked as a duplicate of this bug. ***
Depends on: 149868
The patch in bug 149868 seems to fix this bug.
*** Bug 96024 has been marked as a duplicate of this bug. ***
Resolving as a duplicate, because I believe this is now fixed.
If you still can reproduce, please reopen.

*** This bug has been marked as a duplicate of 149868 ***
Status: NEW → RESOLVED
Closed: 23 years ago22 years ago
No longer depends on: 149868
Resolution: --- → DUPLICATE
Using 2002061303-trunk/Mac OS 9.2 via Squid 2.3 STABLE5-NT,
I cannot reach https://raytheon.benefitcenter.com in Bug 146270,
and https://www.bank128.anser.or.jp/cgi/balance.aic?CCT0080=1559.

If I turn off "Enable TLS", these sites work.
It seems to me the patch is not enough.
Is this another problem ?
Reopening, and confirming HARUNAGA's comment right above. Tested on 6/13 trunk 
Win2000 and Mac OSX. Something has improved here, the 3 sites mentioned 
originally now work, and the sites still appear to be TLS intolerant.
FYI: Raytheon is using this server - 
Axent_Technologies/WebDefender_SecureLink_Bridge/WebDefender_SecureLink_Bridge-2
.0
Bank128 is using this server - iTP Secure WebServer/2.1
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Taking bug.
Assignee: rangansen → kaie
Status: REOPENED → NEW
Attachment #40372 - Attachment is obsolete: true
Attached patch Patch v2 (obsolete) — Splinter Review
I traced this phenomenon, using the servers described in the previous comment.
They both behave identically.

PSM layers itself into the network communication. When going over an proxy, SSL
is not immediately turned on. It gets turned on, as soon as the plaintext
protocol to the proxy is finished, and connection to the real server is
starting.

The TLS intolerant detection uses special logic for the first data write only.
Whether we have seen a data write yet is stored in flag mFirstWrite.

When the network code is about to ask PSM to turn on crypto, it calls a StepUp
function. Within that function, the socket options for encryption are switched
on, and the "mFirstWrite" flag is reset to true, with the intention to
indicate, that no data has been transfered yet.

However, this method also contains an additional action. It "writes zero bytes"
to the network connection. This is described as being a workaround for bug
56924. My understanding is, the intention is to trigger the start of the SSL
handshake immediately.

The problem is: The "write zero bytes" call is done before the mFirstWrite flag
is reset to true.

So what happens is:
- the SSL handshake gets triggered
- the network connection to the remote server is starting
- the first bytes get written, but PSM is not currently aware that it is seeing
the first write, because the flag is not set. Therefore the error code gets
handled incorrectly.
- next the flag gets that, but it is too late

I changed the code to change that order. I moved setting the mFirstWrite flag
before triggering the handshake by executing the zero bytes write.

Using that combination, I can successfully connect to all sites, with or
without proxy.

I was even able to connect to any site by removing that zero bytes write call
completely.

I think we were unable to see this solution earlier, because of bug 149868.
But now, this fix seems obvious to me.

I'm attaching a patch that reorders the code as I described. I think we should
start with that code now, and if it works on the trunk, we should also land it
on the trunk.

(In a future step, we should also remove that zero bytes write call completely
on the trunk - because the comment in the code says, the only reason for that
call was to work around a known problem in NSS, and to my understanding, that
problem has been fixed.)
(One should reread comments before submitting, not after).

Typos:
 next the flag gets that, but it is too late
                     ^^^^
                         => set

I'm attaching a patch that reorders the code as I described. I think we should
start with that code now, and if it works on the trunk,
we should also land it on the trunk.
                              ^^^^^
                                   => 1.0 branch.
Status: NEW → ASSIGNED
Comment on attachment 88937 [details] [diff] [review]
Patch v2

r=javi
Attachment #88937 - Flags: review+
Comment on attachment 88937 [details] [diff] [review]
Patch v2

sr=darin (how did TLS intolerant server detection ever work before?  or was
this only a recent regression?)
Attachment #88937 - Flags: superreview+
> how did TLS intolerant server detection ever work before?
> or was this only a recent regression?)

- what we are patching here is only the tls-intolerant-over-proxy code

- tls-intolerant without proxy has been already working

- tls-intolerant-over-proxy did not really work, as it could be seen in 96024,
for example. But in the end, the general case https-over-proxy did not work well
at all (as recently fixed with bug 149868). Before that, we weren't really able
to successfully understand and solve the special case tls-intolerant-over-proxy.

Short answer: tls-inteloerant-over-proxy probably never has been working reliably.

I hope this patch fixes our problems. I will now check it in to the trunk, and
we should do testing with many sites (only testing using proxy required).

(If for some reason, this simple patch is not the whole part of the solution, we
might again look at the attempt I started in the patches in bug 96024. Darin, we
already attempted to work on that in the past, but all are our attempts were
guaranteed to fail because of 149868.)
Checked in to trunk.
Marking fixed, third attempt.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago22 years ago
Resolution: --- → FIXED
Verified on 6/25 Win2000 trunk. I tested the TLS intolerant sites listed in
tracking bug 59321 with the 6/24 trunk build, but all of the sites have
apparently upgraded their servers in the last year, and work with the 6/24 trunk
build through a proxy. I did find one server which is still TLS intolerant -
https://webmail.wittenberg.edu/. I cannot reach it with the 6/24 build, but can
with the 6/25 trunk build.
www.netcraft.com/sslwhats identifies the server as "unknown", and it is using
the cipher RC4 with MD5 (export version restricted to 40-bit key).
Marking this one as verified per Comment #35 From junruh@netscape.com.
Blocks: 143047
Keywords: adt1.0.1, nsbeta1+
Whiteboard: PDT+ → [adt2 RTM] [ETA 06/26]
Marking verified on trunk.
Status: RESOLVED → VERIFIED
The powers that be, please review bug 155100 and possibly affirm whether the two
are distinctly related.

Thank you
I think bug 155100 is an unrelated issue, because it seems to me that mail/news
does not use the SSL proxy config to connect to the server.

I finally followed the source to see, where the code is that determines whether
a proxy will be used for a given URL or not. I ended at:
  nsProtocolProxyService::ExamineForProxy

The SSL/HTTPS proxy config is never used for any mail/news protocol. It is only
used for https.

I think the proxy UI is confusing. The UI says "SSL proxy". This implies, it
would be used for any kind of SSL connection. But right now the UI would better
say "https proxy".

So for now, bug 155100 can not be related to this bug.
alternatively all ssl connections should use the proxy information.
adding adt1.0.1-.  Let's look at this for the next release.
Keywords: adt1.0.1adt1.0.1-
This has created a very nasty SSL-SMTP regression: trying to send mail via
SSL/SMTP hangs forever, and if you cancel, the rest of necko is subsequently
completely horked (see bug 155431).  Unless there's a good reason not to, I'd
like to back this out.
If it can wait till Kai can take a look I'd prefer that.
Otherwise, go ahead.
I'd argue for backing it out now, for two reasons:

1) the regression seems notably worse than what the patch was intended to fix:
not only is there a loss of connectivity, but necko is left in a non-working state.

2) the patch is so tiny, relanding it shouldn't be hard at all (ie there won't
be any cvs merge conflicts).
Backed out; reopening.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Thanks for your help, Dan.
I think we need to carefully revisit the current logic in PSM's SSL I/O layer.
My understanding is that some people saw the regression 155341 always, while
other's only saw it sometimes. This sounds like timing problems.

This matches what is being reported in bug 153769. The reporter complains about
a race condition in the same code. He suggested a workaround, which "works for
him". But I don't want to try out any more code. Before I make additional
changes, I want to completely understand what's going on.
Attached patch Patch v3 (obsolete) — Splinter Review
Attachment #88937 - Attachment is obsolete: true
My previous attempt to fix this bug failed.
This time I don't want to break it again.


The new patch I just attached tries to fix all of the following:

- make TLS intolerant server detection over proxies work 
  (this bug 87902)
- on failure, only retry without tls when it is really 
  likely to help
  (bug 149910)
- show correct error messages if tls is disabled
- remove obsolete workarounds in SSL i/o layer
  (see removed comments in patch)
- avoid to confuse programmers reading code,
  by renaming TLSStepUp (which means something else)
  to the correct term STARTTLS (what the code is actually doing).
  (As suggested by Nelson)
- if an invalid or expired etc. server certificate is presented,
  a warning is shown. If the user decides to cancel,
  network activity should stop. (we currently warn multiple times)
  (bug 87209)

In order to reach confidence about my patch, I did a lot of testing.

I tested 100 different combinations of configuration and site.

- 20 on Linux without the patch, to document our current incorrect 
  behaviour (only configurations with proxy, as configuration without
  proxy seem to work).
  Note: SMTP/SSL is very similar to a proxy mode connection.

- 40 on Linux with the patch applied

- 40 on Windows with the patch applied

===================
Linux test results:
===================

site / enabled options:          | SSL2 | SSL2 | SSL2 |      |
                                 |      | SSL3 | SSL3 | SSL3 |  
                                 |      |      | TLS  | TLS  | TLS 
---------------------------------|------|------|------|------|-----
https://investing.schwab.com/    |1a    |1b    |1c    |1d    |1e     
no proxy                         | conn | conn | conn | warn | warn 
---------------------------------|------|------|------|------|-----
https://www.verisign.com/        |2a    |2b    |2c    |2d    |2e   
no proxy                         | conn | conn | conn | conn | warn
---------------------------------|------|------|------|------|-----
https://webmail.wittenberg.edu/  |3a    |3b    |3c    |3d    |3e   
no proxy                         | conn | conn | conn | conn | warn
---------------------------------|------|------|------|------|-----
https://www.kuix.de/links/       |4a    |4b    |4c    |4d    |4e   
no proxy                         | stop | stop | stop | stop | stop
---------------------------------|------|------|------|------|-----
https://investing.schwab.com/    |1f    |1g    |1h    |1i    |1j     
with SSL proxy                   | conn | conn | conn | warn | warn  
---------------------------------|------|------|------|------|-----
https://www.verisign.com/        |2f    |2g    |2h    |2i    |2j   
with SSL proxy                   | conn | conn | conn | conn | warn
---------------------------------|------|------|------|------|-----
https://webmail.wittenberg.edu/  |3f    |3g    |3h    |3i    |3j   
with SSL proxy                   | conn | conn | conn | conn | warn
---------------------------------|------|------|------|------|-----
https://www.kuix.de/links/       |4f    |4g    |4h    |4i    |4j   
with SSL proxy                   | stop | stop | stop | stop | stop
---------------------------------|------|------|------|------|-----
send mail                        |5a    |5b    |5c    |5d    |5e   
with SMTP/SSL when available     | fail | conn | conn | conn | conn
---------------------------------|------|------|------|------|-----
send mail                        |5f    |5g    |5h    |5i    |5j   
with SMTP/SSL always             | fail | conn | conn | conn | conn   
---------------------------------|------|------|------|------|-----


=====================
Windows test results:
=====================

site / enabled options:          | SSL2 | SSL2 | SSL2 |      |
                                 |      | SSL3 | SSL3 | SSL3 |
                                 |      |      | TLS  | TLS  | TLS
---------------------------------|------|------|------|------|-----
https://investing.schwab.com/    |1a    |1b    |1c    |1d    |1e
no proxy                         | conn | conn | conn | warn | warn
---------------------------------|------|------|------|------|-----
https://www.verisign.com/        |2a    |2b    |2c    |2d    |2e
no proxy                         | conn | conn | conn | conn | warn
---------------------------------|------|------|------|------|-----
https://webmail.wittenberg.edu/  |3a    |3b    |3c    |3d    |3e   
no proxy                         | conn | conn | conn | conn | warn
---------------------------------|------|------|------|------|-----
https://www.kuix.de/links/       |4a    |4b    |4c    |4d    |4e   
no proxy                         | stop | stop | stop | stop | stop
---------------------------------|------|------|------|------|-----
https://investing.schwab.com/    |1f    |1g    |1h    |1i    |1j
with SSL proxy                   | conn | conn | conn | warn | warn
---------------------------------|------|------|------|------|-----
https://www.verisign.com/        |2f    |2g    |2h    |2i    |2j
with SSL proxy                   | conn | conn | conn | conn | warn
---------------------------------|------|------|------|------|-----
https://webmail.wittenberg.edu/  |3f    |3g    |3h    |3i    |3j   
with SSL proxy                   | conn | conn | conn | conn | warn
---------------------------------|------|------|------|------|-----
https://www.kuix.de/links/       |4f    |4g    |4h    |4i    |4j   
with SSL proxy                   | stop | stop | stop | stop | stop
---------------------------------|------|------|------|------|-----
send mail                        |5a    |5b    |5c    |5d    |5e
with SMTP/SSL when available     | fail | conn | conn | conn | conn
---------------------------------|------|------|------|------|-----
send mail                        |5f    |5g    |5h    |5i    |5j
with SMTP/SSL always             | fail | conn | conn | conn | conn
---------------------------------|------|------|------|------|-----

Note: an inhouse SSL capable proxy is available at carbuncle.mcom.com port 8080



The above test results match the expection, which is described next:

----------------------------------

Site only speaking SSL2:
  https://investing.schwab.com/

1d, 1e, 1i, 1j
expected error message:
  SSL version 2 disabled

1a, 1b, 1c, 1f, 1g, 1h
connection should work
----------------------------------

Site generally able to speak TLS, but having no algorithms 
in common with Mozilla:
  https://www.verisign.com/

2e, 2j:
expected error message:
  no algorithms in common

2a, 2b, 2c, 2d, 2f, 2g, 2h, 2i
connection should work
----------------------------------

Site incompatible with TLS protocol, needs a connection retry
(but the user should not notice that)
  https://webmail.wittenberg.edu/

confirm the warning about the expired cert (if any)
  
3e, 3j expected to see:
  Error, because SSL is disabled

3a, 3b, 3c, 3d, 3f, 3g, 3h, 3i
the first connection in a freshly
started browser session should work.
----------------------------------

Site with untrusted certificate:
  https://www.kuix.de/links/

On connect, a untrusted certificate message will be shown.
Press cancel.

4a-j: no additional error message should be shown.
----------------------------------

Sending mail with SMTP over SSL (STARTTLS).
I used judge.netscape.com as outgoing server.
To my big surprise, tests 5e and 5j failed.
My expectation was, that by definition a STARTTLS
capable SMTP server must be able to speak TLS.
However, the reported error is "server and client
do not have common encryption algorithms".

Nelson confirmed that judge.netscape.com is likely
to run an old version of NSS and the failure is not
a surprise. Also note that this configuration is nothing
new, the existing behaviour without the patch
(see end of comment) is the same.

I tested 5e and 5j with my private SMTP server
at www.kuix.de (need a cert to relay) and it worked.
This server will not relay, but you can send a test
message to k@kuix.de
----------------------------------



Below is a couple of tests I made without the new patch.
I did not test the non-proxy cases, because I assume they work.
It is meant to document the existing behaviour that needs to be fixed.

===============================
Linux test results without fix:
===============================

site / enabled options:          | SSL2 | SSL2 | SSL2 |      |
                                 |      | SSL3 | SSL3 | SSL3 |  
                                 |      |      | TLS  | TLS  | TLS 
---------------------------------|------|------|------|------|-----
https://investing.schwab.com/    |1a    |1b    |1c    |1d    |1e     
no proxy                         |      |      |      |      |      
---------------------------------|------|------|------|------|-----
https://www.verisign.com/        |2a    |2b    |2c    |2d    |2e   
no proxy                         |      |      |      |      |     
---------------------------------|------|------|------|------|-----
https://webmail.wittenberg.edu/  |3a    |3b    |3c    |3d    |3e   
no proxy                         |      |      |      |      |     
---------------------------------|------|------|------|------|-----
https://www.kuix.de/links/       |4a    |4b    |4c    |4d    |4e   
no proxy                         |      |      |      |      |     
---------------------------------|------|------|------|------|-----
https://investing.schwab.com/    |1f    |1g    |1h    |1i    |1j     
with SSL proxy                   | conn | conn | conn | warn | warn  
---------------------------------|------|------|------|------|-----
https://www.verisign.com/        |2f    |2g    |2h    |2i    |2j   
with SSL proxy                   | conn | conn | conn | conn | warn
---------------------------------|------|------|------|------|-----
https://webmail.wittenberg.edu/  |3f    |3g    |3h    |3i    |3j   
with SSL proxy                   | conn | conn |!FAIL |!FAIL |!FAIL
---------------------------------|------|------|------|------|-----
https://www.kuix.de/links/       |4f    |4g    |4h    |4i    |4j   
with SSL proxy                   |!FAIL |!FAIL |!FAIL |!FAIL |!FAIL
---------------------------------|------|------|------|------|-----
send mail                        |5a    |5b    |5c    |5d    |5e   
with SMTP/SSL when available     |      |      |      |      |     
---------------------------------|------|------|------|------|-----
send mail                        |5f    |5g    |5h    |5i    |5j   
with SMTP/SSL always             | fail | conn | conn | conn | conn   
---------------------------------|------|------|------|------|-----



Note for testing:
- in case 3* a cert warning is shown. press ok.
- in case 4* a cert warning is shown. press cancel




conn: connection was possible
warn: the expected warning was shown
stop: as expected, the connection stopped
fail: as expected, the connection failed
!FAIL: This does not meet the expectation



I am unable to test on Mac, because I do not have one.

We can either ask somebody to help out and produce a Mac test build,
or we risk the checkin and test Mac the day after the checkin.
Attached patch Patch v4Splinter Review
Same patch as v3, but has better comments.
Attachment #94549 - Attachment is obsolete: true
Kaie, you should send me a Mac test build, preferably OSX.
Comment on attachment 94584 [details] [diff] [review]
Patch v4

r=javi

Changes in security look good.	Changes under mozilla/netwerk look good to me
as well, but I suggest finding someone from the necko team to look at those
changes as well.
Attachment #94584 - Flags: review+
Javi's Mac OSX build works for me. Proxy is lab212sun.mcom.com:8080.
I forgot to test one more thing, but I just did, and it works for me, too, on
both Linux and Windows.

I re-tested LDAP over SSL, since the patch also touches that code. Configuring
the inhouse LDAP directory nsdirectory and using a SSL connection, a dropdown
menu came up when typing an recipient in mail composer.
John said, LDAP/SSL works with the Mac test build, too.
Darin, Dan, Jean-Francois, can you please review the changes in your modules?

The latest patch makes renaming changes to the following modules:
- netwerk
- directory
- mailnews
See the beginning of section 49 for the motivation of those changes.


All the real code fixes are in security/manager, and have already been reviewed
by Javi.
Comment on attachment 94584 [details] [diff] [review]
Patch v4

r=darin for the netwerk changes.
Comment on attachment 94584 [details] [diff] [review]
Patch v4

R=ducarroz for mailnews changes
Comment on attachment 94584 [details] [diff] [review]
Patch v4

r=dmose for the mozilla/directory changes.
Alec, can you please review?
Comment on attachment 94584 [details] [diff] [review]
Patch v4

sr=alecf
Attachment #94584 - Flags: superreview+
Fixed on trunk.
Status: REOPENED → RESOLVED
Closed: 22 years ago22 years ago
Resolution: --- → FIXED
Blocks: 87209
Blocks: 149910
Verified.
Status: RESOLVED → VERIFIED
Product: PSM → Core
Version: psm2.0 → 1.0 Branch
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: