Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit d4fa0b4

Browse files
committed
Rename a libpq NOT_USED SSL function to
verify_peer_name_matches_certificate(), clarify some of the function's variables and logic, and update a comment. This should make SSL improvements easier in the future.
1 parent e67867b commit d4fa0b4

File tree

1 file changed

+31
-87
lines changed

1 file changed

+31
-87
lines changed

src/interfaces/libpq/fe-secure.c

Lines changed: 31 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
*
1212
*
1313
* IDENTIFICATION
14-
* $PostgreSQL: pgsql/src/interfaces/libpq/fe-secure.c,v 1.102 2008/01/29 02:03:39 tgl Exp $
14+
* $PostgreSQL: pgsql/src/interfaces/libpq/fe-secure.c,v 1.103 2008/02/16 21:03:30 momjian Exp $
1515
*
1616
* NOTES
1717
* [ Most of these notes are wrong/obsolete, but perhaps not all ]
@@ -143,7 +143,7 @@
143143
#endif
144144

145145
#ifdef NOT_USED
146-
static int verify_peer(PGconn *);
146+
static int verify_peer_name_matches_certificate(PGconn *);
147147
#endif
148148
static int verify_cb(int ok, X509_STORE_CTX *ctx);
149149
static int client_cert_cb(SSL *, X509 **, EVP_PKEY **);
@@ -498,18 +498,18 @@ verify_cb(int ok, X509_STORE_CTX *ctx)
498498
* Verify that common name resolves to peer.
499499
*/
500500
static int
501-
verify_peer(PGconn *conn)
501+
verify_peer_name_matches_certificate(PGconn *conn)
502502
{
503-
struct hostent *h = NULL;
504-
struct sockaddr addr;
505-
struct sockaddr_in *sin;
503+
struct hostent *cn_hostentry = NULL;
504+
struct sockaddr server_addr;
505+
struct sockaddr_in *sin (struct sockaddr_in *) &server_addr;
506506
ACCEPT_TYPE_ARG3 len;
507507
char **s;
508508
unsigned long l;
509509

510-
/* get the address on the other side of the socket */
511-
len = sizeof(addr);
512-
if (getpeername(conn->sock, &addr, &len) == -1)
510+
/* Get the address on the other side of the socket. */
511+
len = sizeof(server_addr);
512+
if (getpeername(conn->sock, &server_addr, &len) == -1)
513513
{
514514
char sebuf[256];
515515

@@ -519,10 +519,14 @@ verify_peer(PGconn *conn)
519519
return -1;
520520
}
521521

522-
/* weird, but legal case */
523-
if (addr.sa_family == AF_UNIX)
524-
return 0;
522+
if (server_addr.sa_family != AF_INET)
523+
{
524+
printfPQExpBuffer(&conn->errorMessage,
525+
libpq_gettext("unsupported protocol\n"));
526+
return -1;
527+
}
525528

529+
/* Get the IP addresses of the certificate's common name (CN) */
526530
{
527531
struct hostent hpstr;
528532
char buf[BUFSIZ];
@@ -534,65 +538,29 @@ verify_peer(PGconn *conn)
534538
* convert the pqGethostbyname() function call to use getaddrinfo().
535539
*/
536540
pqGethostbyname(conn->peer_cn, &hpstr, buf, sizeof(buf),
537-
&h, &herrno);
541+
&cn_hostentry, &herrno);
538542
}
539543

540-
/* what do we know about the peer's common name? */
541-
if (h == NULL)
544+
/* Did we get an IP address? */
545+
if (cn_hostentry == NULL)
542546
{
543547
printfPQExpBuffer(&conn->errorMessage,
544548
libpq_gettext("could not get information about host \"%s\": %s\n"),
545549
conn->peer_cn, hstrerror(h_errno));
546550
return -1;
547551
}
548552

