Bug 1578347

Submitted a backport for upstream Ocata: https://review.openstack.org/568557

Should have been upstream Pike: https://review.openstack.org/568558

I requested Release oslo.utils 3.28.3 for Pike:
https://review.openstack.org/#/c/572383/

python-oslo-utils-3.28.3-1.el7ost package is now available for tests.

> "private_key": "-----BEGIN RSA PRIVATE ..."

Oh, I confirm that the fix doesn't work.

"heat-engine.log": so the log comes from Heat, and Heat is supposed to use the oslo_utils module installed on the system.

I checked the package and it includes the patch. I looked again at the bug upstream, and there is a single fix, the one that I backported.

(1) Can you please test manually to call oslo_utils.strutils.mask_password()?

$ python -c 'from oslo_utils import strutils; print(strutils.mask_password({"keypair": {"private_key": "secret"}}))'

{'keypair': {'private_key': '***'}}


(2) Can you please also check if my patch is correctly applied on the system?

$ grep '_SANITIZE_KEYS =' -A3 $(.tox/py3/bin/python -c 'from oslo_utils import strutils; print(strutils.__file__.replace(".pyc", ".py"))')

_SANITIZE_KEYS = ['adminPass', 'admin_pass', 'password', 'admin_password',
                  'auth_token', 'new_pass', 'auth_password', 'secret_uuid',
                  'secret', 'sys_pswd', 'token', 'configdrive',
                  'CHAPPASSWORD', 'encrypted_key', 'private_key']

The fix is the addition of the last item to this _SANITIZE_KEYS list: 'private_key'.

(In reply to Victor Stinner from comment #10)
> > "private_key": "-----BEGIN RSA PRIVATE ..."
> 
> Oh, I confirm that the fix doesn't work.
> 
> "heat-engine.log": so the log comes from Heat, and Heat is supposed to use
> the oslo_utils module installed on the system.
> 
> I checked the package and it includes the patch. I looked again at the bug
> upstream, and there is a single fix, the one that I backported.
> 
> (1) Can you please test manually to call oslo_utils.strutils.mask_password()?
> 
> $ python -c 'from oslo_utils import strutils;
> print(strutils.mask_password({"keypair": {"private_key": "secret"}}))'
> 
> {'keypair': {'private_key': '***'}}
That's successful - The password is astrerixed as expected when I try the command you provided.
> 
> 
> (2) Can you please also check if my patch is correctly applied on the system?
> 
> $ grep '_SANITIZE_KEYS =' -A3 $(.tox/py3/bin/python -c 'from oslo_utils
> import strutils; print(strutils.__file__.replace(".pyc", ".py"))')
> 
> _SANITIZE_KEYS = ['adminPass', 'admin_pass', 'password', 'admin_password',
>                   'auth_token', 'new_pass', 'auth_password', 'secret_uuid',
>                   'secret', 'sys_pswd', 'token', 'configdrive',
>                   'CHAPPASSWORD', 'encrypted_key', 'private_key']
> 
> The fix is the addition of the last item to this _SANITIZE_KEYS list:
> 'private_key'.
That's okay too, the patch seems to be applied

I ain't a coder but I think that mask_password() from strutils is not being used at all when the message is being logged. The log message comes from _http_log_response() from keystoneauth1/session which looks like following on OSP12:

def _http_log_response(self, response=None, json=None,
                           status_code=None, headers=None, text=None,
                           logger=_logger):
        if not logger.isEnabledFor(logging.DEBUG):
            return

        if response is not None:
            if not status_code:
                status_code = response.status_code
            if not headers:
                headers = response.headers
            if not text:
                # NOTE(samueldmq): If the response does not provide enough info
                # about the content type to decide whether it is useful and
                # safe to log it or not, just do not log the body. Trying to
                # read the response body anyways may result on reading a long
                # stream of bytes and getting an unexpected MemoryError. See
                # bug 1616105 for further details.
                content_type = response.headers.get('content-type', None)

                # NOTE(lamt): Per [1], the Content-Type header can be of the
                # form Content-Type := type "/" subtype *[";" parameter]
                # [1] https://www.w3.org/Protocols/rfc1341/4_Content-Type.html
                for log_type in _LOG_CONTENT_TYPES:
                    if content_type is not None and content_type.startswith(
                            log_type):
                        text = self._remove_service_catalog(response.text)
                        break
                else:
                    text = ('Omitted, Content-Type is set to %s. Only '
                            '%s responses have their bodies logged.')
                    text = text % (content_type, ', '.join(_LOG_CONTENT_TYPES))
        if json:
            text = self._json.encode(json)

        string_parts = ['RESP:']

        if status_code:
            string_parts.append('[%s]' % status_code)
        if headers:
            for header in headers.items():
                string_parts.append('%s: %s' % self._process_header(header))
        if text:
            string_parts.append('\nRESP BODY: %s\n' % text)

        logger.debug(' '.join(string_parts))


I cannot see mask_password() being used anyweher, I would expect (probably wrongly) to see sth like:
string_parts.append('\nRESP BODY: %s\n' % strutils.mask_password(text))
at the line before the last line.

> I ain't a coder but I think that mask_password() from strutils is not being used at all when the message is being logged. (...)

Oh, you're right. It seems like the upstream has been closed too early, I commented it to explain that keystoneauth still should be fixed:
https://bugs.launchpad.net/oslo.utils/+bug/1770683

I reassigned this issue to the openstack-keystone component, since keystoneauth should be patched to use mask_password().

I reset the issue status to NEW.

Since that bug requires a fix in both python-oslo-utils and openstack-keystone, I have just clone it [1] to track the python-oslo-utils fix in a dedicated bz.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1612132