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
  */

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.

I'd appreciate your feedback.

Sincerely,
-- 
Alexey Klyukin
diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
new file mode 100644
index 9ba3567..64eab27
*** a/src/interfaces/libpq/fe-secure.c
--- b/src/interfaces/libpq/fe-secure.c
***************
*** 64,69 ****
--- 64,70 ----
  #ifdef USE_SSL_ENGINE
  #include <openssl/engine.h>
  #endif
+ #include <openssl/x509v3.h>
  
  
  #ifndef WIN32
*************** wildcard_certificate_match(const char *p
*** 754,760 ****
  
  
  /*
!  *    Verify that common name resolves to peer.
   */
  static bool
  verify_peer_name_matches_certificate(PGconn *conn)
--- 755,761 ----
  
  
  /*
!  *    Verify that common name or any of the alternative dNSNames resolves to 
peer.
   */
  static bool
  verify_peer_name_matches_certificate(PGconn *conn)
*************** verify_peer_name_matches_certificate(PGc
*** 773,780 ****
  
        /*
         * Extract the common name from the certificate.
-        *
-        * XXX: Should support alternate names here
         */
        /* First find out the name's length and allocate a buffer for it. */
        len = X509_NAME_get_text_by_NID(X509_get_subject_name(conn->peer),
--- 774,779 ----
*************** verify_peer_name_matches_certificate(PGc
*** 837,846 ****
                        result = true;
                else
                {
                        printfPQExpBuffer(&conn->errorMessage,
!                                                         libpq_gettext("server 
common name \"%s\" does not match host name \"%s\"\n"),
                                                          peer_cn, 
conn->pghost);
-                       result = false;
                }
        }
  
--- 836,904 ----
                        result = true;
                else
                {
+                       int                                i;
+                       int                        alt_names_total;
+                       STACK_OF(GENERAL_NAME) *alt_names;
+
+                       /* Get the list and the total number of alternative 
names. */
+                       alt_names = (STACK_OF(GENERAL_NAME) *) 
X509_get_ext_d2i(conn->peer, NID_subject_alt_name, NULL, NULL);
+                       if (alt_names != NULL)
+                               alt_names_total = 
sk_GENERAL_NAME_num(alt_names);
+                       else
+                               alt_names_total = 0;
+
+                       result = false;
+
+                       /*
+                        * Compare the alternative names dnsNames identifies 
against
+                        * the originally given hostname.
+                        */
+                       for (i = 0; i < alt_names_total && !result; i++)
+                       {
+                               const GENERAL_NAME *name = 
sk_GENERAL_NAME_value(alt_names, i);
+                               if (name->type == GEN_DNS)
+                               {
+                                       int    reported_len;
+                                       int    actual_len;
+                                       char  *dns_namedata,
+                                                 *dns_name;
+
+                                       reported_len = 
ASN1_STRING_length(name->d.dNSName);
+                                       dns_namedata = 
ASN1_STRING_data(name->d.dNSName);
+
+                                       dns_name = malloc(reported_len + 1);
+                                       memcpy(dns_name, dns_namedata, 
reported_len);
+                                       dns_name[reported_len] = '\0';
+
+                                       actual_len = strlen(dns_name);
+                                       if (actual_len != reported_len)
+                                       {
+                                               /*
+                                                * Reject embedded NULLs in 
certificate alternative name to prevent attacks
+                                                * like CVE-2009-4034.
+                                                */
+                                               
printfPQExpBuffer(&conn->errorMessage,
+                                                         libpq_gettext("SSL 
certificate's alternative name contains embedded null\n"));
+                                               free(peer_cn);
+                                               free(dns_name);
+                                               
sk_GENERAL_NAMES_free(alt_names);
+                                               return false;
+                                       }
+                                       if (pg_strcasecmp(dns_name, 
conn->pghost) == 0)
+                                               result = true;
+
+                                       free(dns_name);
+                               }
+                       }
+                       sk_GENERAL_NAMES_free(alt_names);
+               }
+
+               if (!result)
+               {
+                       /* Common name did not match and there are no 
alternative names */
                        printfPQExpBuffer(&conn->errorMessage,
!                                                         libpq_gettext("server 
common name \"%s\" and alternatives do not match host name \"%s\"\n"),
                                                          peer_cn, 
conn->pghost);
                }
        }

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to