Skip to content

Commit c1932e5

Browse files
peterehorigutidanielgustafsson
committed
libpq: Allow IP address SANs in server certificates
The current implementation supports exactly one IP address in a server certificate's Common Name, which is brittle (the strings must match exactly). This patch adds support for IPv4 and IPv6 addresses in a server's Subject Alternative Names. Per discussion on-list: - If the client's expected host is an IP address, we allow fallback to the Subject Common Name if an iPAddress SAN is not present, even if a dNSName is present. This matches the behavior of NSS, in violation of the relevant RFCs. - We also, counter-intuitively, match IP addresses embedded in dNSName SANs. From inspection this appears to have been the behavior since the SAN matching feature was introduced in acd08d7. - Unlike NSS, we don't map IPv4 to IPv6 addresses, or vice-versa. Author: Jacob Champion <pchampion@vmware.com> Co-authored-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Co-authored-by: Daniel Gustafsson <daniel@yesql.se> Discussion: https://www.postgresql.org/message-id/flat/9f5f20974cd3a4091a788cf7f00ab663d5fcdffe.camel@vmware.com
1 parent fa25beb commit c1932e5

22 files changed

+635
-17
lines changed

‎configure

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15982,7 +15982,7 @@ fi
1598215982
LIBS_including_readline="$LIBS"
1598315983
LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
1598415984

15985-
for ac_func in backtrace_symbols clock_gettime copyfile fdatasync getifaddrs getpeerucred getrlimit kqueue mbstowcs_l memset_s poll posix_fallocate ppoll pstat pthread_is_threaded_np readlink readv setproctitle setproctitle_fast setsid shm_open strchrnul strsignal symlink syncfs sync_file_range uselocale wcstombs_l writev
15985+
for ac_func in backtrace_symbols clock_gettime copyfile fdatasync getifaddrs getpeerucred getrlimit inet_pton kqueue mbstowcs_l memset_s poll posix_fallocate ppoll pstat pthread_is_threaded_np readlink readv setproctitle setproctitle_fast setsid shm_open strchrnul strsignal symlink syncfs sync_file_range uselocale wcstombs_l writev
1598615986
do :
1598715987
as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
1598815988
ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"

