From 4d78dfaac2edde5efe56b9550955a761894279e7 Mon Sep 17 00:00:00 2001
From: Matt Riedemann <mriedem@us.ibm.com>
Date: Tue, 8 Nov 2016 13:53:32 -0500
Subject: [PATCH] Mask passwords when logging HTTP req/resp bodies

The very specific 'password' is being masked when logging
requests but not when logging response bodies.

This change fixes the response logging to mask passwords and
also makes the request logging more robust as it was just
checking for 'password' but the mask_password method handles
much more than that.

Change-Id: Id8bf583ecdf60eafb50fd69d6a19180ea97bd92c
Closes-Bug: #1640269
---
 cinderclient/client.py                 |  7 ++--
 cinderclient/tests/unit/test_client.py | 44 ++++++++++++++++++++++++++
 test-requirements.txt                  |  1 +
 3 files changed, 47 insertions(+), 5 deletions(-)

diff --git a/cinderclient/client.py b/cinderclient/client.py
index 4ebc88f6a..a4554a291 100644
--- a/cinderclient/client.py
+++ b/cinderclient/client.py
@@ -277,10 +277,7 @@ class HTTPClient(object):
             string_parts.append(header)
 
         if 'data' in kwargs:
-            if "password" in kwargs['data']:
-                data = strutils.mask_password(kwargs['data'])
-            else:
-                data = kwargs['data']
+            data = strutils.mask_password(kwargs['data'])
             string_parts.append(" -d '%s'" % (data))
         self._logger.debug("\nREQ: %s\n" % "".join(string_parts))
 
@@ -291,7 +288,7 @@ class HTTPClient(object):
             "RESP: [%s] %s\nRESP BODY: %s\n",
             resp.status_code,
             resp.headers,
-            resp.text)
+            strutils.mask_password(resp.text))
 
         # if service name is None then use service_type for logging
         service = self.service_name or self.service_type
diff --git a/cinderclient/tests/unit/test_client.py b/cinderclient/tests/unit/test_client.py
index d9edf95b8..e13a77266 100644
--- a/cinderclient/tests/unit/test_client.py
+++ b/cinderclient/tests/unit/test_client.py
@@ -18,6 +18,7 @@ import fixtures
 from keystoneauth1 import adapter
 from keystoneauth1 import exceptions as keystone_exception
 import mock
+from oslo_serialization import jsonutils
 import six
 
 import cinderclient.client
@@ -269,3 +270,46 @@ class ClientTestSensitiveInfo(utils.TestCase):
 
         output = self.logger.output.split('\n')
         self.assertNotIn(secret_auth_token, output[1])
+
+    def test_resp_does_not_log_sensitive_info(self):
+        self.logger = self.useFixture(
+            fixtures.FakeLogger(
+                format="%(message)s",
+                level=logging.DEBUG,
+                nuke_handlers=True
+            )
+        )
+        cs = cinderclient.client.HTTPClient("user", None, None,
+                                            "http://127.0.0.1:5000")
+        resp = mock.Mock()
+        resp.status_code = 200
+        resp.headers = {
+            'x-compute-request-id': 'req-f551871a-4950-4225-9b2c-29a14c8f075e'
+        }
+        auth_password = "kk4qD6CpKFLyz9JD"
+        body = {
+            "connection_info": {
+                "driver_volume_type": "iscsi",
+                "data": {
+                    "auth_password": auth_password,
+                    "target_discovered": False,
+                    "encrypted": False,
+                    "qos_specs": None,
+                    "target_iqn": ("iqn.2010-10.org.openstack:volume-"
+                                   "a2f33dcc-1bb7-45ba-b8fc-5b38179120f8"),
+                    "target_portal": "10.0.100.186:3260",
+                    "volume_id": "a2f33dcc-1bb7-45ba-b8fc-5b38179120f8",
+                    "target_lun": 1,
+                    "access_mode": "rw",
+                    "auth_username": "s4BfSfZ67Bo2mnpuFWY8",
+                    "auth_method": "CHAP"
+                }
+            }
+        }
+        resp.text = jsonutils.dumps(body)
+        cs.http_log_debug = True
+        cs.http_log_resp(resp)
+
+        output = self.logger.output.split('\n')
+        self.assertIn('***', output[1], output)
+        self.assertNotIn(auth_password, output[1], output)
diff --git a/test-requirements.txt b/test-requirements.txt
index 7d6bc86f0..6fb37cbe0 100644
--- a/test-requirements.txt
+++ b/test-requirements.txt
@@ -16,3 +16,4 @@ tempest>=12.1.0 # Apache-2.0
 testtools>=1.4.0 # MIT
 testrepository>=0.0.18 # Apache-2.0/BSD
 os-testr>=0.8.0 # Apache-2.0
+oslo.serialization>=1.10.0              # Apache-2.0