From acd08d764a361dcebd346227281ff0ca62b60936 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Fri, 12 Sep 2014 17:12:11 +0300 Subject: [PATCH] Support Subject Alternative Names in SSL server certificates. This patch makes libpq check the server's hostname against DNS names listed in the X509 subjectAltName extension field in the server certificate. This allows the same certificate to be used for multiple domain names. If there are no SANs in the certificate, the Common Name field is used, like before this patch. If both are given, the Common Name is ignored. That is a bit surprising, but that's the behavior mandated by the relevant RFCs, and it's also what the common web browsers do. This also adds a libpq_ngettext helper macro to allow plural messages to be translated in libpq. Apparently this happened to be the first plural message in libpq, so it was not needed before. Alexey Klyukin, with some kibitzing by me. --- src/interfaces/libpq/fe-misc.c | 18 +- src/interfaces/libpq/fe-secure-openssl.c | 266 +++++++++++++++++------ src/interfaces/libpq/libpq-int.h | 4 + 3 files changed, 216 insertions(+), 72 deletions(-) diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c index fc930bd05b..44f7d03978 100644 --- a/src/interfaces/libpq/fe-misc.c +++ b/src/interfaces/libpq/fe-misc.c @@ -1210,14 +1210,14 @@ PQenv2encoding(void) #ifdef ENABLE_NLS -char * -libpq_gettext(const char *msgid) +static void +libpq_binddomain() { static bool already_bound = false; if (!already_bound) { - /* dgettext() preserves errno, but bindtextdomain() doesn't */ + /* bindtextdomain() does not preserve errno */ #ifdef WIN32 int save_errno = GetLastError(); #else @@ -1237,8 +1237,20 @@ libpq_gettext(const char *msgid) errno = save_errno; #endif } +} +char * +libpq_gettext(const char *msgid) +{ + libpq_binddomain(); return dgettext(PG_TEXTDOMAIN("libpq"), msgid); } +char * +libpq_ngettext(const char *msgid, const char *msgid_plural, unsigned long n) +{ + libpq_binddomain(); + return dngettext(PG_TEXTDOMAIN("libpq"), msgid, msgid_plural, n); +} + #endif /* ENABLE_NLS */ diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c index f950fc3bc3..98d02b6b63 100644 --- a/src/interfaces/libpq/fe-secure-openssl.c +++ b/src/interfaces/libpq/fe-secure-openssl.c @@ -60,9 +60,13 @@ #ifdef USE_SSL_ENGINE #include #endif +#include static bool verify_peer_name_matches_certificate(PGconn *); static int verify_cb(int ok, X509_STORE_CTX *ctx); +static int verify_peer_name_matches_certificate_name(PGconn *conn, + ASN1_STRING *name, + char **store_name); static void destroy_ssl_system(void); static int initialize_SSL(PGconn *conn); static PostgresPollingStatusType open_client_SSL(PGconn *); @@ -473,15 +477,97 @@ wildcard_certificate_match(const char *pattern, const char *string) /* - * Verify that common name resolves to peer. + * Check if a name from a server's certificate matches the peer's hostname. + * + * Returns 1 if the name matches, and 0 if it does not. On error, returns + * -1, and sets the libpq error message. + * + * The name extracted from the certificate is returned in *store_name. The + * caller is responsible for freeing it. + */ +static int +verify_peer_name_matches_certificate_name(PGconn *conn, ASN1_STRING *name_entry, + char **store_name) +{ + int len; + char *name; + unsigned char *namedata; + int result; + + *store_name = NULL; + + /* Should not happen... */ + if (name_entry == NULL) + { + printfPQExpBuffer(&conn->errorMessage, + libpq_gettext("SSL certificate's name entry is missing\n")); + return -1; + } + + /* + * GEN_DNS can be only IA5String, equivalent to US ASCII. + * + * There is no guarantee the string returned from the certificate is + * NULL-terminated, so make a copy that is. + */ + namedata = ASN1_STRING_data(name_entry); + len = ASN1_STRING_length(name_entry); + name = malloc(len + 1); + if (name == NULL) + { + printfPQExpBuffer(&conn->errorMessage, + libpq_gettext("out of memory\n")); + return -1; + } + memcpy(name, namedata, len); + name[len] = '\0'; + + /* + * Reject embedded NULLs in certificate common or alternative name to + * prevent attacks like CVE-2009-4034. + */ + if (len != strlen(name)) + { + free(name); + printfPQExpBuffer(&conn->errorMessage, + libpq_gettext("SSL certificate's name contains embedded null\n")); + return -1; + } + + if (pg_strcasecmp(name, conn->pghost) == 0) + { + /* Exact name match */ + result = 1; + } + else if (wildcard_certificate_match(name, conn->pghost)) + { + /* Matched wildcard name */ + result = 1; + } + else + { + result = 0; + } + + *store_name = name; + return result; +} + +/* + * Verify that the server certificate matches the hostname we connected to. + * + * The certificate's Common Name and Subject Alternative Names are considered. */ static bool verify_peer_name_matches_certificate(PGconn *conn) { - char *peer_cn; - int r; - int len; - bool result; + int names_examined = 0; + bool found_match = false; + bool got_error = false; + char *first_name = NULL; + STACK_OF(GENERAL_NAME) *peer_san; + int i; + int rc; /* * If told not to verify the peer name, don't do it. Return true @@ -490,81 +576,123 @@ verify_peer_name_matches_certificate(PGconn *conn) if (strcmp(conn->sslmode, "verify-full") != 0) return true; - /* - * 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), - NID_commonName, NULL, 0); - if (len == -1) - { - printfPQExpBuffer(&conn->errorMessage, - libpq_gettext("could not get server common name from server certificate\n")); - return false; - } - peer_cn = malloc(len + 1); - if (peer_cn == NULL) - { - printfPQExpBuffer(&conn->errorMessage, - libpq_gettext("out of memory\n")); - return false; - } - - r = X509_NAME_get_text_by_NID(X509_get_subject_name(conn->peer), - NID_commonName, peer_cn, len + 1); - if (r != len) - { - /* Got different length than on the first call. Shouldn't happen. */ - printfPQExpBuffer(&conn->errorMessage, - libpq_gettext("could not get server common name from server certificate\n")); - free(peer_cn); - return false; - } - peer_cn[len] = '\0'; - - /* - * Reject embedded NULLs in certificate common name to prevent attacks - * like CVE-2009-4034. - */ - if (len != strlen(peer_cn)) - { - printfPQExpBuffer(&conn->errorMessage, - libpq_gettext("SSL certificate's common name contains embedded null\n")); - free(peer_cn); - return false; - } - - /* - * We got the peer's common name. Now compare it against the originally - * given hostname. - */ + /* Check that we have a hostname to compare with. */ if (!(conn->pghost && conn->pghost[0] != '\0')) { printfPQExpBuffer(&conn->errorMessage, libpq_gettext("host name must be specified for a verified SSL connection\n")); - result = false; + return false; } + + /* + * First, get the Subject Alternative Names (SANs) from the certificate, + * and compare them against the originally given hostname. + */ + peer_san = (STACK_OF(GENERAL_NAME) *) + X509_get_ext_d2i(conn->peer, NID_subject_alt_name, NULL, NULL); + + if (peer_san) + { + int san_len = sk_GENERAL_NAME_num(peer_san); + + for (i = 0; i < san_len; i++) + { + const GENERAL_NAME *name = sk_GENERAL_NAME_value(peer_san, i); + + if (name->type == GEN_DNS) + { + char *alt_name; + + names_examined++; + rc = verify_peer_name_matches_certificate_name(conn, + name->d.dNSName, + &alt_name); + if (rc == -1) + got_error = true; + if (rc == 1) + found_match = true; + + if (alt_name) + { + if (!first_name) + first_name = alt_name; + else + free(alt_name); + } + } + if (found_match || got_error) + break; + } + sk_GENERAL_NAME_free(peer_san); + } + /* + * If there is no subjectAltName extension, check the Common Name. + * + * (Per RFC 2818 and RFC 6125, if the subjectAltName extension is present, + * the CN must be ignored.) + */ else { - if (pg_strcasecmp(peer_cn, conn->pghost) == 0) - /* Exact name match */ - result = true; - else if (wildcard_certificate_match(peer_cn, conn->pghost)) - /* Matched wildcard certificate */ - result = true; - else + X509_NAME *subject_name; + + subject_name = X509_get_subject_name(conn->peer); + if (subject_name != NULL) { - printfPQExpBuffer(&conn->errorMessage, - libpq_gettext("server common name \"%s\" does not match host name \"%s\"\n"), - peer_cn, conn->pghost); - result = false; + int cn_index; + + cn_index = X509_NAME_get_index_by_NID(subject_name, + NID_commonName, -1); + if (cn_index >= 0) + { + names_examined++; + rc = verify_peer_name_matches_certificate_name( + conn, + X509_NAME_ENTRY_get_data( + X509_NAME_get_entry(subject_name, cn_index)), + &first_name); + + if (rc == -1) + got_error = true; + else if (rc == 1) + found_match = true; + } } } - free(peer_cn); - return result; + if (!found_match && !got_error) + { + /* + * No match. Include the name from the server certificate in the + * error message, to aid debugging broken configurations. If there + * are multiple names, only print the first one to avoid an overly + * long error message. + */ + if (names_examined > 1) + { + printfPQExpBuffer(&conn->errorMessage, + libpq_ngettext("server certificate for \"%s\" (and %d other name) does not match host name \"%s\"\n", + "server certificate for \"%s\" (and %d other names) does not match host name \"%s\"\n", + names_examined - 1), + first_name, names_examined - 1, conn->pghost); + } + else if (names_examined == 1) + { + printfPQExpBuffer(&conn->errorMessage, + libpq_gettext("server certificate for \"%s\" does not match host name \"%s\"\n"), + first_name, conn->pghost); + } + else + { + printfPQExpBuffer(&conn->errorMessage, + libpq_gettext("could not get server's hostname from server certificate\n")); + } + } + + /* clean up */ + if (first_name) + free(first_name); + + return found_match && !got_error; } #ifdef ENABLE_THREAD_SAFETY diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index 60329048f2..4ef46fff47 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -653,8 +653,12 @@ extern ssize_t pgtls_write(PGconn *conn, const void *ptr, size_t len); extern char * libpq_gettext(const char *msgid) __attribute__((format_arg(1))); +extern char * +libpq_ngettext(const char *msgid, const char *msgid_plural, unsigned long n) +__attribute__((format_arg(1))) __attribute__((format_arg(2))); #else #define libpq_gettext(x) (x) +#define libpq_ngettext(s, p, n) ((n) == 1 ? (s) : (p)) #endif /*