From 143d3fbfa1e04778884de5acc08fa6f7fdabb265 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Beraud?= Date: Wed, 3 Nov 2021 17:16:50 +0100 Subject: [PATCH] Fix regex used to mask password Some use cases are poorly handled by the regex used to mask password. Indeed when the password contains quotes or double quotes in the middle such as `pass"word`, the mask_password method will return `***"word`. For more details please see https://bugs.launchpad.net/oslo.utils/+bug/1949623 Closes-Bug: #1949623 Change-Id: I941750b4d49d2d75f0831b24d6dd17f4040f70a2 (cherry picked from commit 6e17ae1f7959c64dfd20a5f67edf422e702426aa) (cherry picked from commit 5ce8a7f0f8ecec7a85a23ec3d7a7fb1cad14ceba) --- oslo_utils/strutils.py | 15 ++++++++++++++- oslo_utils/tests/test_strutils.py | 14 ++++++++++++++ .../fix_mask_password_regex-c0661f95a23369a4.yaml | 7 +++++++ 3 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 releasenotes/notes/fix_mask_password_regex-c0661f95a23369a4.yaml diff --git a/oslo_utils/strutils.py b/oslo_utils/strutils.py index 35b15d30..1a3e8d1f 100644 --- a/oslo_utils/strutils.py +++ b/oslo_utils/strutils.py @@ -73,6 +73,7 @@ _SANITIZE_KEYS = ['adminpass', 'admin_pass', 'password', 'admin_password', # for XML and JSON automatically. _SANITIZE_PATTERNS_2 = {} _SANITIZE_PATTERNS_1 = {} +_SANITIZE_PATTERNS_WILDCARD = {} # NOTE(amrith): Some regular expressions have only one parameter, some # have two parameters. Use different lists of patterns here. @@ -88,6 +89,7 @@ _FORMAT_PATTERNS_2 = [r'(%(key)s[0-9]*\s*[=]\s*[\"\'])[^\"\']*([\"\'])', r'([\'"][^\'"]*%(key)s[0-9]*[\'"]\s*,\s*\'--?[A-z]+' r'\'\s*,\s*u?[\'"])[^\"\']*([\'"])', r'(%(key)s[0-9]*\s*--?[A-z]+\s*)\S+(\s*)'] +_FORMAT_PATTERNS_WILDCARD = [r'([\'\"][^\"\']*%(key)s[0-9]*[\'\"]\s*:\s*u?[\'\"].*[\'\"])[^\"\']*([\'\"])'] # noqa: E501 # NOTE(dhellmann): Keep a separate list of patterns by key so we only # need to apply the substitutions for keys we find using a quick "in" @@ -95,6 +97,7 @@ _FORMAT_PATTERNS_2 = [r'(%(key)s[0-9]*\s*[=]\s*[\"\'])[^\"\']*([\"\'])', for key in _SANITIZE_KEYS: _SANITIZE_PATTERNS_1[key] = [] _SANITIZE_PATTERNS_2[key] = [] + _SANITIZE_PATTERNS_WILDCARD[key] = [] for pattern in _FORMAT_PATTERNS_2: reg_ex = re.compile(pattern % {'key': key}, re.DOTALL | re.IGNORECASE) @@ -104,6 +107,10 @@ for key in _SANITIZE_KEYS: reg_ex = re.compile(pattern % {'key': key}, re.DOTALL | re.IGNORECASE) _SANITIZE_PATTERNS_1[key].append(reg_ex) + for pattern in _FORMAT_PATTERNS_WILDCARD: + reg_ex = re.compile(pattern % {'key': key}, re.DOTALL | re.IGNORECASE) + _SANITIZE_PATTERNS_WILDCARD[key].append(reg_ex) + def int_from_bool_as_string(subject): """Interpret a string as a boolean and return either 1 or 0. @@ -331,6 +338,7 @@ def mask_password(message, secret="***"): # nosec substitute1 = r'\g<1>' + secret substitute2 = r'\g<1>' + secret + r'\g<2>' + substitute_wildcard = r'\g<1>' # NOTE(ldbragst): Check to see if anything in message contains any key # specified in _SANITIZE_KEYS, if not then just return the message since @@ -341,7 +349,12 @@ def mask_password(message, secret="***"): # nosec message = re.sub(pattern, substitute2, message) for pattern in _SANITIZE_PATTERNS_1[key]: message = re.sub(pattern, substitute1, message) - + # NOTE(hberaud): Those case are poorly handled by previous + # patterns. They are passwords with quotes or double quotes. + # They also needs a different way to substitute group this is why + # they aren't fix in the pattern 1 or 2. + for pattern in _SANITIZE_PATTERNS_WILDCARD[key]: + message = re.sub(pattern, substitute_wildcard, message) return message diff --git a/oslo_utils/tests/test_strutils.py b/oslo_utils/tests/test_strutils.py index 0ee35b8f..34e1b479 100644 --- a/oslo_utils/tests/test_strutils.py +++ b/oslo_utils/tests/test_strutils.py @@ -612,11 +612,20 @@ class MaskPasswordTestCase(test_base.BaseTestCase): expected = 'test = "param1" : "value"' self.assertEqual(expected, strutils.mask_password(payload)) + payload = 'test = "original_password" : "aaaaa"aaaa"' + expected = 'test = "original_password" : "***"' + self.assertEqual(expected, strutils.mask_password(payload)) + payload = """{'adminPass':'TL0EfN33'}""" payload = str(payload) expected = """{'adminPass':'***'}""" self.assertEqual(expected, strutils.mask_password(payload)) + payload = """{'adminPass':'TL0E'fN33'}""" + payload = str(payload) + expected = """{'adminPass':'***'}""" + self.assertEqual(expected, strutils.mask_password(payload)) + payload = """{'token':'mytoken'}""" payload = str(payload) expected = """{'token':'***'}""" @@ -692,6 +701,11 @@ class MaskDictionaryPasswordTestCase(test_base.BaseTestCase): self.assertEqual(expected, strutils.mask_dict_password(payload)) + payload = {'password': 'TL0Ef"N33'} + expected = {'password': '***'} + self.assertEqual(expected, + strutils.mask_dict_password(payload)) + payload = {'user': 'admin', 'password': 'TL0EfN33'} expected = {'user': 'admin', 'password': '***'} self.assertEqual(expected, diff --git a/releasenotes/notes/fix_mask_password_regex-c0661f95a23369a4.yaml b/releasenotes/notes/fix_mask_password_regex-c0661f95a23369a4.yaml new file mode 100644 index 00000000..e7bc5582 --- /dev/null +++ b/releasenotes/notes/fix_mask_password_regex-c0661f95a23369a4.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Fix regex used to mask password. The ``strutils.mask_password`` + function will now correctly handle passwords that contain + single or double quotes. Previously, only the characters before the + quote were masked.