Re: Supporting Windows SChannel as OpenSSL replacement

On 06/09/2014 05:39 PM, Martijn van Oosterhout wrote:
> On Mon, Jun 09, 2014 at 03:35:23PM +0200, Magnus Hagander wrote:
>> On Mon, Jun 9, 2014 at 3:19 PM, Andreas Karlsson <andreas(at)proxel(dot)se> wrote:
>>
>>> On 06/09/2014 01:45 PM, Heikki Linnakangas wrote:
>>> There was a patch set for this from Martijn van Oosterhout which was quite
>>> complete.
>>>
>>> http://www.postgresql.org/message-id/20060504134807.GK4752@svana.org
>
> Wow, blast from the past.
>
>> A lot has, unfortunately, changed since 2006. It might be a good
>> startingpoint. But also actively starting from the point of "let's try to
>> support multiple libraries" rather than "let's try to support gnutls" is
>> probably also important.
>
> The patch did provide an API. The idea was that there were a number of
> functions which would need to be defined to support an SSL library.
> Each library would then have a wrapper which wrapped the library and
> based on the results of configure it compiled the right file into the
> backend.
>
> These functions were:
>
> extern void pgtls_initialize(void);
> extern void pgtls_destroy(void);
> extern int pgtls_open_server(Port *);
> extern void pgtls_close(Port *);
> extern ssize_t pgtls_read(Port *port, void *ptr, size_t len);
> extern ssize_t pgtls_write(Port *port, void *ptr, size_t len);
>
> Which should be easy enough to support for any library. These days
> you'd need to add support for verifying certificates, but I don't think
> that that would be difficult (unless the actual certificate formats are
> different).
>
> No switching after compile time, that would just lead to useless
> overhead.

Yeah, that seems like a reasonable design.

I did again the refactoring you did back in 2006, patch attached. One
thing I did differently: I moved the raw, non-encrypted, read/write
functions to separate functions: pqsecure_raw_read and
pqsecure_raw_write. Those functions encapsulate the SIGPIPE handling.
The OpenSSL code implements a custom BIO, which calls to
pqsecure_raw_read/write to do the low-level I/O. Similarly in the
server-side, there are be_tls_raw_read and pg_tls_raw_write functions,
which do the prepare_for_client_read()/client_read_ended() dance, so
that the SSL implementation doesn't need to know about that.

I then implemented a quick proof-of-concept Windows SChannel
implementation of that API. It's client-side only, and there's no
support for validating server certificate or specifying non-default
ciphers or anything yet. But I did implement performing authentication
with a client certificate, as a proof of concept that it's possible to
read the OpenSSL key and certificate files - although in this proof of
concept the key and certificate are hard-coded in the sources, not read
from a file.

The SChannel implementation obviously needs a lot of work, but I'm
fairly confident that it's doable, and the new internal SSL API works
for that. Except for the user-visible PQgetssl() functions and such - I
don't know what to do with those.

>> At some point we should design a new API, so that we can deprecate the old
>> one. Even if we don't hve the code ready, we need to get rid of PQgetssl(),
>> and replace it with something else. I'm thinking probably a functoin that
>> returns both a void pointer and an enum that tells you which library is
>> actually in use. And a boolean just saying "ssl on/off", because that's
>> what a lot of clients are interested in and they don't care aobut more than
>> that.
>>
>> Obviously, we also have to do something about PQinitOpenSSL().
>
> Yeah, I think this was one of the more controversial parts. Support in
> the backend was primarily moving code around and renaming functions,
> fairly straightforward. Even error handling was not so hard (I found
> the gnutls handling of errors much easier than openssl).
>
> One tricky part is that programs like to use libpq for the
> authentication, and then they hijack the connection using PGgetssl().
> The way I dealt with this is defining a new state "passthrough" where
> the caller would get a few function pointers to read/write/check the
> connection. Then the callers would not need to know what library libpq
> was compiled with. And libpq would know the connection was hijacked
> and refuse to act anymore. I don't think everyone was pleased with
> this, but no real alternative was presented (other than requiring
> people hijacking the connection to do the hard work).

Sounds good. If we want to support such hijacking if the first place.

- Heikki