Re: make SSL_shutdown work with non-blocking BIOs
On Tue, Apr 07, 2009, Darryl Miles wrote: > > > With the announcement of OpenSSL 1.0.0 on the way, may I attempt to get > some attention on this issue for which: > > * a patch exists > * a test case exists (that exposes the problem, that verifies the fix > doesn't break anything) > * multiple users have shared their concern over the years (some publicly > some privately) > * which have be brought up and reported to openssl-dev a number of times > requesting attention > > > I'm happy to answer questions and concerns on the issue, please tell the > list publicly what more can be done on the issue. > > I shall have a look into it. I don't see any reference to this issue in the request tracker which I am currently perusing. Steve. -- Dr Stephen N. Henson. Email, S/MIME and PGP keys: see homepage OpenSSL project core developer and freelance consultant. Homepage: http://www.drh-consultancy.demon.co.uk __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
make SSL_shutdown work with non-blocking BIOs
With the announcement of OpenSSL 1.0.0 on the way, may I attempt to get some attention on this issue for which: * a patch exists * a test case exists (that exposes the problem, that verifies the fix doesn't break anything) * multiple users have shared their concern over the years (some publicly some privately) * which have be brought up and reported to openssl-dev a number of times requesting attention I'm happy to answer questions and concerns on the issue, please tell the list publicly what more can be done on the issue. My own feeling is that there does not seem to be any module ownership for the SSL directory and libssl.so amongst the developers; or that the current ownership doesn't have sufficient time to address community needs even when all the hard work has been already done for them. If there is no module owner for SSL/libssl could one be appointed please so at least there is an official point of contact for this part of the library to address matters to. The rest of OpenSSL has worked well with various committers happy to examine patches for modules they are confident in progressing. My best openssl-dev posts to the issues have been: http://www.mail-archive.com/openssl-dev@openssl.org/msg24105.html [recently] The patch: http://marc.info/?t=11515400401&r=1&w=2 [1st followup contains patch] Darryl __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
Re: make SSL_shutdown work with non-blocking BIOs
On Mon, Oct 01, 2007 at 08:06:04PM +0100, Darryl Miles wrote: > > Would Davide be so kind as to look over the following openssl-dev list > post for the semantics I suggest and confirm that logic would work for him: > > http://marc.info/?l=openssl-dev&m=115153998821797&w=2 The archive at marc.info seems to have stripped the patches off this message. Are your patches for ssl_shutdown() available somewhere I could have another look at them? Thanks! Thor __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: make SSL_shutdown work with non-blocking BIOs
Try "next_in_thread", take a look at the thread views (by clicking the subject line) you get to: http://marc.info/?t=11515400401&r=1&w=2 For background reading see also the threads: http://marc.info/?l=openssl-users&m=115088475305680&w=2 http://marc.info/?t=11509972822&r=1&w=2 http://marc.info/?l=openssl-dev&m=116525974320575&w=2 (single email in this thread) Darryl Thor Lancelot Simon wrote: On Mon, Oct 01, 2007 at 08:06:04PM +0100, Darryl Miles wrote: Would Davide be so kind as to look over the following openssl-dev list post for the semantics I suggest and confirm that logic would work for him: http://marc.info/?l=openssl-dev&m=115153998821797&w=2 The archive at marc.info seems to have stripped the patches off this message. Are your patches for ssl_shutdown() available somewhere I could have another look at them? __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
RE: make SSL_shutdown work with non-blocking BIOs
Nanno Langstraat: > Quote chapter and verse of the OpenSSL API documentation, or desist from > such vehement statements. > > You can not scold an API user for violating rules that are not in the > documentation. > > I already claimed that the application programmer is not given the > knowledge that this restriction exists. OpenSSL does its best to document where it fails to make SSL connections look like TCP connections. It does not always document where it succeeds. It is assumed that the programmer is familiar with how TCP works and does not need every similarity documented. > The entire point is that the socket is *not* readable, as you claim it > to be. Incorrect. It is readable. If a dead connection were neither readable nor writable, how would a classic non-blocking application (think 'select') ever discover a dead connection? Even if it wait for both readability and writability, it would never be told that a connection had been closed by the other end if a closed connection were neither readable nor writable. That is why the convention is that dead connections are readable. > The OS kernel has already told OpenSSL that (the downstream direction > of) the socket is shut down. [OpenSSL did read() on the socket and it > returned 0] > > That event is the very reason why SSL_read() decided to return 0 to the > application in the first place! > > The socket is not and never again will be readable. It's passed on. It's > bereft of life. It's not pinin' for the fjords. Etc. Thus it is always readable. Read will always succeed immediately and correctly report that the connection is dead. That is the definition of a readable socket (read will complete immediately). > And more importantly, OpenSSL is the only party who knows this > underlying cause, and SSL_want_read() is the designated channel to for > the application to query the information in order to do a correct > select() or poll(). And OpenSSL is reporting correctly that all it needs to make further progress is a readable socket. It has a readable socket, and further progress can be made immediately. If the socket were not readable, OpenSSL could not make forward progress immediately. > > The application may never call SSL_write. It may have already > > sent all the > > data it ever plans to send. This is not a sensible plan. > Now *That* would be a broken application. An application like that would > not work right when doing non-blocking plain TCP either. Huh? Once a web server has sent all the data it plans to send, it will never call 'write' for that connection again. It may still try to close the connection down cleanly. You only call SSL_write if you have data to send. > All I'm asking for is: either make OpenSSL behaviour comparable to the > Unix socket API, *or* completely document the API as it is, with its > diferent use rules. This is a case where it is comparable. > I should think that this is an eminently reasonable suggestion, either > way, and I don't understand the resistance. What do you see as the difference between OpenSSL's behavior and classic TCP behavior in this case? In both cases, the socket is readable. In both cases, forward progress (which would presumably be closing the socket or tearing down the connection) can be made because the socket is readable. DS __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: make SSL_shutdown work with non-blocking BIOs
Nanno Langstraat wrote:
David Schwartz wrote:
The socket is not and never again will be readable. It's passed on. It's
bereft of life. It's not pinin' for the fjords. Etc.
And more importantly, OpenSSL is the only party who knows this
underlying cause, and SSL_want_read() is the designated channel to for
the application to query the information in order to do a correct
select() or poll().
At kernel/socket level an end-of-stream condition read(fd, buf, 4096)=0
is a permanent readabily event (as signaled by poll/select). Since you
are expected to read it and detect the special case of a return value
being zero as being an EOF and react accordingly.
However this is now what/how I understand SSL_want_{read,write}() to
mean, these are how the situation looks from the opposite end. Meaning
what are the needs of the SSL layer in respect of the BIO layer (below it).
If the SSL layer has uncommitted data it is trying to write to the BIO
layer it will return SSL_want_write()=true.
Maybe this is a workable fix for my SSL_shutdown() problem, where using
the SSL_shutdown()=0 alone is not enough information to be sure the
"shutdown notify packet" was committed to the BIO layer. But if I call
SSL_want_write() after and get back 'false' then I know we're done.
It is unclear from the man page to me what SSL_want_read() indicates,
since in a normal open channel you are always looking for more data,
however the man page sortof indicates that if the SSL layer is in the
middle of processing a packet but is missing the final data then it will
return true, otherwise false.
It is a given that if the channel is open and has no outstanding error
condition then it is always ready, willing and able to process more
received data, as and when it arrives.
The crux tho as David has explained, is that your application observed
an end-of-stream condition from SSL_read(). The SSL_want() man page
indicates it does not account for errors or any other special overriding
circumstances and the values only make sense when the SSL channel is in
a normal condition. I'm sure you will agree that SSL channel is not in
a normal state as soon as it starts handling an end-of-stream in any
direction.
Now *That* would be a broken application. An application like that would
not work right when doing non-blocking plain TCP either.
Urgh. A non-blocking server application that reads telemetry data from
a source would never write anything back the other way.
Darryl
__
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager [EMAIL PROTECTED]
Re: make SSL_shutdown work with non-blocking BIOs
David Schwartz wrote: This goes pear-shaped as follows: The application is broken. Once SSL_read returns 0, the connection is dead. Quote chapter and verse of the OpenSSL API documentation, or desist from such vehement statements. You can not scold an API user for violating rules that are not in the documentation. I already claimed that the application programmer is not given the knowledge that this restriction exists. My claim might be proven wrong of course, but there is naturally a burden on you to provide a hint of evidence for it. I would have expected a relevant quote from the SSL_read() or SSL_want_read() manual pages in your email. * Say "Silly OpenSSL API user! You should have known/guessed that you can't use SSL_want_read() / SSL_want_write() anymore after SSL_read() has returned 0." You can't use them that way. You can call them if you want. SSL_want_read is the correct indication, telling you that you can make SSL calls as soon as the socket is readable, which it already is. There is nothing to wait for, SSL_want_read returning true or false are equally valid, since the socket is readable. The entire point is that the socket is *not* readable, as you claim it to be. The OS kernel has already told OpenSSL that (the downstream direction of) the socket is shut down. [OpenSSL did read() on the socket and it returned 0] That event is the very reason why SSL_read() decided to return 0 to the application in the first place! The socket is not and never again will be readable. It's passed on. It's bereft of life. It's not pinin' for the fjords. Etc. And more importantly, OpenSSL is the only party who knows this underlying cause, and SSL_want_read() is the designated channel to for the application to query the information in order to do a correct select() or poll(). * The OpenSSL code can be updated to handle this nicely: make sure SSL_want_read() and SSL_want_write() return false, and wait for the application to call SSL_write() sooner or later, which will return an error as normal. At that moment the non-blocking application can be expected to "get the idea" and clean up the connection smoothly. The application may never call SSL_write. It may have already sent all the data it ever plans to send. This is not a sensible plan. Now *That* would be a broken application. An application like that would not work right when doing non-blocking plain TCP either. All I'm asking for is: either make OpenSSL behaviour comparable to the Unix socket API, *or* completely document the API as it is, with its diferent use rules. I should think that this is an eminently reasonable suggestion, either way, and I don't understand the resistance. Nanno __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: make SSL_shutdown work with non-blocking BIOs
Nanno Langstraat wrote: It turns out that the problem does *not* directly involve SSL_shutdown(), but it *is* attributable to OpenSSL, and specifically OpenSSL's non-blocking shutdown semantics. Okat thats cleared that up, it is indeed unrelated to the OP of this thread. Please move replies (if any) to a new thread :). Details: the application's event loop is built analogous to a regular TCP event loop. Meaning that the event loop tracks the upstream direction and the downstream independently: the event loop handles the possibility that the downstream direction gets shut down [SSL_read() returned 0] while the upstream direction stays open a while longer [we might still make a few SSL_write() calls]. Just like TCP. This goes pear-shaped as follows: * The SSL connection is made and used * The remote side closes its file descriptor (e.g. process killed, TCP shutdown(RD)) * Local SSL_read() returns 0. The app event loop sets a flag and makes sure it never calls SSL_read() again. * The app event loop prepares for poll() by calling SSL_want_read() and SSL_want_write(). * SSL_want_read() returns 'true'. This is erroneous. * poll() returns immediately. * Repeat the last 3 steps indefinitely. Uses 100% CPU. I can't say I've used those SSL_want_x() methods before. So lets look at the docs. man 3 SSL_want "Unlike SSL_get_error(3), which also evaluates the error queue, the results are obtained by examining an internal state flag only. The information must therefore only be used for normal operation under non-blocking I/O." Oh so we bypass all considerations and simply look at the "internal state flag" because we are expected to be used for a "normal operation". Well handling an end-of-stream condition is not a normal operation or situation in my book for a start. So the question remains, is end-of-stream considered an error condition ? What does SSL_get_error() return in the situation in step 5 above ? I understand the gist if your problem from your point of view. But more detail is needed over step 2 in exactly what you are saying as "process killed" is not the same condition as "TCP shutdown(RD)". One results in a unusable socket (for any purpose, be it read or write) and the other results in a still usable socket. So you can't just get away with lumping those two situations as if either would do. I will spare you the details of why the "process killed" condition is obviously an error no matter what. It would be incorrect for the remote end to issue a "TCP shutdown(RD)" unless that was the result of receiving a "shutdown notify packet" otherwise you just pulled the rug out from under OpenSSL by denying it the ability to securely signal its own end-of-stream condition (it requires to convey data to do that). But if your peer called "TCP shutdown(RD)" as a direct result of receiving a "shutdown notify packet" then you can claim the local end has a bug for SSL_want_write() returning 'true' after it has written its last piece of data to the BIO layer (the "shutdown notify packet"). Since it should already know that it can no longer send any data (not even a re-negotiation response) from the state of its "internal state flags". If there isn't such a flag for this situation then one should exist. So please clarify your step 2, it does matter what and why the other end did what it did, did, did! Darryl __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
RE: make SSL_shutdown work with non-blocking BIOs
> This goes pear-shaped as follows: > > * The SSL connection is made and used > * The remote side closes its file descriptor (e.g. process killed, > TCP shutdown(RD)) > * Local SSL_read() returns 0. The app event loop sets a flag and > makes sure it never calls SSL_read() again. > * The app event loop prepares for poll() by calling SSL_want_read() > and SSL_want_write(). > * SSL_want_read() returns 'true'. This is erroneous. > * poll() returns immediately. > * Repeat the last 3 steps indefinitely. Uses 100% CPU. The application is broken. Once SSL_read returns 0, the connection is dead. > I see three ways to slice this: > > * Say "Silly OpenSSL API user! You should have known/guessed that > you can't use SSL_want_read() / SSL_want_write() anymore after > SSL_read() has returned 0." You can't use them that way. You can call them if you want. SSL_want_read is the correct indication, telling you that you can make SSL calls as soon as the socket is readable, which it already is. There is nothing to wait for, SSL_want_read returning true or false are equally valid, since the socket is readable. > This does not seem reasonable, because as far as I can see this > rule is not mentioned in the API documentation for SSL_read() or > SSL_want_read(). Right? What do you think SSL_want_read should return in this case? The choices are true or false and you don't like true. So I'm guessing you think false. But if the socket is readable, your call will return immediately. Isn't that what SSL_want_read=true means? > * The OpenSSL documentation can be updated to mention this. I suppose so, but I don't see much point. > * The OpenSSL code can be updated to handle this nicely: make sure > SSL_want_read() and SSL_want_write() return false, and wait for > the application to call SSL_write() sooner or later, which will > return an error as normal. At that moment the non-blocking > application can be expected to "get the idea" and clean up the > connection smoothly. The application may never call SSL_write. It may have already sent all the data it ever plans to send. This is not a sensible plan. If both SSL_want_read and SSL_want_write are false, the application will never make the forward progress that it can make. The socket is already readable, and the only progress that can ever be made requires nothing more than that the socket be readable. DS __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: make SSL_shutdown work with non-blocking BIOs
Darryl Miles wrote: Nanno Langstraat wrote: So I can add one more voice to the choir: the current SSL_shutdown() API appears to give trouble to every non-blocking developer (I remember I lost serious time noticing + tracking down this 100% CPU bug), and afterwards things still don't really work right. I can't immediately think of a reason why you'd get 100% CPU, except with a badly constructed event loop (even with non-blocking I/O in use) much as I'd like to see your support :). Them's fighting words :-) But not inconceivable, the event loop juggles several different kinds of file descriptors. So I've taken that event loop from the application and whittled it down to a simple test case, It turns out that the problem does *not* directly involve SSL_shutdown(), but it *is* attributable to OpenSSL, and specifically OpenSSL's non-blocking shutdown semantics. -- Details: the application's event loop is built analogous to a regular TCP event loop. Meaning that the event loop tracks the upstream direction and the downstream independently: the event loop handles the possibility that the downstream direction gets shut down [SSL_read() returned 0] while the upstream direction stays open a while longer [we might still make a few SSL_write() calls]. Just like TCP. This goes pear-shaped as follows: * The SSL connection is made and used * The remote side closes its file descriptor (e.g. process killed, TCP shutdown(RD)) * Local SSL_read() returns 0. The app event loop sets a flag and makes sure it never calls SSL_read() again. * The app event loop prepares for poll() by calling SSL_want_read() and SSL_want_write(). * SSL_want_read() returns 'true'. This is erroneous. * poll() returns immediately. * Repeat the last 3 steps indefinitely. Uses 100% CPU. -- I see three ways to slice this: * Say "Silly OpenSSL API user! You should have known/guessed that you can't use SSL_want_read() / SSL_want_write() anymore after SSL_read() has returned 0." This does not seem reasonable, because as far as I can see this rule is not mentioned in the API documentation for SSL_read() or SSL_want_read(). Right? * The OpenSSL documentation can be updated to mention this. * The OpenSSL code can be updated to handle this nicely: make sure SSL_want_read() and SSL_want_write() return false, and wait for the application to call SSL_write() sooner or later, which will return an error as normal. At that moment the non-blocking application can be expected to "get the idea" and clean up the connection smoothly. In case of interest, I can clean up the test case code further and email it. Regards, Nanno __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: make SSL_shutdown work with non-blocking BIOs
Darryl Miles wrote: 2) SSL_read() already has a return value -1/ZERO_RETURN which indicates end-of-stream. You may then call SSL_shutdown() to look to see if 1 is returned or not. Or even SSL_get_shutdown() and take whatever security action your application needs to take in the event of an improperly shutdown stream. This isn't quite true. I turn off readahead at my initial call to SSL_shutdown() with SSL_set_read_ahead(ssl, 0). Then in my event processor which runs off the readability indicator for the socket that is being shutdown in this handler I call SSL_shutdown() first but if it is still returning 0, I then call SSL_read() a number of times to sink the application data that may exist, I then call SSL_shutdown() again (before processing the last error if there was one to SSL_read()). Turning the read_ahead off may not be necessary for you, I do it because in my app its possible to see unencrypted data behind the last SSL byte (or possible another request to setup a new SSL session). I also only allow SSL_read() to be called a handful of times (in the shutdown handling detailed above) in a tight loop before it returns back to a main event loop this is to stop a form of denial of service where the receiving CPU is lower spec to the sending CPU which may cause starvation of other connections, this would be because its sinking the data and there isn't the usual disconnection due to too much invalid data that you'd get if it were in the main event loop doing request processing. Turning readahead off ensures OpenSSL only reads just as much data as it needs to and my readability events stay armed within the BIO layer if I choose to not read application data to exhaustion. Darryl __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: make SSL_shutdown work with non-blocking BIOs
Nanno Langstraat wrote: So I can add one more voice to the choir: the current SSL_shutdown() API appears to give trouble to every non-blocking developer (I remember I lost serious time noticing + tracking down this 100% CPU bug), and afterwards things still don't really work right. I can't immediately think of a reason why you'd get 100% CPU, except with a badly constructed event loop (even with non-blocking I/O in use) much as I'd like to see your support :). The issue I am claiming is that the event loop has no way of knowing if the SSL_shutdown() managed to write and commit the entire "shutdown notify packet" into the BIO layer as the SSL_shutdown() return value will never indicate a -1/WANT_WRITE error condition, it always returns a value as-if everything has proceeded fine. It also never returns -1/WANT_READ when it has successfully committed the "shutdown notify packet" to the BIO layer but it now waiting to receive and decode a "shutdown notify packet" from the remote end. Knowing the exact state is important for non-blocking, the matters that concern a non-blocking user are: 1) Retrying the SSL_shutdown() when all the necessary data isn't yet been written as we need to get the send "shutdown notify packet" on the wire. 2) Knowing that all the necessary data has been written into the BIO layer so we can now take action to shutdown the sending side of the socket (at socket level, like with shutdown()) and cleanup that half, whilst leaving open the receiving half since we still wish to see the recvd "shutdown notify packet". 3) Allowing an application driven timeout to start from the moment we know we committed the send "shutdown notify packet" to the BIO layer, so we can enforce a timeout based on when we wrote the data to the wire as opposed to when we wanted to start shutdown proceedings. Maybe you got 100% CPU because you were using SSL_shutdown() to look for a return value of 1, meaning its all over. My application uses SSL_read() even after the SSL_shutdown() has been called once. There are 2 reasons for this: 1) It is necessary to read (and discard/sink it if you don't want it) all the encrypted application data that appears before the shutdown. If you stopped calling SSL_read() then I guess 100% CPU results because FD_ISSET(fd, &fdread) always signals there is data and calling SSL_shutdown() alone may not clear it because there is application data waiting to be read that is stalling the inbound stream processing. 2) SSL_read() already has a return value -1/ZERO_RETURN which indicates end-of-stream. You may then call SSL_shutdown() to look to see if 1 is returned or not. Or even SSL_get_shutdown() and take whatever security action your application needs to take in the event of an improperly shutdown stream. So as you can see my guess would be that your application has an incorrect event loop in relation to handling SSL_shutdown(). Darryl __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: make SSL_shutdown work with non-blocking BIOs
Darryl Miles wrote:
David Schwartz wrote:
If I'm misunderstanding the man page and/or the source code
please speak up.
My man page says:
If the underlying BIO is non-blocking, SSL_shutdown() will also
Yes but what SSL_shutdown() actually does is always return 0
This discussion a week ago made me wonder how SSL_shutdown() could work
in our non-blocking I/O codebase.
I just got around to check, and lo, I found the following code snippet:
// HACK! -- There was a bug where 'ssl-handler' took 100% CPU between remote
// disconnect and local peer close(). This was not investigated to the bitter
// end, but the severe suspicion exists that the OpenSSL library does not
// react properly with a half-open connection (read side closed, write side
open)
// with non-blocking sockets. The following 5 lines are a least-invasive fix.
// The original control flow of the program is still intact, even though a
// part is now short-circuited here.
if (st->ssl_in_closed) {
shutdown(st->local_socket, SHUT_RD); // Tell local endpoint that we don't
read anymore
st->local_in_closed = true; // Fake lack of SSL-output-still-to-be-sent
...
So I can add one more voice to the choir: the current SSL_shutdown() API
appears to give trouble to every non-blocking developer (I remember I
lost serious time noticing + tracking down this 100% CPU bug), and
afterwards things still don't really work right.
With regards,
Nanno
__
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager [EMAIL PROTECTED]
Re: make SSL_shutdown work with non-blocking BIOs
On Mon, 1 Oct 2007, Richard Salz wrote: > > If that's an example of "working API" for someone, it's no surprise > > websphere blows. > > There's no need to be rude. > > And WebSphere doesn't use OpenSSL. It was not me that showed up throwing titles, an sigs to look up. I tried to keep the conversation on a technical level, thing that you seem to be avoiding. And I can even understand the reason why you're avoiding it. - Davide __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: make SSL_shutdown work with non-blocking BIOs
> If that's an example of "working API" for someone, it's no surprise > websphere blows. There's no need to be rude. And WebSphere doesn't use OpenSSL. /r$ -- STSM, DataPower Chief Programmer Websphere DataPower SOA Appliances http://www.ibm.com/software/integration/datapower/ __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: make SSL_shutdown work with non-blocking BIOs
On Mon, 1 Oct 2007, Darryl Miles wrote: > Davide Libenzi wrote: > > Can this be worked around? Sure. With some woodoo/ugly magic code in the > > async status code handling. You *cannot* always wait for read&write, since > > you'll be exiting the event selection loop immediately, every time. > > You need to bolt-in the shutdown logic *outside* the shutdown API, and lever > > over the fact that it is internally composed by a write, followed by a read. > > And setup the waited I/O flags accordingly. > > If that's an example of "working API" for someone, it's no surprise > > websphere blows. > > Yes, I'm not sure I follow your workarounds/jargon completely, the possible > workarounds I can suggest would go something like this: [...] There are many ways to do it, al of them require the "extraction" of the shutdown internal logic into the application. One of the simplest and system independent ways, is to wait for WRITE at most two times (given the notify-close packet size, and depends on the TCP stack implementation), and then wait only for READ the following times you get ERROR_SYSCALL. This, of course, only during the SSL_shutdown handling. You have those different case: 1) Write buffer space available - Peer notify-close packet not received In this case, our notify-close has been successfully queue by the TCP/IP stack, and the ERROR_SYSCALL is for a WANT_READ. This is the worst case, and you'll be wasting two (it could be one really, but two puts you on the safe side) cycles waiting for WRITE, while the following wait for READ will behave correctly. 2) Write buffer space available - Peer notify-close packet received This case is trivial, no ERROR_SYSCALL is issued. 3) Write buffer space not available - Peer notify-close packet not received In this case the first WRITE wait will bahave correctly. Then the following WRITE wait will be wasted, and the next READ will be ok. 4) Write buffer space not available - Peer notify-close packet received The first WRITE wait will block correctly. Then no more ERROR_SYSCALL will be issued. Or you can play with SIOCOUTQ, for systems that supports it. But whatever is the chosen method, that does not change the picture. That is, all this is butt-ugly, non conforming to the documentation, and non-coherent with the other OpenSSL APIs (SSL_read, SSL_write, ...). - Davide __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: make SSL_shutdown work with non-blocking BIOs
David Schwartz wrote:
If I'm misunderstanding the man page and/or the source code
please speak up.
My man page says:
If the underlying BIO is non-blocking, SSL_shutdown() will also
return
when the underlying BIO could not satisfy the needs of SSL_shutdown()
to continue the handshake. In this case a call to SSL_get_error()
with
the return value of SSL_shutdown() will yield SSL_ERROR_WANT_READ or
SSL_ERROR_WANT_WRITE. The calling process then must repeat the call
after taking appropriate action to satisfy the needs of
SSL_shutdown().
The action depends on the underlying BIO. When using a non-blocking
socket, nothing is to be done, but select() can be used to check for
the required condition. When using a buffering BIO, like a BIO pair,
data must be written into or retrieved out of the BIO before being
able
to continue.
My man page comes from Red Hat, 0.9.8a-5.4, Sep 28, 2006.
Yes but what SSL_shutdown() actually does is always return 0, in cases
where the above man page states it should return -1/WANT_{READ,WRITE}.
Please compare the man page to my semantics at:
http://marc.info/?l=openssl-dev&m=115153998821797&w=2
and tell me where I'm being contradictory to the documentation.
Now audit SSL_shutdown() before my PATCH2 and after my PATCH2 and see
how the -1 error returns are thrown away in possible scenario where -1
should be returned. In these cases 0 is always returned (which is not
what the man page states should be returned).
The operations of SSL_shutdown() are as following in the following order
of events:
* Flag that SSL_shutdown() has been called at least once, so that any
future calls to SSL_write() will always fail, thats becomes an
application error. [This can operation never fail.]
* Flush all outstanding encrypted application data to the BIO layer,
if there is any. [This can fail (EAGAIN) and should return
-1/WANT_WRITE, I can't remember if this is error is propagated or not]
* Write and flush the "send shutdown notify" packet to the BIO layer.
[This can fail (EAGAIN) and should return -1/WANT_WRITE, but IIRC it
never does from this point onwards 0 is always returned by future calls
to SSL_shutdown(), I propose the first time this was successful and
-1/WANT_READ would ordinarily be returned (due to failure in the next
step) then the 0 is returned, this marks this significant and important
event has been passed on the road to shutting down. The next time
SSL_shutdown() is called and we are still in the waiting for "recvd
shutdown notify" packet mode then -1/WANT_READ shall be returned instead.]
* Then attempt to read the "recvd shutdown notify" packet. [This can
fail, in that we have not seen the packet yet and should return
-1/WANT_READ, but IIRC it never does, it returns 0.]
* Flag the "recvd shutdown notify" packet has been seen so that any
future calls to SSL_read() will always fail. [This operation can never
fail.]
The above operations must be restartable and its important for the
SSL_shutdown() to indicate to the application if/when the it passes the
3rd point above, since the application may wish to do important stuff
with regards to the write side of the BIO (shutdown(fd, SHUT_WR) or
FD_CLR(fd, &fdwrite) forever more).
Darryl
__
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager [EMAIL PROTECTED]
RE: make SSL_shutdown work with non-blocking BIOs
> If I'm misunderstanding the man page and/or the source code > please speak up. My man page says: If the underlying BIO is non-blocking, SSL_shutdown() will also return when the underlying BIO could not satisfy the needs of SSL_shutdown() to continue the handshake. In this case a call to SSL_get_error() with the return value of SSL_shutdown() will yield SSL_ERROR_WANT_READ or SSL_ERROR_WANT_WRITE. The calling process then must repeat the call after taking appropriate action to satisfy the needs of SSL_shutdown(). The action depends on the underlying BIO. When using a non-blocking socket, nothing is to be done, but select() can be used to check for the required condition. When using a buffering BIO, like a BIO pair, data must be written into or retrieved out of the BIO before being able to continue. My man page comes from Red Hat, 0.9.8a-5.4, Sep 28, 2006. DS __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: make SSL_shutdown work with non-blocking BIOs
Davide Libenzi wrote: Can this be worked around? Sure. With some woodoo/ugly magic code in the async status code handling. You *cannot* always wait for read&write, since you'll be exiting the event selection loop immediately, every time. You need to bolt-in the shutdown logic *outside* the shutdown API, and lever over the fact that it is internally composed by a write, followed by a read. And setup the waited I/O flags accordingly. If that's an example of "working API" for someone, it's no surprise websphere blows. Yes, I'm not sure I follow your workarounds/jargon completely, the possible workarounds I can suggest would go something like this: Suggestion One) After the call to SSL_shutdown() that returns 0 starts a periodic timer, which calls SSL_shutdown() again without fail everytime that timer expires. This is because it is not possible to tell if the first SSL_shutdown() that returned 0 actually committed the "send shutdown notify" packet to the BIO layer. The point of the periodic timer is that your suggestion of monitoring fd writability does not work and can result in 100% CPU usage for the normal case that the 1st write to the BIO from the first call to SSL_shutdown() did commit the packet, because its impossible to tell if the "send shutdown packet" was written to the BIO from the return code from SSL_shutdown(). So the periodic time effectively is a form of rate limiting for the call that may restart/flush the data to the BIO. Instead of the normal event loop testing FD_ISSET(fd, &fdwrite) you rate limit to save your CPU. You would also put in place a timeout to ensure your application just gave up eventually. The cons of this workaround is the delay created by the periodic timer when the first attempt to write out the "send shutdown notify" packet failed. Suggestion Two) Implement your own BIO layer, so that you can see what libssl commits to the BIO layer. Then just before you call SSL_shutdown() signal to your BIO layer to be on the look out for a "send shutdown notify" packet. Then your BIO layer perform its own SSL packet decode (or guess the contents base on packet length), or nick/hijack/hack the crypto key and manually decode the packet somehow, probably a futile task with a CBC stream. Then after your SSL_shutdown() returns 0, you ask your custom BIO layer if it saw the "send shutdown notify" packet yet, if it did then you know for sure it got committed and can therefore from this point forward close down the send side shutdown(fd, SHUT_WR) and/or permanently ignore the writability indicator for that fd/BIO. I do not advocate any of these workarounds I'm simply making it clear how ugly a workaround is compared to the fix. I've just re-interpreted the SSL_shutdown() man page and it already documents the behavior my patch is providing (thats my interpretation), so if there is any bug its in someone not following the documentation but following their observation of the implementation (due to the implementation not following the documentation). The documentation as I interpret is good and would work for non-blocking; but the implementation is as this moment is bad. Its easy to audit the SSL_shutdown() code and see that it looses/drops the -1 error return in cases where the man page clearly says it will indicate them back to the application. If I'm misunderstanding the man page and/or the source code please speak up. Darryl __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: make SSL_shutdown work with non-blocking BIOs
On Mon, 1 Oct 2007, Darryl Miles wrote: > Richard Salz wrote: > > > double/triple check over it). Whatever fix you guys come up with, as > > long > > > as SSL_shutdown() ends up having sane (somehow aligned to SSL_read, > > > SSL_write, etc...) semantics WRT non-blocking BIOs. > > > > Nope. Maybe a new shutdown that has your semantics, but do not break > > existing code. > > Great lets do that then have a SSL_shutdown2() based on my patch. > > > Would Davide be so kind as to look over the following openssl-dev list post > for the semantics I suggest and confirm that logic would work for him: > > http://marc.info/?l=openssl-dev&m=115153998821797&w=2 > > > Maybe Davide could also try my PATCH2 and confirm that he can get sane working > non-blocking semantics with the new behavior. I think my patch is easier all > around and does not introduce this new WANT_SHUTDOWN voodoo (which I think is > unnecessary when I've been able to solve this problem in a way inline with the > way OpenSSL is). That is fine. The new WANT_SHUTDOWN can be removed w/out any problems. You could avoid changing ssl3_send_alert() too, and use s->s3->alert_dispatch as indicator of success, in case you want to minimize changes. Bu as I said, I don't care how it gets fixed. Can this be worked around? Sure. With some woodoo/ugly magic code in the async status code handling. You *cannot* always wait for read&write, since you'll be exiting the event selection loop immediately, every time. You need to bolt-in the shutdown logic *outside* the shutdown API, and lever over the fact that it is internally composed by a write, followed by a read. And setup the waited I/O flags accordingly. If that's an example of "working API" for someone, it's no surprise websphere blows. - Davide __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: make SSL_shutdown work with non-blocking BIOs
On Mon, 1 Oct 2007, Darryl Miles wrote: > The "Are you new here?" I find somewhat offputting, even through it was not > directed at me. Richard is obviously old here and set in his ways and thinks > that his OpenSSL library is better than it actually is. Oh, don't worry about that ;) I'm used to ml where ppl goes off the tangent at times, over the rude side. The main different though, is that those ppl somehow have a clue. - Davide __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: make SSL_shutdown work with non-blocking BIOs
Richard Salz wrote: double/triple check over it). Whatever fix you guys come up with, as long as SSL_shutdown() ends up having sane (somehow aligned to SSL_read, SSL_write, etc...) semantics WRT non-blocking BIOs. Nope. Maybe a new shutdown that has your semantics, but do not break existing code. Great lets do that then have a SSL_shutdown2() based on my patch. Would Davide be so kind as to look over the following openssl-dev list post for the semantics I suggest and confirm that logic would work for him: http://marc.info/?l=openssl-dev&m=115153998821797&w=2 Maybe Davide could also try my PATCH2 and confirm that he can get sane working non-blocking semantics with the new behavior. I think my patch is easier all around and does not introduce this new WANT_SHUTDOWN voodoo (which I think is unnecessary when I've been able to solve this problem in a way inline with the way OpenSSL is). I would also be surprised if my patch breaks existing code, since a return value of -1 should be allowed to SSL_shutdown(), all I'm doing is nailing down exactly what -1/WANT_WRITE, -1/WANT_READ, 0 and 1 return codes mean in the context of a shutdown. Yes its a change, yes its going to break someones code, so I'm more than happy with having a new SSL_shutdown2() API call, if you consider this a real compatibility issue. Darryl __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: make SSL_shutdown work with non-blocking BIOs
Davide Libenzi wrote: On Sat, 29 Sep 2007, Richard Salz wrote: I seriously doubt ppl are using SSL_shutdown() with non-blocking BIOs, together with the current API semantics. Seriously. Are you new here? This library has been around for more than a decade. There are *lots* of people using the current API with non-blocking. Which API are you talking about? SSL_shutdown? If yes, please show how elegantly non-blocking BIOs code can cope with the current SSL_shutdown semantics. I have to agree with you Davide. Please examine my sslregress program I posted to the list over the past 16 months which clearly provides a way to demonstrate the insane SSL_shutdown() semantics for non-blocking and clearly demonstrate the problem/bug. This sslregress application can also be used to test, prove, verify, fix any future API breakage that may result from these patches just in case someone is not sure of future breakage then split write your test case up into sslregress and prove it to everyone. The "Are you new here?" I find somewhat offputting, even through it was not directed at me. Richard is obviously old here and set in his ways and thinks that his OpenSSL library is better than it actually is. Maybe Richard should go and take a look at the previous threads and direct any comments in a constructive technical way than to try and guess what he thinks is right/wrong about the current SSL_shutdown() mechanism when applied to a non-blocking socket. So it would seem on this particular issue I would treat Richard's comments with the contempt they deserve. Darryl __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: make SSL_shutdown work with non-blocking BIOs
What is the difference between this an my patch from a year or so ago ? http://marc.info/?t=11509972822&r=1&w=2 '[PATCH] Fix for SSL_shutdown() with non-blocking not returning -1' http://marc.info/?t=11515400401&r=1&w=2 '[PATCH2] Fix for SSL_shutdown() with non-blocking not returning -1' http://marc.info/?t=11512908121&r=1&w=2 'SSL protocol state machine, IO layer, app layer regression testing' http://marc.info/?l=openssl-dev&m=116525974320575&w=2 'Re: Fix for SSL_shutdown() with non-blocking not returning -1' I have had numerous requests and thanks from other OpenSSL users over the past 16 months in connection with this patch. I always request that they themselves petition the maintainers of OpenSSL to deal with this matter. The problem I was exposing was very specific and I provided a test case mechanism to prove my patch fixed my problem. The specific problem I had is that if the write buffer in the kernel is full and you issue a shutdown and that shutdown does not make it into the kernel buffer (write() returns EAGAIN) then your SSL connection and that socket/BIO channel is hosed from that point on. This means its impossible to multiplex SSL connections over a single socket and get correct shutdown conditions. Darryl __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: make SSL_shutdown work with non-blocking BIOs
On Sun, 30 Sep 2007, Richard Salz wrote: > Wait for both, keep your own state. Works well enough. See the products > at the URL in my .sig for proof :) Wow! That *really* impressed me. So, besides throwing titles and sigs, can you show how easily can you cope with the current SSL_shutdown() behaviour when combined to non-blocking BIOs? Please? > The current API works. "Better" is not a reason to change it. Current SSL_shutdown() API works flawlessly with blocking BIOs. Please show the magic you have to do to make it work with non-blocking BIOs. You know what? Knowing the product you're going so proud of in your sigs, being a one-billion-threads design, I even doubt it uses non-blocking BIOs. - Davide __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: make SSL_shutdown work with non-blocking BIOs
Wait for both, keep your own state. Works well enough. See the products at the URL in my .sig for proof :) /r$ -- STSM, DataPower Chief Programmer Websphere DataPower SOA Appliances http://www.ibm.com/software/integration/datapower/ __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: make SSL_shutdown work with non-blocking BIOs
On Sun, 30 Sep 2007, Richard Salz wrote: > Define "elegantly." > > The current API works. "Better" is not a reason to change it. Since you're using SSL_shutdown by many years, would you be so kind to show me a C code snippet that handles the current SSL_shutdown semantics, when used together with non-blocking BIOs? I'm especially curious to know how the caller is going to setup the proper I/O event wait, when the *only* error code returned is ERROR_SYSCALL (for both read-miss and write-miss). - Davide __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: make SSL_shutdown work with non-blocking BIOs
Define "elegantly." The current API works. "Better" is not a reason to change it. /r$ -- STSM, DataPower Chief Programmer Websphere DataPower SOA Appliances http://www.ibm.com/software/integration/datapower/ __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: make SSL_shutdown work with non-blocking BIOs
On Sat, 29 Sep 2007, Richard Salz wrote: > > I seriously doubt ppl are using SSL_shutdown() with non-blocking BIOs, > > together with the current API semantics. Seriously. > > Are you new here? This library has been around for more than a decade. > There are *lots* of people using the current API with non-blocking. Which API are you talking about? SSL_shutdown? If yes, please show how elegantly non-blocking BIOs code can cope with the current SSL_shutdown semantics. - Davide __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: make SSL_shutdown work with non-blocking BIOs
> I seriously doubt ppl are using SSL_shutdown() with non-blocking BIOs, > together with the current API semantics. Seriously. Are you new here? This library has been around for more than a decade. There are *lots* of people using the current API with non-blocking. Seriously. > double/triple check over it). Whatever fix you guys come up with, as long > as SSL_shutdown() ends up having sane (somehow aligned to SSL_read, > SSL_write, etc...) semantics WRT non-blocking BIOs. Nope. Maybe a new shutdown that has your semantics, but do not break existing code. /r$ -- STSM, DataPower Chief Programmer Websphere DataPower SOA Appliances http://www.ibm.com/software/integration/datapower/ __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: make SSL_shutdown work with non-blocking BIOs
On Sat, 29 Sep 2007, Thor Lancelot Simon wrote: > On Sat, Sep 29, 2007 at 03:35:29PM -0700, Davide Libenzi wrote: > > > > I seriously doubt ppl are using SSL_shutdown() with non-blocking BIOs, > > together with the current API semantics. Seriously. > > Well, how do you suppose they're terminating their SSL sessions? If you > look at the archive of this list, you'll see evidence that, in fact, > there are users of the non-blocking interface -- pretty much all of whom > find it poorly designed and implemented, but, they're users nonetheless. > > I find the API resulting from your change considerably better. I just am > concerned about breaking other people's existing code. And, unfortunately, > the text in the RETURN VALUES does actually document how the existing > (stupid) API works. > > On balance maybe it's just best to apply your change. It's hardly up to > _me_ whether that happens in the public OpenSSL distribution! It does > look correct to me and I've applied it to my local tree, since I control > all the code that's linked into. ;-) It doesn't have to be *my* patch to be applied (I actually suggest a double/triple check over it). Whatever fix you guys come up with, as long as SSL_shutdown() ends up having sane (somehow aligned to SSL_read, SSL_write, etc...) semantics WRT non-blocking BIOs. - Davide __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: make SSL_shutdown work with non-blocking BIOs
On Sat, Sep 29, 2007 at 03:35:29PM -0700, Davide Libenzi wrote: > > I seriously doubt ppl are using SSL_shutdown() with non-blocking BIOs, > together with the current API semantics. Seriously. Well, how do you suppose they're terminating their SSL sessions? If you look at the archive of this list, you'll see evidence that, in fact, there are users of the non-blocking interface -- pretty much all of whom find it poorly designed and implemented, but, they're users nonetheless. I find the API resulting from your change considerably better. I just am concerned about breaking other people's existing code. And, unfortunately, the text in the RETURN VALUES does actually document how the existing (stupid) API works. On balance maybe it's just best to apply your change. It's hardly up to _me_ whether that happens in the public OpenSSL distribution! It does look correct to me and I've applied it to my local tree, since I control all the code that's linked into. ;-) Thor __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: make SSL_shutdown work with non-blocking BIOs
On Sat, 29 Sep 2007, Thor Lancelot Simon wrote:
> On Sat, Sep 29, 2007 at 03:11:18PM -0700, Davide Libenzi wrote:
> >
> > Heh? Wait for read&write? Consider such code:
> >
> > for (;;) {
> > err = SSL_shutdown();
> > code = SSL_get_error(ssl, err);
> > if (code == SSL_ERROR_SYSCALL) {
> > select(SSL_get_fd(ssl), RDWR);
>
> This is not how select() works, but I assume you know that and are
> intentionally writing pseudocode. However, what it elides is that,
> generally, applications using select() for nonblocking I/O are managing
> many sockets at once. In practice, selecting on one of those sockets in
> both directions when you only care about one direction is almost zero
> cost.
Yes, I think I know something about select(2), poll(2), and handling many
sockets at a time ;) That was obviously pseudocode.
Handling many sockets at a time does not change the picture though. You'll
permanently exit your I/O scheduler because you get for an event that the
underline API is not waiting for. In the multiple socket example it can be
even worse, since you can have the fd array setup/teardown costs.
> Obviously, if you're shutting down, and the socket's returned ready for
> write once, you select only for read; the corresponsing condition is true
> in the opposite direction.
>
> Yes, it's a stupid API. Yes, it's a nuisance to work around it. But
> changing it is _still_ going to break people's code that switches on what
> were, previously, the only possible error codes that could be returned
> during a shutdown -- which didn't include WANT_READ and WANT_WRITE.
>
> I can easily fix the code _I_ care about to conform to the (better) API
> you're proposing. But I don't think that's a particularly good reason to
> break other people's code, and pretending that it was impossible to get
> the job done isn't tremendously helpful either; there really is a lot of
> code out there already that uses non-blocking BIOs despite all their
> warts (see my complaints about the need for SSL_select() from several
> months ago for another example) and that code really, truly does manage
> to shut down sessions without spinning.
I seriously doubt ppl are using SSL_shutdown() with non-blocking BIOs,
together with the current API semantics. Seriously.
- Davide
__
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager [EMAIL PROTECTED]
RE: make SSL_shutdown work with non-blocking BIOs
Thor Simon wrote:
> On Sat, Sep 29, 2007 at 03:11:18PM -0700, Davide Libenzi wrote:
> >
> > Heh? Wait for read&write? Consider such code:
> >
> > for (;;) {
> > err = SSL_shutdown();
> > code = SSL_get_error(ssl, err);
> > if (code == SSL_ERROR_SYSCALL) {
> > select(SSL_get_fd(ssl), RDWR);
> This is not how select() works, but I assume you know that and are
> intentionally writing pseudocode. However, what it elides is that,
> generally, applications using select() for nonblocking I/O are managing
> many sockets at once. In practice, selecting on one of those sockets in
> both directions when you only care about one direction is almost zero
> cost.
Selecting on one of those sockets repeatedly in a direction for which it is
ready but for which you do not intend to perform an operation has maximal
cost. It will push your CPU consumption right up to 100%.
> Obviously, if you're shutting down, and the socket's returned ready for
> write once, you select only for read; the corresponsing condition is true
> in the opposite direction.
Umm, why? Just becuase you're still shutting down doesn't mean the OpenSSL
library is still trying to perform the same operation. Maybe before it was
trying to send its shutdown but now it's waiting for a response. Maybe
before it was waiting to finish a renegotiation and now it needs to send an
indication to the other side that the connection should be shutdown.
> Yes, it's a stupid API. Yes, it's a nuisance to work around it. But
> changing it is _still_ going to break people's code that switches on what
> were, previously, the only possible error codes that could be returned
> during a shutdown -- which didn't include WANT_READ and WANT_WRITE.
Please explain how you work around it. I think you have some hacks that
"just happen" to work, but no sound way that's guaranteed to work. If you
select for write, you can spin forever. If you don't select for write,
you're simply hoping that OpenSSL didn't actually need to write data.
> I can easily fix the code _I_ care about to conform to the (better) API
> you're proposing. But I don't think that's a particularly good reason to
> break other people's code, and pretending that it was impossible to get
> the job done isn't tremendously helpful either; there really is a lot of
> code out there already that uses non-blocking BIOs despite all their
> warts (see my complaints about the need for SSL_select() from several
> months ago for another example) and that code really, truly does manage
> to shut down sessions without spinning.
This is one of the many reasons I always use BIO pairs.
DS
__
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager [EMAIL PROTECTED]
Re: make SSL_shutdown work with non-blocking BIOs
On Sat, Sep 29, 2007 at 03:11:18PM -0700, Davide Libenzi wrote:
>
> Heh? Wait for read&write? Consider such code:
>
> for (;;) {
> err = SSL_shutdown();
> code = SSL_get_error(ssl, err);
> if (code == SSL_ERROR_SYSCALL) {
> select(SSL_get_fd(ssl), RDWR);
This is not how select() works, but I assume you know that and are
intentionally writing pseudocode. However, what it elides is that,
generally, applications using select() for nonblocking I/O are managing
many sockets at once. In practice, selecting on one of those sockets in
both directions when you only care about one direction is almost zero
cost.
> continue;
> }
> ...
> }
>
> Now consider a typical case, where, just for example, our notify-close is
> already gone and we're wating for the peer notify-close. Output buffer are
> empty, so the select(RDWR) returns immediately every time you call it.
Obviously, if you're shutting down, and the socket's returned ready for
write once, you select only for read; the corresponsing condition is true
in the opposite direction.
Yes, it's a stupid API. Yes, it's a nuisance to work around it. But
changing it is _still_ going to break people's code that switches on what
were, previously, the only possible error codes that could be returned
during a shutdown -- which didn't include WANT_READ and WANT_WRITE.
I can easily fix the code _I_ care about to conform to the (better) API
you're proposing. But I don't think that's a particularly good reason to
break other people's code, and pretending that it was impossible to get
the job done isn't tremendously helpful either; there really is a lot of
code out there already that uses non-blocking BIOs despite all their
warts (see my complaints about the need for SSL_select() from several
months ago for another example) and that code really, truly does manage
to shut down sessions without spinning.
Thor
__
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager [EMAIL PROTECTED]
Re: make SSL_shutdown work with non-blocking BIOs
On Sat, 29 Sep 2007, Thor Lancelot Simon wrote:
> > As far as changes to the existing behaviour, blocking BIOs will never get
> > the new error code (<0). And noone could have used the non-blocking BIOs
> > in a sane way, with the current behavior
> > (lack of proper WANT_READ/WANT_WRITE).
>
> I'm sorry, but your assertion about WANT_READ/WANT_WRITE is false. In
> practice, the way it's been for many years is that if you get
> SSL_ERROR_SYSCALL back, you simply put the socket in your select set for
> all event types (read/write/exception). Yes, it is a rough spot in the
> API. Yes, in an ideal world, it would never have worked this way. The
> problem, as with so much else in OpenSSL, though, is that there's a lot
> of application code out there by now that knows how the API actually
> works -- not how it _should_ work, but how it _does_ -- and adding
> new error returns that weren't possible before will break some of that
> code.
Heh? Wait for read&write? Consider such code:
for (;;) {
err = SSL_shutdown();
code = SSL_get_error(ssl, err);
if (code == SSL_ERROR_SYSCALL) {
select(SSL_get_fd(ssl), RDWR);
continue;
}
...
}
Now consider a typical case, where, just for example, our notify-close is
already gone and we're wating for the peer notify-close. Output buffer are
empty, so the select(RDWR) returns immediately every time you call it.
Same happens in the case where our output buffer are full, we're trying to
send our notify-close, and the peer one is already on our socket buffers.
So, how much CPU is that gonna eat?
It'd be great to see how your code sanely handles SSL_ERROR_SYSCALL with
non-blocking BIOs.
- Davide
__
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager [EMAIL PROTECTED]
Re: make SSL_shutdown work with non-blocking BIOs
On Sat, Sep 29, 2007 at 01:19:38PM -0700, Davide Libenzi wrote: > > But that code *never* returns WANT_READ/WANT_WRITE. Non blocking sockets > always get SSL_ERROR_SYSCALL. Well, unless the case where they both > succeed immediately - but that's like blocking behaviour. Yes, I'm well aware of that. I maintain a great deal of code that uses OpenSSL with non-blocking BIOs, and, yes, when any underlying condition causes the session to not be ready to be shut down, SSL_ERROR_SYSCALL is always the error code returned. No, it's not ideal. Yes, it's what existing application code expects. So it's necessary to be careful about changing this. > As far as changes to the existing behaviour, blocking BIOs will never get > the new error code (<0). And noone could have used the non-blocking BIOs > in a sane way, with the current behavior > (lack of proper WANT_READ/WANT_WRITE). I'm sorry, but your assertion about WANT_READ/WANT_WRITE is false. In practice, the way it's been for many years is that if you get SSL_ERROR_SYSCALL back, you simply put the socket in your select set for all event types (read/write/exception). Yes, it is a rough spot in the API. Yes, in an ideal world, it would never have worked this way. The problem, as with so much else in OpenSSL, though, is that there's a lot of application code out there by now that knows how the API actually works -- not how it _should_ work, but how it _does_ -- and adding new error returns that weren't possible before will break some of that code. Thor __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: make SSL_shutdown work with non-blocking BIOs
On Sat, 29 Sep 2007, Thor Lancelot Simon wrote: > On Sat, Sep 29, 2007 at 12:51:52PM -0700, Davide Libenzi wrote: > > > > The reason I posted the patch was because I noticed a SSL_ERROR_SYSCALL > > back from SSL_get_error(). > > This is what the documentation says: > > > > -- > > If the underlying BIO is non-blocking, SSL_shutdown() will also return > > when the underlying BIO could not satisfy the needs of SSL_shutdown() to > > continue the handshake. In this case a call to SSL_get_error() with the > > return value of SSL_shutdown() will yield SSL_ERROR_WANT_READ or > > SSL_ERROR_WANT_WRITE. The calling process then must repeat the call after > > taking appropriate action to satisfy the needs of SSL_shutdown(). The > > action depends on the underlying BIO. When using a non-blocking socket, > > nothing is to be done, but select() can be used to check for the required > > condition. When using a buffering BIO, like a BIO pair, data must be > > written into or retrieved out of the BIO before being able to continue. > > The documentation *also* says: > > RETURN VALUES >The following return values can occur: > >1 The shutdown was successfully completed. The "close notify" alert >was sent and the peer's "close notify" alert was received. > >0 The shutdown is not yet finished. Call SSL_shutdown() for a second >time, if a bidirectional shutdown shall be performed. The output >of SSL_get_error(3) may be misleading, as an erroneous >SSL_ERROR_SYSCALL may be flagged even though no error occurred. > > I guess the question is whether this SSL_ERROR_SYSCALL is "erroneous" or > not. I assume your intent here is to actually get back WANT_READ or > WANT_WRITE so you can put the underlying socket in the correct select set, > and I agree that that would be a useful thing to be able to do. But I am > concerned that adding possible return values here would break existing > code that switches on the "possible" return values from the SSL_get_error > in this case. I realize that that's not strictly supported by the API as > documented now, but looking through one codebase I work on, I do in fact > see a number of things your change might break if it added new possible > error returns there... But that code *never* returns WANT_READ/WANT_WRITE. Non blocking sockets always get SSL_ERROR_SYSCALL. Well, unless the case where they both succeed immediately - but that's like blocking behaviour. As far as changes to the existing behaviour, blocking BIOs will never get the new error code (<0). And noone could have used the non-blocking BIOs in a sane way, with the current behavior (lack of proper WANT_READ/WANT_WRITE). - Davide __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: make SSL_shutdown work with non-blocking BIOs
On Sat, Sep 29, 2007 at 12:51:52PM -0700, Davide Libenzi wrote: > > The reason I posted the patch was because I noticed a SSL_ERROR_SYSCALL > back from SSL_get_error(). > This is what the documentation says: > > -- > If the underlying BIO is non-blocking, SSL_shutdown() will also return > when the underlying BIO could not satisfy the needs of SSL_shutdown() to > continue the handshake. In this case a call to SSL_get_error() with the > return value of SSL_shutdown() will yield SSL_ERROR_WANT_READ or > SSL_ERROR_WANT_WRITE. The calling process then must repeat the call after > taking appropriate action to satisfy the needs of SSL_shutdown(). The > action depends on the underlying BIO. When using a non-blocking socket, > nothing is to be done, but select() can be used to check for the required > condition. When using a buffering BIO, like a BIO pair, data must be > written into or retrieved out of the BIO before being able to continue. The documentation *also* says: RETURN VALUES The following return values can occur: 1 The shutdown was successfully completed. The "close notify" alert was sent and the peer's "close notify" alert was received. 0 The shutdown is not yet finished. Call SSL_shutdown() for a second time, if a bidirectional shutdown shall be performed. The output of SSL_get_error(3) may be misleading, as an erroneous SSL_ERROR_SYSCALL may be flagged even though no error occurred. I guess the question is whether this SSL_ERROR_SYSCALL is "erroneous" or not. I assume your intent here is to actually get back WANT_READ or WANT_WRITE so you can put the underlying socket in the correct select set, and I agree that that would be a useful thing to be able to do. But I am concerned that adding possible return values here would break existing code that switches on the "possible" return values from the SSL_get_error in this case. I realize that that's not strictly supported by the API as documented now, but looking through one codebase I work on, I do in fact see a number of things your change might break if it added new possible error returns there... Thor __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: make SSL_shutdown work with non-blocking BIOs
On Sat, 29 Sep 2007, Thor Lancelot Simon wrote: > On Sat, Sep 29, 2007 at 11:28:26AM -0700, Davide Libenzi wrote: > > Would it be possible to make SSL_shutdown() on non-blocking BIOs, conform > > to the documentation and aligned to SSL_read, SSL_write, ...? > > > > http://www.openssl.org/docs/ssl/SSL_shutdown.html > > > > I cooked a tentative patch below, that seems to be working here. > > I don't understand -- as far as I can tell, the current implementation > does, in fact, conform to the documentation -- though it does not > work exactly the same way as SSL_read, SSL_write, etc. It is not > documented to work the same way. The reason I posted the patch was because I noticed a SSL_ERROR_SYSCALL back from SSL_get_error(). This is what the documentation says: -- If the underlying BIO is non-blocking, SSL_shutdown() will also return when the underlying BIO could not satisfy the needs of SSL_shutdown() to continue the handshake. In this case a call to SSL_get_error() with the return value of SSL_shutdown() will yield SSL_ERROR_WANT_READ or SSL_ERROR_WANT_WRITE. The calling process then must repeat the call after taking appropriate action to satisfy the needs of SSL_shutdown(). The action depends on the underlying BIO. When using a non-blocking socket, nothing is to be done, but select() can be used to check for the required condition. When using a buffering BIO, like a BIO pair, data must be written into or retrieved out of the BIO before being able to continue. -- If you look at the current code, ssl3_shutdown() returns either zero or one. In case we did not get the peer shutdown yet, it returns zero, even if a BIO-write failed (output buffers full). And this ends up in a SSL_ERROR_SYSCALL back from SSL_get_error(), instead of a WANT_WRITE. Same once we sent the shutdown and we're waiting to receive the peer close-notify packet. We get SSL_ERROR_SYSCALL instead of a WANT_READ. You can try it by yourself if you don't believe. I actually encourage you to try. > Why do you think it would be better if your patch were applied? Are > you sure your change will not break existing code that works with the > current semantics? The patch makes it actualy work as expected/documented for non-blocking BIOs. For blocking BIOs it behaves the same. Patch needs double-check of course. - Davide __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: make SSL_shutdown work with non-blocking BIOs
On Sat, Sep 29, 2007 at 11:28:26AM -0700, Davide Libenzi wrote: > Would it be possible to make SSL_shutdown() on non-blocking BIOs, conform > to the documentation and aligned to SSL_read, SSL_write, ...? > > http://www.openssl.org/docs/ssl/SSL_shutdown.html > > I cooked a tentative patch below, that seems to be working here. I don't understand -- as far as I can tell, the current implementation does, in fact, conform to the documentation -- though it does not work exactly the same way as SSL_read, SSL_write, etc. It is not documented to work the same way. Why do you think it would be better if your patch were applied? Are you sure your change will not break existing code that works with the current semantics? Thor __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
make SSL_shutdown work with non-blocking BIOs
Would it be possible to make SSL_shutdown() on non-blocking BIOs, conform
to the documentation and aligned to SSL_read, SSL_write, ...?
http://www.openssl.org/docs/ssl/SSL_shutdown.html
I cooked a tentative patch below, that seems to be working here.
It definitely need double check from someone that has openssl source code
in sight by more than a few minutes (like myself).
The first shutdown will return (modulo other errors) either <0 with
SSL_get_error=WANT_WRITE, or 0 with SSL_get_error=WANT_SHUTDOWN.
The second shutdown will return (modulo other errors) either <0 with
SSL_get_error=WANT_READ, or 1 with SSL_get_error=NONE.
- Davide
---
ssl/s3_lib.c | 18 +-
ssl/ssl.h |1 +
ssl/ssl_lib.c |9 +
3 files changed, 19 insertions(+), 9 deletions(-)
Index: openssl-0.9.8e/ssl/s3_lib.c
===
--- openssl-0.9.8e.orig/ssl/s3_lib.c2007-09-27 23:56:00.0 -0700
+++ openssl-0.9.8e/ssl/s3_lib.c 2007-09-28 15:02:23.0 -0700
@@ -2241,6 +2241,7 @@
int ssl3_shutdown(SSL *s)
{
+ int err;
/* Don't do anything much if we have not done the handshake or
* we don't want to send messages :-) */
@@ -2258,25 +2259,24 @@
#endif
/* our shutdown alert has been sent now, and if it still needs
* to be written, s->s3->alert_dispatch will be true */
+ if (!s->s3->alert_dispatch)
+ return s->shutdown & SSL_RECEIVED_SHUTDOWN ? 1: 0;
}
- else if (s->s3->alert_dispatch)
+ if (s->s3->alert_dispatch)
{
/* resend it if not sent */
#if 1
- s->method->ssl_dispatch_alert(s);
+ err = s->method->ssl_dispatch_alert(s);
#endif
}
else if (!(s->shutdown & SSL_RECEIVED_SHUTDOWN))
{
/* If we are waiting for a close from our peer, we are closed */
- s->method->ssl_read_bytes(s,0,NULL,0,0);
+ err = s->method->ssl_read_bytes(s,0,NULL,0,0);
}
-
- if ((s->shutdown == (SSL_SENT_SHUTDOWN|SSL_RECEIVED_SHUTDOWN)) &&
- !s->s3->alert_dispatch)
- return(1);
- else
- return(0);
+ if (err < 0)
+ return err;
+ return s->shutdown & SSL_RECEIVED_SHUTDOWN ? 1: 0;
}
int ssl3_write(SSL *s, const void *buf, int len)
Index: openssl-0.9.8e/ssl/ssl.h
===
--- openssl-0.9.8e.orig/ssl/ssl.h 2007-09-27 23:56:00.0 -0700
+++ openssl-0.9.8e/ssl/ssl.h2007-09-28 12:26:16.0 -0700
@@ -1128,6 +1128,7 @@
#define SSL_ERROR_ZERO_RETURN 6
#define SSL_ERROR_WANT_CONNECT 7
#define SSL_ERROR_WANT_ACCEPT 8
+#define SSL_ERROR_WANT_SHUTDOWN9
#define SSL_CTRL_NEED_TMP_RSA 1
#define SSL_CTRL_SET_TMP_RSA 2
Index: openssl-0.9.8e/ssl/ssl_lib.c
===
--- openssl-0.9.8e.orig/ssl/ssl_lib.c 2007-09-27 23:56:00.0 -0700
+++ openssl-0.9.8e/ssl/ssl_lib.c2007-09-28 12:26:16.0 -0700
@@ -2017,6 +2017,15 @@
}
else
{
+ /*
+* SSL_get_error() may be called with code==0 due to the
+* a partially completed SSL_shutdown() returning 0.
+* This is a state where the shutdown packet has been
+* successfully sent, but we didn't get the peer one
yet.
+*/
+ if ((s->shutdown & SSL_SENT_SHUTDOWN) &&
!s->s3->alert_dispatch &&
+ (s->shutdown & SSL_RECEIVED_SHUTDOWN) == 0)
+ return SSL_ERROR_WANT_SHUTDOWN;
if ((s->shutdown & SSL_RECEIVED_SHUTDOWN) &&
(s->s3->warn_alert == SSL_AD_CLOSE_NOTIFY))
return(SSL_ERROR_ZERO_RETURN);
__
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager [EMAIL PROTECTED]
