From 0a8b13b9c6ceb77b0171fe3355b2f0ec145f8491 Mon Sep 17 00:00:00 2001 From: Mark Reynolds Date: Thu, 16 May 2019 00:16:42 +0000 Subject: [PATCH] Ticket 50251 - clear text passwords visable in CLI verbose mode logging Bug Description: If you run any of the CLI tools using "-v", and set a password, that password will be displayed in clear text in the console. Fix Description: Create an internal list of sensitive attributes to filter, and mask them in the operation debug logging. But still allow the password to be seen if you set the env variable DEBUGGING=true We also still print the root DN password if it is a container installation. https://pagure.io/389-ds-base/issue/50251 Reviewed by: spichugi, firstyear, and mhonek (Thanks!!!) Gbp-Pq: Name CVE-2019-10224.patch --- src/lib389/lib389/_constants.py | 6 ++++++ src/lib389/lib389/_entry.py | 7 ++++--- src/lib389/lib389/_mapped_object.py | 12 ++++++------ src/lib389/lib389/instance/setup.py | 6 +++++- src/lib389/lib389/tests/utils_test.py | 11 +++++++++++ src/lib389/lib389/utils.py | 22 +++++++++++++++++++++- 6 files changed, 53 insertions(+), 11 deletions(-) diff --git a/src/lib389/lib389/_constants.py b/src/lib389/lib389/_constants.py index 9b720a6..ee5ed9e 100644 --- a/src/lib389/lib389/_constants.py +++ b/src/lib389/lib389/_constants.py @@ -41,6 +41,12 @@ REPLICATION_BIND_METHOD = RA_METHOD REPLICATION_TRANSPORT = RA_TRANSPORT_PROT REPLICATION_TIMEOUT = RA_TIMEOUT +# Attributes that should be masked from logging output +SENSITIVE_ATTRS = ['userpassword', + 'nsslapd-rootpw', + 'nsds5replicacredentials', + 'nsmultiplexorcredentials'] + TRANS_STARTTLS = "starttls" TRANS_SECURE = "secure" TRANS_NORMAL = "normal" diff --git a/src/lib389/lib389/_entry.py b/src/lib389/lib389/_entry.py index 1f039ff..399e717 100644 --- a/src/lib389/lib389/_entry.py +++ b/src/lib389/lib389/_entry.py @@ -6,7 +6,6 @@ # See LICENSE for details. # --- END COPYRIGHT BLOCK --- -import re import six import logging import ldif @@ -17,12 +16,13 @@ import sys from lib389._constants import * from lib389.properties import * -from lib389.utils import ensure_str, ensure_bytes, ensure_list_bytes +from lib389.utils import (ensure_str, ensure_bytes, ensure_list_bytes, display_log_data) MAJOR, MINOR, _, _, _ = sys.version_info log = logging.getLogger(__name__) + class FormatDict(cidict): def __getitem__(self, name): if name in self: @@ -258,12 +258,13 @@ class Entry(object): def update(self, dct): """Update passthru to the data attribute.""" - log.debug("update dn: %r with %r" % (self.dn, dct)) + log.debug("updating dn: {}".format(self.dn)) for k, v in list(dct.items()): if isinstance(v, list) or isinstance(v, tuple): self.data[k] = v else: self.data[k] = [v] + log.debug("updated dn: {} with {}".format(self.dn, display_log_data(dct))) def __repr__(self): """Convert the Entry to its LDIF representation""" diff --git a/src/lib389/lib389/_mapped_object.py b/src/lib389/lib389/_mapped_object.py index 06a2821..53efaa6 100644 --- a/src/lib389/lib389/_mapped_object.py +++ b/src/lib389/lib389/_mapped_object.py @@ -7,18 +7,18 @@ # See LICENSE for details. # --- END COPYRIGHT BLOCK --- +import os import ldap import ldap.dn from ldap import filter as ldap_filter import logging import json from functools import partial - from lib389._entry import Entry from lib389._constants import DIRSRV_STATE_ONLINE, SER_ROOT_DN, SER_ROOT_PW from lib389.utils import ( ensure_bytes, ensure_str, ensure_int, ensure_list_bytes, ensure_list_str, - ensure_list_int + ensure_list_int, display_log_value, display_log_data ) # This function filter and term generation provided thanks to @@ -360,7 +360,7 @@ class DSLdapObject(DSLogging): action_txt = "UNKNOWN" if value is None or len(value) < 512: - self._log.debug("%s set %s: (%r, %r)" % (self._dn, action_txt, key, value)) + self._log.debug("%s set %s: (%r, %r)" % (self._dn, action_txt, key, display_log_value(key, value))) else: self._log.debug("%s set %s: (%r, value too large)" % (self._dn, action_txt, key)) if self._instance.state != DIRSRV_STATE_ONLINE: @@ -742,11 +742,11 @@ class DSLdapObject(DSLogging): """ assert(len(self._create_objectclasses) > 0) basedn = ensure_str(basedn) - self._log.debug('Checking "%s" under %s : %s' % (rdn, basedn, properties)) + self._log.debug('Checking "%s" under %s : %s' % (rdn, basedn, display_log_data(properties))) # Add the objectClasses to the properties (dn, valid_props) = self._validate(rdn, properties, basedn) # Check if the entry exists or not? .add_s is going to error anyway ... - self._log.debug('Validated dn %s : valid_props %s' % (dn, valid_props)) + self._log.debug('Validated dn {}'.format(dn)) exists = False @@ -774,8 +774,8 @@ class DSLdapObject(DSLogging): e.update({'objectclass': ensure_list_bytes(self._create_objectclasses)}) e.update(valid_props) # We rely on exceptions here to indicate failure to the parent. - self._log.debug('Creating entry %s : %s' % (dn, e)) self._instance.add_ext_s(e, serverctrls=self._server_controls, clientctrls=self._client_controls) + self._log.debug('Created entry %s : %s' % (dn, display_log_data(e.data))) # If it worked, we need to fix our instance dn self._dn = dn return self diff --git a/src/lib389/lib389/instance/setup.py b/src/lib389/lib389/instance/setup.py index 1e3e891..48137c6 100644 --- a/src/lib389/lib389/instance/setup.py +++ b/src/lib389/lib389/instance/setup.py @@ -430,7 +430,7 @@ class SetupDs(object): backend['suffix'] = val break else: - print("The suffix \"{}\" is not a valid DN") + print("The suffix \"{}\" is not a valid DN".format(val)) continue else: backend['suffix'] = suffix @@ -901,6 +901,10 @@ class SetupDs(object): else: self.log.debug("Skipping default SASL maps - no backend found!") + # We need to log the password when containerised + if self.containerised: + self.log.debug("Root DN password: {}".format(slapd['root_password'])) + # Complete. # Change the root password finally ds_instance.config.set('nsslapd-rootpw', diff --git a/src/lib389/lib389/tests/utils_test.py b/src/lib389/lib389/tests/utils_test.py index 8104b62..5378066 100644 --- a/src/lib389/lib389/tests/utils_test.py +++ b/src/lib389/lib389/tests/utils_test.py @@ -134,6 +134,17 @@ def test_formatInfData_withconfigserver(): log.info("content: %r" % ret) +@pytest.mark.parametrize('data', [ + ({'userpaSSwoRd': '1234', 'nsslaPd-rootpw': '5678', 'regularAttr': 'originalvalue'}, + {'userpaSSwoRd': '********', 'nsslaPd-rootpw': '********', 'regularAttr': 'originalvalue'}), + ({'userpassword': ['1', '2', '3'], 'nsslapd-rootpw': ['x']}, + {'userpassword': ['********', '********', '********'], 'nsslapd-rootpw': ['********']}) +]) +def test_get_log_data(data): + before, after = data + assert display_log_data(before) == after + + if __name__ == "__main__": CURRENT_FILE = os.path.realpath(__file__) pytest.main("-s -v %s" % CURRENT_FILE) diff --git a/src/lib389/lib389/utils.py b/src/lib389/lib389/utils.py index 0b90da2..d976282 100644 --- a/src/lib389/lib389/utils.py +++ b/src/lib389/lib389/utils.py @@ -46,7 +46,7 @@ from lib389.paths import Paths from lib389.dseldif import DSEldif from lib389._constants import ( DEFAULT_USER, VALGRIND_WRAPPER, DN_CONFIG, CFGSUFFIX, LOCALHOST, - ReplicaRole, CONSUMER_REPLICAID + ReplicaRole, CONSUMER_REPLICAID, SENSITIVE_ATTRS ) from lib389.properties import ( SER_HOST, SER_USER_ID, SER_GROUP_ID, SER_STRICT_HOSTNAME_CHECKING, SER_PORT, @@ -56,6 +56,8 @@ from lib389.properties import ( MAJOR, MINOR, _, _, _ = sys.version_info +DEBUGGING = os.getenv('DEBUGGING', default=False) + log = logging.getLogger(__name__) # @@ -1170,6 +1172,7 @@ def get_instance_list(prefix=None): insts.sort() return insts + def get_user_is_ds_owner(): # Check if we have permission to administer the DS instance. This is required # for some tasks such as installing, killing, or editing configs for the @@ -1186,3 +1189,20 @@ def get_user_is_ds_owner(): return False +def display_log_value(attr, value, hide_sensitive=True): + # Mask all the sensitive attribute values + if DEBUGGING or not hide_sensitive: + return value + else: + if attr.lower() in SENSITIVE_ATTRS: + if type(value) in (list, tuple): + return list(map(lambda _: '********', value)) + else: + return '********' + else: + return value + + +def display_log_data(data, hide_sensitive=True): + # Take a dict and mask all the sensitive data + return {a: display_log_value(a, v, hide_sensitive) for a, v in data.items()} \ No newline at end of file -- 2.30.2