549-
/* does the address match? */
550-
switch (addr.sa_family)
551-
{
552-
case AF_INET:
553-
sin = (struct sockaddr_in *) & addr;
554-
for (s = h->h_addr_list; *s != NULL; s++)
555-
{
556-
if (!memcmp(&sin->sin_addr.s_addr, *s, h->h_length))
557-
return 0;
558-
}
559-
break;
560-
561-
default:
562-
printfPQExpBuffer(&conn->errorMessage,
563-
libpq_gettext("unsupported protocol\n"));
564-
return -1;
565-
}
566-
567-
/*
568-
* the prior test should be definitive, but in practice it sometimes
569-
* fails. So we also check the aliases.
570-
*/
571-
for (s = h->h_aliases; *s != NULL; s++)
572-
{
573-
if (pg_strcasecmp(conn->peer_cn, *s) == 0)
553+
/* Does one of the CN's IP addresses match the server's IP address? */
554+
for (s = cn_hostentry->h_addr_list; *s != NULL; s++)
555+
if (!memcmp(&sin->sin_addr.s_addr, *s, cn_hostentry->h_length))
574556
return 0;
575-
}
576-
577-
/* generate protocol-aware error message */
578-
switch (addr.sa_family)
579-
{
580-
case AF_INET:
581-
sin = (struct sockaddr_in *) & addr;
582-
l = ntohl(sin->sin_addr.s_addr);
583-
printfPQExpBuffer(&conn->errorMessage,
584-
libpq_gettext(
585-
"server common name \"%s\" does not resolve to %ld.%ld.%ld.%ld\n"),
586-
conn->peer_cn, (l >> 24) % 0x100, (l >> 16) % 0x100,
587-
(l >> 8) % 0x100, l % 0x100);
588-
break;
589-
default:
590-
printfPQExpBuffer(&conn->errorMessage,
591-
libpq_gettext(
592-
"server common name \"%s\" does not resolve to peer address\n"),
593-
conn->peer_cn);
594-
}
595557

558+
l = ntohl(sin->sin_addr.s_addr);
559+
printfPQExpBuffer(&conn->errorMessage,
560+
libpq_gettext(
561+
"server common name \"%s\" does not resolve to %ld.%ld.%ld.%ld\n"),
562+
conn->peer_cn, (l >> 24) % 0x100, (l >> 16) % 0x100,
563+
(l >> 8) % 0x100, l % 0x100);
596564
return -1;
597565
}
598566
#endif /* NOT_USED */
@@ -1049,25 +1017,10 @@ open_client_SSL(PGconn *conn)
10491017
}
10501018
}
10511019

1052-
/* check the certificate chain of the server */
1053-
1054-
#ifdef NOT_USED
1055-
/* CLIENT CERTIFICATES NOT REQUIRED bjm 2002-09-26 */
1056-
10571020
/*
1058-
* this eliminates simple man-in-the-middle attacks and simple
1059-
* impersonations
1021+
* We already checked the server certificate in initialize_SSL()
1022+
* using SSL_CTX_set_verify() if root.crt exists.
10601023
*/
1061-
r = SSL_get_verify_result(conn->ssl);
1062-
if (r != X509_V_OK)
1063-
{
1064-
printfPQExpBuffer(&conn->errorMessage,
1065-
libpq_gettext("certificate could not be validated: %s\n"),
1066-
X509_verify_cert_error_string(r));
1067-
close_SSL(conn);
1068-
return PGRES_POLLING_FAILED;
1069-
}
1070-
#endif
10711024

10721025
/* pull out server distinguished and common names */
10731026
conn->peer = SSL_get_peer_certificate(conn->ssl);
@@ -1091,17 +1044,8 @@ open_client_SSL(PGconn *conn)
10911044
NID_commonName, conn->peer_cn, SM_USER);
10921045
conn->peer_cn[SM_USER] = '\0';
10931046

1094-
/* verify that the common name resolves to peer */
1095-
10961047
#ifdef NOT_USED
1097-
/* CLIENT CERTIFICATES NOT REQUIRED bjm 2002-09-26 */
1098-
1099-
/*
1100-
* this is necessary to eliminate man-in-the-middle attacks and
1101-
* impersonations where the attacker somehow learned the server's private
1102-
* key
1103-
*/
1104-
if (verify_peer(conn) == -1)
1048+
if (verify_peer_name_matches_certificate(conn) == -1)
11051049
{
11061050
close_SSL(conn);
11071051
return PGRES_POLLING_FAILED;

0 commit comments

Comments
 (0)