Debug data isn't sanitized

[Derek Higgins]

From https://bugzilla.redhat.com/show_bug.cgi?id=1346089#c10

the debug output is displaying unsanitized user data and could result in people unintentionally sharing secrets, it should be passed through strutils.mask_password in oslo_util to sanitize all known secrets

In this case the password "a12345" is been logged in debug mode,
$ ironic --debug node-create -d pxe_ipmitool -i ipmi_password=$PASSWORD
<snip/>
DEBUG (session:337) REQ: curl -g -i -X POST http://192.0.2.1:6385/v1/nodes -H "X-OpenStack-Ironic-API-Version: 1.9" -H "User-Agent: python-ironicclient" -H "Content-Type: application/json" -H "Accept: application/json" -H "X-Auth-Token: {SHA1}874b5da517e12f3a17e3afaa15be8eabffc190a3" -d '{"driver": "pxe_ipmitool", "driver_info": {"ipmi_password": "a12345"}}'
INFO (connectionpool:213) Starting new HTTP connection (1): 192.0.2.1
<snip/>

+-------------------+--------------------------------------+
| Property | Value |
+-------------------+--------------------------------------+
| chassis_uuid | |
| driver | pxe_ipmitool |
| driver_info | {u'ipmi_password': u'******'} |
| extra | {} |
| name | None |
| network_interface | |
| properties | {} |
| resource_class | |
| uuid | 7c45c974-6462-48fe-9bfe-98ea864409b3 |
+-------------------+--------------------------------------+

[Derek Higgins (derekh)]

possible fix attached

[Morgan Fainberg (mdrnstm)]

I am speaking as a member of the keystonecoresec team not as a VMT member (as Keystoneauth is not officially covered by the VMT).

We cannot add any oslo dependencies to keystoneauth. You can however pass your own logger to keystoneauth's session when making the request the does explicit masking. https://github.com/openstack/keystoneauth/blob/01e0122a14c9a61a50038df67c008455f6cffd90/keystoneauth1/session.py#L395

Oslo is explicitly banned in this project due to the volume of transient dependencies that come from the libraries. This is to ensure keystoneauth does not grow dependencies that bog down the loading / instantiation of the session objects and plugins.

As a final note, we typically do not view "debug" logs as something that needs to explicitly be sanitized (it is a nice-to have). Ideally services should not be run in debug and masking all the data at a debug level can hamper development work.

