This is an automated email from the ASF dual-hosted git repository. asf-gitbox-commits pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/qpid-proton.git
commit d793f97e4a2d0f960eda32724a2e867236468337 Author: Andrew Stitcher <[email protected]> AuthorDate: Mon Sep 28 21:20:01 2026 -0400 PROTON-2976: Make pn_ssl_domain_set_trusted_ca_db replace the default CAs pn_ssl_domain_set_trusted_cs_db is using SSL_CTX_load_verify_locations(). This loads the new CAs into the store that already holds the system certificates, so adding to those CAs rather than overriding them. Overriding is what the documentation has always described, and what SChannel already does. Install a fresh X509_STORE instead, in both the ssl and tls backends. Assisted-By: Claude Opus 5 <[email protected]> --- c/include/proton/ssl.h | 5 +++ c/include/proton/tls.h | 5 +++ c/src/ssl/openssl.c | 21 ++++++++++-- c/src/tls/openssl.c | 21 ++++++++++-- python/tests/proton_tests/ssl.py | 74 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 122 insertions(+), 4 deletions(-) diff --git a/c/include/proton/ssl.h b/c/include/proton/ssl.h index 550410886..20b44f1c6 100644 --- a/c/include/proton/ssl.h +++ b/c/include/proton/ssl.h @@ -174,6 +174,11 @@ PN_EXTERN int pn_ssl_domain_set_credentials(pn_ssl_domain_t *domain, * will depend on how the OS is set up. When using the Windows SChannel implementation the default * will be the users default trusted certificate store. * + * @note The database given here replaces that default rather than adding to it, so after + * this call the certificates named here are the only trust anchors the domain will accept. + * Repeating the call replaces the database again; to trust more than one CA, name a + * directory or a file holding all of them. + * * @param[in] domain the ssl domain that will use the database. * @param[in] certificate_db database of trusted CAs, used to authenticate the peer. * @return 0 on success diff --git a/c/include/proton/tls.h b/c/include/proton/tls.h index 6ae42c78c..f5e70ef8d 100644 --- a/c/include/proton/tls.h +++ b/c/include/proton/tls.h @@ -170,6 +170,11 @@ PN_TLS_EXTERN int pn_tls_config_set_credentials(pn_tls_config_t *domain, * will depend on how the OS is set up. When using the Windows SChannel implementation the default * will be the users default trusted certificate store. * + * @note The database given here replaces that default rather than adding to it, so after + * this call the certificates named here are the only trust anchors the domain will accept. + * Repeating the call replaces the database again; to trust more than one CA, name a + * directory or a file holding all of them. + * * @param[in] domain the tls domain that will use the database. * @param[in] certificate_db database of trusted CAs, used to authenticate the peer. * @return 0 on success diff --git a/c/src/ssl/openssl.c b/c/src/ssl/openssl.c index a9d19081e..baabec0f1 100644 --- a/c/src/ssl/openssl.c +++ b/c/src/ssl/openssl.c @@ -802,11 +802,28 @@ int pn_ssl_domain_set_trusted_ca_db(pn_ssl_domain_t *domain, file = certificate_db; } - if (SSL_CTX_load_verify_locations( domain->ctx, file, dir ) != 1) { - ssl_log_error("SSL_CTX_load_verify_locations( %s ) failed", certificate_db); + /* Replace the context's store rather than loading into it: the store already holds the + system default certificates, installed when the domain was created, and the documented + contract is that naming a database overrides that default rather than adding to it. + Loading into the existing store would leave every system CA a trust anchor, so a peer + holding any publicly issued certificate would verify against a domain configured to + trust one private CA. SSL_CTX_set_cert_store() takes ownership and frees the old + store. */ + X509_STORE *store = X509_STORE_new(); + if (!store) { + ssl_log_error("Unable to allocate certificate store"); + return -1; + } + // Support intermediate/subordinate CAs as trust anchors, as the replaced store did. + X509_STORE_set_flags(store, X509_V_FLAG_PARTIAL_CHAIN); + + if (X509_STORE_load_locations( store, file, dir ) != 1) { + ssl_log_error("X509_STORE_load_locations( %s ) failed", certificate_db); + X509_STORE_free(store); return -1; } + SSL_CTX_set_cert_store( domain->ctx, store ); return 0; } diff --git a/c/src/tls/openssl.c b/c/src/tls/openssl.c index 021495460..8a3d07c21 100644 --- a/c/src/tls/openssl.c +++ b/c/src/tls/openssl.c @@ -912,11 +912,28 @@ int pn_tls_config_set_trusted_certs(pn_tls_config_t *domain, file = certificate_db; } - if (SSL_CTX_load_verify_locations( domain->ctx, file, dir ) != 1) { - ssl_log_error("SSL_CTX_load_verify_locations( %s ) failed", certificate_db); + /* Replace the context's store rather than loading into it: the store already holds the + system default certificates, installed when the domain was created, and the documented + contract is that naming a database overrides that default rather than adding to it. + Loading into the existing store would leave every system CA a trust anchor, so a peer + holding any publicly issued certificate would verify against a domain configured to + trust one private CA. SSL_CTX_set_cert_store() takes ownership and frees the old + store. */ + X509_STORE *store = X509_STORE_new(); + if (!store) { + ssl_log_error("Unable to allocate certificate store"); + return -1; + } + // Support intermediate/subordinate CAs as trust anchors, as the replaced store did. + X509_STORE_set_flags(store, X509_V_FLAG_PARTIAL_CHAIN); + + if (X509_STORE_load_locations( store, file, dir ) != 1) { + ssl_log_error("X509_STORE_load_locations( %s ) failed", certificate_db); + X509_STORE_free(store); return -1; } + SSL_CTX_set_cert_store( domain->ctx, store ); return 0; } diff --git a/python/tests/proton_tests/ssl.py b/python/tests/proton_tests/ssl.py index 903104ba8..56b9d35e7 100644 --- a/python/tests/proton_tests/ssl.py +++ b/python/tests/proton_tests/ssl.py @@ -229,6 +229,80 @@ class SslTest(common.Test): server.connection.close() self._pump(client, server) + def _system_ca_domains(self, system_ca): + """ Build a fresh client/server domain pair with the system default trust store + pointed at system_ca. + + OpenSSL reads SSL_CERT_FILE when the domain installs the system defaults, so the + variable has to be in place before the domains are constructed - hence the fresh + pair rather than the ones setUp() made. SChannel takes its default from the + Windows certificate store and ignores the variable. + """ + old = os.environ.get("SSL_CERT_FILE") + os.environ["SSL_CERT_FILE"] = system_ca + try: + return (SSLDomain(SSLDomain.MODE_SERVER), SSLDomain(SSLDomain.MODE_CLIENT)) + finally: + if old is None: + del os.environ["SSL_CERT_FILE"] + else: + os.environ["SSL_CERT_FILE"] = old + + def test_trusted_ca_db_replaces_system_default(self): + """ A configured CA database overrides the system default rather than adding to it. + + Without this the system store stays in the trust set, so a certificate issued by + any CA the machine happens to trust verifies against a domain configured to trust + one private CA. bad-server is self-signed, so treating it as a system root makes + it exactly such a certificate. + """ + if os.name == "nt": + raise Skipped("Windows SChannel does not use SSL_CERT_FILE.") + server_domain, client_domain = self._system_ca_domains( + self._testpath("bad-server-certificate.pem")) + + server_domain.set_credentials(self._testpath("bad-server-certificate.pem"), + self._testpath("bad-server-private-key.pem"), + "server-password") + client_domain.set_trusted_ca_db(self._testpath("ca-certificate.pem")) + client_domain.set_peer_authentication(SSLDomain.VERIFY_PEER) + + server = SslTest.SslTestConnection(server_domain, mode=Transport.SERVER) + client = SslTest.SslTestConnection(client_domain) + + client.connection.open() + server.connection.open() + self._pump(client, server) + + assert client.transport.closed + assert server.transport.closed + assert client.connection.state & Endpoint.REMOTE_UNINIT + assert server.connection.state & Endpoint.REMOTE_UNINIT + + def test_system_default_ca_db_used_when_unconfigured(self): + """ With no CA database configured the system default is still the trust set. + """ + if os.name == "nt": + raise Skipped("Windows SChannel does not use SSL_CERT_FILE.") + server_domain, client_domain = self._system_ca_domains( + self._testpath("bad-server-certificate.pem")) + + server_domain.set_credentials(self._testpath("bad-server-certificate.pem"), + self._testpath("bad-server-private-key.pem"), + "server-password") + client_domain.set_peer_authentication(SSLDomain.VERIFY_PEER) + + server = SslTest.SslTestConnection(server_domain, mode=Transport.SERVER) + client = SslTest.SslTestConnection(client_domain) + + client.connection.open() + server.connection.open() + self._pump(client, server) + assert client.ssl.protocol_name() is not None + client.connection.close() + server.connection.close() + self._pump(client, server) + def test_intermediate_ca(self): """ Ensure an intermediate/subordinate certificate can be used as a CA for validation. """ --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
