I have applied the attached patch which renames a libpq NOT_USED SSL
function to verify_peer_name_matches_certificate(), clarifies some of
the function's variables and logic, and updates a comment.  This should
make SSL improvements easier in the future.

-- 
  Bruce Momjian  <[EMAIL PROTECTED]>        http://momjian.us
  EnterpriseDB                             http://postgres.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +
Index: src/interfaces/libpq/fe-secure.c
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-secure.c,v
retrieving revision 1.102
diff -c -c -r1.102 fe-secure.c
*** src/interfaces/libpq/fe-secure.c	29 Jan 2008 02:03:39 -0000	1.102
--- src/interfaces/libpq/fe-secure.c	14 Feb 2008 03:46:30 -0000
***************
*** 143,149 ****
  #endif
  
  #ifdef NOT_USED
! static int	verify_peer(PGconn *);
  #endif
  static int	verify_cb(int ok, X509_STORE_CTX *ctx);
  static int	client_cert_cb(SSL *, X509 **, EVP_PKEY **);
--- 143,149 ----
  #endif
  
  #ifdef NOT_USED
! static int	verify_peer_name_matches_certificate(PGconn *);
  #endif
  static int	verify_cb(int ok, X509_STORE_CTX *ctx);
  static int	client_cert_cb(SSL *, X509 **, EVP_PKEY **);
***************
*** 498,515 ****
   *	Verify that common name resolves to peer.
   */
  static int
! verify_peer(PGconn *conn)
  {
! 	struct hostent *h = NULL;
! 	struct sockaddr addr;
! 	struct sockaddr_in *sin;
  	ACCEPT_TYPE_ARG3 len;
  	char	  **s;
  	unsigned long l;
  
! 	/* get the address on the other side of the socket */
! 	len = sizeof(addr);
! 	if (getpeername(conn->sock, &addr, &len) == -1)
  	{
  		char		sebuf[256];
  
--- 498,515 ----
   *	Verify that common name resolves to peer.
   */
  static int
! verify_peer_name_matches_certificate(PGconn *conn)
  {
! 	struct hostent *cn_hostentry = NULL;
! 	struct sockaddr server_addr;
! 	struct sockaddr_in *sin (struct sockaddr_in *) &server_addr;
  	ACCEPT_TYPE_ARG3 len;
  	char	  **s;
  	unsigned long l;
  
! 	/* Get the address on the other side of the socket. */
! 	len = sizeof(server_addr);
! 	if (getpeername(conn->sock, &server_addr, &len) == -1)
  	{
  		char		sebuf[256];
  
***************
*** 519,528 ****
  		return -1;
  	}
  
! 	/* weird, but legal case */
! 	if (addr.sa_family == AF_UNIX)
! 		return 0;
  
  	{
  		struct hostent hpstr;
  		char		buf[BUFSIZ];
--- 519,532 ----
  		return -1;
  	}
  
! 	if (server_addr.sa_family != AF_INET)
! 	{
! 		printfPQExpBuffer(&conn->errorMessage,
! 						  libpq_gettext("unsupported protocol\n"));
! 		return -1;
! 	}
  
+ 	/* Get the IP addresses of the certificate's common name (CN) */
  	{
  		struct hostent hpstr;
  		char		buf[BUFSIZ];
***************
*** 534,544 ****
  		 * convert the pqGethostbyname() function call to use getaddrinfo().
  		 */
  		pqGethostbyname(conn->peer_cn, &hpstr, buf, sizeof(buf),
! 						&h, &herrno);
  	}
  
! 	/* what do we know about the peer's common name? */
! 	if (h == NULL)
  	{
  		printfPQExpBuffer(&conn->errorMessage,
  		  libpq_gettext("could not get information about host \"%s\": %s\n"),
--- 538,548 ----
  		 * convert the pqGethostbyname() function call to use getaddrinfo().
  		 */
  		pqGethostbyname(conn->peer_cn, &hpstr, buf, sizeof(buf),
! 						&cn_hostentry, &herrno);
  	}
  
! 	/* Did we get an IP address? */
! 	if (cn_hostentry == NULL)
  	{
  		printfPQExpBuffer(&conn->errorMessage,
  		  libpq_gettext("could not get information about host \"%s\": %s\n"),
***************
*** 546,598 ****
  		return -1;
  	}
  
! 	/* does the address match? */
! 	switch (addr.sa_family)
! 	{
! 		case AF_INET:
! 			sin = (struct sockaddr_in *) & addr;
! 			for (s = h->h_addr_list; *s != NULL; s++)
! 			{
! 				if (!memcmp(&sin->sin_addr.s_addr, *s, h->h_length))
! 					return 0;
! 			}
! 			break;
! 
! 		default:
! 			printfPQExpBuffer(&conn->errorMessage,
! 							  libpq_gettext("unsupported protocol\n"));
! 			return -1;
! 	}
! 
! 	/*
! 	 * the prior test should be definitive, but in practice it sometimes
! 	 * fails.  So we also check the aliases.
! 	 */
! 	for (s = h->h_aliases; *s != NULL; s++)
! 	{
! 		if (pg_strcasecmp(conn->peer_cn, *s) == 0)
  			return 0;
- 	}
- 
- 	/* generate protocol-aware error message */
- 	switch (addr.sa_family)
- 	{
- 		case AF_INET:
- 			sin = (struct sockaddr_in *) & addr;
- 			l = ntohl(sin->sin_addr.s_addr);
- 			printfPQExpBuffer(&conn->errorMessage,
- 							  libpq_gettext(
- 											"server common name \"%s\" does not resolve to %ld.%ld.%ld.%ld\n"),
- 						 conn->peer_cn, (l >> 24) % 0x100, (l >> 16) % 0x100,
- 							  (l >> 8) % 0x100, l % 0x100);
- 			break;
- 		default:
- 			printfPQExpBuffer(&conn->errorMessage,
- 							  libpq_gettext(
- 			 "server common name \"%s\" does not resolve to peer address\n"),
- 							  conn->peer_cn);
- 	}
  
  	return -1;
  }
  #endif   /* NOT_USED */
--- 550,566 ----
  		return -1;
  	}
  
! 	/* Does one of the CN's IP addresses match the server's IP address? */
! 	for (s = cn_hostentry->h_addr_list; *s != NULL; s++)
! 		if (!memcmp(&sin->sin_addr.s_addr, *s, cn_hostentry->h_length))
  			return 0;
  
+ 	l = ntohl(sin->sin_addr.s_addr);
+ 	printfPQExpBuffer(&conn->errorMessage,
+ 					  libpq_gettext(
+ 					    "server common name \"%s\" does not resolve to %ld.%ld.%ld.%ld\n"),
+ 					  conn->peer_cn, (l >> 24) % 0x100, (l >> 16) % 0x100,
+ 					  (l >> 8) % 0x100, l % 0x100);
  	return -1;
  }
  #endif   /* NOT_USED */
***************
*** 1049,1073 ****
  		}
  	}
  
