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/09 17:03:41 UTC
[qpid-dispatch] branch master updated: DISPATCH-1440 - Deprecated
passwordFile attribute in sslProfile and modified the password field to
accept openssl style prefixes. This closes #582.
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 7bcd40a DISPATCH-1440 - Deprecated passwordFile attribute in sslProfile and modified the password field to accept openssl style prefixes. This closes #582.
7bcd40a is described below
commit 7bcd40aec8bb059e1ea200c7b126635ba0903139
Author: Ganesh Murthy <gm...@apache.org>
AuthorDate: Fri Oct 4 14:30:17 2019 -0400
DISPATCH-1440 - Deprecated passwordFile attribute in sslProfile and modified the password field to accept openssl style prefixes. This closes #582.
---
docs/books/user-guide/configuration-reference.adoc | 4 +-
docs/books/user-guide/configuration-security.adoc | 2 +-
...ecting-using-mutual-ssl-tls-authentication.adoc | 5 +-
.../modules/enabling-ssl-tls-encryption.adoc | 7 +-
.../securing-connections-between-routers.adoc | 7 +-
python/qpid_dispatch/management/qdrouter.json | 6 +-
src/connection_manager.c | 145 ++++++++++++++-------
tests/system_tests_user_id.py | 22 +++-
8 files changed, 134 insertions(+), 64 deletions(-)
diff --git a/docs/books/user-guide/configuration-reference.adoc b/docs/books/user-guide/configuration-reference.adoc
index c25cd2c..5d09083 100644
--- a/docs/books/user-guide/configuration-reference.adoc
+++ b/docs/books/user-guide/configuration-reference.adoc
@@ -76,8 +76,8 @@ Attributes for setting SSL/TLS configuration for connections.
* *_caCertFile_* (path) : The absolute path to the database that contains the public certificates of trusted certificate authorities (CA).
* *_certFile_* (path) : The absolute path to the file containing the PEM-formatted public certificate to be used on the local end of any connections using this profile.
* *_privateKeyFile_* (path) : The absolute path to the file containing the PEM-formatted private key for the above certificate.
-* *_passwordFile_* (path) : If the above private key is password protected, this is the absolute path to a file containing the password that unlocks the certificate key.
-* *_password_* (string) : An alternative to storing the password in a file referenced by passwordFile is to supply the password right here in the configuration file. This option can be used by supplying the password in the ‘password’ option. Don’t use both password and passwordFile in the same profile.
+* *_passwordFile_* (path) : (DEPRECATED) If the above private key is password protected, this is the absolute path to the file containing the password that unlocks the certificate key. This file should be permission protected to limit access. This has been deprecated. Use the file: prefix in the password field to specify the absolute path of the file containing the password. If both password and passwordFile are provided, the passwordFile is ignored.
+* *_password_* (string) : Password that unlocks the certificate key. Supports three prefixes namely - env:, file: pass:. Also supports the legacy literal: prefix. env:var obtains the password from the environment variable var. Since the environment of other processes is visible on certain platforms (e.g. ps under certain Unix OSes) this option should be used with caution. file:absolutepath obtains password from the absolute path of the file containing the password. This option is the saf [...]
* *_uidFormat_* (string) : A list of x509 client certificate fields that will be used to build a string that will uniquely identify the client certificate owner. For example, a value of ‘cou’ indicates that the uid will consist of c - common name concatenated with o - organization-company name concatenated with u - organization unit; or a value of ‘oF’ indicates that the uid will consist of o (organization name) concatenated with F (the sha256 fingerprint of the entire certificate) . All [...]
* *_uidNameMappingFile_* (string) : The absolute path to the file containing the unique id to display name mapping.
* *_name_* (string) : The name of the profile used for referencing it from _listener_ and _connector_ sections.
diff --git a/docs/books/user-guide/configuration-security.adoc b/docs/books/user-guide/configuration-security.adoc
index d0e4018..8008da8 100644
--- a/docs/books/user-guide/configuration-security.adoc
+++ b/docs/books/user-guide/configuration-security.adoc
@@ -133,7 +133,7 @@ For example:
privateKeyFile: /qdrouterd/ssl_certs/router-key-pwd.pem
----
-`passwordFile` or `password`:: If the private key is password-protected, you must provide the password by either specifying the absolute path to a file containing the password that unlocks the certificate key, or entering the password directly in the configuration file.
+`passwordFile` or `password`:: If the private key is password-protected, you must provide the password by either specifying the absolute path to a file containing the password that unlocks the certificate key, or entering the password directly in the configuration file. Entering the password directly in the configuration file is unsafe. passwordFile has been deprecated. Use password.
+
For example:
+
diff --git a/docs/books/user-guide/modules/connecting-using-mutual-ssl-tls-authentication.adoc b/docs/books/user-guide/modules/connecting-using-mutual-ssl-tls-authentication.adoc
index 6bc9e05..cf773e6 100644
--- a/docs/books/user-guide/modules/connecting-using-mutual-ssl-tls-authentication.adoc
+++ b/docs/books/user-guide/modules/connecting-using-mutual-ssl-tls-authentication.adoc
@@ -48,6 +48,7 @@ sslProfile {
certFile: /etc/qpid-dispatch-certs/tls.crt
privateKeyFile: /etc/qpid-dispatch-certs/tls.key
caCertFile: /etc/qpid-dispatch-certs/ca.crt
+ password: file:/etc/qpid-dispatch-certs/
...
}
----
@@ -55,9 +56,11 @@ sslProfile {
`certFile`:: The absolute path to the file containing the public certificate for this router.
+`caCertFile`:: The absolute path to the CA certificate that was used to sign the router's certificate.
+
`privateKeyFile`:: The absolute path to the file containing the private key for this router's public certificate.
-`caCertFile`:: The absolute path to the CA certificate that was used to sign the router's certificate.
+`password`:: The password to unlock the certificate key. Not specified if certificate key has no password. The prefix file: is used to specify the absolute path of the file containing the password.
--
. Configure the `connector` for this connection to use the `sslProfile` that you created.
diff --git a/docs/books/user-guide/modules/enabling-ssl-tls-encryption.adoc b/docs/books/user-guide/modules/enabling-ssl-tls-encryption.adoc
index 5720f6c..ba32dec 100644
--- a/docs/books/user-guide/modules/enabling-ssl-tls-encryption.adoc
+++ b/docs/books/user-guide/modules/enabling-ssl-tls-encryption.adoc
@@ -46,8 +46,9 @@ This `sslProfile` contains the locations of the private key and certificates tha
sslProfile {
name: service-tls
certFile: /etc/qpid-dispatch-certs/normal/tls.crt
- privateKeyFile: /etc/qpid-dispatch-certs/normal/tls.key
caCertFile: /etc/qpid-dispatch-certs/client-ca/ca.crt
+ privateKeyFile: /etc/qpid-dispatch-certs/normal/tls.key
+ password: file:/etc/qpid-dispatch-certs/inter-router/password.txt
...
}
----
@@ -55,9 +56,11 @@ sslProfile {
`certFile`:: The absolute path to the file containing the public certificate for this router.
+`caCertFile`:: The absolute path to the CA certificate that was used to sign the router's certificate.
+
`privateKeyFile`:: The absolute path to the file containing the private key for this router's public certificate.
-`caCertFile`:: The absolute path to the CA certificate that was used to sign the router's certificate.
+`password`:: The password to unlock the certificate key. Not specified if certificate key has no password. The prefix file: is used to specify the absolute path of the file containing the password.
--
. Configure the `listener` for this connection to use SSL/TLS to encrypt the connection.
diff --git a/docs/books/user-guide/modules/securing-connections-between-routers.adoc b/docs/books/user-guide/modules/securing-connections-between-routers.adoc
index 10e1460..f69c2f8 100644
--- a/docs/books/user-guide/modules/securing-connections-between-routers.adoc
+++ b/docs/books/user-guide/modules/securing-connections-between-routers.adoc
@@ -56,8 +56,9 @@ This `sslProfile` contains the locations of the private key and certificates tha
sslProfile {
name: inter-router-tls
certFile: /etc/qpid-dispatch-certs/inter-router/tls.crt
- privateKeyFile: /etc/qpid-dispatch-certs/inter-router/tls.key
caCertFile: /etc/qpid-dispatch-certs/inter-router/ca.crt
+ privateKeyFile: /etc/qpid-dispatch-certs/inter-router/tls.key
+ password: file:/etc/qpid-dispatch-certs/inter-router/password.txt
...
}
----
@@ -65,9 +66,11 @@ sslProfile {
`certFile`:: The absolute path to the file containing the public certificate for this router.
+`caCertFile`:: The absolute path to the CA certificate that was used to sign the router's certificate.
+
`privateKeyFile`:: The absolute path to the file containing the private key for this router's public certificate.
-`caCertFile`:: The absolute path to the CA certificate that was used to sign the router's certificate.
+`password`:: The password to unlock the certificate key. Not specified if certificate key has no password. The prefix file: is used to specify the absolute path of the file containing the password.
--
.. Configure the inter-router `connector` for this connection to use the `sslProfile` that you created.
diff --git a/python/qpid_dispatch/management/qdrouter.json b/python/qpid_dispatch/management/qdrouter.json
index 0b8262c..3cd092f 100644
--- a/python/qpid_dispatch/management/qdrouter.json
+++ b/python/qpid_dispatch/management/qdrouter.json
@@ -639,14 +639,14 @@
},
"passwordFile": {
"type": "path",
- "description": "If the above private key is password protected, this is the absolute path to a file containing the password that unlocks the certificate key. This file should be permission protected to limit access",
+ "description": "(DEPRECATED) If the above private key is password protected, this is the absolute path to the file containing the password that unlocks the certificate key. This file should be permission protected to limit access. This has been deprecated. Use the file: prefix in the password field to specify the absolute path of the file containing the password. If both password and passwordFile are provided, the passwordFile is ignored",
+ "deprecated": true,
"create": true
},
"password": {
"type": "string",
- "description": "(DEPRECATED) An alternative to storing the password in a file referenced by passwordFile is to supply the password right here in the configuration file. This takes precedence over the passwordFile if both are specified. This attribute has been deprecated because it is unsafe to store plain text passwords in config files. Use the passwordFile instead",
- "deprecated": true,
+ "description": "Password that unlocks the certificate key. Supports three prefixes namely - env:, file: pass:. Also supports the legacy literal: prefix. env:var obtains the password from the environment variable var. Since the environment of other processes is visible on certain platforms (e.g. ps under certain Unix OSes) this option should be used with caution. file:absolutepath obtains the password from the absolute path of the file containing the password. This op [...]
"create": true
},
diff --git a/src/connection_manager.c b/src/connection_manager.c
index b571555..9593198 100644
--- a/src/connection_manager.c
+++ b/src/connection_manager.c
@@ -32,6 +32,7 @@
#include <string.h>
#include <stdio.h>
#include <inttypes.h>
+#include <errno.h>
struct qd_config_ssl_profile_t {
DEQ_LINKS(qd_config_ssl_profile_t);
@@ -104,6 +105,44 @@ static qd_config_ssl_profile_t *qd_find_ssl_profile(qd_connection_manager_t *cm,
}
/**
+ * Read the file from the password_file location on the file system and populate password_field with the
+ * contents of the file.
+ */
+static void qd_set_password_from_file(char *password_file, char **password_field, qd_log_source_t *log_source)
+{
+ if (password_file) {
+ FILE *file = fopen(password_file, "r");
+
+ if (file == NULL) {
+ //
+ // The global variable errno (found in <errno.h>) contains information about what went wrong; you can use perror() to print that information as a readable string
+ //
+ qd_log(log_source, QD_LOG_ERROR, "Unable to open password file %s, error: %s", password_file, strerror(errno));
+ return;
+ }
+
+ char buffer[200];
+
+ int c;
+ int i=0;
+
+ while (i < 200 - 1) {
+ c = fgetc(file);
+ if (c == EOF || c == '\n')
+ break;
+ buffer[i++] = c;
+ }
+
+ if (i != 0) {
+ buffer[i] = '\0';
+ free(*password_field);
+ *password_field = strdup(buffer);
+ }
+ fclose(file);
+ }
+}
+
+/**
* Search the list of config_sasl_plugins for an sasl-profile that matches the passed in name
*/
static qd_config_sasl_plugin_t *qd_find_sasl_plugin(qd_connection_manager_t *cm, char *name)
@@ -210,9 +249,8 @@ 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_ssl_profile_process_password(qd_config_ssl_profile_t* ssl_profile)
+static void qd_config_process_password(char **actual_val, char *pw, bool *is_file, qd_log_source_t *log_source)
{
- char *pw = ssl_profile->ssl_password;
if (!pw)
return;
@@ -230,29 +268,52 @@ static void qd_config_ssl_profile_process_password(qd_config_ssl_profile_t* ssl_
//
// Replace the allocated directive with the looked-up password
//
- free(ssl_profile->ssl_password);
- ssl_profile->ssl_password = strdup(passwd);
+ *actual_val = strdup(passwd);
} else {
qd_error(QD_ERROR_NOT_FOUND, "Failed to find a password in the environment variable");
}
}
//
- // If the "password" starts with "literal:" then
+ // If the "password" starts with "literal:" or "pass:" then
// the remaining text is the password and the heading should be
// stripped off
//
- else if (strncmp(pw, "literal:", 8) == 0) {
- // skip the "literal:" header
- pw += 8;
+ else if (strncmp(pw, "literal:", 8) == 0 || 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) {
+ // skip the "literal:" header
+ pw += 8;
+ }
+ else {
+ // skip the "pass:" header
+ pw += 5;
+ }
- // skip the whitespace if it is there
- while (*pw == ' ') ++pw;
+ //
+ // Replace the password with a copy of the string after "literal: or "pass:"
+ //
+ char *copy = strdup(pw);
+ *actual_val = copy;
+ }
+ //
+ // If the password starts with a file: literal set the is_file to true.
+ //
+ else if (strncmp(pw, "file:", 5) == 0) {
+ pw += 5;
- // Replace the password with a copy of the string after "literal:"
+ // Replace the password with a copy of the string after "file:"
char *copy = strdup(pw);
- free(ssl_profile->ssl_password);
- ssl_profile->ssl_password = copy;
+ *actual_val = copy;
+ *is_file = true;
+ }
+ else {
+ //
+ // THe password field does not have any prefixes. Use it as plain text
+ //
+ qd_log(log_source, QD_LOG_WARNING, "It is unsafe to provide plain text passwords in the config file");
+
}
}
@@ -508,42 +569,33 @@ qd_config_ssl_profile_t *qd_dispatch_configure_ssl_profile(qd_dispatch_t *qd, qd
ssl_profile->ssl_certificate_file = qd_entity_opt_string(entity, "certFile", 0); CHECK();
ssl_profile->ssl_private_key_file = qd_entity_opt_string(entity, "privateKeyFile", 0); CHECK();
ssl_profile->ssl_password = qd_entity_opt_string(entity, "password", 0); CHECK();
+ char *password_file = qd_entity_opt_string(entity, "passwordFile", 0); CHECK();
if (ssl_profile->ssl_password) {
- qd_log(cm->log_source, QD_LOG_WARNING, "Attribute password of entity sslProfile has been deprecated. Use passwordFile instead.");
+ //
+ // Process the password to handle any modifications or lookups needed
+ //
+ 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();
+ if (actual_pass && is_file_path) {
+ qd_set_password_from_file(actual_pass, &ssl_profile->ssl_password, cm->log_source);
+ }
+ else if (actual_pass) {
+ free(ssl_profile->ssl_password);
+ ssl_profile->ssl_password = actual_pass;
+ }
+ }
+ else if (password_file) {
+ //
+ // Warn the user that the passwordFile attribute has been deprecated.
+ //
+ qd_log(cm->log_source, QD_LOG_WARNING, "Attribute passwordFile of entity sslProfile has been deprecated. Use password field with the file: prefix instead.");
+ qd_set_password_from_file(password_file, &ssl_profile->ssl_password, cm->log_source);
}
+ free(password_file);
- if (!ssl_profile->ssl_password) {
- // SSL password not provided. Check if passwordFile property is specified.
- char *password_file = qd_entity_opt_string(entity, "passwordFile", 0); CHECK();
-
- if (password_file) {
- FILE *file = fopen(password_file, "r");
-
- if (file) {
- char buffer[200];
-
- int c;
- int i=0;
-
- while (i < 200 - 1) {
- c = fgetc(file);
- if (c == EOF || c == '\n')
- break;
- buffer[i++] = c;
- }
-
- if (i != 0) {
- buffer[i] = '\0';
- free(ssl_profile->ssl_password);
- ssl_profile->ssl_password = strdup(buffer);
- }
- fclose(file);
- }
- }
- free(password_file);
- }
ssl_profile->ssl_ciphers = qd_entity_opt_string(entity, "ciphers", 0); CHECK();
ssl_profile->ssl_protocols = qd_entity_opt_string(entity, "protocols", 0); CHECK();
ssl_profile->ssl_trusted_certificate_db = qd_entity_opt_string(entity, "caCertFile", 0); CHECK();
@@ -551,11 +603,6 @@ qd_config_ssl_profile_t *qd_dispatch_configure_ssl_profile(qd_dispatch_t *qd, qd
ssl_profile->ssl_uid_format = qd_entity_opt_string(entity, "uidFormat", 0); CHECK();
ssl_profile->uid_name_mapping_file = qd_entity_opt_string(entity, "uidNameMappingFile", 0); CHECK();
- //
- // Process the password to handle any modifications or lookups needed
- //
- qd_config_ssl_profile_process_password(ssl_profile); CHECK();
-
qd_log(cm->log_source, QD_LOG_INFO, "Created SSL Profile with name %s ", ssl_profile->name);
return ssl_profile;
diff --git a/tests/system_tests_user_id.py b/tests/system_tests_user_id.py
index 827cd0f..7798c68 100644
--- a/tests/system_tests_user_id.py
+++ b/tests/system_tests_user_id.py
@@ -38,6 +38,8 @@ class QdSSLUseridTest(TestCase):
def setUpClass(cls):
super(QdSSLUseridTest, cls).setUpClass()
+ os.environ["TLS_SERVER_PASSWORD"] = "server-password"
+
ssl_profile1_json = os.path.join(DIR, 'displayname_files', 'profile_names1.json')
ssl_profile2_json = os.path.join(DIR, 'displayname_files', 'profile_names2.json')
@@ -90,7 +92,9 @@ class QdSSLUseridTest(TestCase):
'certFile': cls.ssl_file('server-certificate.pem'),
'privateKeyFile': cls.ssl_file('server-private-key.pem'),
'uidFormat': 'cs5',
- 'password': 'server-password'}),
+ # Use the env: prefix TLS_SERVER_PASSWORD. The TLS_SERVER_PASSWORD
+ # is set to 'server-password'
+ 'password': 'env:TLS_SERVER_PASSWORD'}),
# no fingerprint field
('sslProfile', {'name': 'server-ssl7',
@@ -113,7 +117,9 @@ class QdSSLUseridTest(TestCase):
'caCertFile': cls.ssl_file('ca-certificate.pem'),
'certFile': cls.ssl_file('server-certificate.pem'),
'privateKeyFile': cls.ssl_file('server-private-key.pem'),
- 'password': 'server-password'}),
+ # Use the prefix 'file:' for the password. This should read the file and
+ # use the password from the file.
+ 'password': 'file:' + cls.ssl_file('server-password-file.txt')}),
# one component of uidFormat is invalid (x), this will result in an error in the fingerprint calculation.
# The user_id will fall back to proton's pn_transport_get_user
@@ -131,7 +137,9 @@ class QdSSLUseridTest(TestCase):
'certFile': cls.ssl_file('server-certificate.pem'),
'privateKeyFile': cls.ssl_file('server-private-key.pem'),
'uidFormat': 'abxd',
- 'password': 'server-password'}),
+ # Use the prefix 'literal:'. This makes sure we maintain
+ #backward compatability
+ 'password': 'literal:server-password'}),
('sslProfile', {'name': 'server-ssl12',
'caCertFile': cls.ssl_file('ca-certificate.pem'),
@@ -139,7 +147,8 @@ class QdSSLUseridTest(TestCase):
'privateKeyFile': cls.ssl_file('server-private-key.pem'),
'uidFormat': '1',
'uidNameMappingFile': ssl_profile1_json,
- 'password': 'server-password'}),
+ # Use the pass: followed by the actual password
+ 'password': 'pass:server-password'}),
# should translate a display name
# specifying both passwordFile and password, password takes precedence.
@@ -150,6 +159,9 @@ class QdSSLUseridTest(TestCase):
'uidFormat': '2',
'uidNameMappingFile': ssl_profile2_json,
'password': 'server-password',
+ # If both passwordFile and password are provided, the passwordFile is ignored.
+ # Here we specify a bad password in the passwordFile and
+ # it is ignored.
'passwordFile': cls.ssl_file('server-password-file-bad.txt')}),
('sslProfile', {'name': 'server-ssl14',
@@ -158,6 +170,8 @@ class QdSSLUseridTest(TestCase):
'privateKeyFile': cls.ssl_file('server-private-key.pem'),
'uidFormat': '1',
'uidNameMappingFile': ssl_profile1_json,
+ # Use just the deprecated passwordFile entity to make sure it is backward
+ # compatible.
'passwordFile': cls.ssl_file('server-password-file.txt')}),
('listener', {'port': cls.tester.get_port(), 'sslProfile': 'server-ssl1', 'authenticatePeer': 'yes',
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org