Re: PQgetssl() and alternative SSL implementations

On 08/19/2014 05:48 PM, Stephen Frost wrote:
> * Heikki Linnakangas (hlinnakangas@vmware.com) wrote:
>>    server_cert_valid: Did the server present a valid certificate?
>> "yes" or "no"
>>
>>    server_cert_matches_host: Does the Common Name of the certificate
>> match the host connected to? "yes" or "no"
>
> Aren't these questions addressed by sslmode?

Sort of. In sslmode=verify-ca, libpq checks that the server cert was 
valid (the first attribute) and rejects the connection if not. In 
verify-full mode, it also checks that the hostname matches (the second 
attribute). But in sslmode=require, it's possible to connect to a server 
with an invalid server cert. (to be precise in sslmode=require mode 
libpq checks the server cert if a root CA cert was given, but if no root 
CA cert is configured it will allow connecting anyway).

I think it would be nice to be able to query those attributes 
explicitly, rather than just expect libpq to reject the connection if 
something's wrong. For example, I'm thinking that an interactive client 
might present an annoying pop-up window to the user if the server cert 
is not valid, asking if he wants to connect anyway, and perhaps remember 
the certificate and not ask again (TOFU).

We don't actually have such functionality today; you can query the 
OpenSSL structs for those things, but the checks that libpq performs are 
not exactly the same that OpenSSL does. We have our own function to 
check if a wildcard cert matches a hostname, for example, and libpq 
knows that "host" and "hostaddr" can be different. So this would 
actually be a new feature, probably best to be implemented as a separate 
patch. (I grabbed the idea for those attributes from Martijn's ancient 
gnutls patch.)

>> Exposing the SSL information as generic key/value pairs allows
>> adding more attributes in the future, without breaking the ABI, and
>> it also allows exposing implementation-specific information in a
>> generic way. The attributes listed above cover the needs of psql.
>> What else do we need?
>
> At first blush, I'd say a whole bunch..  Off the top of my head I can
> think of:
>
> For all certificates:
> (client, server, cert that signed each, any intermediate CAs, root CAs)
>    Certificate itself (perhaps in DER, PEM, X509 formats..)
>    Fingerprint
>    Signed-By info
>    Common Name
>    Organization (et al)
>    Alternate names
>    Issue date, expiration date
>    CRL info, OCSP info
>    Allowed usage (encryption, signing, etc)

Hmm. That seems a bit too much. Perhaps provide just the certificate 
itself in DER/PEM format, and have the client parse it (using OpenSSL or 
something else) if it wants more details.

> CRL checking done?

I guess, although you know implicitly that it was if the sslcrl option 
was given.

> OCSP used?

We don't support OCSP.

>> I think it would also be nice to get more information from the
>> server's certificate, like the hostname and the organization its
>> issued to, and expiration date, so that an interactive client like
>> pgAdmin or even psql could display that information like a web
>> browser does. Would it be best to add those as extra attributes in
>> the above list, perhaps with a "server_cert_*" prefix, or add a new
>> function for extracting server cert's attributes?
>
> This really shouldn't be for *just* the server's certificate but rather
> available for all certificates involved- on both sides.

Ok, but why? All the other stuff is readily available in the 
configuration you use to connect. I guess it doesn't hurt to expose them 
through this interface as well, but I can't immediately think of an 
example that would use them.

> Have you looked at how this change will play out with the ODBC driver..?
> Especially on Windows with the SSL library you're proposing we use
> there..  I recall that at one point the ODBC driver simply used libpq to
> handle the authentication and set everything up, and then switched to
> talking directly without libpq.  In any case, it'd probably be good to
> make sure the attributes you're suggesting are sufficient to meet the
> needs of the ODBC driver too.

Indeed, the ODBC driver only uses libpq for authentication, then calls 
PQgetssl(), and takes over the whole show calling SSL_read() and 
SSL_write() itself. Ideally, we'd modify psqlodbc to stop doing that, 
but that's not an easy job. In the short-term, I think we need to export 
pqsecure_read() and pqsecure_write() functions in libpq, so that the 
ODBC driver can use those instead of SSL_read() and SSL_write().

- Heikki