This is an automated email from the ASF dual-hosted git repository. cliffjansen pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/qpid-proton.git
The following commit(s) were added to refs/heads/main by this push: new 605bc58 PROTON-2397: make client TLS connection verification defaults consistent: verify peer certificate and name 605bc58 is described below commit 605bc58009549dff2678961455a7ec86b0acede4 Author: Cliff Jansen <cliffjan...@apache.org> AuthorDate: Thu Jun 17 23:04:21 2021 -0700 PROTON-2397: make client TLS connection verification defaults consistent: verify peer certificate and name --- c/include/proton/ssl.h | 5 +++-- c/src/ssl/openssl.c | 6 ------ c/src/ssl/schannel.cpp | 2 ++ c/tests/proactor_test.cpp | 35 +++++++++++++++++++++++++++++++---- python/proton/_reactor.py | 12 +++++++----- python/proton/_transport.py | 3 ++- python/tests/proton_tests/connect.py | 25 +++++++++++++++++++------ python/tests/proton_tests/sasl.py | 6 ++++++ python/tests/proton_tests/ssl.py | 3 ++- ruby/examples/ssl_send.rb | 3 ++- ruby/lib/core/ssl_domain.rb | 7 ++++--- 11 files changed, 78 insertions(+), 29 deletions(-) diff --git a/c/include/proton/ssl.h b/c/include/proton/ssl.h index 03acbf5..a84a9b5 100644 --- a/c/include/proton/ssl.h +++ b/c/include/proton/ssl.h @@ -195,7 +195,7 @@ PN_EXTERN int pn_ssl_domain_set_trusted_ca_db(pn_ssl_domain_t *domain, * identity as contained in the certificate to be valid (see * ::pn_ssl_set_peer_hostname). * - * ANONYMOUS_PEER is configured by default. + * VERIFY_PEER_NAME is configured by default. */ typedef enum { PN_SSL_VERIFY_NULL = 0, /**< internal use only */ @@ -208,7 +208,8 @@ typedef enum { * Configure the level of verification used on the peer certificate. * * This method controls how the peer's certificate is validated, if at all. By default, - * neither servers nor clients attempt to verify their peers (PN_SSL_ANONYMOUS_PEER). + * servers do not attempt to verify their peers (PN_SSL_ANONYMOUS_PEER) but + * clients attempt to verify both the certificate and peer name (PN_SSL_VERIFY_PEER_NAME). * Once certificates and trusted CAs are configured, peer verification can be enabled. * * @note In order to verify a peer, a trusted CA must be configured. See diff --git a/c/src/ssl/openssl.c b/c/src/ssl/openssl.c index 3a98200..b4b7564 100644 --- a/c/src/ssl/openssl.c +++ b/c/src/ssl/openssl.c @@ -562,12 +562,6 @@ pn_ssl_domain_t *pn_ssl_domain( pn_ssl_mode_t mode ) return NULL; } - // Insecure, but backward compatible client default - if (mode==PN_SSL_MODE_CLIENT && - pn_ssl_domain_set_peer_authentication(domain, PN_SSL_ANONYMOUS_PEER, NULL)) { - free(domain); - return NULL; - } return domain; } diff --git a/c/src/ssl/schannel.cpp b/c/src/ssl/schannel.cpp index 3056890..d167678 100644 --- a/c/src/ssl/schannel.cpp +++ b/c/src/ssl/schannel.cpp @@ -486,6 +486,8 @@ pn_ssl_domain_t *pn_ssl_domain( pn_ssl_mode_t mode ) pn_ssl_domain_free(domain); return NULL; } + if (mode == PN_SSL_MODE_CLIENT) + domain->verify_mode == PN_SSL_VERIFY_PEER_NAME; return domain; } diff --git a/c/tests/proactor_test.cpp b/c/tests/proactor_test.cpp index e13fcf4..73e285e 100644 --- a/c/tests/proactor_test.cpp +++ b/c/tests/proactor_test.cpp @@ -533,9 +533,21 @@ TEST_CASE("proactor_ssl") { pn_listener_t *l = p.listen(":0", &listener); REQUIRE_RUN(p, PN_LISTENER_OPEN); - /* Basic SSL connection */ + /* Not Anonymous by default */ p.connect(l, &client); - /* Open ok at both ends */ + REQUIRE_RUN(p, PN_TRANSPORT_ERROR); + CHECK_THAT(*client.last_condition, + cond_matches("amqp:connection:framing-error", "SSL")); + REQUIRE_RUN(p, PN_TRANSPORT_CLOSED); + REQUIRE_RUN(p, PN_TRANSPORT_ERROR); + REQUIRE_RUN(p, PN_TRANSPORT_CLOSED); + + /* Deliberate use of Anonymous */ + pn_ssl_domain_t *cd = client.ssl_domain; + REQUIRE(0 == pn_ssl_domain_set_peer_authentication( + cd, PN_SSL_ANONYMOUS_PEER, NULL)); + pn_connection_t *c = pn_connection(); + p.connect(l, &client, c); REQUIRE_RUN(p, PN_CONNECTION_REMOTE_OPEN); REQUIRE_RUN(p, PN_CONNECTION_REMOTE_OPEN); CHECK_THAT(*server.last_condition, cond_empty()); @@ -544,11 +556,10 @@ TEST_CASE("proactor_ssl") { REQUIRE_RUN(p, PN_TRANSPORT_CLOSED); /* Verify peer with good hostname */ - pn_ssl_domain_t *cd = client.ssl_domain; REQUIRE(0 == pn_ssl_domain_set_trusted_ca_db(cd, CERTIFICATE("tserver"))); REQUIRE(0 == pn_ssl_domain_set_peer_authentication( cd, PN_SSL_VERIFY_PEER_NAME, NULL)); - pn_connection_t *c = pn_connection(); + c = pn_connection(); pn_connection_set_hostname(c, "test_server"); p.connect(l, &client, c); REQUIRE_RUN(p, PN_CONNECTION_REMOTE_OPEN); @@ -565,6 +576,22 @@ TEST_CASE("proactor_ssl") { REQUIRE_RUN(p, PN_TRANSPORT_ERROR); CHECK_THAT(*client.last_condition, cond_matches("amqp:connection:framing-error", "SSL")); + REQUIRE_RUN(p, PN_TRANSPORT_CLOSED); + REQUIRE_RUN(p, PN_TRANSPORT_ERROR); + REQUIRE_RUN(p, PN_TRANSPORT_CLOSED); + + /* Can ignore bad hostname */ + REQUIRE(0 == pn_ssl_domain_set_peer_authentication( + cd, PN_SSL_VERIFY_PEER, NULL)); + c = pn_connection(); + pn_connection_set_hostname(c, "wrongname"); + p.connect(l, &client, c); + REQUIRE_RUN(p, PN_CONNECTION_REMOTE_OPEN); + REQUIRE_RUN(p, PN_CONNECTION_REMOTE_OPEN); + CHECK_THAT(*server.last_condition, cond_empty()); + CHECK_THAT(*client.last_condition, cond_empty()); + REQUIRE_RUN(p, PN_TRANSPORT_CLOSED); + REQUIRE_RUN(p, PN_TRANSPORT_CLOSED); } TEST_CASE("proactor_addr") { diff --git a/python/proton/_reactor.py b/python/proton/_reactor.py index a56c57e..28d02c9 100644 --- a/python/proton/_reactor.py +++ b/python/proton/_reactor.py @@ -1199,9 +1199,10 @@ class Container(Reactor): * ``ca``: Path to a database of trusted CAs that the server will advertise. * ``cert``: Path to a file/database containing the identifying certificate. * ``key``: An optional key to access the identifying certificate. - * ``verify``: If ``True``, verify the peer name - (:const:`proton.SSLDomain.VERIFY_PEER_NAME`) and certificate using the - ``ca`` above. + * ``verify``: If ``False``, do not verify the peer name + (:const:`proton.SSLDomain.ANONYMOUS_PEER`) or certificate. By default + (or if ``True``) verify the peer name and certificate using the + ``ca`` above (:const:`proton.SSLDomain.VERIFY_PEER_NAME`). :param url: URL string of process to connect to :type url: ``str`` @@ -1302,10 +1303,11 @@ class Container(Reactor): ca = tls_config.get('ca') cert = tls_config.get('cert') key = tls_config.get('key') + verify = tls_config.get('verify', True) if ca: _ssl_domain.set_trusted_ca_db(str(ca)) - if tls_config.get('verify', True): - _ssl_domain.set_peer_authentication(SSLDomain.VERIFY_PEER_NAME, str(ca)) + if not verify: + _ssl_domain.set_peer_authentication(SSLDomain.ANONYMOUS_PEER, None) if cert and key: _ssl_domain.set_credentials(str(cert), str(key), None) diff --git a/python/proton/_transport.py b/python/proton/_transport.py index 57f75e8..5764384 100644 --- a/python/proton/_transport.py +++ b/python/proton/_transport.py @@ -832,7 +832,8 @@ class SSLDomain(object): def set_peer_authentication(self, verify_mode, trusted_CAs=None): """ This method controls how the peer's certificate is validated, if at all. By default, - neither servers nor clients attempt to verify their peers (PN_SSL_ANONYMOUS_PEER). + servers do not attempt to verify their peers (PN_SSL_ANONYMOUS_PEER) but + clients attempt to verify both the certificate and peer name (PN_SSL_VERIFY_PEER_NAME). Once certificates and trusted CAs are configured, peer verification can be enabled. .. note:: In order to verify a peer, a trusted CA must be configured. See diff --git a/python/tests/proton_tests/connect.py b/python/tests/proton_tests/connect.py index 024f69a..bc18885 100644 --- a/python/tests/proton_tests/connect.py +++ b/python/tests/proton_tests/connect.py @@ -68,10 +68,10 @@ class Client(MessagingHandler): class ConnectConfigTest(Test): def test_port(self): ensureCanTestExtendedSASL() - server = Server() + server = Server(scheme='amqp') container = Container(server) client = Client() - write_connect_conf({'port': server.port}) + write_connect_conf({'port': server.port, 'scheme': 'amqp'}) container.connect(handler=client, reconnect=False) container.run() assert client.opened @@ -82,8 +82,20 @@ class ConnectConfigTest(Test): password = 'password' server = Server(user) container = Container(server) + container.ssl.server.set_credentials(_testpath('server-certificate.pem'), + _testpath('server-private-key.pem'), + 'server-password') client = Client() - write_connect_conf({'port': server.port, 'user': user, 'password': password}) + config = { + 'user': user, + 'password': password, + 'port': server.port, + 'tls': { + 'verify': False, + 'ca': _testpath('ca-certificate.pem') + } + } + write_connect_conf(config) container.connect(handler=client, reconnect=False) container.run() assert client.opened @@ -93,15 +105,16 @@ class ConnectConfigTest(Test): ensureCanTestExtendedSASL() server = Server(scheme='amqps') container = Container(server) - container.ssl.server.set_credentials(_testpath('server-certificate.pem'), - _testpath('server-private-key.pem'), + container.ssl.server.set_credentials(_testpath('server-certificate-lh.pem'), + _testpath('server-private-key-lh.pem'), 'server-password') client = Client() config = { 'scheme': 'amqps', 'port': server.port, 'tls': { - 'verify': False + 'verify': True, + 'ca': _testpath('ca-certificate.pem') } } write_connect_conf(config) diff --git a/python/tests/proton_tests/sasl.py b/python/tests/proton_tests/sasl.py index 55f83f5..c5a6e72 100644 --- a/python/tests/proton_tests/sasl.py +++ b/python/tests/proton_tests/sasl.py @@ -412,6 +412,8 @@ class SSLSASLTest(Test): self.c1.password = 'password' self.c1.hostname = 'localhost' + self.client_domain.set_peer_authentication(SSLDomain.ANONYMOUS_PEER) + ssl1 = _sslConnection(self.client_domain, self.t1, self.c1) ssl2 = _sslConnection(self.server_domain, self.t2, self.c2) @@ -431,6 +433,8 @@ class SSLSASLTest(Test): self.c1.password = 'password' self.c1.hostname = 'localhost' + self.client_domain.set_peer_authentication(SSLDomain.ANONYMOUS_PEER) + ssl1 = _sslConnection(self.client_domain, self.t1, self.c1) ssl2 = _sslConnection(self.server_domain, self.t2, self.c2) @@ -448,6 +452,8 @@ class SSLSASLTest(Test): self.c1.password = 'password' self.c1.hostname = 'localhost' + self.client_domain.set_peer_authentication(SSLDomain.ANONYMOUS_PEER) + ssl1 = _sslConnection(self.client_domain, self.t1, self.c1) ssl2 = _sslConnection(self.server_domain, self.t2, self.c2) diff --git a/python/tests/proton_tests/ssl.py b/python/tests/proton_tests/ssl.py index 04dc4df..a2efeb2 100644 --- a/python/tests/proton_tests/ssl.py +++ b/python/tests/proton_tests/ssl.py @@ -97,12 +97,13 @@ class SslTest(common.Test): server.connection.close() self._pump(client, server) - def test_defaults(self): + def test_anonymous_cipher(self): if os.name == "nt": raise Skipped("Windows SChannel lacks anonymous cipher support.") """ By default, both the server and the client support anonymous ciphers - they should connect without need for a certificate. """ + self.client_domain.set_peer_authentication(SSLDomain.ANONYMOUS_PEER) server = SslTest.SslTestConnection(self.server_domain, mode=Transport.SERVER) client = SslTest.SslTestConnection(self.client_domain) diff --git a/ruby/examples/ssl_send.rb b/ruby/examples/ssl_send.rb index 4ff5da6..9e437bf 100644 --- a/ruby/examples/ssl_send.rb +++ b/ruby/examples/ssl_send.rb @@ -32,8 +32,9 @@ class SimpleSend < Qpid::Proton::MessagingHandler end def on_container_start(container) - # Use a default client SSL domain + # Use anonymous client SSL domain ssl_domain = Qpid::Proton::SSLDomain.new(Qpid::Proton::SSLDomain::MODE_CLIENT) + ssl_domain.peer_authentication(Qpid::Proton::SSLDomain::ANONYMOUS_PEER) c = container.connect(@url, { :ssl_domain => ssl_domain }) c.open_sender(@address) end diff --git a/ruby/lib/core/ssl_domain.rb b/ruby/lib/core/ssl_domain.rb index 813fc1a..8a45b64 100644 --- a/ruby/lib/core/ssl_domain.rb +++ b/ruby/lib/core/ssl_domain.rb @@ -105,9 +105,10 @@ module Qpid::Proton # Configures the level of verification used on the peer certificate. # # This method congtrols how the peer's certificate is validated, if at all. - # By default, neither servers nor clients attempt to verify their peers - # (*ANONYMOUS_PEER*). Once certficates and trusted CAs are configured, peer - # verification can be enabled. + # By default, servers do not attempt to verify their peers + # (*ANONYMOUS_PEER*) but clients attempt to verify both the certificate and + # peer name (*VERIFY_PEER_NAME*). Once certficates and trusted CAs are + # configured, peer verification can be enabled. # # *NOTE:* In order to verify a peer, a trusted CA must be configured. # --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@qpid.apache.org For additional commands, e-mail: commits-h...@qpid.apache.org