Allow to use system CA pool for certificate verification

This adds a new option to libpq's sslrootcert, "system", which will load
the system trusted CA roots for certificate verification. This is a more
convenient way to achieve this than pointing to the system CA roots
manually since the location can differ by installation and be locally
adjusted by env vars in OpenSSL.

When sslrootcert is set to system, sslmode is forced to be verify-full
as weaker modes aren't providing much security for public CAs.

Changing the location of the system roots by setting environment vars is
not supported by LibreSSL so the tests will use a heuristic to determine
if the system being tested is LibreSSL or OpenSSL.

The workaround in .cirrus.yml is required to handle a strange interaction
between homebrew and the openssl@3 formula; hopefully this can be removed
in the near future.

The original patch was written by Thomas Habets, which was later revived
by Jacob Champion.

Author: Jacob Champion <jchampion@timescale.com>
Author: Thomas Habets <thomas@habets.se>
Reviewed-by: Jelte Fennema <postgres@jeltef.nl>
Reviewed-by: Andrew Dunstan <andrew@dunslane.net>
Reviewed-by: Magnus Hagander <magnus@hagander.net>
Discussion: https://www.postgresql.org/message-id/flat/CA%2BkHd%2BcJwCUxVb-Gj_0ptr3_KZPwi3%2B67vK6HnLFBK9MzuYrLA%40mail.gmail.com

6 files changed, 205 insertions, 7 deletions

diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index bb7347cb0c0..40fef0e2c81 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -1465,6 +1465,23 @@ connectOptions2(PGconn *conn)
 			goto oom_error;
 	}
 
+#ifndef USE_SSL
+
+	/*
+	 * sslrootcert=system is not supported. Since setting this changes the
+	 * default sslmode, check this _before_ we validate sslmode, to avoid
+	 * confusing the user with errors for an option they may not have set.
+	 */
+	if (conn->sslrootcert
+		&& strcmp(conn->sslrootcert, "system") == 0)
+	{
+		conn->status = CONNECTION_BAD;
+		libpq_append_conn_error(conn, "sslrootcert value \"%s\" invalid when SSL support is not compiled in",
+								conn->sslrootcert);
+		return false;
+	}
+#endif
+
 	/*
 	 * validate sslmode option
 	 */
@@ -1511,6 +1528,22 @@ connectOptions2(PGconn *conn)
 			goto oom_error;
 	}
 
+#ifdef USE_SSL
+
+	/*
+	 * If sslrootcert=system, make sure our chosen sslmode is compatible.
+	 */
+	if (conn->sslrootcert
+		&& strcmp(conn->sslrootcert, "system") == 0
+		&& strcmp(conn->sslmode, "verify-full") != 0)
+	{
+		conn->status = CONNECTION_BAD;
+		libpq_append_conn_error(conn, "weak sslmode \"%s\" may not be used with sslrootcert=system (use verify-full)",
+								conn->sslmode);
+		return false;
+	}
+#endif
+
 	/*
 	 * Validate TLS protocol versions for ssl_min_protocol_version and
 	 * ssl_max_protocol_version.
@@ -6236,6 +6269,8 @@ static bool
 conninfo_add_defaults(PQconninfoOption *options, PQExpBuffer errorMessage)
 {
 	PQconninfoOption *option;
+	PQconninfoOption *sslmode_default = NULL,
+			   *sslrootcert = NULL;
 	char	   *tmp;
 
 	/*
@@ -6252,6 +6287,9 @@ conninfo_add_defaults(PQconninfoOption *options, PQExpBuffer errorMessage)
 	 */
 	for (option = options; option->keyword != NULL; option++)
 	{
+		if (strcmp(option->keyword, "sslrootcert") == 0)
+			sslrootcert = option;	/* save for later */
+
 		if (option->val != NULL)
 			continue;			/* Value was in conninfo or service */
 
@@ -6294,6 +6332,13 @@ conninfo_add_defaults(PQconninfoOption *options, PQExpBuffer errorMessage)
 				}
 				continue;
 			}
+
+			/*
+			 * sslmode is not specified. Let it be filled in with the compiled
+			 * default for now, but if sslrootcert=system, we'll override the
+			 * default later before returning.
+			 */
+			sslmode_default = option;
 		}
 
 		/*
@@ -6326,6 +6371,27 @@ conninfo_add_defaults(PQconninfoOption *options, PQExpBuffer errorMessage)
 		}
 	}
 
+	/*
+	 * Special handling for sslrootcert=system with no sslmode explicitly
+	 * defined. In this case we want to strengthen the default sslmode to
+	 * verify-full.
+	 */
+	if (sslmode_default && sslrootcert)
+	{
+		if (sslrootcert->val && strcmp(sslrootcert->val, "system") == 0)
+		{
+			free(sslmode_default->val);
+
+			sslmode_default->val = strdup("verify-full");
+			if (!sslmode_default->val)
+			{
+				if (errorMessage)
+					libpq_append_error(errorMessage, "out of memory");
+				return false;
+			}
+		}
+	}
+
 	return true;
 }
 
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index 4d1e4009ef1..00b203cbfa3 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -1060,8 +1060,29 @@ initialize_SSL(PGconn *conn)
 	else
 		fnbuf[0] = '\0';
 
