You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by gm...@apache.org on 2019/10/17 17:46:13 UTC

[qpid-dispatch] branch master updated: DISPATCH-1445 - Modeled saslPassword to be more or less in line with sslProfile's password field

This is an automated email from the ASF dual-hosted git repository.

gmurthy pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/qpid-dispatch.git


The following commit(s) were added to refs/heads/master by this push:
     new c6cbac8  DISPATCH-1445 - Modeled saslPassword to be more or less in line with sslProfile's password field
c6cbac8 is described below

commit c6cbac8fb748096259e0c28864b262a724f7b089
Author: Ganesh Murthy <gm...@apache.org>
AuthorDate: Tue Oct 15 13:28:27 2019 -0400

    DISPATCH-1445 - Modeled saslPassword to be more or less in line with sslProfile's password field
---
 docs/books/user-guide/configuration-reference.adoc |   2 +-
 docs/books/user-guide/configuration-security.adoc  |   2 +-
 ...ing-using-username-password-authentication.adoc |   4 +-
 python/qpid_dispatch/management/qdrouter.json      |   2 +-
 src/connection_manager.c                           |  23 ++-
 tests/sasl_files/password.txt                      |   1 +
 tests/system_tests_sasl_plain.py                   | 210 ++++++++++++++++++++-
 tests/system_tests_ssl.py                          |   8 +-
 8 files changed, 232 insertions(+), 20 deletions(-)

diff --git a/docs/books/user-guide/configuration-reference.adoc b/docs/books/user-guide/configuration-reference.adoc
index 5d09083..7cbe31f 100644
--- a/docs/books/user-guide/configuration-reference.adoc
+++ b/docs/books/user-guide/configuration-reference.adoc
@@ -144,7 +144,7 @@ Establishes an outgoing connection from the router.
 * *_linkCapacity_* (integer) : 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_* (boolean, default=True) : yes: Ensures that when initiating a connection (as a client) the hostname in the URL to which this connector connects to matches the hostname in the digital certificate that the peer sends back as part of the SSL/TLS connection; no: Does not perform hostname verification
 * *_saslUsername_* (string) : The username that the connector is using to connect to a peer.
-* *_saslPassword_* (string) : The password that the connector is using to connect to a peer.
+* *_saslPassword_* (string) : The password that the connector is using to connect to a peer. You can specify the password by specifying an environment variable that stores the password, a file that stores the password, or by entering the password in clear text. To use an environment variable, specify "saslPassword: env:". Use this option with caution, because the environment of other processes is visible on certain platforms (for example, "ps" on certain Unix OSs). To use a file, specify [...]
 * *_sslProfile_* (string) : The name of the _sslProfile_ entity to use in order to have SSL/TLS configuration.
 
 [id='router-configuration-file-log']
