database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-06-25 01:02:05 +03:00
Code Activity

Refactor client-side SSL certificate checking code

Browse Source 
Separate the parts specific to the SSL library from the general logic.

The previous code structure was

open_client_SSL()
calls verify_peer_name_matches_certificate()
calls verify_peer_name_matches_certificate_name()
calls wildcard_certificate_match()

and was completely in fe-secure-openssl.c.  The new structure is

open_client_SSL() [openssl]
calls pq_verify_peer_name_matches_certificate() [generic]
calls pgtls_verify_peer_name_matches_certificate_guts() [openssl]
calls openssl_verify_peer_name_matches_certificate_name() [openssl]
calls pq_verify_peer_name_matches_certificate_name() [generic]
calls wildcard_certificate_match() [generic]

Move the generic functions into a new file fe-secure-common.c, so the
calls generally go fe-connect.c -> fe-secure.c -> fe-secure-${impl}.c ->
fe-secure-common.c, although there is a bit of back-and-forth between
the last two.

Reviewed-by: Michael Paquier <michael.paquier@gmail.com>
This commit is contained in:
Peter Eisentraut
2018-01-27 13:47:52 -05:00
parent 38d485fdaa
commit f75a959155
7 changed files with 271 additions and 187 deletions
Download Patch File Download Diff File Expand all files Collapse all files

210
src/interfaces/libpq/fe-secure-openssl.c
View File

@ -28,6 +28,7 @@
#include "libpq-fe.h"
#include "fe-auth.h"
#include "fe-secure-common.h"
#include "libpq-int.h"
#ifdef WIN32
@ -60,9 +61,8 @@
#endif
#include <openssl/x509v3.h>
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,
static int openssl_verify_peer_name_matches_certificate_name(PGconn *conn,
ASN1_STRING *name,
char **store_name);
static void destroy_ssl_system(void);
@ -492,76 +492,16 @@ verify_cb(int ok, X509_STORE_CTX *ctx)
/*
* Check if a wildcard certificate matches the server hostname.
*
* The rule for this is:
* 1. We only match the '*' character as wildcard
* 2. We match only wildcards at the start of the string
* 3. The '*' character does *not* match '.', meaning that we match only
* a single pathname component.
* 4. We don't support more than one '*' in a single pattern.
*
* This is roughly in line with RFC2818, but contrary to what most browsers
* appear to be implementing (point 3 being the difference)
*
* Matching is always case-insensitive, since DNS is case insensitive.
* OpenSSL-specific wrapper around
* pq_verify_peer_name_matches_certificate_name(), converting the ASN1_STRING
* into a plain C string.
*/
static int
wildcard_certificate_match(const char *pattern, const char *string)
{
int lenpat = strlen(pattern);
int lenstr = strlen(string);
/* If we don't start with a wildcard, it's not a match (rule 1 & 2) */
if (lenpat < 3 ||
pattern[0] != '*' ||
pattern[1] != '.')
return 0;
if (lenpat > lenstr)
/* If pattern is longer than the string, we can never match */
return 0;
if (pg_strcasecmp(pattern + 1, string + lenstr - lenpat + 1) != 0)
/*
* If string does not end in pattern (minus the wildcard), we don't
* match
*/
return 0;
if (strchr(string, '.') < string + lenstr - lenpat)
/*
* If there is a dot left of where the pattern started to match, we
* don't match (rule 3)
*/
return 0;
/* String ended with pattern, and didn't have a dot before, so we match */
return 1;
}
/*
* 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)
openssl_verify_peer_name_matches_certificate_name(PGconn *conn, ASN1_STRING *name_entry,
char **store_name)
{
int len;
char *name;
const unsigned char *namedata;
int result;
char *host = PQhost(conn);
*store_name = NULL;
/* Should not happen... */
if (name_entry == NULL)
@ -573,9 +513,6 @@ verify_peer_name_matches_certificate_name(PGconn *conn, ASN1_STRING *name_entry,
/*
* 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.
*/
#ifdef HAVE_ASN1_STRING_GET0_DATA
namedata = ASN1_STRING_get0_data(name_entry);
@ -583,45 +520,9 @@ verify_peer_name_matches_certificate_name(PGconn *conn, ASN1_STRING *name_entry,
namedata = ASN1_STRING_data(name_entry);
#endif
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, host) == 0)
{
/* Exact name match */
result = 1;
}
else if (wildcard_certificate_match(name, host))
{
/* Matched wildcard name */
result = 1;
}
else
{
result = 0;
}
*store_name = name;
return result;
/* OK to cast from unsigned to plain char, since it's all ASCII. */
return pq_verify_peer_name_matches_certificate_name(conn, (const char *) namedata, len, store_name);
}
/*
@ -629,33 +530,14 @@ verify_peer_name_matches_certificate_name(PGconn *conn, ASN1_STRING *name_entry,
*
* The certificate's Common Name and Subject Alternative Names are considered.
*/
static bool
verify_peer_name_matches_certificate(PGconn *conn)
int
pgtls_verify_peer_name_matches_certificate_guts(PGconn *conn,
int *names_examined,
char **first_name)
{
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;
char *host = PQhost(conn);
/*
* If told not to verify the peer name, don't do it. Return true
* indicating that the verification was successful.
*/
if (strcmp(conn->sslmode, "verify-full") != 0)
return true;
/* Check that we have a hostname to compare with. */
if (!(host && host[0] != '\0'))
{
printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("host name must be specified for a verified SSL connection\n"));
return false;
}
int rc = 0;
/*
* First, get the Subject Alternative Names (SANs) from the certificate,
@ -676,24 +558,20 @@ verify_peer_name_matches_certificate(PGconn *conn)
{
char *alt_name;
names_examined++;
rc = verify_peer_name_matches_certificate_name(conn,
(*names_examined)++;
rc = openssl_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;
if (!*first_name)
*first_name = alt_name;
else
free(alt_name);
}
}
if (found_match || got_error)
if (rc != 0)
break;
}
sk_GENERAL_NAME_free(peer_san);
@ -706,7 +584,7 @@ verify_peer_name_matches_certificate(PGconn *conn)
* (Per RFC 2818 and RFC 6125, if the subjectAltName extension of type
* dNSName is present, the CN must be ignored.)
*/
if (names_examined == 0)
if (*names_examined == 0)
{
X509_NAME *subject_name;
@ -719,55 +597,17 @@ verify_peer_name_matches_certificate(PGconn *conn)
NID_commonName, -1);
if (cn_index >= 0)
{
names_examined++;
rc = verify_peer_name_matches_certificate_name(
(*names_examined)++;
rc = openssl_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;
first_name);
}
}
}
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, host);
}
else if (names_examined == 1)
{
printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("server certificate for \"%s\" does not match host name \"%s\"\n"),
first_name, host);
}
else
{
printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not get server's host name from server certificate\n"));
}
}
/* clean up */
if (first_name)
free(first_name);
return found_match && !got_error;
return rc;
}
#if defined(ENABLE_THREAD_SAFETY) && defined(HAVE_CRYPTO_LOCK)
@ -1441,7 +1281,7 @@ open_client_SSL(PGconn *conn)
return PGRES_POLLING_FAILED;
}
if (!verify_peer_name_matches_certificate(conn))
if (!pq_verify_peer_name_matches_certificate(conn))
{
pgtls_close(conn);
return PGRES_POLLING_FAILED;