Mask passwords in exceptions and error messages

When a ProcessExecutionError is thrown by processutils.execute(), the
exception may contain information such as password. Upstream
applications that just log the message (as several appear to do) could
inadvertently expose these passwords to a user with read access to the
log files. It is therefore considered prudent to invoke
strutils.mask_password() on the command, stdout and stderr in the
exception. A test case has been added to ensure that all three are
properly masked.

OSSA is aware of this change request.

Submitted to oslo.concurrency in
Ie122db5f19802f519b96ed024ab3f2b5eede3eee

Change-Id: I173dfb865e84eb7dee54a22c76db1e4f125a0a8a
Closes-Bug: #1343604
This commit is contained in:
Amrith Kumar 2014-07-24 17:04:42 -04:00
parent a9e1122f44
commit 63c99a0fd5
2 changed files with 41 additions and 6 deletions

View File

@ -150,12 +150,12 @@ def execute(*cmd, **kwargs):
cmd = shlex.split(root_helper) + list(cmd)
cmd = map(str, cmd)
sanitized_cmd = strutils.mask_password(' '.join(cmd))
while attempts > 0:
attempts -= 1
try:
LOG.log(loglevel, 'Running cmd (subprocess): %s',
strutils.mask_password(' '.join(cmd)))
LOG.log(loglevel, _('Running cmd (subprocess): %s'), sanitized_cmd)
_PIPE = subprocess.PIPE # pylint: disable=E1101
if os.name == 'nt':
@ -192,16 +192,18 @@ def execute(*cmd, **kwargs):
LOG.log(loglevel, 'Result was %s' % _returncode)
if not ignore_exit_code and _returncode not in check_exit_code:
(stdout, stderr) = result
sanitized_stdout = strutils.mask_password(stdout)
sanitized_stderr = strutils.mask_password(stderr)
raise ProcessExecutionError(exit_code=_returncode,
stdout=stdout,
stderr=stderr,
cmd=' '.join(cmd))
stdout=sanitized_stdout,
stderr=sanitized_stderr,
cmd=sanitized_cmd)
return result
except ProcessExecutionError:
if not attempts:
raise
else:
LOG.log(loglevel, '%r failed. Retrying.', cmd)
LOG.log(loglevel, _('%r failed. Retrying.'), sanitized_cmd)
if delay_on_retry:
greenthread.sleep(random.randint(20, 200) / 100.0)
finally:

View File

@ -18,6 +18,7 @@ from __future__ import print_function
import errno
import multiprocessing
import os
import stat
import tempfile
import fixtures
@ -27,6 +28,14 @@ import six
from openstack.common import processutils
TEST_EXCEPTION_AND_MASKING_SCRIPT = """#!/bin/bash
# This is to test stdout and stderr
# and the command returned in an exception
# when a non-zero exit code is returned
echo onstdout --password='"secret"'
echo onstderr --password='"secret"' 1>&2
exit 38"""
class UtilsTest(test_base.BaseTestCase):
# NOTE(jkoelker) Moar tests from nova need to be ported. But they
@ -217,6 +226,30 @@ grep foo
self.assertIn('SUPER_UNIQUE_VAR=The answer is 42', out)
def test_exception_and_masking(self):
tmpfilename = self.create_tempfiles(
[["test_exceptions_and_masking",
TEST_EXCEPTION_AND_MASKING_SCRIPT]], ext='bash')[0]
os.chmod(tmpfilename, (stat.S_IRWXU |
stat.S_IRGRP |
stat.S_IXGRP |
stat.S_IROTH |
stat.S_IXOTH))
err = self.assertRaises(processutils.ProcessExecutionError,
processutils.execute,
tmpfilename, 'password="secret"',
'something')
self.assertEqual(38, err.exit_code)
self.assertEqual(err.stdout, 'onstdout --password="***"\n')
self.assertEqual(err.stderr, 'onstderr --password="***"\n')
self.assertEqual(err.cmd, ' '.join([tmpfilename,
'password="***"',
'something']))
self.assertNotIn('secret', str(err))
def fake_execute(*cmd, **kwargs):
return 'stdout', 'stderr'