Skip to content

Commit acd08d7

Browse files
hlinnakahlinnaka
committed
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.
1 parent 774a78f commit acd08d7

File tree

3 files changed

+202
-58
lines changed

3 files changed

+202
-58
lines changed

‎src/interfaces/libpq/fe-misc.c

+15-3
Original file line numberDiff line numberDiff line change
@@ -1210,14 +1210,14 @@ PQenv2encoding(void)
12101210

12111211
#ifdef ENABLE_NLS
12121212

1213-
char *
1214-
libpq_gettext(const char *msgid)
1213+
static void
1214+
libpq_binddomain()
12151215
{
12161216
static bool already_bound = false;
12171217

12181218
if (!already_bound)
12191219
{
1220-
/* dgettext() preserves errno, but bindtextdomain() doesn't */
1220+
/* bindtextdomain() does not preserve errno */
12211221
#ifdef WIN32
12221222
int save_errno = GetLastError();
12231223
#else
@@ -1237,8 +1237,20 @@ libpq_gettext(const char *msgid)
12371237
errno = save_errno;
12381238
#endif
12391239
}
1240+
}
12401241

1242+
char *
1243+
libpq_gettext(const char *msgid)
1244+
{
1245+
libpq_binddomain();
12411246
return dgettext(PG_TEXTDOMAIN("libpq"), msgid);
12421247
}
12431248

1249+
char *
1250+
libpq_ngettext(const char *msgid, const char *msgid_plural, unsigned long n)
1251+
{
1252+
libpq_binddomain();
1253+
return dngettext(PG_TEXTDOMAIN("libpq"), msgid, msgid_plural, n);
1254+
}
1255+
12441256
#endif /* ENABLE_NLS */

‎src/interfaces/libpq/fe-secure-openssl.c

+183-55
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,13 @@
6060
#ifdef USE_SSL_ENGINE
6161
#include <openssl/engine.h>
6262
#endif
63+
#include <openssl/x509v3.h>
6364

6465
static bool verify_peer_name_matches_certificate(PGconn *);
6566
static int verify_cb(int ok, X509_STORE_CTX *ctx);
67+
static int verify_peer_name_matches_certificate_name(PGconn *conn,
68+
ASN1_STRING *name,
69+
char **store_name);
6670
static void destroy_ssl_system(void);
6771
static int initialize_SSL(PGconn *conn);
6872
static PostgresPollingStatusType open_client_SSL(PGconn *);
@@ -473,98 +477,222 @@ wildcard_certificate_match(const char *pattern, const char *string)
473477

474478

475479
/*
476-
* Verify that common name resolves to peer.
480+
* Check if a name from a server's certificate matches the peer's hostname.
481+
*
482+
* Returns 1 if the name matches, and 0 if it does not. On error, returns
483+
* -1, and sets the libpq error message.
484+
*
485+
* The name extracted from the certificate is returned in *store_name. The
486+
* caller is responsible for freeing it.
477487
*/
478-
static bool
479-
verify_peer_name_matches_certificate(PGconn *conn)
488+
static int
489+
verify_peer_name_matches_certificate_name(PGconn *conn, ASN1_STRING *name_entry,
490+
char **store_name)
480491
{
481-
char *peer_cn;
482-
int r;
483492
int len;
484-
bool result;
493+
char *name;
494+
unsigned char *namedata;
495+
int result;
485496

486-
/*
487-
* If told not to verify the peer name, don't do it. Return true
488-
* indicating that the verification was successful.
489-
*/
490-
if (strcmp(conn->sslmode, "verify-full") != 0)
491-
return true;
497+
*store_name = NULL;
498+
499+
/* Should not happen... */
500+
if (name_entry == NULL)
501+
{
502+
printfPQExpBuffer(&conn->errorMessage,
503+
libpq_gettext("SSL certificate's name entry is missing\n"));
504+
return -1;
505+
}
492506

493507
/*
494-
* Extract the common name from the certificate.
508+
* GEN_DNS can be only IA5String, equivalent to US ASCII.
495509
*
496-
* XXX: Should support alternate names here
510+
* There is no guarantee the string returned from the certificate is
511+
* NULL-terminated, so make a copy that is.
497512
*/
498-
/* First find out the name's length and allocate a buffer for it. */
499-
len = X509_NAME_get_text_by_NID(X509_get_subject_name(conn->peer),
500-
NID_commonName, NULL, 0);
501-
if (len == -1)
513+
namedata = ASN1_STRING_data(name_entry);
514+
len = ASN1_STRING_length(name_entry);
515+
name = malloc(len + 1);
516+
if (name == NULL)
502517
{
503518
printfPQExpBuffer(&conn->errorMessage,
504-
libpq_gettext("could not get server common name from server certificate\n"));
505-
return false;
519+
libpq_gettext("out of memory\n"));
520+
return -1;
506521
}
507-
peer_cn = malloc(len + 1);
508-
if (peer_cn == NULL)
522+
memcpy(name, namedata, len);
523+
name[len] = '\0';
524+
525+
/*
526+
* Reject embedded NULLs in certificate common or alternative name to
527+
* prevent attacks like CVE-2009-4034.
528+
*/
529+
if (len != strlen(name))
509530
{
531+
free(name);
510532
printfPQExpBuffer(&conn->errorMessage,
511-
libpq_gettext("out of memory\n"));
512-
return false;
533+
libpq_gettext("SSL certificate's name contains embedded null\n"));
534+
return -1;
513535
}
514536

