Re: [PATCH] Avoid mixing custom and OpenSSL BIO functions

> On 3 Sep 2024, at 14:18, Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:

> Attached is a v4 rebase over the recent OpenSSL 1.0.2 removal which made this
> patch no longer apply. I've just started to dig into it so have no comments on
> it right now, but wanted to get a cleaned up version into the CFBot.

CFBot building green for this, I just have a few small questions/comments:

+ my_bio_index |= BIO_TYPE_SOURCE_SINK;

According to the OpenSSL docs we should set BIO_TYPE_DESCRIPTOR as well as this
BIO is socket based, but it's not entirely clear to me why. Is there a
specific reason it was removed?

+ bio_method = port_bio_method();
if (bio_method == NULL)
{
SSLerr(SSL_F_SSL_SET_FD, ERR_R_BUF_LIB);

SSL_F_SSL_SET_FD is no longer the correct function context for this error
reporting. In OpenSSL 3.x it means nothing since SSLerr throws away the
function when calling ERR_raise_data, but we still support 1.1.0+. I think the
correct error would be BIOerr(BIO_F_BIO_METH_NEW..) but I wonder if we should
just remove it since BIO_meth_new and BIO_new already set errors for us to
consume? It doesn't seem to make sense to add more errors on the queue here?
The same goes for the frontend part.

The attached v5 is a fresh rebase with my comments from above as 0002 to
illustrate.

--
Daniel Gustafsson