From e0425691d90bce0bbe847a9ff49468ce0fab5486 Mon Sep 17 00:00:00 2001 From: Davanum Srinivas Date: Fri, 15 Aug 2014 13:53:57 -0400 Subject: [PATCH] Make strutils.mask_password more secure Make some enhancements to strutils.mask_password to allow it to catch more cases of passwords in strings. Test cases have been added to test for these newly added situations. The following is a listing of patterns that will be handled. The keyword that mask_password uses (a list of four now) is represented by and the password is shown as . Quotes (both single and double) are represented as . -- -- = = All existing tests and patterns are still handled. Originally submitted in If5ea2d91b1d87c995f50d07a1281879493bd7adb Change-Id: Ifa9a753821484defb5784b136470e3a78ebed3e3 Partial-Bug: #1345233 --- oslo/utils/strutils.py | 42 +++++++++++++++++++++++++++------------ tests/test_strutils.py | 45 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 13 deletions(-) diff --git a/oslo/utils/strutils.py b/oslo/utils/strutils.py index 2b1ff851..5550e772 100644 --- a/oslo/utils/strutils.py +++ b/oslo/utils/strutils.py @@ -57,19 +57,30 @@ _SANITIZE_KEYS = ['adminPass', 'admin_pass', 'password', 'admin_password'] # _SANITIZE_KEYS we already have. This way, we only have to add the new key # to the list of _SANITIZE_KEYS and we can generate regular expressions # for XML and JSON automatically. -_SANITIZE_PATTERNS = [] -_FORMAT_PATTERNS = [r'(%(key)s\s*[=]\s*[\"\']).*?([\"\'])', - r'(<%(key)s>).*?()', - r'([\"\']%(key)s[\"\']\s*:\s*[\"\']).*?([\"\'])', - r'([\'"].*?%(key)s[\'"]\s*:\s*u?[\'"]).*?([\'"])', - r'([\'"].*?%(key)s[\'"]\s*,\s*\'--?[A-z]+\'\s*,\s*u?[\'"])' - '.*?([\'"])', - r'(%(key)s\s*--?[A-z]+\s*)\S+(\s*)'] +_SANITIZE_PATTERNS_2 = [] +_SANITIZE_PATTERNS_1 = [] + +# NOTE(amrith): Some regular expressions have only one parameter, some +# have two parameters. Use different lists of patterns here. +_FORMAT_PATTERNS_1 = [r'(%(key)s\s*[=]\s*)[^\s^\'^\"]+'] +_FORMAT_PATTERNS_2 = [r'(%(key)s\s*[=]\s*[\"\']).*?([\"\'])', + r'(%(key)s\s+[\"\']).*?([\"\'])', + r'([-]{2}%(key)s\s+)[^\'^\"^=^\s]+([\s]*)', + r'(<%(key)s>).*?()', + r'([\"\']%(key)s[\"\']\s*:\s*[\"\']).*?([\"\'])', + r'([\'"].*?%(key)s[\'"]\s*:\s*u?[\'"]).*?([\'"])', + r'([\'"].*?%(key)s[\'"]\s*,\s*\'--?[A-z]+\'\s*,\s*u?' + '[\'"]).*?([\'"])', + r'(%(key)s\s*--?[A-z]+\s*)\S+(\s*)'] for key in _SANITIZE_KEYS: - for pattern in _FORMAT_PATTERNS: + for pattern in _FORMAT_PATTERNS_2: reg_ex = re.compile(pattern % {'key': key}, re.DOTALL) - _SANITIZE_PATTERNS.append(reg_ex) + _SANITIZE_PATTERNS_2.append(reg_ex) + + for pattern in _FORMAT_PATTERNS_1: + reg_ex = re.compile(pattern % {'key': key}, re.DOTALL) + _SANITIZE_PATTERNS_1.append(reg_ex) def int_from_bool_as_string(subject): @@ -218,7 +229,12 @@ def mask_password(message, secret="***"): if not any(key in message for key in _SANITIZE_KEYS): return message - secret = r'\g<1>' + secret + r'\g<2>' - for pattern in _SANITIZE_PATTERNS: - message = re.sub(pattern, secret, message) + substitute = r'\g<1>' + secret + r'\g<2>' + for pattern in _SANITIZE_PATTERNS_2: + message = re.sub(pattern, substitute, message) + + substitute = r'\g<1>' + secret + for pattern in _SANITIZE_PATTERNS_1: + message = re.sub(pattern, substitute, message) + return message diff --git a/tests/test_strutils.py b/tests/test_strutils.py index c9b23718..c8b1fbb5 100644 --- a/tests/test_strutils.py +++ b/tests/test_strutils.py @@ -457,6 +457,51 @@ class MaskPasswordTestCase(test_base.BaseTestCase): self.assertEqual(expected, strutils.mask_password(payload, secret='111')) + payload = 'mysqld --password "aaaaaa"' + expected = 'mysqld --password "****"' + self.assertEqual(expected, + strutils.mask_password(payload, secret='****')) + + payload = 'mysqld --password aaaaaa' + expected = 'mysqld --password ???' + self.assertEqual(expected, + strutils.mask_password(payload, secret='???')) + + payload = 'mysqld --password = "aaaaaa"' + expected = 'mysqld --password = "****"' + self.assertEqual(expected, + strutils.mask_password(payload, secret='****')) + + payload = "mysqld --password = 'aaaaaa'" + expected = "mysqld --password = '****'" + self.assertEqual(expected, + strutils.mask_password(payload, secret='****')) + + payload = "mysqld --password = aaaaaa" + expected = "mysqld --password = ****" + self.assertEqual(expected, + strutils.mask_password(payload, secret='****')) + + payload = "test = password = aaaaaa" + expected = "test = password = 111" + self.assertEqual(expected, + strutils.mask_password(payload, secret='111')) + + payload = "test = password= aaaaaa" + expected = "test = password= 111" + self.assertEqual(expected, + strutils.mask_password(payload, secret='111')) + + payload = "test = password =aaaaaa" + expected = "test = password =111" + self.assertEqual(expected, + strutils.mask_password(payload, secret='111')) + + payload = "test = password=aaaaaa" + expected = "test = password=111" + self.assertEqual(expected, + strutils.mask_password(payload, secret='111')) + payload = 'test = "original_password" : "aaaaaaaaa"' expected = 'test = "original_password" : "***"' self.assertEqual(expected, strutils.mask_password(payload))