>   To answer my own question: No.  Here is an amended version.

        While I believe your code is okay, it can be improved in a few ways. It
contains some assumptions that are not always true, and it will work better
without those assumptions.

> > for(cp = connobjs; cp; cp = cp->next)
> >     cp->want_write = false;
> >
> > while(!quit) {
> >   for(cp = connobjs; cp; cp = cp->next) {
> >     if(cp->want_write || cp->output_buffer) {

        Here your code contains the assumption that because you want to write
encrypted data to OpenSSL, SSL will need to write to the socket. It probably
will, but that's just as assumption. The test for cp->output_buffer should
not be there.

> >             /* either openssl wants to say something, or
> >              * we want to send something */
> >             FD_SET(cp->fd, &writefds);
> >     } else {
> >             FD_SET(cp->fd, &readfds);
> >     }
> >   }
> >
> >   select(maxfd + 1, &readfds, &writefds, NULL, &timeout);
> >
> >   for(cp = connobjs; cp; cp = cp->next) {
> >     if(cp->want_write || cp->output_buffer) {
> >             if(FD_ISSET(&cp->fd, &writefds) {
> >                     if(cp->output_buffer) {
> >                             byteswr = SSL_write(cp->sslobj,
> >                                     cp->output_buffer,
> >                                     cp->output_bufsz);
> >                             if(byteswr <= 0) {
> >                                     err = ERR_get_error(cp->sslobj);
> >                                     if(err == SSL_ERROR_WANT_WRITE)
> >                                             cp->want_write = true;
> >                             } else {
> >
> remove_bytes_from_beginning_of_buffer(cp->output_buffer,
> >                                                     cp->output_bufsz);
> >                             }

>                       } else {

        You don't call SSL_read if the socket is writable. That doesn't make any
sense.

>                               bytesrd = SSL_read(cp->sslobj,
>                                               cp->input_buffer,
>                                               cp->input_bufsz);
>                               if(bytesrd <= 0) {
>                                       err = ERR_get_error(cp->sslobj);
>                                       if(err == SSL_ERROR_WANT_WRITE)
>                                               cp->want_write = true;
>                               } else {
>                                       do_something_with_buffer(cp);
>                               }
>                       }
>               } else if(FD_ISSET(cp->fd, &readfds)) {
>                       bytesrd = SSL_read(cp->sslobj,
>                                       cp->input_buffer,
>                                       cp->input_bufsz);
>                       if(bytesrd <= 0) {
>                               err = ERR_get_error(cp->sslobj);
>                               if(err == SSL_ERROR_WANT_WRITE)
>                                       cp->want_write = true;
>                       } else {
>                               do_something_with_buffer(cp);
>                       }
>               }
> >     }
> >     }
> > }

        You should 'select' for writability if and only if you get a WANT_WRITE
indication, whether it comes from SSL_read or SSL_write. This is the *only*
thing that means that you would block while trying to send encrypted data to
the socket. (Whether or not you are trying to currently send any unencrypted
data has nothing to do with it.)

        If you get a 'select' hit, whether for readability or writability, you
should retry *all* operations, whether reads or writes. (Obviously, don't
call SSL_write unless you have some data to write!)

        Your code assumes that there is some correlation between the motion of
encrypted data and the motion of decrypted data. While, of course, there is,
your code should pretend there is not and let the SSL engine figure that
out.

        Where do you clear 'want_write'?

        Again, I also recommend trying an SSL_read on any hit, whether for
readability or writability, before doing anything else. This will 100%
ensure you don't deadlock (as long as you call SSL_read after each SSL_write
until it runs out) nd it will give you better behavior under edge conditions
like memory pressure.

>Are you suggesting that the write hit from select is an
>indication that the descriptor is writable and hence the data
>that SSL required to write before it could serve our read
>request must have been written. And so we would expect a
>successful SSL_read after this write hit from select?

        If you got a WANT_WRITE, that means the operation could not proceed 
until
encrypted data could be written. Once the socket is writable, the operation
can proceed. So you should retry it. No harm in retrying all current
operations (always SSL_read, but SSL_write if there's unencrypted data to
send).

Moreover, should we not wait again for a read hit from select
before we SSL_read()?

        That would be a disaster. The corresponding encrypted data might have
already been read. You should *never* assume no inbound data will be
available until the socket is readable unless SSL_read just returned
WANT_READ and you have no called SSL_write since then.

        The SSL engine may have already read the encrypted data, during a 
previous
call to SSL_read or a previous call to SSL_write! So waiting for more data
to be received can deadlock you.

        Do not assume that no unencrypted data can be read until encrypted data 
can
be read unless you know this for a fact. The only way you can know this for
a fact is if two things are the case:

        1) Your last SSL_read returned WANT_READ, *and*

        2) You have not called SSL_write since then.

        Otherwise, you should *not* wait for readability before calling 
SSL_read.
The encrypted data may have already been read.

        Note that if an SSL_write returns WANT_READ, you should 'select' for
readability and then retry the SSL_write. No harm in doing an SSL_read
before or after that. In fact, there are good reasons for doing one both
before and after, assuming the first SSL_read doesn't return 'WANT_READ'.

        DS


______________________________________________________________________
OpenSSL Project                                 http://www.openssl.org
User Support Mailing List                    openssl-users@openssl.org
Automated List Manager                           [EMAIL PROTECTED]

Reply via email to