Fix libpq certificate validation for SSL connections.

Add config parameter "sslverify" to control the verification. Default
is to do full verification.

Clean up some old SSL code that never really worked.

5 files changed, 139 insertions, 77 deletions

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 6f977f9083e..0b220ab514b 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1,4 +1,4 @@
-<!-- $PostgreSQL: pgsql/doc/src/sgml/libpq.sgml,v 1.268 2008/11/09 00:28:34 tgl Exp $ -->
+<!-- $PostgreSQL: pgsql/doc/src/sgml/libpq.sgml,v 1.269 2008/11/13 09:45:24 mha Exp $ -->
 
 <chapter id="libpq">
  <title><application>libpq</application> - C Library</title>
@@ -260,6 +260,33 @@
         </varlistentry>
 
         <varlistentry>
+         <term><literal>sslverify</literal></term>
+         <listitem>
+          <para>
+           This option controls how libpq verifies the certificate on the
+           server when performing an <acronym>SSL</> connection. There are
+           three options: <literal>none</> disables verification completely
+           (not recommended!); <literal>cert</> enables verification that
+           the certificate chains to a known CA only; <literal>cn</> will
+           both verify that the certificate chains to a known CA and that
+           the <literal>cn</> attribute of the certificate matches the
+           hostname the connection is being made to (default).
+          </para>
+
+          <para>
+           It is always recommended to use the <literal>cn</> value for
+           this parameter, since this is the only option that prevents
+           man-in-the-middle attacks. Note that this requires the server
+           name on the certificate to match exactly with the host name
+           used for the connection, and therefore does not support connections
+           to aliased names. It can be used with pure IP address connections
+           only if the certificate also has just the IP address in the
+           <literal>cn</> field.
+          </para>
+         </listitem>
+        </varlistentry>
+
+        <varlistentry>
          <term><literal>requiressl</literal></term>
          <listitem>
           <para>
@@ -5682,6 +5709,22 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough)
     <listitem>
      <para>
       <indexterm>
+       <primary><envar>PGSSLVERIFY</envar></primary>
+      </indexterm>
+      <envar>PGSSLVERIFY</envar> controls how libpq verifies the certificate on the
+       server when performing an <acronym>SSL</> connection. There are
+       three options: <literal>none</> disables verification completely
+       (not recommended!); <literal>cert</> enables verification that
+       the certificate chains to a known CA only; <literal>cn</> will
+       both verify that the certificate chains to a known CA and that
+       the <literal>cn</> attribute of the certificate matches the
+       hostname the connection is being made to (default).
+     </para>
+    </listitem>
+
+    <listitem>
+     <para>
+      <indexterm>
        <primary><envar>PGREQUIRESSL</envar></primary>
       </indexterm>
       <envar>PGREQUIRESSL</envar> sets whether or not the connection must
@@ -6026,9 +6069,11 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough)
   </para>
 
   <para>
-   To verify the server certificate is trustworthy, place certificates of
-   the certificate authorities (<acronym>CA</acronym>) you trust in the
-   file <filename>~/.postgresql/root.crt</> in the user's home directory.
+   When the <literal>sslverify</> parameter is set to <literal>cn</> or
+   <literal>cert</>, libpq will verify that the server certificate is
+   trustworthy by checking the certificate chain up to a <acronym>CA</>.
+   For this to work, place the certificate of a trusted <acronym>CA</>
+   in the file <filename>~/.postgresql/root.crt</> in the user's home directory.
    (On Microsoft Windows the file is named
    <filename>%APPDATA%\postgresql\root.crt</filename>.)
    <application>libpq</application> will then verify that the server's
diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml
index a46f59cf1fc..b04de288c33 100644
--- a/doc/src/sgml/runtime.sgml
+++ b/doc/src/sgml/runtime.sgml
@@ -1,4 +1,4 @@
-<!-- $PostgreSQL: pgsql/doc/src/sgml/runtime.sgml,v 1.419 2008/11/04 04:18:50 momjian Exp $ -->
+<!-- $PostgreSQL: pgsql/doc/src/sgml/runtime.sgml,v 1.420 2008/11/13 09:45:24 mha Exp $ -->
 
 <chapter Id="runtime">
  <title>Operating System Environment</title>
@@ -1418,9 +1418,9 @@ $ <userinput>kill -INT `head -1 /usr/local/pgsql/data/postmaster.pid`</userinput
    <filename>server.key</filename> (key) and
    <filename>server.crt</filename> (certificate) files (<xref
    linkend="ssl-tcp">). The TCP client must connect using
-   <literal>sslmode='require'</> (<xref linkend="libpq-connect">) and have
-   a <filename>~/.postgresql/root.crt</> SSL certificate (<xref
-   linkend="libpq-ssl">).
+   <literal>sslmode='require'</>, specify <literal>sslverify='cn'</>
+   or <literal>sslverify='cert'</> and have the required certificate
+   files present (<xref linkend="libpq-connect">).
   </para>
  </sect1>
   
