Re: implement subject alternative names support for SSL connections

On Fri, Jul 25, 2014 at 6:10 PM, Alexey Klyukin <alexk(at)hintbits(dot)com> wrote:
> Greetings,
>
> I'd like to propose a patch for checking subject alternative names entry in
> the SSL certificate for DNS names during SSL authentication.
>
> When the client PGSSLMODE is set to verify-full, the common name (CN) of the
> PostgreSQL server in the certificate is matched against the actual host
> name supplied by the client. A successful connection is only established
> when those names match.
>
> If a failover schema with a floating IP is used, a single DNS name may point
> to different hosts after failover. In order to maintain the correct pair of
> (client connection FQDN, FQDN in the certificate) the certificate from the
> master should also be transferred to the slave. Unfortunately, it makes
> failover more complicated, since the server restart is required when the new
> certificate is installed.
>
> The other option is to issue a single certificate covering all the hosts
> that may participate in the failover.
>
> So far the only way to cover more than one server name with a single
> certificate is to use wildcards in the server common name, i.e. *.db.example
> com. Unfortunately, this schema only works for names like
> master.db.example.com, slave.db.example.com, but not for the example.com and
> db-master.example.com and db-slave.example.com or other more complex naming
> schemas.
>
> The other way to issue a single certificate for multiple names is to set the
> subject alternative names, described in the RFC 3280 4.2.1.7. SAN allows
> binding multiple identities to the certificate, including DNS names. For the
> names above the SAN with look like this:
>
> X509v3 Subject Alternative Name:
> DNS:db-master.example.com, DNS:db-slave.example.com.
>
> At the moment PostgreSQL doesn't support SANs, but should, according to the
> following comment in the code in verify_peer_name_matches_certificate:
>
> /*
> * Extract the common name from the certificate.
> *
> * XXX: Should support alternate names here
> */

I believe that comment is mine, and yes, it should definitely be done.
So absolutely +1 on the feature :)

> The attached patch adds support for checking the client supplied names
> against the dNSName-s in SAN, making it easier to set secure HA environments
> using PostgreSQL. If SAN section is present, the DNS names from that section
> are checked against the peer name in addition to checking the common name
> (CN) from the certificate. The match is successful if at least one of those
> names match the name supplied by the peer.
>
> I gave it a spin and it works in our environment (Linux database servers,
> Linux and Mac clients). I don't have either Windows or old versions of
> OpenSSL, it's not tested against those systems.

We definitely need testing for older openssl. I don't think Windows
will make a difference, but openssl definitely may - there is a fairly
large chance we need a configure check.
>
> I'd appreciate your feedback.

I just took a very quick look at the code, and just noticed one thing:

Why keep looping once you've found a match? When you set result=true
you should break; from the loop I think. Not necessarily for
performance, but there might be something about a different extension
we can't parse for example, no need to fail in that case.

Please add it to the next CF - this was just a very quick review, and
it needs a proper one along with openssl version testing :)

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/