Miloslav Trmac, an engineer at Red Hat, reported the following in https://bugzilla.redhat.com/show_bug.cgi?id=468603 I propose we move the discussion over here, I'd like to avoid tweaking the deep internals of NSS' libSSL behavior, but instead see whether his proposed change makes sense in general, and can be accepted for cvs commit. Below is what mitr said, quoting for the bugzilla mentioned aboved: Description of problem: I need to implement a "nested", temporary TLS session on a connection (running unencrypted at first, then TLS, then again unencrypted). I'm using SSL_ImportFD(PR_ImportTCPSocket(dup(mysocket))) to create a NSS SSL socket. Switching from unencrypted to encrypted works fine, but to switch back I need to reliably detect when the TLS session has ended. When any of the sides closes the SSL socket, nss sends a close_notify alert, but it does not wait until it receives a close_notify alert from the other side. Ideally there should be an interface that lets me do this (see (man SSL_shutdown) to see how OpenSSL handles it) - please consider this a feature request. Because the second unencrypted communication is unidirectional in my case, I decided to PR_Close() the socket in the writer (and ignore any incoming close_notify), and to PR_Recv() in the reader, which should return 0 upon receiving the close_notify. PR_Recv() actually processes the close_notify, but then waits for more data anyway, consuming data from the unencrypted communication. The attached patch fixes the receive side to stop reading data after receiving a close_notify, which is sufficient for my current application - but please add a general interface to reliably shut down the TLS session.

I'm trying to understand the requirement here. Are you trying to have SSL terminate cleanly, but have the TCP connection continue in an unencrypted state? If so, that is a significant new requirement, and probably requires a greater change than the attached patch. It would require having a way to get the SSL protocol to go through all the motions of a PR_Close, without closing the underlying socket. That's not an entirely unreasonable request, IMO, but if that's what you're trying to accomplish, then this bug should be an RFE asking for that capability. It would probably require defining another settable SSL socket option. See also the large block comment about close_notify at http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/ssl/sslsecur.c&rev=1.42&mark=999#999

Yes, that's what the application was trying to do. (BTW, it turned out I don't need this functionality for my application after all. So feel free to close this report if you don't think the functionality is likely to be useful.) I'm not concerned with PR_Close() right now - dup() can take care of that - I'd just like the SSL layer on one side to read exactly data written by the SSL layer on the other side. The block comment seems to describe only an implementation difficulty - the code can (in principle, perhaps the current organization makes it difficult) simply ignore ECONNRESET, and sending the close_notify in non-blocking mode is no worse than the current implementation. Leaving all that aside, the specific submitted patch looks "obviously correct" to me and AFAICS it can not harm anything, and although it doesn't do everything I'd like, it makes some applications possible. On the other hand, I don't understand SSL all that much and I might be missing something important.

Miloslav, Does your application attempt to handshake as a client? or as a server? Does dup() overcome the effects of a shutdown call on the underlying socket?

One as client, one as server :) The one that was doing the PR_Read() was handshaking as a client. Both the original file handle and the file handle returned by dup() are affected by PR_Shutdown(). PR_Close(), which I used, does not shut down the socket at all.

Are you willing to accept the limitation that ANY failure of the SSL handshake will leave the underlying socket in a state where the unencrypted connection cannot be resumed reliably (or perhaps, ever)?

Perfectly. I was using the nested SSL session to authenticate the previous unencrypted context.

Comment on attachment 346063 [details] [diff] [review] proposed patch v1 (3 added lines, lots of context) > rv = ssl3_HandleRecord(ss, &cText, &ss->gs.buf); > if (rv < 0) { > return ss->recvdCloseNotify ? 0 : rv; > } >+ if (ss->recvdCloseNotify) { >+ return 0; >+ } I'm not opposed to this change in principle. You can rearrange it a little though, so that only one code path tests ss->recvdCloseNotify, but the results are the same. This will make the code smaller and easier to understand. Please do that. When that is done correctly, I'll give it r+

Here is an updated patch.

FWIW, I used this (with Fedora's python-nss) to test the behavior. Correct output is: 'foo' 'bar' 'baz' Incorrect output is: 'foo' 'bar' ''

Comment on attachment 346630 [details] [diff] [review] Stop PR_Recv() on close_notify (backed out) r=nelson I'm willing to give this a try. My only concern for it is that it might change behavior of some program that is presently working without this change, and that change might be a change for the worse. But I'm willing to give it a try.

Checking in ssl3gthr.c; new revision: 1.8; previous revision: 1.7

I backed out this patch, because all the Windows builds that began after it was checked in failed their SSL tests. The current hypothesis is that somehow this patch keeps selfserv from shutting down, and selfserv keeps holding on to the bound port, so all subsequent attempts to use it get EADDRINUSE. It will be necessary for someone to login to the Windows Tinderbox systems and kill any leftover selfserv processes. Then on the next build cycle, if everything goes back to green, that will confirm that this patch was the cause. It's strange that this is only an issue on Windows. My own test builds on Windows with the Win95 flavor of NSPR all continue to pass all.sh, so it may be related to the use of the WinNT flavor of NSPR, or to the use of the MinGW shell (I still use MKSNT).

I gather that Miloslav is not working on this any more...

Comment on attachment 346630 [details] [diff] [review] Stop PR_Recv() on close_notify (backed out) Marking this patch obsolete because it failed and was backed out