You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by GitBox <gi...@apache.org> on 2020/09/02 21:06:30 UTC

[GitHub] [qpid-dispatch] ChugR commented on a change in pull request #843: DISPATCH-1762: Connector ssl config errors must prohibit connections

ChugR commented on a change in pull request #843:
URL: https://github.com/apache/qpid-dispatch/pull/843#discussion_r482469927



##########
File path: tests/system_tests_ssl.py
##########
@@ -780,5 +781,148 @@ def test_connected_tls_sasl_routers(self):
         self.assertEqual(len(router_nodes), expected_nodes)
 
 
+class RouterTestSslInterRouterVerifyHostname(RouterTestSslBase):
+    """
+    DISPATCH-1762
+    Starts 2 routers:
+       Router A listener serves a normal, good certificate
+       Router B connector is configured with a bad CA in its profile but verifyHostname is false.
+    Test proves:
+       Router B must not connect to A.
+
+    DISPATCH-1762 fixes several issues related to failed ssl setup
+    allowing a connector to connect anyway.
+    This test checks only one of the failures.
+    """
+    # Listener ports for each TLS protocol definition
+    PORT_NO_SSL  = 0
+    PORT_TLS_ALL = 0
+
+    @classmethod
+    def setUpClass(cls):
+        """
+        Prepares 2 routers to form a network.
+        """
+        super(RouterTestSslInterRouterVerifyHostname, cls).setUpClass()
+
+        if not SASL.extended():
+            return
+
+        os.environ["ENV_SASL_PASSWORD"] = "password"
+
+        # Generate authentication DB
+        super(RouterTestSslInterRouterVerifyHostname, cls).create_sasl_files()
+
+        # Router expected to be connected
+        cls.connected_tls_sasl_routers = []
+
+        # Generated router list
+        cls.routers = []
+
+        # Saving listener ports for each TLS definition
+        cls.PORT_NO_SSL = cls.tester.get_port()
+        cls.PORT_TLS_ALL = cls.tester.get_port()
+
+        config_a = Qdrouterd.Config([
+            ('router', {'id': 'QDR.A',
+                        'mode': 'interior',
+                        'saslConfigName': 'tests-mech-PLAIN',
+                        'saslConfigDir': os.getcwd()}),
+            # No auth and no SSL for management access
+            ('listener', {'host': '0.0.0.0', 'role': 'normal', 'port': cls.PORT_NO_SSL}),
+            # All TLS versions and normal, good sslProfile config
+            ('listener', {'host': '0.0.0.0', 'role': 'inter-router', 'port': cls.PORT_TLS_ALL,
+                          'saslMechanisms': 'PLAIN',
+                          'requireEncryption': 'yes', 'requireSsl': 'yes',
+                          'sslProfile': 'ssl-profile-tls-all'}),
+            # SSL Profile for all TLS versions (protocols element not defined)
+            ('sslProfile', {'name': 'ssl-profile-tls-all',
+                            'caCertFile': cls.ssl_file('ca-certificate.pem'),
+                            'certFile': cls.ssl_file('server-certificate.pem'),
+                            'privateKeyFile': cls.ssl_file('server-private-key.pem'),
+                            'ciphers': 'ECDH+AESGCM:DH+AESGCM:ECDH+AES256:DH+AES256:ECDH+AES128:' \
+                                       'DH+AES:RSA+AESGCM:RSA+AES:!aNULL:!MD5:!DSS',
+                            'password': 'server-password'})
+        ])
+
+        # Router B has a connector to listener that allows all protocols but will not verify hostname.
+        # The sslProfile has a bad caCertFile name and this router should not connect.
+        config_b = Qdrouterd.Config([
+            ('router', {'id': 'QDR.B',
+                        'mode': 'interior'}),
+            # Connector to All TLS versions allowed listener
+            ('connector', {'host': '0.0.0.0', 'role': 'inter-router', 'port': cls.PORT_TLS_ALL,
+                           'verifyHostname': 'no', 'saslMechanisms': 'PLAIN',
+                           'saslUsername': 'test@domain.com', 'saslPassword': 'pass:password',
+                           'sslProfile': 'ssl-profile-tls-all'}),
+            # SSL Profile with bad caCertFile name. The correct name here would allow this
+            # router to connect. The object is to trigger a specific failure in the ssl
+            # setup chain of calls to pn_ssl_domain_* functions.
+            ('sslProfile', {'name': 'ssl-profile-tls-all',
+                            'caCertFile': cls.ssl_file('ca-certificate-BAD.pem'),
+                            'certFile': cls.ssl_file('client-certificate.pem'),
+                            'privateKeyFile': cls.ssl_file('client-private-key.pem'),
+                            'ciphers': 'ECDH+AESGCM:DH+AESGCM:ECDH+AES256:DH+AES256:ECDH+AES128:' \
+                                       'DH+AES:RSA+AESGCM:RSA+AES:!aNULL:!MD5:!DSS',
+                            'password': 'client-password'})
+        ])
+
+        cls.routers.append(cls.tester.qdrouterd("A", config_a, wait=False))
+        cls.routers.append(cls.tester.qdrouterd("B", config_b, wait=False))
+
+        # Wait until A is running
+        cls.routers[0].wait_ports()
+
+        # Can't wait until B is connected because it's not supposed to connect.
+
+    def get_router_nodes(self):
+        """
+        Retrieves connected router nodes from QDR.A
+        :return: list of connected router id's
+        """
+        if not SASL.extended():
+            self.skipTest("Cyrus library not available. skipping test")
+
+        url = Url("amqp://0.0.0.0:%d/$management" % self.PORT_NO_SSL)
+        node = Node.connect(url)
+        response = node.query(type="org.apache.qpid.dispatch.router.node", attribute_names=["id"])
+        router_nodes = []
+        for resp in response.get_dicts():
+            router_nodes.append(resp['id'])
+        node.close()
+        return router_nodes
+
+    @SkipIfNeeded(RouterTestSslBase.DISABLE_SSL_TESTING or not SASL.extended(),
+                  "Cyrus library not available. skipping test")
+    def test_connected_misconfigured_tls_sasl_routers(self):
+        """
+        Prove that improperly configured ssl-enabled connector prevents the router
+        from joining the network
+        """
+        if not SASL.extended():
+            self.skipTest("Cyrus library not available. skipping test")
+
+        # Poll for a while until the connector error shows up in router B's log
+        pattern = " SERVER (error) SSL CA configuration failed"
+        sleep_time = 0.1 # seconds
+        poll_duration = 10.0 # seconds
+        verified = False
+        for tries in range(int(poll_duration / sleep_time)):
+            with  open('../setUpClass/B.log', 'r') as router_log:
+                log_lines = router_log.read().split("\n")
+            e_lines = [s for s in log_lines if pattern in s]
+            verified = len(e_lines) > 0
+            if verified:
+                break;
+            time.sleep(sleep_time)
+        self.assertTrue(verified, "Log line containing '%s' not seen in QDR.B log" % pattern)
+
+        # Show that router A does not have router B in its network
+        router_nodes = self.get_router_nodes()
+        self.assertTrue(router_nodes)
+        node = "QDR.B"
+        self.assertTrue(node not in router_nodes, "%s should not be connected" % node)

Review comment:
       fixed




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org