@@ -1544,8 +1544,8 @@ $ <userinput>kill -INT `head -1 /usr/local/pgsql/data/postmaster.pid`</userinput
 
    <listitem>
     <para> 
-     It is possible for both the client and server to provide SSL keys
-     or certificates to each other. It takes some extra configuration
+     It is possible for both the client and server to provide SSL
+     certificates to each other. It takes some extra configuration
      on each side, but this provides stronger verification of identity
      than the mere use of passwords. It prevents a computer from
      pretending to be the server just long enough to read the password
@@ -1757,7 +1757,9 @@ chmod og-rwx server.key
     A self-signed certificate can be used for testing, but a certificate
     signed by a certificate authority (<acronym>CA</>) (either one of the
     global <acronym>CAs</> or a local one) should be used in production
-    so the client can verify the server's identity.
+    so the client can verify the server's identity. If all the clients
+    are local to the organization, using a local <acronym>CA</> is
+    recommended.
    </para>
 
   </sect2>
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index d8b243b8d69..e1376dc0173 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/interfaces/libpq/fe-connect.c,v 1.367 2008/11/09 00:28:35 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/interfaces/libpq/fe-connect.c,v 1.368 2008/11/13 09:45:24 mha Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -92,8 +92,10 @@ static int ldapServiceLookup(const char *purl, PQconninfoOption *options,
 #define DefaultPassword		  ""
 #ifdef USE_SSL
 #define DefaultSSLMode	"prefer"
+#define DefaultSSLVerify "cn"
 #else
 #define DefaultSSLMode	"disable"
+#define DefaultSSLVerify "none"
 #endif
 
 /* ----------
@@ -181,6 +183,9 @@ static const PQconninfoOption PQconninfoOptions[] = {
 	{"sslmode", "PGSSLMODE", DefaultSSLMode, NULL,
 	"SSL-Mode", "", 8},			/* sizeof("disable") == 8 */
 
+	{"sslverify", "PGSSLVERIFY", DefaultSSLVerify, NULL,
+	"SSL-Verify", "", 5},		/* sizeof("chain") == 5 */
+
 #if defined(KRB5) || defined(ENABLE_GSS) || defined(ENABLE_SSPI)
 	/* Kerberos and GSSAPI authentication support specifying the service name */
 	{"krbsrvname", "PGKRBSRVNAME", PG_KRB_SRVNAM, NULL,
@@ -415,6 +420,8 @@ connectOptions1(PGconn *conn, const char *conninfo)
 	conn->connect_timeout = tmp ? strdup(tmp) : NULL;
 	tmp = conninfo_getval(connOptions, "sslmode");
 	conn->sslmode = tmp ? strdup(tmp) : NULL;
+	tmp = conninfo_getval(connOptions, "sslverify");
+	conn->sslverify = tmp ? strdup(tmp) : NULL;
 #ifdef USE_SSL
 	tmp = conninfo_getval(connOptions, "requiressl");
 	if (tmp && tmp[0] == '1')
@@ -530,6 +537,24 @@ connectOptions2(PGconn *conn)
 		conn->sslmode = strdup(DefaultSSLMode);
 
 	/*
+	 * Validate sslverify option
+	 */
+	if (conn->sslverify)
+	{
+		if (strcmp(conn->sslverify, "none") != 0
+			&& strcmp(conn->sslverify, "cert") != 0
+			&& strcmp(conn->sslverify, "cn") != 0)
+		{
+			conn->status = CONNECTION_BAD;
+			printfPQExpBuffer(&conn->errorMessage,
+							libpq_gettext("invalid sslverify value: \"%s\"\n"),
+							  conn->sslverify);
+			return false;
+		}
+	}
+
+
+	/*
 	 * Only if we get this far is it appropriate to try to connect. (We need a
 	 * state flag, rather than just the boolean result of this function, in
 	 * case someone tries to PQreset() the PGconn.)
@@ -2008,6 +2033,8 @@ freePGconn(PGconn *conn)
 		free(conn->pgpass);
 	if (conn->sslmode)
 		free(conn->sslmode);
+	if (conn->sslverify)
+		free(conn->sslverify);
 #if defined(KRB5) || defined(ENABLE_GSS) || defined(ENABLE_SSPI)
 	if (conn->krbsrvname)
 		free(conn->krbsrvname);
diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
index af85257a86b..1cc7c5cbfb0 100644
--- a/src/interfaces/libpq/fe-secure.c
+++ b/src/interfaces/libpq/fe-secure.c
@@ -11,7 +11,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/interfaces/libpq/fe-secure.c,v 1.106 2008/10/24 12:29:11 mha Exp $
+ *	  $PostgreSQL: pgsql/src/interfaces/libpq/fe-secure.c,v 1.107 2008/11/13 09:45:25 mha Exp $
  *
  * NOTES
  *
@@ -87,9 +87,7 @@
 #define ERR_pop_to_mark()	((void) 0)
 #endif
 
-#ifdef NOT_USED
-static int	verify_peer_name_matches_certificate(PGconn *);
-#endif
+static bool verify_peer_name_matches_certificate(PGconn *);
 static int	verify_cb(int ok, X509_STORE_CTX *ctx);
 static int	client_cert_cb(SSL *, X509 **, EVP_PKEY **);
 static int	init_ssl_system(PGconn *conn);
@@ -438,77 +436,44 @@ verify_cb(int ok, X509_STORE_CTX *ctx)
 	return ok;
 }
 
-#ifdef NOT_USED
 /*
  *	Verify that common name resolves to peer.
  */
-static int
+static bool
 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];
-
-		printfPQExpBuffer(&conn->errorMessage,
-						  libpq_gettext("error querying socket: %s\n"),
-						  SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
-		return -1;
-	}
+	/*
+	 * If told not to verify the peer name, don't do it. Return
+	 * 0 indicating that the verification was successful.
+	 */
+	if(strcmp(conn->sslverify, "cn") != 0)
+		return true;
 
-	if (server_addr.sa_family != AF_INET)
+	if (conn->pghostaddr)
 	{
 		printfPQExpBuffer(&conn->errorMessage,
-						  libpq_gettext("unsupported protocol\n"));
-		return -1;
+						  libpq_gettext("verified SSL connections are only supported when connecting to a hostname"));
+		return false;
 	}