‎configure.ac

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1787,6 +1787,7 @@ AC_CHECK_FUNCS(m4_normalize([
17871787
getifaddrs
17881788
getpeerucred
17891789
getrlimit
1790+
inet_pton
17901791
kqueue
17911792
mbstowcs_l
17921793
memset_s

‎doc/src/sgml/libpq.sgml

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8356,16 +8356,31 @@ ldap://ldap.acme.com/cn=dbserver,cn=hosts?pgconnectinfo?base?(objectclass=*)
83568356

83578357
<para>
83588358
In <literal>verify-full</literal> mode, the host name is matched against the
8359-
certificate's Subject Alternative Name attribute(s), or against the
8360-
Common Name attribute if no Subject Alternative Name of type <literal>dNSName</literal> is
8359+
certificate's Subject Alternative Name attribute(s) (SAN), or against the
8360+
Common Name attribute if no SAN of type <literal>dNSName</literal> is
83618361
present. If the certificate's name attribute starts with an asterisk
83628362
(<literal>*</literal>), the asterisk will be treated as
83638363
a wildcard, which will match all characters <emphasis>except</emphasis> a dot
83648364
(<literal>.</literal>). This means the certificate will not match subdomains.
83658365
If the connection is made using an IP address instead of a host name, the
8366-
IP address will be matched (without doing any DNS lookups).
8366+
IP address will be matched (without doing any DNS lookups) against SANs of
8367+
type <literal>iPAddress</literal> or <literal>dNSName</literal>. If no
8368+
<literal>iPAddress</literal> SAN is present and no
8369+
matching <literal>dNSName</literal> SAN is present, the host IP address is
8370+
matched against the Common Name attribute.
83678371
</para>
83688372

8373+
<note>
8374+
<para>
8375+
For backward compatibility with earlier versions of PostgreSQL, the host
8376+
IP address is verified in a manner different
8377+
from <ulink url="https://tools.ietf.org/html/rfc6125">RFC 6125</ulink>.
8378+
The host IP address is always matched against <literal>dNSName</literal>
8379+
SANs as well as <literal>iPAddress</literal> SANs, and can be matched
8380+
against the Common Name attribute if no relevant SANs exist.
8381+
</para>
8382+
</note>
8383+
83698384
<para>
83708385
To allow server certificate verification, one or more root certificates
83718386
must be placed in the file <filename>~/.postgresql/root.crt</filename>

‎src/include/pg_config.h.in

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,9 @@
283283
/* Define to 1 if you have the `inet_aton' function. */
284284
#undef HAVE_INET_ATON
285285

286+
/* Define to 1 if you have the `inet_pton' function. */
287+
#undef HAVE_INET_PTON
288+
286289
/* Define to 1 if the system has the type `int64'. */
287290
#undef HAVE_INT64
288291

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

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919

2020
#include "postgres_fe.h"
2121

22+
#include <arpa/inet.h>
23+
2224
#include "fe-secure-common.h"
2325

2426
#include "libpq-int.h"
@@ -144,6 +146,108 @@ pq_verify_peer_name_matches_certificate_name(PGconn *conn,
144146
return result;
145147
}
146148

149+
/*
150+
* Check if an IP address from a server's certificate matches the peer's
151+
* hostname (which must itself be an IPv4/6 address).
152+
*
153+
* Returns 1 if the address matches, and 0 if it does not. On error, returns
154+
* -1, and sets the libpq error message.
155+
*
156+
* A string representation of the certificate's IP address is returned in
157+
* *store_name. The caller is responsible for freeing it.
158+
*/
159+
int
160+
pq_verify_peer_name_matches_certificate_ip(PGconn *conn,
161+
const unsigned char *ipdata,
162+
size_t iplen,
163+
char **store_name)
164+
{
165+
char *addrstr;
166+
int match = 0;
167+
char *host = conn->connhost[conn->whichhost].host;
168+
int family;
169+
char tmp[sizeof "ffff:ffff:ffff:ffff:ffff:ffff:255.255.255.255"];
170+
char sebuf[PG_STRERROR_R_BUFLEN];
171+
172+
*store_name = NULL;
173+
174+
if (!(host && host[0] != '\0'))
175+
{
176+
appendPQExpBufferStr(&conn->errorMessage,
177+
libpq_gettext("host name must be specified\n"));
178+
return -1;
179+
}
180+
181+
/*
182+
* The data from the certificate is in network byte order. Convert our
183+
* host string to network-ordered bytes as well, for comparison. (The host
184+
* string isn't guaranteed to actually be an IP address, so if this
185+
* conversion fails we need to consider it a mismatch rather than an
186+
* error.)
187+
*/
188+
if (iplen == 4)
189+
{
190+
/* IPv4 */
191+
struct in_addr addr;
192+
193+
family = AF_INET;
194+
195+
/*
196+
* The use of inet_aton() is deliberate; we accept alternative IPv4
197+
* address notations that are accepted by inet_aton() but not
198+
* inet_pton() as server addresses.
199+
*/
200+
if (inet_aton(host, &addr))
201+
{
202+
if (memcmp(ipdata, &addr.s_addr, iplen) == 0)
203+
match = 1;
204+
}
205+
}
206+
/*
207+
* If they don't have inet_pton(), skip this. Then, an IPv6 address in a
208+
* certificate will cause an error.
209+
*/
210+
#ifdef HAVE_INET_PTON
211+
else if (iplen == 16)
212+
{
213+
/* IPv6 */
214+
struct in6_addr addr;
215+
216+
family = AF_INET6;
217+
218+
if (inet_pton(AF_INET6, host, &addr) == 1)
219+
{
220+
if (memcmp(ipdata, &addr.s6_addr, iplen) == 0)
221+
match = 1;
222+
}
223+
}
224+
#endif
225+
else
226+
{
227+
/*
228+
* Not IPv4 or IPv6. We could ignore the field, but leniency seems
229+
* wrong given the subject matter.
230+
*/
231+
appendPQExpBuffer(&conn->errorMessage,
232+
libpq_gettext("certificate contains IP address with invalid length %lu\n"),
233+
(unsigned long) iplen);
234+
return -1;
235+
}
236+
237+
/* Generate a human-readable representation of the certificate's IP. */
238+
addrstr = pg_inet_net_ntop(family, ipdata, 8 * iplen, tmp, sizeof(tmp));
239+
if (!addrstr)
240+
{
241+
appendPQExpBuffer(&conn->errorMessage,
242+
libpq_gettext("could not convert certificate's IP address to string: %s\n"),
243+
strerror_r(errno, sebuf, sizeof(sebuf)));
244+
return -1;
245+
}
246+
247+
*store_name = strdup(addrstr);
248+
return match;
249+
}
250+
147251
/*
148252
* Verify that the server certificate matches the hostname we connected to.
149253
*

‎src/interfaces/libpq/fe-secure-common.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,10 @@
2121
extern int pq_verify_peer_name_matches_certificate_name(PGconn *conn,
2222
const char *namedata, size_t namelen,
2323
char **store_name);
24+
extern int pq_verify_peer_name_matches_certificate_ip(PGconn *conn,
25+
const unsigned char *addrdata,
26+
size_t addrlen,
27+
char **store_name);
2428
extern bool pq_verify_peer_name_matches_certificate(PGconn *conn);
2529

2630
#endif /* FE_SECURE_COMMON_H */

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

Lines changed: 130 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,9 @@ static int verify_cb(int ok, X509_STORE_CTX *ctx);
7272
static int openssl_verify_peer_name_matches_certificate_name(PGconn *conn,
7373
ASN1_STRING *name,
7474
char **store_name);
75+
static int openssl_verify_peer_name_matches_certificate_ip(PGconn *conn,
76+
ASN1_OCTET_STRING *addr_entry,
77+
char **store_name);
7578
static void destroy_ssl_system(void);
7679
static int initialize_SSL(PGconn *conn);
7780
static PostgresPollingStatusType open_client_SSL(PGconn *);
@@ -509,6 +512,56 @@ openssl_verify_peer_name_matches_certificate_name(PGconn *conn, ASN1_STRING *nam
509512
return pq_verify_peer_name_matches_certificate_name(conn, (const char *) namedata, len, store_name);
510513
}
511514

515+
/*
516+
* OpenSSL-specific wrapper around
517+
* pq_verify_peer_name_matches_certificate_ip(), converting the
518+
* ASN1_OCTET_STRING into a plain C string.
519+
*/
520+
static int
521+
openssl_verify_peer_name_matches_certificate_ip(PGconn *conn,
522+
ASN1_OCTET_STRING *addr_entry,
523+
char **store_name)
524+
{
525+
int len;
526+
const unsigned char *addrdata;
527+
528+
/* Should not happen... */
529+
if (addr_entry == NULL)
530+
{
531+
appendPQExpBufferStr(&conn->errorMessage,
532+
libpq_gettext("SSL certificate's address entry is missing\n"));
533+
return -1;
534+
}
535+
536+
/*
537+
* GEN_IPADD is an OCTET STRING containing an IP address in network byte
538+
* order.
539+
*/
540+
#ifdef HAVE_ASN1_STRING_GET0_DATA
541+
addrdata = ASN1_STRING_get0_data(addr_entry);
542+
#else
543+
addrdata = ASN1_STRING_data(addr_entry);
544+
#endif
545+
len = ASN1_STRING_length(addr_entry);
546+
547+
return pq_verify_peer_name_matches_certificate_ip(conn, addrdata, len, store_name);
548+
}
549+
550+
static bool
551+
is_ip_address(const char *host)
552+
{
553+
struct in_addr dummy4;
554+
#ifdef HAVE_INET_PTON
555+
struct in6_addr dummy6;
556+
#endif
557+
558+
return inet_aton(host, &dummy4)
559+
#ifdef HAVE_INET_PTON
560+
|| (inet_pton(AF_INET6, host, &dummy6) == 1)
561+
#endif
562+
;
563+
}
564+
512565
/*
513566
* Verify that the server certificate matches the hostname we connected to.
514567
*
@@ -522,6 +575,36 @@ pgtls_verify_peer_name_matches_certificate_guts(PGconn *conn,
522575
STACK_OF(GENERAL_NAME) * peer_san;
523576
int i;
524577
int rc = 0;
578+
char *host = conn->connhost[conn->whichhost].host;
579+
int host_type;
580+
bool check_cn = true;
581+
582+
Assert(host && host[0]); /* should be guaranteed by caller */
583+
584+
/*
585+
* We try to match the NSS behavior here, which is a slight departure from
586+
* the spec but seems to make more intuitive sense:
587+
*
588+
* If connhost contains a DNS name, and the certificate's SANs contain any
589+
* dNSName entries, then we'll ignore the Subject Common Name entirely;
590+
* otherwise, we fall back to checking the CN. (This behavior matches the
591+
* RFC.)
592+
*
593+
* If connhost contains an IP address, and the SANs contain iPAddress
594+
* entries, we again ignore the CN. Otherwise, we allow the CN to match,
595+
* EVEN IF there is a dNSName in the SANs. (RFC 6125 prohibits this: "A
596+
* client MUST NOT seek a match for a reference identifier of CN-ID if the
597+
* presented identifiers include a DNS-ID, SRV-ID, URI-ID, or any
598+
* application-specific identifier types supported by the client.")
599+
*
600+
* NOTE: Prior versions of libpq did not consider iPAddress entries at
601+
* all, so this new behavior might break a certificate that has different
602+
* IP addresses in the Subject CN and the SANs.
603+
*/
604+
if (is_ip_address(host))
605+
host_type = GEN_IPADD;
606+
else
607+
host_type = GEN_DNS;
525608

526609
/*
527610
* First, get the Subject Alternative Names (SANs) from the certificate,
@@ -537,38 +620,62 @@ pgtls_verify_peer_name_matches_certificate_guts(PGconn *conn,
537620
for (i = 0; i < san_len; i++)
538621
{
539622
const GENERAL_NAME *name = sk_GENERAL_NAME_value(peer_san, i);
623+
char *alt_name = NULL;
540624

541-
if (name->type == GEN_DNS)
625+
if (name->type == host_type)
542626
{
543-
char *alt_name;
627+
/*
628+
* This SAN is of the same type (IP or DNS) as our host name,
629+
* so don't allow a fallback check of the CN.
630+
*/
631+
check_cn = false;
632+
}
544633

634+
if (name->type == GEN_DNS)
635+
{
545636
(*names_examined)++;
546637
rc = openssl_verify_peer_name_matches_certificate_name(conn,
547638
name->d.dNSName,
548639
&alt_name);
640+
}
641+
else if (name->type == GEN_IPADD)
642+
{
643+
(*names_examined)++;
644+
rc = openssl_verify_peer_name_matches_certificate_ip(conn,
645+
name->d.iPAddress,
646+
&alt_name);
647+
}
549648

550-
if (alt_name)
551-
{
552-
if (!*first_name)
553-
*first_name = alt_name;
554-
else
555-
free(alt_name);
556-
}
649+
if (alt_name)
650+
{
651+
if (!*first_name)
652+
*first_name = alt_name;
653+
else
654+
free(alt_name);
557655
}
656+
558657
if (rc != 0)
658+
{
659+
/*
660+
* Either we hit an error or a match, and either way we should
661+
* not fall back to the CN.
662+
*/
663+
check_cn = false;
559664
break;
665+
}
560666
}
561667
sk_GENERAL_NAME_pop_free(peer_san, GENERAL_NAME_free);
562668
}
563669