515-
r = X509_NAME_get_text_by_NID(X509_get_subject_name(conn->peer),
516-
NID_commonName, peer_cn, len + 1);
517-
if (r != len)
537+
if (pg_strcasecmp(name, conn->pghost) == 0)
518538
{
519-
/* Got different length than on the first call. Shouldn't happen. */
520-
printfPQExpBuffer(&conn->errorMessage,
521-
libpq_gettext("could not get server common name from server certificate\n"));
522-
free(peer_cn);
523-
return false;
539+
/* Exact name match */
540+
result = 1;
541+
}
542+
else if (wildcard_certificate_match(name, conn->pghost))
543+
{
544+
/* Matched wildcard name */
545+
result = 1;
546+
}
547+
else
548+
{
549+
result = 0;
524550
}
525-
peer_cn[len] = '\0';
551+
552+
*store_name = name;
553+
return result;
554+
}
555+
556+
/*
557+
* Verify that the server certificate matches the hostname we connected to.
558+
*
559+
* The certificate's Common Name and Subject Alternative Names are considered.
560+
*/
561+
static bool
562+
verify_peer_name_matches_certificate(PGconn *conn)
563+
{
564+
int names_examined = 0;
565+
bool found_match = false;
566+
bool got_error = false;
567+
char *first_name = NULL;
568+
STACK_OF(GENERAL_NAME) *peer_san;
569+
int i;
570+
int rc;
526571

527572
/*
528-
* Reject embedded NULLs in certificate common name to prevent attacks
529-
* like CVE-2009-4034.
573+
* If told not to verify the peer name, don't do it. Return true
574+
* indicating that the verification was successful.
530575
*/
531-
if (len != strlen(peer_cn))
576+
if (strcmp(conn->sslmode, "verify-full") != 0)
577+
return true;
578+
579+
/* Check that we have a hostname to compare with. */
580+
if (!(conn->pghost && conn->pghost[0] != '\0'))
532581
{
533582
printfPQExpBuffer(&conn->errorMessage,
534-
libpq_gettext("SSL certificate's common name contains embedded null\n"));
535-
free(peer_cn);
583+
libpq_gettext("host name must be specified for a verified SSL connection\n"));
536584
return false;
537585
}
538586

