since-v42.diff.txt

text/plain

Filename: since-v42.diff.txt
Type: text/plain
Part: 0
Message: Re: Support for NSS as a libpq TLS backend
commit ec0fb068da39236602e8e1de8bbf8487a1093590
Author: Jacob Champion <pchampion@vmware.com>
Date:   Mon Sep 27 12:26:29 2021 -0700

    squash! nss: Support libnss as TLS library in libpq
    
    Don't send SNI if the user has disabled it. We can remove the IP address
    check since NSS does that natively.

diff --git a/src/interfaces/libpq/fe-secure-nss.c b/src/interfaces/libpq/fe-secure-nss.c
index 0a9751c22b..51f09fb7fa 100644
--- a/src/interfaces/libpq/fe-secure-nss.c
+++ b/src/interfaces/libpq/fe-secure-nss.c
@@ -183,7 +183,6 @@ pgtls_open_client(PGconn *conn)
 	PRFileDesc *model;
 	NSSInitParameters params;
 	SSLVersionRange desired_range;
-	const char *host;
 
 #ifdef ENABLE_THREAD_SAFETY
 #ifdef WIN32
@@ -450,15 +449,16 @@ pgtls_open_client(PGconn *conn)
 
 	/*
 	 * Specify which hostname we are expecting to talk to for the ClientHello
-	 * SNI extension. TODO: if the hostname is a literal IP address we skip
-	 * setting it, and check for SNI status in the cert callback. Is this the
-	 * correct course of action?
+	 * SNI extension, unless the user has disabled it. NSS will suppress IP
+	 * addresses in SNIs, so we don't need to filter those explicitly like we do
+	 * for OpenSSL.
 	 */
-	host = conn->connhost[conn->whichhost].host;
-	if (host && host[0] &&
-		!(strspn(host, "0123456789.") == strlen(host) ||
-		  strchr(host, ':')))
-		SSL_SetURL(conn->pr_fd, host);
+	if (conn->sslsni && conn->sslsni[0] == '1')
+	{
+		const char *host = conn->connhost[conn->whichhost].host;
+		if (host && host[0])
+			SSL_SetURL(conn->pr_fd, host);
+	}
 
 	status = SSL_ForceHandshake(conn->pr_fd);
 
@@ -678,12 +678,16 @@ pgtls_verify_peer_name_matches_certificate_guts(PGconn *conn,
 	CERTGeneralName *san_list;
 	CERTGeneralName *cn;
 
-	server_hostname = SSL_RevealURL(conn->pr_fd);
+	/*
+	 * In the "standard" NSS use case, we'd call SSL_RevealURL() to get the
+	 * hostname. In our case, SSL_SetURL() won't have been called if the user
+	 * has disabled SNI, so we always pull the hostname from connhost instead.
+	 */
+	server_hostname = conn->connhost[conn->whichhost].host;
 	if (!server_hostname || server_hostname[0] == '\0')
 	{
-		/* If SNI is enabled we must have a hostname set */
-		if (conn->sslsni && conn->sslsni[0])
-			status = SECFailure;
+		/* verify-full shouldn't progress this far without a hostname. */
+		status = SECFailure;
 		goto done;
 	}
 
@@ -762,7 +766,6 @@ done:
 		PORT_FreeArena(arena, PR_TRUE);
 	if (server_cert)
 		CERT_DestroyCertificate(server_cert);
-	PR_Free(server_hostname);
 
 	if (status == SECSuccess)
 		return 1;