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

> On 11 Feb 2024, at 19:19, David Benjamin <davidben@google.com> wrote:
>
> Hi all,
>
> I've attached a patch for the master branch to fix up the custom BIOs used by PostgreSQL, in light of the issues with
theOpenSSL update recently. While c82207a548db47623a2bfa2447babdaa630302b9 (switching from BIO_get_data to
BIO_get_app_data)resolved the immediate conflict, I don't think it addressed the root cause, which is that PostgreSQL
wasmixing up two BIO implementations, each of which expected to be driving the BIO. 

Thanks for your patch, I'm still digesting it so will provide more of a review
later.

> It turns out the parts that came from the OpenSSL socket BIO were a no-op, and in fact PostgreSQL is relying on it
beinga no-op. Instead, it's cleaner to just define a custom BIO the normal way, which then leaves the more standard
BIO_get_datamechanism usable. This also avoids the risk that a future OpenSSL will add a now BIO_ctrl to the socket
type,with libssl calling into it, and then break some assumptions made by PostgreSQL. 

+        case BIO_CTRL_FLUSH:
+            /* libssl expects all BIOs to support BIO_flush. */
+            res = 1;
+            break;

Will this always be true?  Returning 1 implies that we have flushed all data on
the socket, but what if we just before called BIO_set_retry..XX()?

> I've attached a patch which does that. The existing SSL tests pass with it, tested on Debian stable. (Though it took
mea few iterations to figure out how to run the SSL tests, so it's possible I've missed something.) 

We've done a fair bit of work on making them easier to run, so I'm curious if
you saw any room for improvements there as someone coming to them for the first
time?

--
Daniel Gustafsson