564670
/*
565-
* If there is no subjectAltName extension of type dNSName, check the
671+
* If there is no subjectAltName extension of the matching type, check the
566672
* Common Name.
567673
*
568674
* (Per RFC 2818 and RFC 6125, if the subjectAltName extension of type
569-
* dNSName is present, the CN must be ignored.)
675+
* dNSName is present, the CN must be ignored. We break this rule if host
676+
* is an IP address; see the comment above.)
570677
*/
571-
if (*names_examined == 0)
678+
if (check_cn)
572679
{
573680
X509_NAME *subject_name;
574681

@@ -581,10 +688,20 @@ pgtls_verify_peer_name_matches_certificate_guts(PGconn *conn,
581688
NID_commonName, -1);
582689
if (cn_index >= 0)
583690
{
691+
char *common_name = NULL;
692+
584693
(*names_examined)++;
585694
rc = openssl_verify_peer_name_matches_certificate_name(conn,
586695
X509_NAME_ENTRY_get_data(X509_NAME_get_entry(subject_name, cn_index)),
587-
first_name);
696+
&common_name);
697+
698+
if (common_name)
699+
{
700+
if (!*first_name)
701+
*first_name = common_name;
702+
else
703+
free(common_name);
704+
}
588705
}
589706
}
590707
}

0 commit comments

Comments
 (0)