From 957a011364f4ed73d043217eae8d18ac4d543573 Mon Sep 17 00:00:00 2001 From: Jacob Champion Date: Mon, 24 Oct 2022 15:30:25 -0700 Subject: [PATCH v10 1/2] libpq: add sslrootcert=system to use default CAs Based on a patch by Thomas Habets. Note the workaround in .cirrus.yml for a strange interaction between brew and the openssl@3 formula; hopefully this can be removed in the near future. Discussion: https://www.postgresql.org/message-id/flat/CA%2BkHd%2BcJwCUxVb-Gj_0ptr3_KZPwi3%2B67vK6HnLFBK9MzuYrLA%40mail.gmail.com --- .cirrus.yml | 14 ++++++- doc/src/sgml/libpq.sgml | 25 ++++++++++++ doc/src/sgml/runtime.sgml | 6 ++- src/interfaces/libpq/fe-secure-openssl.c | 29 ++++++++++++-- src/test/ssl/ssl/server-cn-only+server_ca.crt | 38 +++++++++++++++++++ src/test/ssl/sslfiles.mk | 6 ++- src/test/ssl/t/001_ssltests.pl | 31 +++++++++++++++ 7 files changed, 142 insertions(+), 7 deletions(-) create mode 100644 src/test/ssl/ssl/server-cn-only+server_ca.crt diff --git a/.cirrus.yml b/.cirrus.yml index 04786174ed..d3f88821a8 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -475,16 +475,28 @@ task: llvm \ lz4 \ make \ meson \ openldap \ - openssl \ + openssl@3 \ python \ tcl-tk \ zstd brew cleanup -s # to reduce cache size + + # brew cleanup removes the empty certs directory in OPENSSLDIR, causing + # OpenSSL to report unexpected errors ("unregistered scheme") during + # verification failures. Put it back for now as a workaround. + # + # https://github.com/orgs/Homebrew/discussions/4030 + # + # Note that $(brew --prefix openssl) will give us the opt/ prefix but not + # the etc/ prefix, so we hardcode the full path here. openssl@3 is pinned + # above to try to minimize the chances of this changing beneath us, but it's + # brittle... + mkdir -p "/opt/homebrew/etc/openssl@3/certs" upload_caches: homebrew ccache_cache: folder: $CCACHE_DIR configure_script: | diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index 9f72dd29d8..29ef0ae75d 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -1874,10 +1874,35 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname certificate authority (CA) certificate(s). If the file exists, the server's certificate will be verified to be signed by one of these authorities. The default is ~/.postgresql/root.crt. + + The special value system may be specified instead, in + which case the system's trusted CA roots will be loaded. The exact + locations of these root certificates differ by SSL implementation and + platform. For OpenSSL in particular, the + locations may be further modified by the SSL_CERT_DIR + and SSL_CERT_FILE environment variables. + + + + When using sslrootcert=system, it is critical to + also use the strongest certificate verification method, + sslmode=verify-full. In most cases it is trivial for + anyone to obtain a certificate trusted by the system for a hostname + they control, rendering the verify-ca mode useless. + + + + + The magic system value will take precedence over a + local certificate file with the same name. If for some reason you find + yourself in this situation, use an alternative path like + sslrootcert=./system instead. + + sslcrl diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml index 149e9b33d4..b93184537a 100644 --- a/doc/src/sgml/runtime.sgml +++ b/doc/src/sgml/runtime.sgml @@ -2005,11 +2005,15 @@ pg_dumpall -p 5432 | psql -d postgres -p 5433 must be configured to accept only hostssl connections () and have SSL key and certificate files (). The TCP client must connect using sslmode=verify-ca or verify-full and have the appropriate root certificate - file installed (). + file installed (). Alternatively the + system CA pool can be used using sslrootcert=system; in + this case, sslmode=verify-full must be used for safety, + since it is generally trivial to obtain certificates which are signed by a + public CA. To prevent spoofing with GSSAPI, the server must be configured to accept only hostgssenc connections diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c index 4d1e4009ef..ac0c27a926 100644 --- a/src/interfaces/libpq/fe-secure-openssl.c +++ b/src/interfaces/libpq/fe-secure-openssl.c @@ -1058,12 +1058,33 @@ initialize_SSL(PGconn *conn) else if (have_homedir) snprintf(fnbuf, sizeof(fnbuf), "%s/%s", homedir, ROOT_CERT_FILE); 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; if (SSL_CTX_load_verify_locations(SSL_context, fnbuf, NULL) != 1) { @@ -1120,14 +1141,14 @@ initialize_SSL(PGconn *conn) * pqGetHomeDirectory failed. That's a sufficiently unusual case * that it seems worth having a specialized error message for it. */ 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; } have_rootcert = false; } 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 0000000000..9870e8c17a --- /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 e63342469d..f7ababe42c 100644 --- a/src/test/ssl/sslfiles.mk +++ b/src/test/ssl/sslfiles.mk @@ -59,11 +59,12 @@ COMBINATIONS := \ ssl/both-cas-2.crt \ ssl/root+server_ca.crt \ 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) STANDARD_KEYS := $(CERTIFICATES:%=ssl/%.key) CRLS := ssl/root.crl \ @@ -148,10 +149,13 @@ ssl/root+server_ca.crt: ssl/root_ca.crt ssl/server_ca.crt 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 ssl/root+client.crl: ssl/root.crl ssl/client.crl diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl index dc43b8f81a..3781a25e39 100644 --- a/src/test/ssl/t/001_ssltests.pl +++ b/src/test/ssl/t/001_ssltests.pl @@ -31,10 +31,16 @@ sub sslkey 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 accomodating 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 # hostname, because the server certificate is always for the domain # postgresql-ssl-regression.test. @@ -459,10 +465,35 @@ $node->connect_fails( . "sslmode=verify-full host=common-name.pg-ssltest.test", "server certificate without CN or SANs sslmode=verify-full", 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/); + +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"); +} + # Test that the CRL works switch_server_cert($node, certfile => 'server-revoked'); $common_connstr = "$default_ssl_connstr user=ssltestuser dbname=trustdb hostaddr=$SERVERHOSTADDR host=common-name.pg-ssltest.test"; -- 2.25.1