[openssl-dev] [openssl.org #4572] SSL_set_bio and friends

[David Benjamin]

On Fri, Jul 29, 2016 at 9:21 AM Matt Caswell via RT <rt at openssl.org> wrote:

> On Tue Jun 14 20:30:09 2016, davidben at google.com wrote:
> > I recently made some changes around BoringSSL's SSL_set_bio, etc.
> > which you
> > all might be interested in. The BIO management has two weird behaviors
> > right now:
> >
> > 1. The existence of bbio is leaked in the public API when it should be
> > an
> > implementation detail. (Otherwise you're stuck with it for DTLS where
> > it's
> > really messy.) SSL_get_wbio will return it, and SSL_set_bio messes up
> > when
> > the bbio is active.
>
> Fixed by 2e7dc7cd688.
>
> > 2. SSL_set_bio's object ownership story is a mess. It also doesn't
> > quite
> > work. This crashes:
> > SSL_set_fd(ssl, 1);
> > SSL_set_rfd(ssl, 2);
> > But this does not:
> > SSL_set_fd(ssl, 1);
> > SSL_set_wfd(ssl, 2);
> > Not that anyone would do such a thing, but the asymmetry is off.
>
> Fixed by 2e7dc7cd688 and in the docs by e040a42e44.
>
> I also added a test, which I verified against the original 1.0.2
> implementation
> of SSL_set_bio(), in 7fb4c82035.
>
> I found I needed to make some tweaks to the implementation of SSL_set_bio()
> from your version in order to preserve the behaviour between 1.0.2 and
> master.
> Possibly your version was a deliberate simplification.


Hrm. It's been a while, but I believe it was a deliberate simplification to
fix the SSL_set_rfd crash above.

My interpretation was that SSL_set_rfd and SSL_set_wfd's calling
pattern, insane
as it is, is definitively correct. Thus, the asymmetry in SSL_set_bio is a
bug and was a deliberate change on my part.

It seems your interpretation was that SSL_set_bio's behavior, insane as it
is, is definitively correct. Thus the bug lies in SSL_set_[rw]fd using
SSL_set_bio wrong, so you changed those functions and kept SSL_set_bio's
behavior as-is. (I'm not sure SSL_set_bio actually lets you implement
SSL_set_rfd, but you have the new side-specific setters and just used that.)

I like my interpretation slightly better if only because it makes
SSL_set_bio's behavior a little more sane. Judging by SSL_set_[rw]fd, it
also seems to have been the original intent. It is a behavior change, but
one I'm sure will break no one.

(If you still prefer yours, I will make BoringSSL match since I see no
reason for us to diverge here.)

David
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mta.openssl.org/pipermail/openssl-dev/attachments/20160730/a52bf86e/attachment.html>