-
-	/* Get the IP addresses of the certificate's common name (CN) */
+	else
 	{
-		struct hostent hpstr;
-		char		buf[BUFSIZ];
-		int			herrno = 0;
-
 		/*
-		 * Currently, pqGethostbyname() is used only on platforms that don't
-		 * have getaddrinfo().	If you enable this function, you should
-		 * convert the pqGethostbyname() function call to use getaddrinfo().
+		 * Connect by hostname.
+		 *
+		 * XXX: Should support alternate names here
+		 * XXX: Should support wildcard certificates here
 		 */
-		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"),
-						  conn->peer_cn, hstrerror(h_errno));
-		return -1;
+		if (pg_strcasecmp(conn->peer_cn, conn->pghost) != 0)
+		{
+			printfPQExpBuffer(&conn->errorMessage,
+							  libpq_gettext("server common name '%s' does not match hostname '%s'"),
+							  conn->peer_cn, conn->pghost);
+			return false;
+		}
+		else
+			return true;
 	}
-
-	/* 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 */
 
 /*
  *	Callback used by SSL to load client cert and key.
@@ -846,6 +811,12 @@ initialize_SSL(PGconn *conn)
 	if (init_ssl_system(conn))
 		return -1;
 
+	/*
+	 * If sslverify is set to anything other than "none", perform certificate
+	 * verification. If set to "cn" we will also do further verifications after
+	 * the connection has been completed.
+	 */
+
 	/* Set up to verify server cert, if root.crt is present */
 	if (pqGetHomeDirectory(homedir, sizeof(homedir)))
 	{
@@ -889,6 +860,24 @@ initialize_SSL(PGconn *conn)
 
 			SSL_CTX_set_verify(SSL_context, SSL_VERIFY_PEER, verify_cb);
 		}
+		else
+		{
+			if (strcmp(conn->sslverify, "none") != 0)
+			{
+				printfPQExpBuffer(&conn->errorMessage,
+								  libpq_gettext("root certificate file (%s) not found"), fnbuf);
+				return -1;
+			}
+		}
+	}
+	else
+	{
+		if (strcmp(conn->sslverify, "none") != 0)
+		{
+			printfPQExpBuffer(&conn->errorMessage,
+							  libpq_gettext("cannot find home directory to locate root certificate file"));
+			return -1;
+		}
 	}
 
 	/* set up mechanism to provide client certificate, if available */
@@ -1004,13 +993,11 @@ open_client_SSL(PGconn *conn)
 							  NID_commonName, conn->peer_cn, SM_USER);
 	conn->peer_cn[SM_USER] = '\0';
 
-#ifdef NOT_USED
-	if (verify_peer_name_matches_certificate(conn) == -1)
+	if (!verify_peer_name_matches_certificate(conn))
 	{
 		close_SSL(conn);
 		return PGRES_POLLING_FAILED;
 	}
-#endif
 
 	/* SSL handshake is complete */
 	return PGRES_POLLING_OK;
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index d5ec8ce13fc..adeaa35d0bf 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -12,7 +12,7 @@
  * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/interfaces/libpq/libpq-int.h,v 1.136 2008/10/28 12:10:44 mha Exp $
+ * $PostgreSQL: pgsql/src/interfaces/libpq/libpq-int.h,v 1.137 2008/11/13 09:45:25 mha Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -291,6 +291,7 @@ struct pg_conn
 	char	   *pguser;			/* Postgres username and password, if any */
 	char	   *pgpass;
 	char	   *sslmode;		/* SSL mode (require,prefer,allow,disable) */
+	char	   *sslverify;		/* Verify server SSL certificate (none,chain,cn) */
 #if defined(KRB5) || defined(ENABLE_GSS) || defined(ENABLE_SSPI)
 	char	   *krbsrvname;		/* Kerberos service name */
 #endif