539587
/*
540-
* We got the peer's common name. Now compare it against the originally
541-
* given hostname.
588+
* First, get the Subject Alternative Names (SANs) from the certificate,
589+
* and compare them against the originally given hostname.
542590
*/
543-
if (!(conn->pghost && conn->pghost[0] != '\0'))
591+
peer_san = (STACK_OF(GENERAL_NAME) *)
592+
X509_get_ext_d2i(conn->peer, NID_subject_alt_name, NULL, NULL);
593+
594+
if (peer_san)
544595
{
545-
printfPQExpBuffer(&conn->errorMessage,
546-
libpq_gettext("host name must be specified for a verified SSL connection\n"));
547-
result = false;
596+
int san_len = sk_GENERAL_NAME_num(peer_san);
597+
598+
for (i = 0; i < san_len; i++)
599+
{
600+
const GENERAL_NAME *name = sk_GENERAL_NAME_value(peer_san, i);
601+
602+
if (name->type == GEN_DNS)
603+
{
604+
char *alt_name;
605+
606+
names_examined++;
607+
rc = verify_peer_name_matches_certificate_name(conn,
608+
name->d.dNSName,
609+
&alt_name);
610+
if (rc == -1)
611+
got_error = true;
612+
if (rc == 1)
613+
found_match = true;
614+
615+
if (alt_name)
616+
{
617+
if (!first_name)
618+
first_name = alt_name;
619+
else
620+
free(alt_name);
621+
}
622+
}
623+
if (found_match || got_error)
624+
break;
625+
}
626+
sk_GENERAL_NAME_free(peer_san);
548627
}
628+
/*
629+
* If there is no subjectAltName extension, check the Common Name.
630+
*
631+
* (Per RFC 2818 and RFC 6125, if the subjectAltName extension is present,
632+
* the CN must be ignored.)
633+
*/
549634
else
550635
{
551-
if (pg_strcasecmp(peer_cn, conn->pghost) == 0)
552-
/* Exact name match */
553-
result = true;
554-
else if (wildcard_certificate_match(peer_cn, conn->pghost))
555-
/* Matched wildcard certificate */
556-
result = true;
636+
X509_NAME *subject_name;
637+
638+
subject_name = X509_get_subject_name(conn->peer);
639+
if (subject_name != NULL)
640+
{
641+
int cn_index;
642+
643+
cn_index = X509_NAME_get_index_by_NID(subject_name,
644+
NID_commonName, -1);
645+
if (cn_index >= 0)
646+
{
647+
names_examined++;
648+
rc = verify_peer_name_matches_certificate_name(
649+
conn,
650+
X509_NAME_ENTRY_get_data(
651+
X509_NAME_get_entry(subject_name, cn_index)),
652+
&first_name);
653+
654+
if (rc == -1)
655+
got_error = true;
656+
else if (rc == 1)
657+
found_match = true;
658+
}
659+
}
660+
}
661+
662+
if (!found_match && !got_error)
663+
{
664+
/*
665+
* No match. Include the name from the server certificate in the
666+
* error message, to aid debugging broken configurations. If there
667+
* are multiple names, only print the first one to avoid an overly
668+
* long error message.
669+
*/
670+
if (names_examined > 1)
671+
{
672+
printfPQExpBuffer(&conn->errorMessage,
673+
libpq_ngettext("server certificate for \"%s\" (and %d other name) does not match host name \"%s\"\n",
674+
"server certificate for \"%s\" (and %d other names) does not match host name \"%s\"\n",
675+
names_examined - 1),
676+
first_name, names_examined - 1, conn->pghost);
677+
}
678+
else if (names_examined == 1)
679+
{
680+
printfPQExpBuffer(&conn->errorMessage,
681+
libpq_gettext("server certificate for \"%s\" does not match host name \"%s\"\n"),
682+
first_name, conn->pghost);
683+
}
557684
else
558685
{
559686
printfPQExpBuffer(&conn->errorMessage,
560-
libpq_gettext("server common name \"%s\" does not match host name \"%s\"\n"),
561-
peer_cn, conn->pghost);
562-
result = false;
687+
libpq_gettext("could not get server's hostname from server certificate\n"));
563688
}
564689
}
565690

566-
free(peer_cn);
567-
return result;
691+
/* clean up */
692+
if (first_name)
693+
free(first_name);
694+
695+
return found_match && !got_error;
568696
}
569697

570698
#ifdef ENABLE_THREAD_SAFETY

‎src/interfaces/libpq/libpq-int.h

+4
Original file line numberDiff line numberDiff line change
@@ -653,8 +653,12 @@ extern ssize_t pgtls_write(PGconn *conn, const void *ptr, size_t len);
653653
extern char *
654654
libpq_gettext(const char *msgid)
655655
__attribute__((format_arg(1)));
656+
extern char *
657+
libpq_ngettext(const char *msgid, const char *msgid_plural, unsigned long n)
658+
__attribute__((format_arg(1))) __attribute__((format_arg(2)));
656659
#else
657660
#define libpq_gettext(x) (x)
661+
#define libpq_ngettext(s, p, n) ((n) == 1 ? (s) : (p))
658662
#endif
659663

660664
/*

0 commit comments

Comments
 (0)