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 <[email protected]>
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: [email protected]
For additional commands, e-mail: [email protected]