-	if (fnbuf[0] != '\0' &&
-		stat(fnbuf, &buf) == 0)
+	if (strcmp(fnbuf, "system") == 0)
+	{
+		/*
+		 * The "system" sentinel value indicates that we should load whatever
+		 * root certificates are installed for use by OpenSSL; these locations
+		 * differ by platform. Note that the default system locations may be
+		 * further overridden by the SSL_CERT_DIR and SSL_CERT_FILE
+		 * environment variables.
+		 */
+		if (SSL_CTX_set_default_verify_paths(SSL_context) != 1)
+		{
+			char	   *err = SSLerrmessage(ERR_get_error());
+
+			libpq_append_conn_error(conn, "could not load system root certificate paths: %s",
+									err);
+			SSLerrfree(err);
+			SSL_CTX_free(SSL_context);
+			return -1;
+		}
+		have_rootcert = true;
+	}
+	else if (fnbuf[0] != '\0' &&
+			 stat(fnbuf, &buf) == 0)
 	{
 		X509_STORE *cvstore;
 
@@ -1122,10 +1143,10 @@ initialize_SSL(PGconn *conn)
 			 */
 			if (fnbuf[0] == '\0')
 				libpq_append_conn_error(conn, "could not get home directory to locate root certificate file\n"
-								   "Either provide the file or change sslmode to disable server certificate verification.");
+										"Either provide the file, use the system's trusted roots with sslrootcert=system, or change sslmode to disable server certificate verification.");
 			else
 				libpq_append_conn_error(conn, "root certificate file \"%s\" does not exist\n"
-								   "Either provide the file or change sslmode to disable server certificate verification.", fnbuf);
+										"Either provide the file, use the system's trusted roots with sslrootcert=system, or change sslmode to disable server certificate verification.", fnbuf);
 			SSL_CTX_free(SSL_context);
 			return -1;
 		}
diff --git a/src/interfaces/libpq/t/001_uri.pl b/src/interfaces/libpq/t/001_uri.pl
index 2ab537f97f1..cd659bc1b0f 100644
--- a/src/interfaces/libpq/t/001_uri.pl
+++ b/src/interfaces/libpq/t/001_uri.pl
@@ -8,7 +8,9 @@ use IPC::Run;
 
 
 # List of URIs tests. For each test the first element is the input string, the