This bug can probably be marked as public / not under embargo as the bug should not be explicitly exploitable. To use the VMT taxonomy this would be a Class B3 ( https://security.openstack.org/vmt-process.html#incident-report-taxonomy ).

In an effort to follow closely to the VMT process (until Keystoneauth is under VMT management officially) this bug will remain private until further comments from keystone-coresec who has been subscribed.

[David Stanek (dstanek)]

I agree with all of Morgan's points. We don't spend too much time thinking about debug logging since we view it is bad production practice to run a production instance in debug mode. I'm sure if you look at the debug logs over time we'd also be leaking PII.

[Steve Martinelli (stevemar)]

Agree with David and Morgan. We could probably copy the oslo.utils masking code over to keystoneauth utils and maintain a copy, I agree with Morgan that we don't want to depend on oslo.*.

[Morgan Fainberg (mdrnstm)]

I also want to point out the red hat bug is public, which means this bug should be made public.

I would prefer not to carry a copy of the mask utils from oslo and lean on the consumers of keystoneauth to pass a logger that does the masking... however, I wouldn't block the fix to carry a copy of the mask_password code.

So, tl;dr:

* Lets make this public and get the fix(es) developed and submitted to gerrit instead of under an embargo (since red hat bug is public)

* Lets evaluate the options (carry a copy of mask_password or provide clear documentation on how to mask sensitive info in a logger and get the consumers of keystoneauth to use oslo's mask_password where possible) - both options are valid fixes.

[Lance Bragstad (lbragstad)]

One thing we could do would be to officially document Morgan's logging solution. I like that idea because it doesn't require duplicated code between keystoneauth and oslo, especially since it deals with masking sensitive information. If we get into copying that code over to keystoneauth, we'll have to be mindful that we should keep both libraries in the same state, which can be a pain for maintainer and frustrating for users when/if there are inconsistencies.

[Morgan Fainberg (mdrnstm)]

Added Dinesh Bhor, as this bug is a duplicate of a reported bug he opened.

[Morgan Fainberg (mdrnstm)]

This bug should be moved to public. It has been referenced in different formats publicly now: #1704515 This also is referring to debug output.

[Lance Bragstad (lbragstad)]

I agree with Morgan's assessment about public exposure. While exposing this information through debug logging certainly isn't secure, the workaround and recommended way in production is to not use debug.

Give we're right up against the wall of Pike, I doubt we'll be able to get a fix in Pike. This might be something we can incorporate early in Queens though.

[Morgan Fainberg (mdrnstm)]

Marked as public. Thanks Lance.

[Dinesh Bhor (dinesh-bhor)]

Hi all,

I have tried below approach which is similar to what Morgan has suggested.
By adding a custom filter in oslo logger and passing that logger from masakari to python-novaclient while creating it's object I am able to mask the required sensitive information in keystoneauth.

Below are the steps:

1] I have added custom PasswordMaskingFilter in oslo_log/log.py.
   This filter masks the sensitive information using oslo_utils "strutils.mask_password" method.

   I have added this filter to the oslo logger which we get by calling "getLogger" method of
   oslo_log/log.py module.

diff --git a/oslo_log/log.py b/oslo_log/log.py
index 827a57d..16aade6 100644
--- a/oslo_log/log.py
+++ b/oslo_log/log.py
@@ -40,6 +40,7 @@ except ImportError:

 from oslo_config import cfg
 from oslo_utils import importutils
+from oslo_utils import strutils
 import six
 from six import moves

@@ -421,6 +422,17 @@ def get_loggers():
     return _loggers.copy()

+class PasswordMaskingFilter(logging.Filter):
+ """Demonstrate how to filter sensitive data:"""
+
+ def filter(self, record):
+ # The call signature matches string interpolation: args can be a tuple
+ # or a lone dict
+
+ # Use oslo_utils password masking method to sanitize data
+ record.msg = strutils.mask_password(record.msg)
+ return True
+
+
 def getLogger(name=None, project='unknown', version='unknown'):
     """Build a logger with the given name.

@@ -442,7 +454,9 @@ def getLogger(name=None, project='unknown', version='unknown'):
     if name and name.startswith('oslo_'):
         name = 'oslo.' + name[5:]
     if name not in _loggers:
- _loggers[name] = KeywordArgumentAdapter(logging.getLogger(name),
+ masking_logger = logging.getLogger(name)
+ masking_logger.addFilter(PasswordMaskingFilter())
+ _loggers[name] = KeywordArgumentAdapter(masking_logger,
                                                 {'project': project,
                                                  'version': version})

[2] Used this oslo_logger in masakari while creating novaclient:

diff --git a/masakari/compute/nova.py b/masakari/compute/nova.py
index 56a12c6..830c5a5 100644
--- a/masakari/compute/nova.py
+++ b/masakari/compute/nova.py
@@ -119,7 +119,8 @@ def novaclient(context, timeout=None):
                                     region_name=CONF.os_region_name,
                                     endpoint_type=endpoint_type,
                                     cacert=CONF.nova_ca_certificates_file,
- extensions=nova_extensions)
+ extensions=nova_extensions,
+ logger=LOG)

The above "LOG" variable is of oslo_logger itself.

The disadvantage of this solution is it checks every log message for certain password fields further degrading the performance.
We should pass some info in the log message indicating there is a need to mask the password fields.

Please let me know your opinion about this and also any other solution you have.

[Dinesh Bhor (dinesh-bhor)]

Please see the paste file having steps of above approach for better reading: http://paste.openstack.org/show/617093/

[Dinesh Bhor (dinesh-bhor)]

Hi All,

Morgan's solution:

Provide clear documentation on how to mask sensitive info in a logger and get the consumers of
keystoneauth to use Oslo's mask_password wherever possible.

The solution I have proposed in comment #11 and #12 follows the same way but has a disadvantage like
that solution would check each and every log message for certain password fields further degrading the
performance.

To overcome this disadvantage I have thought of passing mask_password=True keyword argument to the
logger statement. If the mask_password is set then only the information will be masked at the time
of logging.

I have explained this approach in below paste file with code snippet:
http://paste.openstack.org/show/618019/

With these changes I am able to mask the sensitive information in keystoneauth successfully without
using the external oslo lib explicitly.

Now the problem with this solution is:
If you forget to pass mask_password=True for logging messages where password related information
is present, then those fields won't be masked with ***. But this can be clearly documented as
suggested by Morgan and Lance.

[Dinesh Bhor (dinesh-bhor)]

Hi All,

There are four solutions to fix this issue:

1) Carry a copy of mask_password() method to keystoneauth from oslo_utils [1]:
Pros:
A. keystoneauth will use already tested and used version of mask_password.

Cons:
A. keystoneauth will have to keep the version of mask_password() method sync with
   oslo_utils version. If there are any new "_SANITIZE_KEYS" added to oslo_utils
   mask_password then those should be added in keystoneauth mask_password also.
B. Copying the "mask_password" will also require to copy its supporting code [2]
   which is huge.

2) Use Oslo.utils mask_password() method in keystoneauth:
Pros:
A) No synching issue as described in solution #1. keystoneauth will directly use
   mask_password() method from Oslo.utils.

Cons:
A) You will need oslo.utils library to use keystoneauth.
Objection by community:
- keystoneauth community don't want any dependency on any of OpenStack common oslo
  libraries.
Please refer to the comment from Morgan:
https://bugs.launchpad.net/keystoneauth/+bug/1700751/comments/3

3] Add a custom logging filter in oslo logger
Please refer to POC sample here: http://paste.openstack.org/show/617093/

OpenStack core services using any OpenStack individual python-*client (for e.g
python-cinderclient used in nova service) will need to pass oslo_logger object
during its initialization which will do the work of masking sensitive information.

Note: In nova, oslo.logger object is not passed during cinderclient initialization (https://github.com/openstack/nova/blob/master/nova/volume/cinder.py#L135-L141),
In this case, sensitive information will not be masked as it isn’t using Oslo.logger.

Pros:
A) No changes required in oslo.logger or any OpenStack services if mask_password
   method is modified in oslo.utils.

Cons:
A) Every log message will be scanned for certain password fields degrading the performance.
B) If consumer of keystoneauth doesn’t use oslo_logger, then the sensitive information
   will not be masked.
C) Will need to make changes wherever applicable to the OpenStack core services to pass
   oslo.logger object during python-novaclient initialization.

4] Add mask_password formatter parameter in oslo_log:
Add "mask_password" formatter to sanitize sensitive data and pass it as a keyword argument
to the log statement. If the mask_password is set, then only the sensitive information
will be masked at the time of logging.

The log statement will look like below:

logger.debug("'adminPass': 'Now you see me'"), mask_password=True)

Please refer to the POC code here: http://paste.openstack.org/show/618019/

Pros:
A) No changes required in oslo.logger or any OpenStack services if mask_password method is
modified in oslo.utils.

Cons:
A) If consumer of keystoneauth doesn’t use oslo_logger, then the sensitive information
will not be masked.
B) If you forget to pass mask_password=True for logging messages where sensitive information
is present, then those fields won't be masked with ***. But this can be clearly documented
as suggested by Morgan and Lance.
C) This solution requires you to add a below check in keystoneauth to avoid from an
exception being raised in case logger is pure python...

[Lance Bragstad (lbragstad)]

Jamie also responded to a similar thread on the mailing list with an alternative [0].

[0] http://lists.openstack.org/pipermail/openstack-dev/2017-October/123024.html

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/512522

[OpenStack Infra (hudson-openstack)]

Change abandoned by Morgan Fainberg (<email address hidden>) on branch: master
Review: https://review.openstack.org/512522
Reason: Administratively abandoning this. We're not moving forward with this direction.

[Morgan Fainberg (mdrnstm)]

The path forward is to pass a logger that has sanitization code to KeystoneAuth's session object.