Build ID: 2002-03-25, all OSs (Windows 2000, Mac OS 9.2, Mandrake 8.1, Mac OS X 10.1.3). Summary: Connection to IMAP over SSL and SNEWS fails using SOCKS proxy V5. Steps to Reproduce: 1. If you're inside the firewall enable SOCKS Proxy V5 support via Edit | Preferences | Advanced | Proxies, under 'Manual Proxy Configuration', enter socks5qa.mcom.com in the SOCKS Host section and 1080 for the port. 2. Shutdown/restart. 3. Either click on snews://secnews.netscape.com/netscape.public.mozilla.mail-news or setup an IMAP account and check the "Use SSL" option for the Server panel in Account Manager. 4. Try to connect to either the secure IMAP or SNEWS server. Actual Results: It seems at first glance that a connection is made, but it just halts. Expected Results: Connection is made (handshake) and then transparently, you can connect to either server. (If you open netstat you *should* see: carbuncle.nscp.aoltw.net:1080 ESTABLISHED).

I looked into this. Its failing because mailnews uses ssl-forcehandshake for a socket type, while https (which does work over socks) uses ssl (or tlsstepup over a proxy) It appears that the ssl-forcehandshake type was added by stuart to fix bug 39154. His patch in that bug doesn't include stuff added to nss, so I'm not sure if the nss backend was added before that. The bug it claims to work around (bug 66706) has now been fixed. I'm not sure why this breaks it, though. I havne't done detailed tests, just checked wht was different between https-over-socks and mailnews-over-socks. Is there some other reason to do ssl-forcehandshake? Or can we remove that code entirely, and just use ssl instead? just plain "ssl" works with and without socks for nntps - I didn't test other protocols. Kaie, stuart, darin - any ideas on what this code actually does, and why it would fail with socks? mailnews people: Was there another reason why this socket type is used? I'm still not sure why bug 66706 would have affected mailnews but not http in the first place. I asked stephend to assign this to me, but anyone else is welcome to take this bug. The fix is just doing s/ssl-forcehandshake/ssl/ in the few places which use this, and then probably removing the backed code added by stuart, since noone else uses it. Although I could just leave that in anyway, I guess.

QA--> myself, for testing/verification when this is fixed.

I don't know about NNTP, but IMAP requires force handshake. the client must send a message to the server before any data will come through, otherwise it will hand forever.

cc ssaux

I played with this a bit more. My only way of talking to an imap/ssl server is to use stunnel. However, stunnel works fine on an unpatched mozilla for both nntp and ssl. Anyone know a free imap/ssl server I can play with? This could just be a netscape server issue, rather than a mozilla one. Stuart: I don't follow your logic. The only extra code for force-handshake that I can see just writes 0 bytes. There is an SSL_ForceHandshake function, but the only callers of that are test programs. stephend: can you try changing the ssl-forcehandshake to just ssl in nsImapProtocol.cpp, and see if that fixes this for you?

Stunnel must just be doing something different - with a cyrus imap server, ssl-forcehandshake fails for socks, works for normal. 'ssl' works for both

ssl-forcehandshake should no longer be needed. The bug it worked around has been fixed.

the university lab, i used to work in, uses a package called sslwrap to setup a SSL imap server on redhat linux. seems to work great.

nominating for mozilla1.0. if possible, it'd be nice to have.

bradley, what are the chances that this bug will have a patch in the next few days?

So I don't know if the attached patch fixes the problem reported, but it does remove the guts of the forcehandshake workaround and IMAP/SSL still works with it applied.

Comment on attachment 76449 [details] [diff] [review] Proposed fix I don't want to remove that without the security team saying that that is OK. For 1.0, I'd prefer to just change ssl-forcehandshake to ssl, and file a separate bug on pulling this code out.

In answer to Bradley's question in comment 1 above, the reason that mail/news protocols (e.g. imap, pop3, smtp, nntp) use ssl-forcehandshake sockets and http doesn't is that the mail/news protocols are all "server speaks first" protocols, and http is a "client speaks first" protocol. A simple (?) change to those protocols would have obviated the special PSM socket type, without making the protocol code behavior conditional on the use of SSL, but that change was never made. Instead a portion of SSL was rewritten for NSS 3.3 that changed the behavior of PR_Poll on SSL sockets. That change is relevant to the proposed fix, and may be relevant to the cause of the problem reported in this bug. This bug seems to be a case where a workaround/fix has been found, but the real cause of the problem has not been found. I'm quite troubled that there is a case where ssl-forcehandshake type sockets don't work as well as straight ssl sockets. IMO, ssl-forceHandshake type sockets should work as well as ordinary SSL sockets in all cases. The extra zero-length write should never be a problem, ever. I worry that this problem is perhaps a regression caused by the rewrite of ssl_Poll that was done for bugs 56924, 56926 and 66706, and that was new in NSS 3.3. I wonder if the problem is related to experiencing PR_WOULD_BLOCK_ERROR on the first write in the SSL handshake. I'd like to see a complete ssltap trace or other (e.g. tcpdump) trace of the network traffic prior to the hang. I have no problem with getting rid of PSM's ssl-forcehandshake sockets. IMO, they were _never_ needed. I believe it was always possible to write the higher layer application code (e.g. the nntp, smtp, imap, or pop3 code) in such a way that it worked correctly for both SSL and non-SSL sockets without having any conditional SSL behavior (that is, without having to take a different code path in the protocol for SSL than for non-SSL) and without the huge changes that were made to ssl's poll function in NSS 3.4. I will be happy to explain to anyone how to do that (but not in this bug report). But I wish that we could understand this problem before we apply the fix. Unfortunately, my current duties/priorities don't allow me to take the lead in tracking it down. But I'm willing to study ssltap or tcpdump output. Knowing the last value returned by PR_Poll for the affected socket prior to the hang would (might) also be helpful.

