On Fri, Jul 29, 2016 at 9:21 AM Matt Caswell via RT <r...@openssl.org> wrote:
> On Tue Jun 14 20:30:09 2016, david...@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 -- Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4572 Please log in as guest with password guest if prompted -- openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