- 	/* check the certificate chain of the server */
- 
- #ifdef NOT_USED
- 	/* CLIENT CERTIFICATES NOT REQUIRED  bjm 2002-09-26 */
- 
  	/*
! 	 * this eliminates simple man-in-the-middle attacks and simple
! 	 * impersonations
  	 */
- 	r = SSL_get_verify_result(conn->ssl);
- 	if (r != X509_V_OK)
- 	{
- 		printfPQExpBuffer(&conn->errorMessage,
- 				   libpq_gettext("certificate could not be validated: %s\n"),
- 						  X509_verify_cert_error_string(r));
- 		close_SSL(conn);
- 		return PGRES_POLLING_FAILED;
- 	}
- #endif
  
  	/* pull out server distinguished and common names */
  	conn->peer = SSL_get_peer_certificate(conn->ssl);
--- 1017,1026 ----
  		}
  	}
  
  	/*
! 	 * We already checked the server certificate in initialize_SSL()
! 	 * using SSL_CTX_set_verify() if root.crt exists.
  	 */
  
  	/* pull out server distinguished and common names */
  	conn->peer = SSL_get_peer_certificate(conn->ssl);
***************
*** 1091,1107 ****
  							  NID_commonName, conn->peer_cn, SM_USER);
  	conn->peer_cn[SM_USER] = '\0';
  
- 	/* verify that the common name resolves to peer */
- 
  #ifdef NOT_USED
! 	/* CLIENT CERTIFICATES NOT REQUIRED  bjm 2002-09-26 */
! 
! 	/*
! 	 * this is necessary to eliminate man-in-the-middle attacks and
! 	 * impersonations where the attacker somehow learned the server's private
! 	 * key
! 	 */
! 	if (verify_peer(conn) == -1)
  	{
  		close_SSL(conn);
  		return PGRES_POLLING_FAILED;
--- 1044,1051 ----
  							  NID_commonName, conn->peer_cn, SM_USER);
  	conn->peer_cn[SM_USER] = '\0';
  
  #ifdef NOT_USED
! 	if (verify_peer_name_matches_certificate(conn) == -1)
  	{
  		close_SSL(conn);
  		return PGRES_POLLING_FAILED;
---------------------------(end of broadcast)---------------------------
TIP 5: don't forget to increase your free space map settings

Reply via email to