-# second the expected stdout and the third the expected stderr.
+# second the expected stdout and the third the expected stderr. Optionally,
+# additional arguments may specify key/value pairs which will override
+# environment variables for the duration of the test.
 my @tests = (
 	[
 		q{postgresql://uri-user:secret@host:12345/db},
@@ -209,20 +211,44 @@ my @tests = (
 		q{postgres://%2Fvar%2Flib%2Fpostgresql/dbname},
 		q{dbname='dbname' host='/var/lib/postgresql' (local)},
 		q{},
+	],
+	# Usually the default sslmode is 'prefer' (for libraries with SSL) or
+	# 'disable' (for those without). This default changes to 'verify-full' if
+	# the system CA store is in use.
+	[
+		q{postgresql://host?sslmode=disable},
+		q{host='host' sslmode='disable' (inet)},
+		q{},
+		PGSSLROOTCERT => "system",
+	],
+	[
+		q{postgresql://host?sslmode=prefer},
+		q{host='host' sslmode='prefer' (inet)},
+		q{},
+		PGSSLROOTCERT => "system",
+	],
+	[
+		q{postgresql://host?sslmode=verify-full},
+		q{host='host' (inet)},
+		q{},
+		PGSSLROOTCERT => "system",
 	]);
 
 # test to run for each of the above test definitions
 sub test_uri
 {
 	local $Test::Builder::Level = $Test::Builder::Level + 1;
+	local %ENV = %ENV;
 
 	my $uri;
 	my %expect;
+	my %envvars;
 	my %result;
 
-	($uri, $expect{stdout}, $expect{stderr}) = @$_;
+	($uri, $expect{stdout}, $expect{stderr}, %envvars) = @$_;
 
 	$expect{'exit'} = $expect{stderr} eq '';
+	%ENV = (%ENV, %envvars);
 
 	my $cmd = [ 'libpq_uri_regress', $uri ];
 	$result{exit} = IPC::Run::run $cmd, '>', \$result{stdout}, '2>',
diff --git a/src/test/ssl/ssl/server-cn-only+server_ca.crt b/src/test/ssl/ssl/server-cn-only+server_ca.crt
new file mode 100644
index 00000000000..9870e8c17aa
--- /dev/null
+++ b/src/test/ssl/ssl/server-cn-only+server_ca.crt
@@ -0,0 +1,38 @@
+-----BEGIN CERTIFICATE-----
+MIIDAzCCAesCCCAhAwMUEgcBMA0GCSqGSIb3DQEBCwUAMEIxQDA+BgNVBAMMN1Rl
+c3QgQ0EgZm9yIFBvc3RncmVTUUwgU1NMIHJlZ3Jlc3Npb24gdGVzdCBzZXJ2ZXIg
+Y2VydHMwHhcNMjEwMzAzMjIxMjA3WhcNNDgwNzE5MjIxMjA3WjBGMR4wHAYDVQQL
+DBVQb3N0Z3JlU1FMIHRlc3Qgc3VpdGUxJDAiBgNVBAMMG2NvbW1vbi1uYW1lLnBn
+LXNzbHRlc3QudGVzdDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBANWz
+VPMk7i5f+W0eEadRE+TTAtsIK08CkLMUnjs7zJkxnnm6RGBXPx6vK3AkAIi+wG4Y
+mXjYP3GuMiXaLjnWh2kzBSfIRQyNbTThnhSu3nDjAVkPexsSrPyiKimFuNgDfkGe
+5dQKa9Ag2SuVU4vd9SYxOMAiIFIC4ts4MLWWJf5D/PehdSuc0e5Me+91Nnbz90nl
+ds4lHvuDR+aKnZlTHmch3wfhXv7lNQImIBzfwl36Kd/bWB0fAEVFse3iZWmigaI/
+9FKh//WIq43TNLxn68OCQoyMe/HGjZDR/Xwo3rE6jg6/iAwSWib9yabfYPKbqq2G
+oFy6aYmmEquaDgLuX7kCAwEAATANBgkqhkiG9w0BAQsFAAOCAQEA2AZrD9cTQXTW
+4j2tT8N/TTc6WK2ncN4h22NTte6vK7MVwsZJCtw5ndYkmxcWkXAqiclzWyMdayds
+WOa12CEH7jKAhivF4Hcw3oO3JHM5BA6KzLWBVz9uZksOM6mPqn29DTKvA/Y1V8tj
+mxK/KUA68h/u6inu3mo4ywBpb/tqHxxg2cjyR0faCmM0pwRM0HBr/16fUMfO83nj
+QG8g9J/bybu5sYso/aSoC5nUNp4XjmDMdVLdqg/nTe/ejS8IfFr0WQxBlqooqFgx
+MSE+kX2e2fHsuOWSU/9eClt6FpQrwoC2C8F+/4g1Uz7Liqc4yMHPwjgeP9ewrrLO
+iIhlNNPqpQ==
+-----END CERTIFICATE-----
+-----BEGIN CERTIFICATE-----
+MIIDFDCCAfygAwIBAgIIICEDAxQSBwAwDQYJKoZIhvcNAQELBQAwQDE+MDwGA1UE
+Aww1VGVzdCByb290IENBIGZvciBQb3N0Z3JlU1FMIFNTTCByZWdyZXNzaW9uIHRl
+c3Qgc3VpdGUwHhcNMjEwMzAzMjIxMjA3WhcNNDgwNzE5MjIxMjA3WjBCMUAwPgYD
+VQQDDDdUZXN0IENBIGZvciBQb3N0Z3JlU1FMIFNTTCByZWdyZXNzaW9uIHRlc3Qg
+c2VydmVyIGNlcnRzMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA4kp2
+GW5nPb6QrSrtbClfZeutyQnHrm4TMPBoNepFdIVxBX/04BguM5ImDRze/huOWA+z
+atJAQqt3R7dsDwnOnPKUKCOuHX6a1aj5L86HtVgaWTXrZFE5NtL9PIzXkWu13UW0
+UesHtbPVRv6a6fB7Npph6hHy7iPZb009A8/lTJnxSPC39u/K/sPqjrVZaAJF+wDs
+qCxCZTUtAUFvWFnR/TeXLWlFzBupS1djgI7PltbJqSn6SKTAgNZTxpRJbu9Icp6J
+/50ELwT++0n0KWVXNHrDNfI5Gaa+SxClAsPsei2jLKpgR6QFC3wn38Z9q9LjAVuC
++FWhoN1uhYeoricEXwIDAQABoxAwDjAMBgNVHRMEBTADAQH/MA0GCSqGSIb3DQEB
+CwUAA4IBAQCdCA/EoXrustoV4jJGbkdXDuOUkBurwggSNBAqUBSDvCohRoD77Ecb
+QVuzPNxWKG+E4PwfUq2ha+2yPONEJ28ZgsbHq5qlJDMJ43wlcjn6wmmAJNeSpO8F
+0V9d2X/4wNZty9/zbwTnw26KChgDHumQ0WIbCoBtdqy8KDswYOvpgws6dqc021I7
+UrFo6vZek7VoApbJgkDL6qYADa6ApfW43ThH4sViFITeYt/kSHgmy2Udhs34jMM8
+xsFP/uYpRi1b1glenwSIKiHjD4/C9vnWQt5K3gRBvYukEj2Bw9VkNRpBVCi0cOoA
+OuwX3bwzNYNbZQv4K66oRpvuoEjCNeHg
+-----END CERTIFICATE-----
diff --git a/src/test/ssl/sslfiles.mk b/src/test/ssl/sslfiles.mk
index e63342469d3..f7ababe42c9 100644
--- a/src/test/ssl/sslfiles.mk
+++ b/src/test/ssl/sslfiles.mk
@@ -61,7 +61,8 @@ COMBINATIONS := \
 	ssl/root+server.crl \
 	ssl/root+client_ca.crt \
 	ssl/root+client.crl \
-	ssl/client+client_ca.crt
+	ssl/client+client_ca.crt \
+	ssl/server-cn-only+server_ca.crt
 
 CERTIFICATES := root_ca server_ca client_ca $(SERVERS) $(CLIENTS)
 STANDARD_CERTS := $(CERTIFICATES:%=ssl/%.crt)
@@ -150,6 +151,9 @@ ssl/root+client_ca.crt: ssl/root_ca.crt ssl/client_ca.crt
 # and for the client, to present to the server
 ssl/client+client_ca.crt: ssl/client.crt ssl/client_ca.crt
 
+# for the server, to present to a client that only knows the root
+ssl/server-cn-only+server_ca.crt: ssl/server-cn-only.crt ssl/server_ca.crt
+
 # If a CRL is used, OpenSSL requires a CRL file for *all* the CAs in the
 # chain, even if some of them are empty.
 ssl/root+server.crl: ssl/root.crl ssl/server.crl
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index dc43b8f81aa..6fdc040cfc2 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -33,6 +33,12 @@ sub switch_server_cert
 {
 	$ssl_server->switch_server_cert(@_);
 }
+
+# Determine whether this build uses OpenSSL or LibreSSL. As a heuristic, the
+# HAVE_SSL_CTX_SET_CERT_CB macro isn't defined for LibreSSL. (Nor for OpenSSL
+# 1.0.1, but that's old enough that accommodating it isn't worth the cost.)
+my $libressl = not check_pg_config("#define HAVE_SSL_CTX_SET_CERT_CB 1");
+
 #### Some configuration
 
 # This is the hostname used to connect to the server. This cannot be a
@@ -461,6 +467,43 @@ $node->connect_fails(
 	expected_stderr =>
 	  qr/could not get server's host name from server certificate/);
 
+# Test system trusted roots.
+switch_server_cert($node,
+	certfile => 'server-cn-only+server_ca',
+	keyfile => 'server-cn-only',
+	cafile => 'root_ca');
+$common_connstr =
+  "$default_ssl_connstr user=ssltestuser dbname=trustdb sslrootcert=system hostaddr=$SERVERHOSTADDR";
+
+# By default our custom-CA-signed certificate should not be trusted.
+$node->connect_fails(
+	"$common_connstr sslmode=verify-full host=common-name.pg-ssltest.test",
+	"sslrootcert=system does not connect with private CA",
+	expected_stderr => qr/SSL error: certificate verify failed/);
+
+# Modes other than verify-full cannot be mixed with sslrootcert=system.
+$node->connect_fails(
+	"$common_connstr sslmode=verify-ca host=common-name.pg-ssltest.test",
+	"sslrootcert=system only accepts sslmode=verify-full",
+	expected_stderr => qr/weak sslmode "verify-ca" may not be used with sslrootcert=system/);
+
+SKIP:
+{
+	skip "SSL_CERT_FILE is not supported with LibreSSL" if $libressl;
+
+	# We can modify the definition of "system" to get it trusted again.
+	local $ENV{SSL_CERT_FILE} = $node->data_dir . "/root_ca.crt";
+	$node->connect_ok(
+		"$common_connstr sslmode=verify-full host=common-name.pg-ssltest.test",
+		"sslrootcert=system connects with overridden SSL_CERT_FILE");
+
+	# verify-full mode should be the default for system CAs.
+	$node->connect_fails(
+		"$common_connstr host=common-name.pg-ssltest.test.bad",
+		"sslrootcert=system defaults to sslmode=verify-full",
+		expected_stderr => qr/server certificate for "common-name.pg-ssltest.test" does not match host name "common-name.pg-ssltest.test.bad"/);
+}
+
 # Test that the CRL works
 switch_server_cert($node, certfile => 'server-revoked');