From fb677c1f808041c963b2b5bfee6814c001f2809f Mon Sep 17 00:00:00 2001 From: Ganesh Murthy Date: Wed, 11 May 2016 10:51:13 -0400 Subject: [PATCH] DISPATCH-320 - Added a new connector property called verifyHostName which will verify host name on SSL connections --- include/qpid/dispatch/server.h | 6 + python/qpid_dispatch/management/qdrouter.json | 6 + src/connection_manager.c | 32 ++-- src/server.c | 11 ++ tests/system_test.py | 9 +- tests/system_tests_sasl_plain.py | 173 +++++++++++++++++- 6 files changed, 218 insertions(+), 19 deletions(-) diff --git a/include/qpid/dispatch/server.h b/include/qpid/dispatch/server.h index 2a8e34a02f..935cb171eb 100644 --- a/include/qpid/dispatch/server.h +++ b/include/qpid/dispatch/server.h @@ -310,6 +310,12 @@ typedef struct qd_server_config_t { */ bool requireEncryption; + /** + * Ensures that when initiating a connection (as a client) the host name in the URL to which this connector + * connects to matches the host name in the digital certificate that the peer sends back as part of the SSL connection + */ + bool verify_host_name; + /** * If true, strip the inbound qpid dispatch specific message annotations. This only applies to ingress and egress routers. * Annotations generated by inter-router messages will be untouched. diff --git a/python/qpid_dispatch/management/qdrouter.json b/python/qpid_dispatch/management/qdrouter.json index c804f72133..699031a022 100644 --- a/python/qpid_dispatch/management/qdrouter.json +++ b/python/qpid_dispatch/management/qdrouter.json @@ -745,6 +745,12 @@ "required": false, "description": "The capacity of links within this connection, in terms of message deliveries. The capacity is the number of messages that can be in-flight concurrently for each link." }, + "verifyHostName": { + "type": "boolean", + "default": false, + "description": "yes: Ensures that when initiating a connection (as a client) the host name in the URL to which this connector connects to matches the host name in the digital certificate that the peer sends back as part of the SSL connection; no: Does not perform host name verification", + "create": true + }, "saslUsername": { "type": "string", "required": false, diff --git a/src/connection_manager.c b/src/connection_manager.c index 2479189d0e..43caf1847c 100644 --- a/src/connection_manager.c +++ b/src/connection_manager.c @@ -127,27 +127,28 @@ static qd_error_t load_server_config(qd_dispatch_t *qd, qd_server_config_t *conf { qd_error_clear(); - bool authenticatePeer = qd_entity_opt_bool(entity, "authenticatePeer", false); CHECK(); - char *stripAnnotations = qd_entity_opt_string(entity, "stripAnnotations", 0); CHECK(); - bool requireEncryption = qd_entity_opt_bool(entity, "requireEncryption", false); CHECK(); - bool requireSsl = qd_entity_opt_bool(entity, "requireSsl", false); CHECK(); - bool depRequirePeerAuth = qd_entity_opt_bool(entity, "requirePeerAuth", false); CHECK(); + bool authenticatePeer = qd_entity_opt_bool(entity, "authenticatePeer", false); CHECK(); + bool verifyHostName = qd_entity_opt_bool(entity, "verifyHostName", false); CHECK(); + char *stripAnnotations = qd_entity_opt_string(entity, "stripAnnotations", 0); CHECK(); + bool requireEncryption = qd_entity_opt_bool(entity, "requireEncryption", false); CHECK(); + bool requireSsl = qd_entity_opt_bool(entity, "requireSsl", false); CHECK(); + bool depRequirePeerAuth = qd_entity_opt_bool(entity, "requirePeerAuth", false); CHECK(); bool depAllowUnsecured = qd_entity_opt_bool(entity, "allowUnsecured", !requireSsl); CHECK(); memset(config, 0, sizeof(*config)); - config->host = qd_entity_get_string(entity, "addr"); CHECK(); - config->port = qd_entity_get_string(entity, "port"); CHECK(); - config->name = qd_entity_opt_string(entity, "name", 0); CHECK(); - config->role = qd_entity_get_string(entity, "role"); CHECK(); - config->inter_router_cost = qd_entity_opt_long(entity, "cost", 1); CHECK(); + config->host = qd_entity_get_string(entity, "addr"); CHECK(); + config->port = qd_entity_get_string(entity, "port"); CHECK(); + config->name = qd_entity_opt_string(entity, "name", 0); CHECK(); + config->role = qd_entity_get_string(entity, "role"); CHECK(); + config->inter_router_cost = qd_entity_opt_long(entity, "cost", 1); CHECK(); config->protocol_family = qd_entity_opt_string(entity, "protocolFamily", 0); CHECK(); - config->max_frame_size = qd_entity_get_long(entity, "maxFrameSize"); CHECK(); - config->idle_timeout_seconds = qd_entity_get_long(entity, "idleTimeoutSeconds"); CHECK(); - config->sasl_username = qd_entity_opt_string(entity, "saslUsername", 0); CHECK(); - config->sasl_password = qd_entity_opt_string(entity, "saslPassword", 0); CHECK(); + config->max_frame_size = qd_entity_get_long(entity, "maxFrameSize"); CHECK(); + config->idle_timeout_seconds = qd_entity_get_long(entity, "idleTimeoutSeconds"); CHECK(); + config->sasl_username = qd_entity_opt_string(entity, "saslUsername", 0); CHECK(); + config->sasl_password = qd_entity_opt_string(entity, "saslPassword", 0); CHECK(); config->sasl_mechanisms = qd_entity_opt_string(entity, "saslMechanisms", 0); CHECK(); + config->link_capacity = qd_entity_opt_long(entity, "linkCapacity", 0); CHECK(); config->ssl_enabled = has_attrs(entity, ssl_attributes, ssl_attributes_count); - config->link_capacity = qd_entity_opt_long(entity, "linkCapacity", 0); CHECK(); // // Handle the defaults for link capacity. @@ -160,6 +161,7 @@ static qd_error_t load_server_config(qd_dispatch_t *qd, qd_server_config_t *conf // user community, we can revisit this later. // config->allowInsecureAuthentication = true; + config->verify_host_name = verifyHostName; load_strip_annotations(config, stripAnnotations); diff --git a/src/server.c b/src/server.c index 3ddef991c4..9e7a0d8170 100644 --- a/src/server.c +++ b/src/server.c @@ -1163,12 +1163,14 @@ static void cxtr_try_open(void *context) // if (config->ssl_enabled) { pn_ssl_domain_t *domain = pn_ssl_domain(PN_SSL_MODE_CLIENT); + if (!domain) { qd_error(QD_ERROR_RUNTIME, "SSL domain failed for connection to %s:%s", ct->config->host, ct->config->port); /* TODO aconway 2014-07-15: Close the connection, clean up. */ return; } + /* TODO aconway 2014-07-15: error handling on all SSL calls. */ // set our trusted database for checking the peer's cert: @@ -1205,6 +1207,15 @@ static void cxtr_try_open(void *context) } } + //If ssl is enabled and verify_host_name is true, instruct proton to verify peer name + if (config->verify_host_name) { + if (pn_ssl_domain_set_peer_authentication(domain, PN_SSL_VERIFY_PEER_NAME, NULL)) { + qd_log(ct->server->log_source, QD_LOG_ERROR, + "SSL peer host name verification failed for %s:%s", + ct->config->host, ct->config->port); + } + } + ctx->ssl = pn_ssl(tport); pn_ssl_init(ctx->ssl, domain, 0); pn_ssl_domain_free(domain); diff --git a/tests/system_test.py b/tests/system_test.py index dc21c189d3..460afd967e 100755 --- a/tests/system_test.py +++ b/tests/system_test.py @@ -472,19 +472,23 @@ def get_host(self, protocol_family): else: return '127.0.0.1' + def wait_ports(self, **retry_kwargs): + wait_ports(self.ports_family, **retry_kwargs) + def wait_connectors(self, **retry_kwargs): """ Wait for all connectors to be connected @param retry_kwargs: keyword args for L{retry} """ for c in self.config.sections('connector'): - assert retry(lambda: self.is_connected(port=c['port'], host=self.get_host(c.get('protocolFamily'))), **retry_kwargs), "Port not connected %s" % c['port'] + assert retry(lambda: self.is_connected(port=c['port'], host=self.get_host(c.get('protocolFamily'))), + **retry_kwargs), "Port not connected %s" % c['port'] def wait_ready(self, **retry_kwargs): """Wait for ports and connectors to be ready""" if not self._wait_ready: self._wait_ready = True - wait_ports(self.ports_family, **retry_kwargs) + self.wait_ports(**retry_kwargs) self.wait_connectors(**retry_kwargs) return self @@ -501,7 +505,6 @@ def is_router_connected(self, router_id, **retry_kwargs): except: return False - def wait_router_connected(self, router_id, **retry_kwargs): retry(lambda: self.is_router_connected(router_id), **retry_kwargs) diff --git a/tests/system_tests_sasl_plain.py b/tests/system_tests_sasl_plain.py index a01ea33dab..6872c11df6 100644 --- a/tests/system_tests_sasl_plain.py +++ b/tests/system_tests_sasl_plain.py @@ -17,7 +17,7 @@ # under the License. # -import unittest, os +import unittest, os, time from subprocess import PIPE, Popen from system_test import TestCase, Qdrouterd, main_module, DIR, TIMEOUT @@ -215,6 +215,177 @@ def test_inter_router_plain_over_ssl_exists(self): # user must be test@domain.com self.assertEqual(u'test@domain.com', local_node.query(type='org.apache.qpid.dispatch.connection').results[0][16]) +class RouterTestVerifyHostNameYes(RouterTestPlainSaslCommon): + + @staticmethod + def ssl_file(name): + return os.path.join(DIR, 'ssl_certs', name) + + @classmethod + def setUpClass(cls): + """ + Tests the verifyHostName property of the connector. The hostname on the server certificate we use is + A1.Good.Server.domain.com and the host is 0.0.0.0 on the client router initiating the SSL connection. + Since the host names do not match and the verifyHostName is set to true, the client router + will NOT be able make a successful SSL connection the server router. + """ + super(RouterTestVerifyHostNameYes, cls).setUpClass() + + super(RouterTestVerifyHostNameYes, cls).createSaslFiles() + + cls.routers = [] + + x_listener_port = cls.tester.get_port() + y_listener_port = cls.tester.get_port() + + super(RouterTestVerifyHostNameYes, cls).router('X', [ + ('listener', {'addr': '0.0.0.0', 'role': 'inter-router', 'port': x_listener_port, + 'sslProfile':'server-ssl-profile', + 'saslMechanisms':'PLAIN', 'authenticatePeer': 'yes'}), + # This unauthenticated listener is for qdstat to connect to it. + ('listener', {'addr': '0.0.0.0', 'role': 'normal', 'port': cls.tester.get_port(), + 'authenticatePeer': 'no'}), + ('sslProfile', {'name': 'server-ssl-profile', + 'cert-db': cls.ssl_file('ca-certificate.pem'), + 'cert-file': cls.ssl_file('server-certificate.pem'), + 'key-file': cls.ssl_file('server-private-key.pem'), + 'password': 'server-password'}), + ('router', {'workerThreads': 1, + 'routerId': 'QDR.X', + 'mode': 'interior', + 'saslConfigName': 'tests-mech-PLAIN', + 'saslConfigPath': os.getcwd()}), + ]) + + super(RouterTestVerifyHostNameYes, cls).router('Y', [ + ('connector', {'addr': '127.0.0.1', 'role': 'inter-router', 'port': x_listener_port, + 'ssl-profile': 'client-ssl-profile', + 'verifyHostName': 'yes', + 'saslMechanisms': 'PLAIN', + 'saslUsername': 'test@domain.com', 'saslPassword': 'password'}), + ('router', {'workerThreads': 1, + 'mode': 'interior', + 'routerId': 'QDR.Y'}), + ('listener', {'addr': '0.0.0.0', 'role': 'normal', 'port': y_listener_port}), + ('sslProfile', {'name': 'client-ssl-profile', + 'cert-db': cls.ssl_file('ca-certificate.pem'), + 'cert-file': cls.ssl_file('client-certificate.pem'), + 'key-file': cls.ssl_file('client-private-key.pem'), + 'password': 'client-password'}), + ]) + + cls.routers[0].wait_ports() + cls.routers[1].wait_ports() + try: + # This will time out because there is no inter-router connection + cls.routers[1].wait_connectors(timeout=3) + except: + pass + + def test_no_inter_router_connection(self): + """ + Tests to make sure that there are no 'inter-router' connections. + The connection to the other router will not happen because the connection failed + due to setting 'verifyHostName': 'yes' + """ + local_node = Node.connect(self.routers[1].addresses[0], timeout=TIMEOUT) + + # There should be only two connections. + # There will be no inter-router connection + self.assertEqual(2, len(local_node.query(type='org.apache.qpid.dispatch.connection').results)) + self.assertEqual('in', local_node.query(type='org.apache.qpid.dispatch.connection').results[0][15]) + self.assertEqual('normal', local_node.query(type='org.apache.qpid.dispatch.connection').results[0][9]) + self.assertEqual('anonymous', local_node.query(type='org.apache.qpid.dispatch.connection').results[0][16]) + + self.assertEqual('normal', local_node.query(type='org.apache.qpid.dispatch.connection').results[1][9]) + self.assertEqual('anonymous', local_node.query(type='org.apache.qpid.dispatch.connection').results[1][16]) + +class RouterTestVerifyHostNameNo(RouterTestPlainSaslCommon): + + @staticmethod + def ssl_file(name): + return os.path.join(DIR, 'ssl_certs', name) + + @classmethod + def setUpClass(cls): + """ + Tests the verifyHostName property of the connector. The hostname on the server certificate we use is + A1.Good.Server.domain.com and the host is 0.0.0.0 on the client router initiating the SSL connection. + Since the host names do not match but verifyHostName is set to false, the client router + will be successfully able to make an SSL connection the server router. + """ + super(RouterTestVerifyHostNameNo, cls).setUpClass() + + super(RouterTestVerifyHostNameNo, cls).createSaslFiles() + + cls.routers = [] + + x_listener_port = cls.tester.get_port() + y_listener_port = cls.tester.get_port() + + super(RouterTestVerifyHostNameNo, cls).router('X', [ + ('listener', {'addr': '0.0.0.0', 'role': 'inter-router', 'port': x_listener_port, + 'sslProfile':'server-ssl-profile', + 'saslMechanisms':'PLAIN', 'authenticatePeer': 'yes'}), + # This unauthenticated listener is for qdstat to connect to it. + ('listener', {'addr': '0.0.0.0', 'role': 'normal', 'port': cls.tester.get_port(), + 'authenticatePeer': 'no'}), + ('sslProfile', {'name': 'server-ssl-profile', + 'cert-db': cls.ssl_file('ca-certificate.pem'), + 'cert-file': cls.ssl_file('server-certificate.pem'), + 'key-file': cls.ssl_file('server-private-key.pem'), + 'password': 'server-password'}), + ('router', {'workerThreads': 1, + 'routerId': 'QDR.X', + 'mode': 'interior', + 'saslConfigName': 'tests-mech-PLAIN', + 'saslConfigPath': os.getcwd()}), + ]) + + super(RouterTestVerifyHostNameNo, cls).router('Y', [ + # This router will act like a client. First an SSL connection will be established and then + # we will have SASL plain authentication over SSL. + ('connector', {'addr': '127.0.0.1', 'role': 'inter-router', 'port': x_listener_port, + 'ssl-profile': 'client-ssl-profile', + # Provide a sasl user name and password to connect to QDR.X + 'saslMechanisms': 'PLAIN', + 'verifyHostName': 'no', + 'saslUsername': 'test@domain.com', 'saslPassword': 'password'}), + ('router', {'workerThreads': 1, + 'mode': 'interior', + 'routerId': 'QDR.Y'}), + ('listener', {'addr': '0.0.0.0', 'role': 'normal', 'port': y_listener_port}), + ('sslProfile', {'name': 'client-ssl-profile', + 'cert-db': cls.ssl_file('ca-certificate.pem'), + 'cert-file': cls.ssl_file('client-certificate.pem'), + 'key-file': cls.ssl_file('client-private-key.pem'), + 'password': 'client-password'}), + ]) + + cls.routers[0].wait_ports() + cls.routers[1].wait_ports() + cls.routers[1].wait_connectors(timeout=3) + + def test_inter_router_plain_over_ssl_exists(self): + """ + Tests to make sure that an inter-router connection exists between the routers since verifyHostName is 'no'. + """ + local_node = Node.connect(self.routers[1].addresses[0], timeout=TIMEOUT) + + self.assertEqual(3, len(local_node.query(type='org.apache.qpid.dispatch.connection').results)) + + # sslProto should be TLSv1/SSLv3 + self.assertEqual(u'TLSv1/SSLv3', local_node.query(type='org.apache.qpid.dispatch.connection').results[0][4]) + + # role should be inter-router + self.assertEqual(u'inter-router', local_node.query(type='org.apache.qpid.dispatch.connection').results[0][9]) + + # sasl must be plain + self.assertEqual(u'PLAIN', local_node.query(type='org.apache.qpid.dispatch.connection').results[0][12]) + + # user must be test@domain.com + self.assertEqual(u'test@domain.com', local_node.query(type='org.apache.qpid.dispatch.connection').results[0][16]) + if __name__ == '__main__': unittest.main(main_module())