Re: SSL BIO wrappers

Attached patch replaces the SSL BIO wrapper code we have now, with one
that directly calls the send() and recv() functions instead. THis means
that they get passed through the rewrite macros to our internal
functions on Win32, and I think this will fix some of the strange errors
that seem to be platform specific there (there are some really hard to
reproduce bug reports around that).

They're of course tightly modeled around the code from OpenSSL - found
here: http://cvs.openssl.org/fileview?f=openssl/crypto/bio/bss_sock.c&v=1.15

(functions sock_read and sock_write)

So:

1) Thoughts in general?

2) Per my_sock_write - should we do the prepare read there as well, even
though it's a write? :-)

//Magnus

Magnus Hagander <magnus(at)hagander(dot)net> writes:
> Attached patch replaces the SSL BIO wrapper code we have now, with one
> that directly calls the send() and recv() functions instead. THis means
> that they get passed through the rewrite macros to our internal
> functions on Win32, and I think this will fix some of the strange errors
> that seem to be platform specific there (there are some really hard to
> reproduce bug reports around that).

Hmm. Basically what this is doing is exactly what the comment says we
didn't want to do, namely copy-and-paste the implementations of
OpenSSL's socket BIO functions. How stable is that code? If the
functions haven't changed textually in a long time (at least across all
the OpenSSL versions we claim to support) then maybe it's okay.

regards, tom lane

Tom Lane wrote:
> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>> Attached patch replaces the SSL BIO wrapper code we have now, with one
>> that directly calls the send() and recv() functions instead. THis means
>> that they get passed through the rewrite macros to our internal
>> functions on Win32, and I think this will fix some of the strange errors
>> that seem to be platform specific there (there are some really hard to
>> reproduce bug reports around that).
>
> Hmm. Basically what this is doing is exactly what the comment says we
> didn't want to do, namely copy-and-paste the implementations of
> OpenSSL's socket BIO functions. How stable is that code? If the
> functions haven't changed textually in a long time (at least across all
> the OpenSSL versions we claim to support) then maybe it's okay.

The logic in it is identical to the original import of code in OpenSSL.
It originally had #ifdefs around how the BIO interface worked. That was
tidied up in a commit back in 2001. I think it's fair to say it's been
pretty stable.

I don't read the comment as saying that, fwiw. It just says we may
eventually need to do what I did now, but for other reasons.

Do you have a comment around the "should we prepare for read even though
it's a write" part?

//Magnus

Magnus Hagander <magnus(at)hagander(dot)net> writes:
> The logic in it is identical to the original import of code in OpenSSL.
> It originally had #ifdefs around how the BIO interface worked. That was
> tidied up in a commit back in 2001. I think it's fair to say it's been
> pretty stable.

Fair enough.

> Do you have a comment around the "should we prepare for read even though
> it's a write" part?

We shouldn't; in fact that would be quite wrong.

regards, tom lane