From 4bac2b282e3a01e7a9a035d8b8c921024ecb6261 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=A9dric=20Jeanneret?= <cjeanner@redhat.com>
Date: Fri, 1 Nov 2019 11:47:35 +0100
Subject: [PATCH] Ensure we mask sensitive data from Mistral Action logs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Mistral didn't make use of the oslo_utils "mask_password" methods,
leading in sensitive data leakage in its logs.

This patch corrects this security issue.
Note that it depends on oslo_utils patch adding new patterns, and
ensuring it's case-insensitive.

Change-Id: I544d3c172f2dea02c62c49c311c4b5954413ae15
Related-Bug: #1850843
Co-Authored-By: Dougal Matthews <dougal@redhat.com>
Signed-off-by: Cédric Jeanneret <cjeanner@redhat.com>
---
 mistral_lib/actions/types.py                    |  5 ++++-
 mistral_lib/tests/test_utils.py                 | 17 +++++++++++++++++
 mistral_lib/utils/__init__.py                   | 11 +++++++++++
 .../notes/mask-password-6899d868d213f722.yaml   |  5 +++++
 4 files changed, 37 insertions(+), 1 deletion(-)
 create mode 100644 releasenotes/notes/mask-password-6899d868d213f722.yaml

diff --git a/mistral_lib/actions/types.py b/mistral_lib/actions/types.py
index cd8bf28..a77b96f 100644
--- a/mistral_lib/actions/types.py
+++ b/mistral_lib/actions/types.py
@@ -32,8 +32,11 @@ class Result(serialization.MistralSerializable):
         )
 
     def cut_repr(self):
+        _data = utils.mask_data(self.data)
+        _error = utils.mask_data(self.error)
+        _cancel = utils.mask_data(self.cancel)
         return 'Result [data=%s, error=%s, cancel=%s]' % (
-            utils.cut(self.data), utils.cut(self.error), str(self.cancel)
+            utils.cut(_data), utils.cut(_error), str(_cancel)
         )
 
     def is_cancel(self):
diff --git a/mistral_lib/tests/test_utils.py b/mistral_lib/tests/test_utils.py
index 0cba20b..23bdb3d 100644
--- a/mistral_lib/tests/test_utils.py
+++ b/mistral_lib/tests/test_utils.py
@@ -250,3 +250,20 @@ class TestUtils(tests_base.TestCase):
             d[i] = {'value': 'This is a string that exceeds 35 characters'}
         s = utils.cut(d, 65500)
         self.assertThat(len(s), ttm.Not(ttm.GreaterThan(65500)))
+
+    def test_mask_data(self):
+        payload = {'adminPass': 'fooBarBaz'}
+        expected = {'adminPass': '***'}
+        self.assertEqual(expected, utils.mask_data(payload))
+
+        payload = """adminPass='fooBarBaz'"""
+        expected = """adminPass='***'"""
+        self.assertEqual(expected, utils.mask_data(payload))
+
+        payload = [{'adminPass': 'fooBarBaz'}, {"new_pass": "blah"}]
+        expected = [{'adminPass': '***'}, {"new_pass": "***"}]
+        self.assertEqual(expected, utils.mask_data(payload))
+
+        payload = ["adminPass", 'fooBarBaz']
+        expected = ["adminPass", 'fooBarBaz']
+        self.assertEqual(expected, utils.mask_data(payload))
diff --git a/mistral_lib/utils/__init__.py b/mistral_lib/utils/__init__.py
index f08f15f..db9838b 100644
--- a/mistral_lib/utils/__init__.py
+++ b/mistral_lib/utils/__init__.py
@@ -28,6 +28,8 @@ import threading
 import eventlet
 from eventlet import corolocal
 from oslo_log import log as logging
+from oslo_utils.strutils import mask_dict_password
+from oslo_utils.strutils import mask_password
 from oslo_utils import timeutils
 from oslo_utils import uuidutils
 import pkg_resources as pkg
@@ -484,3 +486,12 @@ def generate_string(length):
         string.ascii_uppercase + string.digits)
         for _ in range(length)
     )
+
+
+def mask_data(obj):
+    if isinstance(obj, dict):
+        return mask_dict_password(obj)
+    elif isinstance(obj, list):
+        return [mask_data(i) for i in obj]
+    else:
+        return mask_password(obj)
diff --git a/releasenotes/notes/mask-password-6899d868d213f722.yaml b/releasenotes/notes/mask-password-6899d868d213f722.yaml
new file mode 100644
index 0000000..5178a04
--- /dev/null
+++ b/releasenotes/notes/mask-password-6899d868d213f722.yaml
@@ -0,0 +1,5 @@
+---
+security:
+  - Ensure we mask sensitive data before logging Action return values
+fixes:
+  - https://bugs.launchpad.net/tripleo/+bug/1850843