Make mask_password case insensitive, and add new patterns

It appears that Mistral service logs everything, and doesn't use yet
the mask_password (nor mask_dict_password) method. In order to ensure
all is properly masked, we have to add some new patterns, and make it
case insensitive in order to simplify and avoid duplicated entries.

Change-Id: Icc19b7c8bdb6a3182939d5e9fdef21288b19f43d
Related-Bug: #1850843
Signed-off-by: Cédric Jeanneret <cjeanner@redhat.com>
This commit is contained in:
Cédric Jeanneret 2019-11-01 11:27:37 +01:00 committed by Dougal Matthews
parent c08b1006ef
commit b41268417c
3 changed files with 64 additions and 14 deletions

View File

@ -59,7 +59,7 @@ _SANITIZE_KEYS = ['adminPass', 'admin_pass', 'password', 'admin_password',
'auth_token', 'new_pass', 'auth_password', 'secret_uuid', 'auth_token', 'new_pass', 'auth_password', 'secret_uuid',
'secret', 'sys_pswd', 'token', 'configdrive', 'secret', 'sys_pswd', 'token', 'configdrive',
'CHAPPASSWORD', 'encrypted_key', 'private_key', 'CHAPPASSWORD', 'encrypted_key', 'private_key',
'encryption_key_id'] 'encryption_key_id', 'fernetkey', 'sslkey', 'passphrase']
# NOTE(ldbragst): Let's build a list of regex objects using the list of # NOTE(ldbragst): Let's build a list of regex objects using the list of
# _SANITIZE_KEYS we already have. This way, we only have to add the new key # _SANITIZE_KEYS we already have. This way, we only have to add the new key
@ -70,17 +70,18 @@ _SANITIZE_PATTERNS_1 = {}
# NOTE(amrith): Some regular expressions have only one parameter, some # NOTE(amrith): Some regular expressions have only one parameter, some
# have two parameters. Use different lists of patterns here. # have two parameters. Use different lists of patterns here.
_FORMAT_PATTERNS_1 = [r'(%(key)s\s*[=]\s*)[^\s^\'^\"]+'] _FORMAT_PATTERNS_1 = [r'(%(key)s[0-9]*\s*[=]\s*)[^\s^\'^\"]+']
_FORMAT_PATTERNS_2 = [r'(%(key)s\s*[=]\s*[\"\'])[^\"\']*([\"\'])', _FORMAT_PATTERNS_2 = [r'(%(key)s[0-9]*\s*[=]\s*[\"\'])[^\"\']*([\"\'])',
r'(%(key)s\s+[\"\'])[^\"\']*([\"\'])', r'(%(key)s[0-9]*\s+[\"\'])[^\"\']*([\"\'])',
r'([-]{2}%(key)s\s+)[^\'^\"^=^\s]+([\s]*)', r'([-]{2}%(key)s[0-9]*\s+)[^\'^\"^=^\s]+([\s]*)',
r'(<%(key)s>)[^<]*(</%(key)s>)', r'(<%(key)s[0-9]*>)[^<]*(</%(key)s[0-9]*>)',
r'([\"\']%(key)s[\"\']\s*:\s*[\"\'])[^\"\']*([\"\'])', r'([\"\']%(key)s[0-9]*[\"\']\s*:\s*[\"\'])[^\"\']*'
r'([\'"][^"\']*%(key)s[\'"]\s*:\s*u?[\'"])[^\"\']*' '([\"\'])',
r'([\'"][^"\']*%(key)s[0-9]*[\'"]\s*:\s*u?[\'"])[^\"\']*'
'([\'"])', '([\'"])',
r'([\'"][^\'"]*%(key)s[\'"]\s*,\s*\'--?[A-z]+\'\s*,\s*u?' r'([\'"][^\'"]*%(key)s[0-9]*[\'"]\s*,\s*\'--?[A-z]+'
'[\'"])[^\"\']*([\'"])', '\'\s*,\s*u?[\'"])[^\"\']*([\'"])',
r'(%(key)s\s*--?[A-z]+\s*)\S+(\s*)'] r'(%(key)s[0-9]*\s*--?[A-z]+\s*)\S+(\s*)']
# NOTE(dhellmann): Keep a separate list of patterns by key so we only # 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" # need to apply the substitutions for keys we find using a quick "in"
@ -90,11 +91,11 @@ for key in _SANITIZE_KEYS:
_SANITIZE_PATTERNS_2[key] = [] _SANITIZE_PATTERNS_2[key] = []
for pattern in _FORMAT_PATTERNS_2: for pattern in _FORMAT_PATTERNS_2:
reg_ex = re.compile(pattern % {'key': key}, re.DOTALL) reg_ex = re.compile(pattern % {'key': key}, re.DOTALL | re.IGNORECASE)
_SANITIZE_PATTERNS_2[key].append(reg_ex) _SANITIZE_PATTERNS_2[key].append(reg_ex)
for pattern in _FORMAT_PATTERNS_1: for pattern in _FORMAT_PATTERNS_1:
reg_ex = re.compile(pattern % {'key': key}, re.DOTALL) reg_ex = re.compile(pattern % {'key': key}, re.DOTALL | re.IGNORECASE)
_SANITIZE_PATTERNS_1[key].append(reg_ex) _SANITIZE_PATTERNS_1[key].append(reg_ex)
@ -329,7 +330,7 @@ def mask_password(message, secret="***"): # nosec
# specified in _SANITIZE_KEYS, if not then just return the message since # specified in _SANITIZE_KEYS, if not then just return the message since
# we don't have to mask any passwords. # we don't have to mask any passwords.
for key in _SANITIZE_KEYS: for key in _SANITIZE_KEYS:
if key in message: if key.lower() in message.lower():
for pattern in _SANITIZE_PATTERNS_2[key]: for pattern in _SANITIZE_PATTERNS_2[key]:
message = re.sub(pattern, substitute2, message) message = re.sub(pattern, substitute2, message)
for pattern in _SANITIZE_PATTERNS_1[key]: for pattern in _SANITIZE_PATTERNS_1[key]:

View File

@ -353,6 +353,43 @@ class MaskPasswordTestCase(test_base.BaseTestCase):
payload = """{ 'token' : 'token' }""" payload = """{ 'token' : 'token' }"""
expected = """{ 'token' : '***' }""" expected = """{ 'token' : '***' }"""
self.assertEqual(expected, strutils.mask_password(payload)) self.assertEqual(expected, strutils.mask_password(payload))
# Test 'fernetkey'
payload = """{ 'fernetkey' : 'token' }"""
expected = """{ 'fernetkey' : '***' }"""
self.assertEqual(expected, strutils.mask_password(payload))
# Test 'FernetKey'
payload = """{ 'FernetKey' : 'token' }"""
expected = """{ 'FernetKey' : '***' }"""
self.assertEqual(expected, strutils.mask_password(payload))
# Test 'sslkey'
payload = """{ 'sslkey' : 'token' }"""
expected = """{ 'sslkey' : '***' }"""
self.assertEqual(expected, strutils.mask_password(payload))
# Test 'SslKey'
payload = """{ 'SslKey' : 'token' }"""
expected = """{ 'SslKey' : '***' }"""
self.assertEqual(expected, strutils.mask_password(payload))
# Test 'passphrase'
payload = """{ 'passphrase' : 'token' }"""
expected = """{ 'passphrase' : '***' }"""
self.assertEqual(expected, strutils.mask_password(payload))
# Test 'PassPhrase'
payload = """{ 'PassPhrase' : 'token' }"""
expected = """{ 'PassPhrase' : '***' }"""
self.assertEqual(expected, strutils.mask_password(payload))
# Some real-life cases
# Test 'KeystoneFernetKey1'
payload = """{ 'KeystoneFernetKey1' : 'token' }"""
expected = """{ 'KeystoneFernetKey1' : '***' }"""
self.assertEqual(expected, strutils.mask_password(payload))
# Test 'OctaviaCaKeyPassword'
payload = """{ 'OctaviaCaKeyPassword' : 'token' }"""
expected = """{ 'OctaviaCaKeyPassword' : '***' }"""
self.assertEqual(expected, strutils.mask_password(payload))
# Test 'OctaviaCaKeyPassphrase'
payload = """{ 'OctaviaCaKeyPassphrase' : 'token' }"""
expected = """{ 'OctaviaCaKeyPassphrase' : '***' }"""
self.assertEqual(expected, strutils.mask_password(payload))
def test_xml(self): def test_xml(self):
# Test 'adminPass' w/o spaces # Test 'adminPass' w/o spaces
@ -395,6 +432,10 @@ class MaskPasswordTestCase(test_base.BaseTestCase):
</password>""" </password>"""
expected = """<password>***</password>""" expected = """<password>***</password>"""
self.assertEqual(expected, strutils.mask_password(payload)) self.assertEqual(expected, strutils.mask_password(payload))
# Test 'Password1' - case-insensitive + number
payload = """<Password1>TL0EfN33</Password1>"""
expected = """<Password1>***</Password1>"""
self.assertEqual(expected, strutils.mask_password(payload))
def test_xml_attribute(self): def test_xml_attribute(self):
# Test 'adminPass' w/o spaces # Test 'adminPass' w/o spaces

View File

@ -0,0 +1,8 @@
---
security:
- This patch ensures we actually mask sensitive data, even if case doesn't
match the static entry we have in the patterns.
- It also ensures that some fancy names with a common base, but added
number are actually taken care of.
fixes:
- https://bugs.launchpad.net/tripleo/+bug/1850843