diff --git a/docs/books/user-guide/configuration-security.adoc b/docs/books/user-guide/configuration-security.adoc
index 8008da8..f66eec7 100644
--- a/docs/books/user-guide/configuration-security.adoc
+++ b/docs/books/user-guide/configuration-security.adoc
@@ -378,7 +378,7 @@ connector {
 +
 For a full list of supported Cyrus SASL authentication mechanisms, see link:https://www.cyrusimap.org/sasl/sasl/authentication_mechanisms.html[Authentication Mechanisms^].
 `saslUsername`:: If any of the SASL mechanisms uses username/password authentication, then provide the username to connect to the external container.
-`saslPassword`:: If any of the SASL mechanisms uses username/password authentication, then provide the password to connect to the external container.
+`saslPassword`:: If any of the SASL mechanisms uses username/password authentication, then provide the password to connect to the external container. You can specify the password by specifying an environment variable that stores the password, a file that stores the password, or by entering the password in clear text. To use an environment variable, specify "saslPassword: env:". Use this option with caution, because the environment of other processes is visible on certain platforms (for e [...]
 --
 
 [id='integrating-with-kerberos']
diff --git a/docs/books/user-guide/modules/connecting-using-username-password-authentication.adoc b/docs/books/user-guide/modules/connecting-using-username-password-authentication.adoc
index cf3f254..2e31e05 100644
--- a/docs/books/user-guide/modules/connecting-using-username-password-authentication.adoc
+++ b/docs/books/user-guide/modules/connecting-using-username-password-authentication.adoc
@@ -41,7 +41,7 @@ include::{FragmentDir}/fragment-router-sasl-para.adoc[]
 
 include::{FragmentDir}/fragment-router-open-config-file-step.adoc[]
 
-. Configure the `connector` for this connection to provide user name and password credentials to the external AMQP container.
+. Configure the `connector` for this connection to provide user name and password credentials to the external AMQP container. Note that the saslPassword used to attach to an external container uses the file: prefix to specify the absolute path of the file containing the the password. The file permissions need to be set on the file to prevent general access. Other prefixes like pass: or env: are also supported by generally unsafe.
 +
 --
 [options="nowrap",subs="+quotes"]
@@ -52,7 +52,7 @@ connector {
     role: route-container
     saslMechanisms: PLAIN
     saslUsername: user
-    saslPassword: password
+    saslPassword: file:/path/to/file/password.txt
     }
 ----
 --
diff --git a/python/qpid_dispatch/management/qdrouter.json b/python/qpid_dispatch/management/qdrouter.json
index 69d1bed..e297d58 100644
--- a/python/qpid_dispatch/management/qdrouter.json
+++ b/python/qpid_dispatch/management/qdrouter.json
@@ -1008,7 +1008,7 @@
                 "saslPassword": {
                     "type": "string",
                     "required": false,
-                    "description": "The password that the connector is using to connect to a peer.",
+                    "description": "The password that the connector is using to connect to a peer. You can specify the password by specifying an environment variable that stores the password, a file that stores the password, or by entering the password in clear text. To use an environment variable, specify 'saslPassword: env:'. Use this option with caution, because the environment of other processes is visible on certain platforms (for example, ps on certain Unix OSs). To use a file, spe [...]
                     "create": true,
                     "hidden": true
                 },
diff --git a/src/connection_manager.c b/src/connection_manager.c
index 4a8b0b2..788fba6 100644
--- a/src/connection_manager.c
+++ b/src/connection_manager.c
@@ -249,7 +249,7 @@ static void set_config_host(qd_server_config_t *config, qd_entity_t* entity)
     snprintf(config->host_port, hplen, "%s:%s", config->host, config->port);
 }
 
-static void qd_config_process_password(char **actual_val, char *pw, bool *is_file, qd_log_source_t *log_source)
+static void qd_config_process_password(char **actual_val, char *pw, bool *is_file, bool allow_literal_prefix, qd_log_source_t *log_source)
 {
     if (!pw)
         return;
@@ -279,7 +279,7 @@ static void qd_config_process_password(char **actual_val, char *pw, bool *is_fil
     // the remaining text is the password and the heading should be
     // stripped off
     //
-    else if (strncmp(pw, "literal:", 8) == 0 || strncmp(pw, "pass:", 5) == 0) {
+    else if ( (strncmp(pw, "literal:", 8) == 0 && allow_literal_prefix) || strncmp(pw, "pass:", 5) == 0) {
         qd_log(log_source, QD_LOG_WARNING, "It is unsafe to provide plain text passwords in the config file");
 
         if (strncmp(pw, "l", 1) == 0) {
@@ -396,6 +396,23 @@ static qd_error_t load_server_config(qd_dispatch_t *qd, qd_server_config_t *conf
     config->policy_vhost         = qd_entity_opt_string(entity, "policyVhost", 0);    CHECK();
     set_config_host(config, entity);
 
+    if (config->sasl_password) {
+        //
+        //Process the sasl password field and set the right values based on prefixes.
+        //
+        char *actual_pass = 0;
+        bool is_file_path = 0;
+        qd_config_process_password(&actual_pass, config->sasl_password, &is_file_path, false, qd->connection_manager->log_source);
+        if (actual_pass && is_file_path) {
+            qd_set_password_from_file(actual_pass, &config->sasl_password, qd->connection_manager->log_source);
+
+        }
+        else if (actual_pass) {
+            free(config->sasl_password);
+            config->sasl_password = actual_pass;
+        }
+    }
+
     //
     // Handle the defaults for various settings
     //
@@ -577,7 +594,7 @@ qd_config_ssl_profile_t *qd_dispatch_configure_ssl_profile(qd_dispatch_t *qd, qd
         //
         char *actual_pass = 0;
         bool is_file_path = 0;
-        qd_config_process_password(&actual_pass, ssl_profile->ssl_password, &is_file_path, cm->log_source); CHECK();
+        qd_config_process_password(&actual_pass, ssl_profile->ssl_password, &is_file_path, true, cm->log_source); CHECK();
         if (actual_pass && is_file_path) {
             qd_set_password_from_file(actual_pass, &ssl_profile->ssl_password, cm->log_source);
         }
diff --git a/tests/sasl_files/password.txt b/tests/sasl_files/password.txt
new file mode 100644
index 0000000..f3097ab
--- /dev/null
+++ b/tests/sasl_files/password.txt
@@ -0,0 +1 @@
+password
diff --git a/tests/system_tests_sasl_plain.py b/tests/system_tests_sasl_plain.py
index 2b7d1ee..f35371c 100644
--- a/tests/system_tests_sasl_plain.py
+++ b/tests/system_tests_sasl_plain.py
@@ -22,15 +22,14 @@ from __future__ import division
 from __future__ import absolute_import
 from __future__ import print_function
 
-import os
-from subprocess import PIPE, Popen
-from system_test import TestCase, Qdrouterd, main_module, DIR, TIMEOUT, SkipIfNeeded
-from system_test import unittest
+import os, json
+from subprocess import PIPE, STDOUT, Popen
+from system_test import TestCase, Qdrouterd, main_module, DIR, TIMEOUT, SkipIfNeeded, Process
+from system_test import unittest, QdManager
 from qpid_dispatch.management.client import Node
 from proton import SASL
 
 class RouterTestPlainSaslCommon(TestCase):
-
     @classmethod
     def router(cls, name, connection):
 
@@ -58,6 +57,189 @@ mech_list: ANONYMOUS DIGEST-MD5 EXTERNAL PLAIN
 sql_select: dummy select
 """)
 
+
+class RouterTestPlainSaslFailure(RouterTestPlainSaslCommon):
+    @staticmethod
+    def sasl_file(name):
+        return os.path.join(DIR, 'sasl_files', name)
+
+
+    @classmethod
+    def setUpClass(cls):
+        """
+        Tests the sasl_username, sasl_password property of the dispatch router.
+
+        Creates two routers (QDR.X and QDR.Y) and sets up PLAIN authentication on QDR.X.
+        QDR.Y connects to QDR.X by providing a sasl_username and a bad sasl_password
+        as a non-existent file.
+
+        """
+        super(RouterTestPlainSaslFailure, cls).setUpClass()
+
+        if not SASL.extended():
+            return
+
+        super(RouterTestPlainSaslFailure, cls).createSaslFiles()
+
+        cls.routers = []
+
+        x_listener_port = cls.tester.get_port()
+        y_listener_port = cls.tester.get_port()
+
+        super(RouterTestPlainSaslFailure, cls).router('X', [
+                     ('listener', {'host': '0.0.0.0', 'role': 'inter-router', 'port': x_listener_port,
+                                   'saslMechanisms':'PLAIN', 'authenticatePeer': 'yes'}),
+                     # This unauthenticated listener is for qdstat to connect to it.
+                     ('listener', {'host': '0.0.0.0', 'role': 'normal', 'port': cls.tester.get_port(),
+                                   'authenticatePeer': 'no'}),
+                     ('listener', {'host': '0.0.0.0', 'role': 'normal', 'port': cls.tester.get_port(),
+                                   'saslMechanisms':'PLAIN', 'authenticatePeer': 'yes'}),
+                     ('router', {'workerThreads': 1,
+                                 'id': 'QDR.X',
+                                 'mode': 'interior',
+                                 'saslConfigName': 'tests-mech-PLAIN',
+                                 # Leave as saslConfigPath for testing backward compatibility
+                                 'saslConfigPath': os.getcwd()}),
+        ])
+
+        super(RouterTestPlainSaslFailure, cls).router('Y', [
+                     ('connector', {'host': '0.0.0.0', 'role': 'inter-router', 'port': x_listener_port,
+                                    # Provide a sasl user name and password to connect to QDR.X
+                                   'saslMechanisms': 'PLAIN',
+                                    'saslUsername': 'test@domain.com',
+                                    # Provide a non-existen file.
+                                    'saslPassword': 'file:' + cls.sasl_file('non-existent-password-file.txt')}),
+                     ('router', {'workerThreads': 1,
+                                 'mode': 'interior',
+                                 'id': 'QDR.Y'}),
+                     ('listener', {'host': '0.0.0.0', 'role': 'normal', 'port': y_listener_port}),
+        ])
+
+        cls.routers[0].wait_ports()
+        cls.routers[1].wait_ports()
+        try:
+            # This will time out in 5 seconds because there is no inter-router connection
+            cls.routers[1].wait_connectors(timeout=5)
+        except:
+            pass
+
+    def test_inter_router_sasl_fail(self):
+        passed = False
+        long_type = 'org.apache.qpid.dispatch.connection'
+        qd_manager = QdManager(self, address=self.routers[1].addresses[0])
+        connections = qd_manager.query(long_type)
+        for connection in connections:
+            if connection['role'] == 'inter-router':
+                passed = True
+                break
+
+        # There was no inter-router connection established.
+        self.assertFalse(passed)
+
+        qd_manager = QdManager(self, address=self.routers[1].addresses[0])
+        logs = qd_manager.get_log()
+
+        sasl_failed = False
+        file_open_failed = False
+        for log in logs:
+            if log[0] == 'SERVER' and log[1] == "info" and "amqp:unauthorized-access Authentication failed [mech=PLAIN]" in log[2]:
+                sasl_failed = True
+            if log[0] == "CONN_MGR" and log[1] == "error" and "Unable to open password file" in log[2] and "error: No such file or directory" in log[2]:
+                file_open_failed = True
+
+        self.assertTrue(sasl_failed)
+        self.assertTrue(file_open_failed)
+
+
+class RouterTestPlainSaslFailureUsingLiteral(RouterTestPlainSaslCommon):
+    @staticmethod
+    def sasl_file(name):
+        return os.path.join(DIR, 'sasl_files', name)
+
+
+    @classmethod
+    def setUpClass(cls):
+        """
+        Tests the sasl_username, sasl_password property of the dispatch router.
+
+        Creates two routers (QDR.X and QDR.Y) and sets up PLAIN authentication on QDR.X.
+        QDR.Y connects to QDR.X by providing a sasl_username and a bad sasl_password
+        using the literal: prefix.
+
+        """
+        super(RouterTestPlainSaslFailureUsingLiteral, cls).setUpClass()
+
+        if not SASL.extended():
+            return
+
+        super(RouterTestPlainSaslFailureUsingLiteral, cls).createSaslFiles()
+
+        cls.routers = []
+
+        x_listener_port = cls.tester.get_port()
+        y_listener_port = cls.tester.get_port()
+
+        super(RouterTestPlainSaslFailureUsingLiteral, cls).router('X', [
+                     ('listener', {'host': '0.0.0.0', 'role': 'inter-router', 'port': x_listener_port,
+                                   'saslMechanisms':'PLAIN', 'authenticatePeer': 'yes'}),
+                     # This unauthenticated listener is for qdstat to connect to it.
+                     ('listener', {'host': '0.0.0.0', 'role': 'normal', 'port': cls.tester.get_port(),
+                                   'authenticatePeer': 'no'}),
+                     ('listener', {'host': '0.0.0.0', 'role': 'normal', 'port': cls.tester.get_port(),
+                                   'saslMechanisms':'PLAIN', 'authenticatePeer': 'yes'}),
+                     ('router', {'workerThreads': 1,
+                                 'id': 'QDR.X',
+                                 'mode': 'interior',
+                                 'saslConfigName': 'tests-mech-PLAIN',
+                                 # Leave as saslConfigPath for testing backward compatibility
+                                 'saslConfigPath': os.getcwd()}),
+        ])
+
+        super(RouterTestPlainSaslFailureUsingLiteral, cls).router('Y', [
+                     ('connector', {'host': '0.0.0.0', 'role': 'inter-router', 'port': x_listener_port,
+                                    # Provide a sasl user name and password to connect to QDR.X
+                                   'saslMechanisms': 'PLAIN',
+                                    'saslUsername': 'test@domain.com',
+                                    # Provide the password with a prefix of literal. This should fail..
+                                    'saslPassword': 'literal:password'}),
+                     ('router', {'workerThreads': 1,
+                                 'mode': 'interior',
+                                 'id': 'QDR.Y'}),
+                     ('listener', {'host': '0.0.0.0', 'role': 'normal', 'port': y_listener_port}),
+        ])
+
+        cls.routers[0].wait_ports()
+        cls.routers[1].wait_ports()
+        try:
+            # This will time out in 5 seconds because there is no inter-router connection
+            cls.routers[1].wait_connectors(timeout=5)
+        except:
+            pass
+
+    def test_inter_router_sasl_fail(self):
+        passed = False
+        long_type = 'org.apache.qpid.dispatch.connection'
+
+        qd_manager = QdManager(self, address=self.routers[1].addresses[0])
+        connections = qd_manager.query(long_type)
+
+        for connection in connections:
+            if connection['role'] == 'inter-router':
+                passed = True
+                break
+
+        # There was no inter-router connection established.
+        self.assertFalse(passed)
+        logs = qd_manager.get_log()
+
+        sasl_failed = False
+        for log in logs:
+            if log[0] == 'SERVER' and log[1] == "info" and "amqp:unauthorized-access Authentication failed [mech=PLAIN]" in log[2]:
+                sasl_failed = True
+
+        self.assertTrue(sasl_failed)
+
+
 class RouterTestPlainSasl(RouterTestPlainSaslCommon):
 
     @classmethod
@@ -74,6 +256,8 @@ class RouterTestPlainSasl(RouterTestPlainSaslCommon):
         if not SASL.extended():
             return
 
+        os.environ["ENV_SASL_PASSWORD"] = "password"
+
         super(RouterTestPlainSasl, cls).createSaslFiles()
 
         cls.routers = []
@@ -102,7 +286,7 @@ class RouterTestPlainSasl(RouterTestPlainSaslCommon):
                                     # Provide a sasl user name and password to connect to QDR.X
                                    'saslMechanisms': 'PLAIN',
                                     'saslUsername': 'test@domain.com',
-                                    'saslPassword': 'password'}),
+                                    'saslPassword': 'env:ENV_SASL_PASSWORD'}),
                      ('router', {'workerThreads': 1,
                                  'mode': 'interior',
                                  'id': 'QDR.Y'}),
@@ -195,6 +379,10 @@ class RouterTestPlainSaslOverSsl(RouterTestPlainSaslCommon):
     def ssl_file(name):
         return os.path.join(DIR, 'ssl_certs', name)
 
+    @staticmethod
+    def sasl_file(name):
+        return os.path.join(DIR, 'sasl_files', name)
+
     @classmethod
     def setUpClass(cls):
         """
@@ -249,7 +437,7 @@ class RouterTestPlainSaslOverSsl(RouterTestPlainSaslCommon):
                                     # Provide a sasl user name and password to connect to QDR.X
                                     'saslMechanisms': 'PLAIN',
                                     'saslUsername': 'test@domain.com',
-                                    'saslPassword': 'password'}),
+                                    'saslPassword': 'file:' + cls.sasl_file('password.txt')}),
                      ('router', {'workerThreads': 1,
                                  'mode': 'interior',
                                  'id': 'QDR.Y'}),
@@ -328,6 +516,10 @@ class RouterTestVerifyHostNameYes(RouterTestPlainSaslCommon):
     def ssl_file(name):
         return os.path.join(DIR, 'ssl_certs', name)
 
+    @staticmethod
+    def sasl_file(name):
+        return os.path.join(DIR, 'sasl_files', name)
+
     @classmethod
     def setUpClass(cls):
         """
@@ -375,7 +567,7 @@ class RouterTestVerifyHostNameYes(RouterTestPlainSaslCommon):
                                     'verifyHostName': 'yes',
                                     'saslMechanisms': 'PLAIN',
                                     'saslUsername': 'test@domain.com',
-                                    'saslPassword': 'password'}),
+                                    'saslPassword': 'file:' + cls.sasl_file('password.txt')}),
                      ('router', {'workerThreads': 1,
                                  'mode': 'interior',
                                  'id': 'QDR.Y'}),
@@ -473,7 +665,7 @@ class RouterTestVerifyHostNameNo(RouterTestPlainSaslCommon):
                                     # Provide a sasl user name and password to connect to QDR.X
                                     'saslMechanisms': 'PLAIN',
                                     'verifyHostname': 'no',
-                                    'saslUsername': 'test@domain.com', 'saslPassword': 'password'}),
+                                    'saslUsername': 'test@domain.com', 'saslPassword': 'pass:password'}),
                      ('router', {'workerThreads': 1,
                                  'mode': 'interior',
                                  'id': 'QDR.Y'}),
diff --git a/tests/system_tests_ssl.py b/tests/system_tests_ssl.py
index 67ffca4..fa53fea 100644
--- a/tests/system_tests_ssl.py
+++ b/tests/system_tests_ssl.py
@@ -576,6 +576,8 @@ class RouterTestSslInterRouter(RouterTestSslBase):
         if not SASL.extended():
             return
 
+        os.environ["ENV_SASL_PASSWORD"] = "password"
+
         # Generate authentication DB
         super(RouterTestSslInterRouter, cls).create_sasl_files()
 
@@ -649,7 +651,7 @@ class RouterTestSslInterRouter(RouterTestSslBase):
             # 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': 'password',
+                           'saslUsername': 'test@domain.com', 'saslPassword': 'pass:password',
                            'sslProfile': 'ssl-profile-tls-all'}),
             # SSL Profile for all TLS versions (protocols element not defined)
             ('sslProfile', {'name': 'ssl-profile-tls-all',
@@ -668,7 +670,7 @@ class RouterTestSslInterRouter(RouterTestSslBase):
             # Connector to listener that allows TLSv1.2 only
             ('connector', {'host': '0.0.0.0', 'role': 'inter-router', 'port': cls.PORT_TLS12,
                            'verifyHostname': 'no', 'saslMechanisms': 'PLAIN',
-                           'saslUsername': 'test@domain.com', 'saslPassword': 'password',
+                           'saslUsername': 'test@domain.com', 'saslPassword': 'env:ENV_SASL_PASSWORD',
                            'sslProfile': 'ssl-profile-tls12'}),
             # SSL Profile for TLSv1.2
             ('sslProfile', {'name': 'ssl-profile-tls12',
@@ -688,7 +690,7 @@ class RouterTestSslInterRouter(RouterTestSslBase):
             # Connector to listener that allows TLSv1 and TLSv1.2 only
             ('connector', {'host': '0.0.0.0', 'role': 'inter-router', 'port': cls.PORT_TLS1_TLS12,
                            'verifyHostname': 'no', 'saslMechanisms': 'PLAIN',
-                           'saslUsername': 'test@domain.com', 'saslPassword': 'password',
+                           'saslUsername': 'test@domain.com', 'saslPassword': 'pass:password',
                            'sslProfile': 'ssl-profile-tls1'}),
             # SSL Profile for TLSv1
             ('sslProfile', {'name': 'ssl-profile-tls1',


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