From f273e1b751b91dd1b55463bd3bd9aabccea30b3e Mon Sep 17 00:00:00 2001 From: Stanislaw Pitucha Date: Tue, 18 Jun 2013 13:24:50 +0000 Subject: [PATCH] Don't log the credentials by default Even in case of DEBUG level logging credentials (especially those that give admin level access) should not be saved into log files. This way of handling it has the side effect that if someone uses password "password", it will be replaced in another place too... but password "password" or some other keyword that can be found in the request itself was a pretty bad idea to begin with. Shell utilities are not affected and the verbose mode will still display the passwords to make debugging easy. Implements: blueprint limit-credentials-logging Change-Id: I50d0ebbfbd44c7a5b162d9334b4fdbda67e5c28d --- neutronclient/client.py | 21 +++++++++++++++++-- neutronclient/common/clientmanager.py | 22 +++++++++++--------- neutronclient/shell.py | 3 ++- tests/unit/test_auth.py | 29 +++++++++++++++++++++++++++ tests/unit/test_ssl.py | 6 ++++-- 5 files changed, 67 insertions(+), 14 deletions(-) diff --git a/neutronclient/client.py b/neutronclient/client.py index daea317fe..0549965ac 100644 --- a/neutronclient/client.py +++ b/neutronclient/client.py @@ -99,7 +99,8 @@ class HTTPClient(httplib2.Http): token=None, region_name=None, timeout=None, endpoint_url=None, insecure=False, endpoint_type='publicURL', - auth_strategy='keystone', ca_cert=None, **kwargs): + auth_strategy='keystone', ca_cert=None, log_credentials=False, + **kwargs): super(HTTPClient, self).__init__(timeout=timeout, ca_certs=ca_cert) self.username = username @@ -113,6 +114,7 @@ class HTTPClient(httplib2.Http): self.content_type = 'application/json' self.endpoint_url = endpoint_url self.auth_strategy = auth_strategy + self.log_credentials = log_credentials # httplib2 overrides self.disable_ssl_certificate_validation = insecure @@ -132,7 +134,13 @@ class HTTPClient(httplib2.Http): kargs['body'] = kwargs['body'] args = utils.safe_encode_list(args) kargs = utils.safe_encode_dict(kargs) - utils.http_log_req(_logger, args, kargs) + + if self.log_credentials: + log_kargs = kargs + else: + log_kargs = self._strip_credentials(kargs) + + utils.http_log_req(_logger, args, log_kargs) try: resp, body = self.request(*args, **kargs) except httplib2.SSLHandshakeError as e: @@ -150,6 +158,15 @@ class HTTPClient(httplib2.Http): raise exceptions.Forbidden(message=body) return resp, body + def _strip_credentials(self, kwargs): + if kwargs.get('body') and self.password: + log_kwargs = kwargs.copy() + log_kwargs['body'] = kwargs['body'].replace(self.password, + 'REDACTED') + return log_kwargs + else: + return kwargs + def authenticate_and_fetch_endpoint_url(self): if not self.auth_token: self.authenticate() diff --git a/neutronclient/common/clientmanager.py b/neutronclient/common/clientmanager.py index ca8962737..51f01b950 100644 --- a/neutronclient/common/clientmanager.py +++ b/neutronclient/common/clientmanager.py @@ -57,6 +57,7 @@ class ClientManager(object): auth_strategy=None, insecure=False, ca_cert=None, + log_credentials=False, ): self._token = token self._url = url @@ -72,19 +73,22 @@ class ClientManager(object): self._auth_strategy = auth_strategy self._insecure = insecure self._ca_cert = ca_cert + self._log_credentials = log_credentials return def initialize(self): if not self._url: - httpclient = client.HTTPClient(username=self._username, - tenant_name=self._tenant_name, - tenant_id=self._tenant_id, - password=self._password, - region_name=self._region_name, - auth_url=self._auth_url, - endpoint_type=self._endpoint_type, - insecure=self._insecure, - ca_cert=self._ca_cert) + httpclient = client.HTTPClient( + username=self._username, + tenant_name=self._tenant_name, + tenant_id=self._tenant_id, + password=self._password, + region_name=self._region_name, + auth_url=self._auth_url, + endpoint_type=self._endpoint_type, + insecure=self._insecure, + ca_cert=self._ca_cert, + log_credentials=self._log_credentials) httpclient.authenticate() # Populate other password flow attributes self._token = httpclient.auth_token diff --git a/neutronclient/shell.py b/neutronclient/shell.py index f9a3ee1d5..e1190762d 100644 --- a/neutronclient/shell.py +++ b/neutronclient/shell.py @@ -529,7 +529,8 @@ class NeutronShell(app.App): auth_strategy=self.options.os_auth_strategy, endpoint_type=self.options.endpoint_type, insecure=self.options.insecure, - ca_cert=self.options.os_cacert, ) + ca_cert=self.options.os_cacert, + log_credentials=True) return def initialize_app(self, argv): diff --git a/tests/unit/test_auth.py b/tests/unit/test_auth.py index 88d7c7640..b168c8604 100644 --- a/tests/unit/test_auth.py +++ b/tests/unit/test_auth.py @@ -25,6 +25,7 @@ import testtools from neutronclient import client from neutronclient.common import exceptions +from neutronclient.common import utils USERNAME = 'testuser' @@ -323,6 +324,34 @@ class CLITestAuthKeystone(testtools.TestCase): self.client._extract_service_catalog, resources) + def test_strip_credentials_from_log(self): + def verify_no_credentials(kwargs): + return ('REDACTED' in kwargs['body']) and ( + self.client.password not in kwargs['body']) + + def verify_credentials(body): + return 'REDACTED' not in body and self.client.password in body + + self.mox.StubOutWithMock(self.client, "request") + self.mox.StubOutWithMock(utils, "http_log_req") + + res200 = self.mox.CreateMock(httplib2.Response) + res200.status = 200 + + utils.http_log_req(mox.IgnoreArg(), mox.IgnoreArg(), mox.Func( + verify_no_credentials)) + self.client.request( + mox.IsA(str), mox.IsA(str), body=mox.Func(verify_credentials), + headers=mox.IgnoreArg() + ).AndReturn((res200, json.dumps(KS_TOKEN_RESULT))) + utils.http_log_req(mox.IgnoreArg(), mox.IgnoreArg(), mox.IgnoreArg()) + self.client.request( + mox.IsA(str), mox.IsA(str), headers=mox.IsA(dict) + ).AndReturn((res200, '')) + self.mox.ReplayAll() + + self.client.do_request('/resource', 'GET') + class CLITestAuthKeystoneWithId(CLITestAuthKeystone): diff --git a/tests/unit/test_ssl.py b/tests/unit/test_ssl.py index 0370ae657..d7154d74e 100644 --- a/tests/unit/test_ssl.py +++ b/tests/unit/test_ssl.py @@ -60,7 +60,8 @@ class TestSSL(testtools.TestCase): tenant_name=mox.IgnoreArg(), token=mox.IgnoreArg(), url=mox.IgnoreArg(), - username=mox.IgnoreArg() + username=mox.IgnoreArg(), + log_credentials=mox.IgnoreArg(), ) openstack_shell.NeutronShell.interact().AndReturn(0) self.mox.ReplayAll() @@ -88,7 +89,8 @@ class TestSSL(testtools.TestCase): tenant_name=mox.IgnoreArg(), token=mox.IgnoreArg(), url=mox.IgnoreArg(), - username=mox.IgnoreArg() + username=mox.IgnoreArg(), + log_credentials=mox.IgnoreArg(), ) openstack_shell.NeutronShell.interact().AndReturn(0) self.mox.ReplayAll()