The problem here is that we're actually talking to ssl via a socks server. This could be a socks layer bug, where the socks socket type doesn't like the 0 length write.

The usual configuration of socks and SSL is that SSL "tunnels" through socks. This is also expressed as "SSL over SOCKS". E.g. Layers: Application SSL SOCKS socket Is that not the configuration you're using? If it is, then SOCKS should never see the zero length write because it will be intercepted by SSL.

Right, but I'm wondering if the ssl code in mozilla is interacting with the socks code in some wierd way. This change definately fixed it for me, so something is wrong. You can grab tcpdumps by following stephend's instructions in comment 0, with and without my change. I can attach full traces, but not until late tomorrow.

My guess, unsupported by evidence, is that the 0-byte write is happening before the SOCKS I/O layer is inserted onto the stack, causing the SSL initial handshake request to get sent to the SOCKS server before the SOCKS set-up-a-tunnel request.

My socks server logs did show a connection through it, but since this problem doesn't occur with stunnel, I can't say whether anything gets through. There was a couple of K worth of traffic on the failing connection, though The force-connect just writes 0 bytes, to the socket, right?

cc javi

The 0-byte write to the SSL layer causes the SSL layer to try to make progress. In this case, it causes the SSL layer to write the first handshake record to the lower layer, be that the SOCKS layer or the raw socket.

Hmm. Well, if the force handshake stuff was just to work arround another bug, I'd prefer that we just change ssl-forcehandshake to ssl, and confirm that that doesn't break anything. Are there any side effects? If someone wants to work out the real cause, feel free.

What do you mean by "change ssl-forcehandshake to ssl"? The proposed patch changes the implementation of ssl-forcehandshake to do the same thing as ssl, which is the right first step. What is the work that is needed on the patch?

That makes teh force-handshake stuff silently do nothing. I want to change mailnews, instead.

The patch makes ssl-forcehandshake do the same thing as ssl. It does not make it do "nothing". The "-forcehandshake" part of "ssl-forcehandshake" means "in the protocol that runs over this SSL connection, the server speaks first." It's a crappy name for the concept, but that's what pavlov called it. Changing mailnews would be step 2. Until the implementation of ssl-forcehandshake has been made to be the same as ssl, changing mailnews is likely to require review from both mailnews and crypto, which is less likely to be done in time than a patch that only needs review from crypto. Changes to implementations are a lot less risky than changes to interfaces.

What I meant was that it silently will do nothing different. Either remove teh Get/Set forcehandshake stuff, or keep it working and disable it for mailnews, don't do half of each - thast just confusing.

Changing the interface is risker and requires changing more modules. The forcehandshake stuff is already confusing, this patch makes it less so by making it clearer that it does nothing usefully different.

Comment on attachment 76449 [details] [diff] [review] Proposed fix Patch looks ok to me. I looked at the Mozilla sources, and it seems that only imap/pop3/nntp are using forcehandshake. My only concern is that we are leaving obsolete code around. This should probably get removed, and that does not require changing interfaces. I.e., we should remove mForceHandshake from .h and .cpp, more code from nsSSLIOLayerConnect, etc. However, we can delay this if you want - in that case we should file a bug that reminds us to clean up. r=kaie.

Bug 135023 tracks the rest of the cleanup work.

-> patch author

*** Bug 136131 has been marked as a duplicate of this bug. ***

Comment on attachment 76449 [details] [diff] [review] Proposed fix sr=brendan@mozilla.org

*** Bug 79448 has been marked as a duplicate of this bug. ***

Any plans of checking this in for 1.0/MachV? If so, I won't relnote this, but if not, I'll need to get on that promptly. Thanks.

I have sent a request to drivers@mozilla.org for approval but have not received a response one way or the other.

If its been more than a couple of days, try emailing again

Comment on attachment 76449 [details] [diff] [review] Proposed fix a=asa (on behalf of drivers) for checkin to the 1.0 branch

please check this into the trunk, let it bake a day, and if it passes, renominate adt1.0.0 please.

Fix checked into branch and trunk.

Verified FIXED using: TRUNK - Mandrake 8.1 - 2002-04-17-14 Mac OS 9.2.2 - 2002-04-17-10 Mac OS X 10.1.3 - 2002-04-17-03 Windows 2000 - 2002-04-17-18 BRANCH - Mandrake 8.1 - 2002-04-17-11 Mac OS 9.2.2 - 2002-04-17-11 Mac OS X 10.1.3 - 2002-04-17-12 Windows 2000 - 2002-04-17-11 Since this is a 1-2 knock-out punch for *both* trunk and branch, replacing fixed1.0.0 with verified1.0.0 and setting bug status to VERIFIED FIXED. Also, I'll make a note in bug 133795 to remove the release note for this, as it now works. Thanks for everyone's work on this!

*** Bug 143541 has been marked as a duplicate of this bug. ***

Yesterday, I had submitted bug 143541, describing a problem sending mail via SMTP via SSL via SOCKS. The bug was categorized as a duplicate of this bug. I have read through this bug report, and it is similar to my situation, but does not address the issue I reported. I can confirm that the IMAP over SSL over SOCKS problem was resolved by this bug fix, because I remember 0.9.x versions that did not implement it properly, and a subsuquent version did. I was happy to add encryption of my IMAP retrieval. Thanks for fixing it! However, I have just downloaded 1.0RC2 and I confirm the SMTP over SSL over SOCKS issue still remains. I have also taken the nightly build from yesterday, installed it, and reproduced the problem. Do I need to take a different branch? Should I reopen the bug marked as duplicate?