Make strutils.mask_password more secure0.2.0

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
<key> and the password is shown as <password>. Quotes (both single and
double) are represented as <quote>.

--<key> <password>
--<key> <quote><password><quote>
<key> = <password>
<key> = <quote><password><quote>

All existing tests and patterns are still handled.

Originally submitted in If5ea2d91b1d87c995f50d07a1281879493bd7adb

Change-Id: Ifa9a753821484defb5784b136470e3a78ebed3e3
Partial-Bug: #1345233

2 files changed, 74 insertions, 13 deletions

diff --git a/oslo/utils/strutils.py b/oslo/utils/strutils.py
index 2b1ff85..5550e77 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>).*?(</%(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>).*?(</%(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 c9b2371..c8b1fbb 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))