diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c index 6b1365c6d3efa944952e4410cf17a2360cbff53f..dce0eaa20e208314c14fda855994f071c5831c34 100644 --- a/src/backend/libpq/be-secure.c +++ b/src/backend/libpq/be-secure.c @@ -73,6 +73,7 @@ #include "libpq/libpq.h" #include "tcop/tcopprot.h" +#include "utils/memutils.h" #ifdef USE_SSL @@ -945,44 +946,54 @@ aloop: port->count = 0; - /* get client certificate, if available. */ + /* Get client certificate, if available. */ port->peer = SSL_get_peer_certificate(port->ssl); - if (port->peer == NULL) - { - strlcpy(port->peer_dn, "(anonymous)", sizeof(port->peer_dn)); - strlcpy(port->peer_cn, "(anonymous)", sizeof(port->peer_cn)); - } - else + + /* and extract the Common Name from it. */ + port->peer_cn = NULL; + if (port->peer != NULL) { - X509_NAME_oneline(X509_get_subject_name(port->peer), - port->peer_dn, sizeof(port->peer_dn)); - port->peer_dn[sizeof(port->peer_dn) - 1] = '\0'; - r = X509_NAME_get_text_by_NID(X509_get_subject_name(port->peer), - NID_commonName, port->peer_cn, sizeof(port->peer_cn)); - port->peer_cn[sizeof(port->peer_cn) - 1] = '\0'; - if (r == -1) - { - /* Unable to get the CN, set it to blank so it can't be used */ - port->peer_cn[0] = '\0'; - } - else + int len; + + len = X509_NAME_get_text_by_NID(X509_get_subject_name(port->peer), + NID_commonName, NULL, 0); + if (len != -1) { + char *peer_cn; + + peer_cn = MemoryContextAlloc(TopMemoryContext, len + 1); + r = X509_NAME_get_text_by_NID(X509_get_subject_name(port->peer), + NID_commonName, peer_cn, len + 1); + peer_cn[len] = '\0'; + if (r != len) + { + /* shouldn't happen */ + pfree(peer_cn); + close_SSL(port); + return -1; + } + /* * Reject embedded NULLs in certificate common name to prevent * attacks like CVE-2009-4034. */ - if (r != strlen(port->peer_cn)) + if (len != strlen(peer_cn)) { ereport(COMMERROR, (errcode(ERRCODE_PROTOCOL_VIOLATION), errmsg("SSL certificate's common name contains embedded null"))); + pfree(peer_cn); close_SSL(port); return -1; } + + port->peer_cn = peer_cn; } } + ereport(DEBUG2, - (errmsg("SSL connection from \"%s\"", port->peer_cn))); + (errmsg("SSL connection from \"%s\"", + port->peer_cn ? port->peer_cn : "(anonymous)"))); /* set up debugging/info callback */ SSL_CTX_set_info_callback(SSL_context, info_cb); @@ -1008,6 +1019,12 @@ close_SSL(Port *port) X509_free(port->peer); port->peer = NULL; } + + if (port->peer_cn) + { + pfree(port->peer_cn); + port->peer_cn = NULL; + } } /* diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h index ad670c3216f66bebb07b98f217dec1c02ca444e0..4d92c18974bc56b402445671e4ff84d7c7628daa 100644 --- a/src/include/libpq/libpq-be.h +++ b/src/include/libpq/libpq-be.h @@ -175,8 +175,7 @@ typedef struct Port #ifdef USE_SSL SSL *ssl; X509 *peer; - char peer_dn[128 + 1]; - char peer_cn[SM_USER + 1]; + char *peer_cn; unsigned long count; #endif } Port; diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c index 3c1ca8c97fae2736ecf65479ab6b888f2eab9cb7..5c4d73c3acf4a214185bdf6c49881e050fa74e66 100644 --- a/src/interfaces/libpq/fe-secure.c +++ b/src/interfaces/libpq/fe-secure.c @@ -733,6 +733,11 @@ wildcard_certificate_match(const char *pattern, const char *string) static bool verify_peer_name_matches_certificate(PGconn *conn) { + char *peer_cn; + int r; + int len; + bool result; + /* * If told not to verify the peer name, don't do it. Return true * indicating that the verification was successful. @@ -740,33 +745,81 @@ 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. + */ if (!(conn->pghost && conn->pghost[0] != '\0')) { printfPQExpBuffer(&conn->errorMessage, libpq_gettext("host name must be specified for a verified SSL connection\n")); - return false; + result = false; } else { - /* - * Compare CN to originally given hostname. - * - * XXX: Should support alternate names here - */ - if (pg_strcasecmp(conn->peer_cn, conn->pghost) == 0) + if (pg_strcasecmp(peer_cn, conn->pghost) == 0) /* Exact name match */ - return true; - else if (wildcard_certificate_match(conn->peer_cn, conn->pghost)) + result = true; + else if (wildcard_certificate_match(peer_cn, conn->pghost)) /* Matched wildcard certificate */ - return true; + result = true; else { printfPQExpBuffer(&conn->errorMessage, libpq_gettext("server common name \"%s\" does not match host name \"%s\"\n"), - conn->peer_cn, conn->pghost); - return false; + peer_cn, conn->pghost); + result = false; } } + + free(peer_cn); + return result; } #ifdef ENABLE_THREAD_SAFETY @@ -1372,7 +1425,7 @@ open_client_SSL(PGconn *conn) * SSL_CTX_set_verify(), if root.crt exists. */ - /* pull out server distinguished and common names */ + /* get server certificate */ conn->peer = SSL_get_peer_certificate(conn->ssl); if (conn->peer == NULL) { @@ -1386,33 +1439,6 @@ open_client_SSL(PGconn *conn) return PGRES_POLLING_FAILED; } - X509_NAME_oneline(X509_get_subject_name(conn->peer), - conn->peer_dn, sizeof(conn->peer_dn)); - conn->peer_dn[sizeof(conn->peer_dn) - 1] = '\0'; - - r = X509_NAME_get_text_by_NID(X509_get_subject_name(conn->peer), - NID_commonName, conn->peer_cn, SM_USER); - conn->peer_cn[SM_USER] = '\0'; /* buffer is SM_USER+1 chars! */ - if (r == -1) - { - /* Unable to get the CN, set it to blank so it can't be used */ - conn->peer_cn[0] = '\0'; - } - else - { - /* - * Reject embedded NULLs in certificate common name to prevent attacks - * like CVE-2009-4034. - */ - if (r != strlen(conn->peer_cn)) - { - printfPQExpBuffer(&conn->errorMessage, - libpq_gettext("SSL certificate's common name contains embedded null\n")); - close_SSL(conn); - return PGRES_POLLING_FAILED; - } - } - if (!verify_peer_name_matches_certificate(conn)) { close_SSL(conn); diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index 987311efc8dd63d15b1849e13f58770f581af208..2103af88329894d5cecee7f4de0214ab8613bbe4 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -406,8 +406,6 @@ struct pg_conn * attempting normal connection */ SSL *ssl; /* SSL status, if have SSL connection */ X509 *peer; /* X509 cert of server */ - char peer_dn[256 + 1]; /* peer distinguished name */ - char peer_cn[SM_USER + 1]; /* peer common name */ #ifdef USE_SSL_ENGINE ENGINE *engine; /* SSL engine, if any */ #else