Merge "Add logging for xmlsec1 installation"

This commit is contained in:
Zuul 2018-03-21 23:58:14 +00:00 committed by Gerrit Code Review
commit d4f3160334
2 changed files with 93 additions and 28 deletions

View File

@ -382,6 +382,38 @@ class SAMLGenerator(object):
return signature
def _verify_assertion_binary_is_installed():
"""Make sure the specified xmlsec binary is installed.
If the binary specified in configuration isn't installed, make sure we
leave some sort of useful error message for operators since the absense of
it is going to throw an HTTP 500.
"""
try:
# `check_output` just returns the output of whatever is passed in
# (hence the name). We don't really care about where the location of
# the binary exists, though. We just want to make sure it's actually
# installed and if an `CalledProcessError` isn't thrown, it is.
subprocess.check_output( # nosec : The contents of this command are
# coming from either the default
# configuration value for
# CONF.saml.xmlsec1_binary or an operator
# supplied location for that binary. In
# either case, it is safe to assume this
# input is coming from a trusted source and
# not a possible attacker (over the API).
['/usr/bin/which', CONF.saml.xmlsec1_binary]
)
except subprocess.CalledProcessError:
msg = (
'Unable to locate %(binary)s binary on the system. Check to make '
'sure it is installed.' % {'binary': CONF.saml.xmlsec1_binary}
)
LOG.error(msg)
raise exception.SAMLSigningError(reason=msg)
def _sign_assertion(assertion):
"""Sign a SAML assertion.
@ -416,6 +448,13 @@ def _sign_assertion(assertion):
'idp_private_key': CONF.saml.keyfile,
}
# Verify that the binary used to create the assertion actually exists on
# the system. If it doesn't, log a warning for operators to go and install
# it. Requests for assertions will fail with HTTP 500s until the package is
# installed, so providing something useful in the logs is about the best we
# can do.
_verify_assertion_binary_is_installed()
command_list = [
CONF.saml.xmlsec1_binary, '--sign', '--privkey-pem', certificates,
'--id-attr:ID', 'Assertion']

View File

@ -3740,12 +3740,13 @@ class SAMLGenerationTests(test_v3.RestfulTestCase):
def test_assertion_using_explicit_namespace_prefixes(self):
def mocked_subprocess_check_output(*popenargs, **kwargs):
# the last option is the assertion file to be signed
filename = popenargs[0][-1]
with open(filename, 'r') as f:
assertion_content = f.read()
# since we are not testing the signature itself, we can return
# the assertion as is without signing it
return assertion_content
if popenargs[0] != ['/usr/bin/which', CONF.saml.xmlsec1_binary]:
filename = popenargs[0][-1]
with open(filename, 'r') as f:
assertion_content = f.read()
# since we are not testing the signature itself, we can return
# the assertion as is without signing it
return assertion_content
with mock.patch.object(subprocess, 'check_output',
side_effect=mocked_subprocess_check_output):
@ -4026,37 +4027,46 @@ class SAMLGenerationTests(test_v3.RestfulTestCase):
create_class_mock.assert_called_with(saml.Assertion, 'fakeoutput')
@mock.patch('oslo_utils.fileutils.write_to_tempfile')
@mock.patch.object(subprocess, 'check_output')
def test_sign_assertion_exc(self, check_output_mock,
write_to_tempfile_mock):
def test_sign_assertion_exc(self, write_to_tempfile_mock):
# If the command fails the command output is logged.
write_to_tempfile_mock.return_value = 'tmp_path'
sample_returncode = 1
sample_output = self.getUniqueString()
check_output_mock.side_effect = subprocess.CalledProcessError(
returncode=sample_returncode, cmd=CONF.saml.xmlsec1_binary,
output=sample_output)
write_to_tempfile_mock.return_value = 'tmp_path'
logger_fixture = self.useFixture(fixtures.LoggerFixture())
self.assertRaises(exception.SAMLSigningError,
keystone_idp._sign_assertion,
self.signed_assertion)
# The function __str__ in subprocess.CalledProcessError is different
# between py3.6 and lower python version.
expected_log = (
"Error when signing assertion, reason: Command '%s' returned "
"non-zero exit status %s\.? %s\n" %
(CONF.saml.xmlsec1_binary, sample_returncode, sample_output))
def side_effect(*args, **kwargs):
if args[0] == ['/usr/bin/which', CONF.saml.xmlsec1_binary]:
return '/usr/bin/xmlsec1\n'
else:
raise subprocess.CalledProcessError(
returncode=sample_returncode, cmd=CONF.saml.xmlsec1_binary,
output=sample_output
)
self.assertRegex(logger_fixture.output,
re.compile(r'%s' % expected_log))
with mock.patch.object(subprocess, 'check_output',
side_effect=side_effect):
logger_fixture = self.useFixture(fixtures.LoggerFixture())
self.assertRaises(
exception.SAMLSigningError,
keystone_idp._sign_assertion,
self.signed_assertion
)
# The function __str__ in subprocess.CalledProcessError is
# different between py3.6 and lower python version.
expected_log = (
"Error when signing assertion, reason: Command '%s' returned "
"non-zero exit status %s\.? %s\n" %
(CONF.saml.xmlsec1_binary, sample_returncode, sample_output))
self.assertRegex(logger_fixture.output,
re.compile(r'%s' % expected_log))
@mock.patch('oslo_utils.fileutils.write_to_tempfile')
def test_sign_assertion_fileutils_exc(self, write_to_tempfile_mock):
@mock.patch.object(subprocess, 'check_output')
def test_sign_assertion_fileutils_exc(self, check_output_mock,
write_to_tempfile_mock):
exception_msg = 'fake'
write_to_tempfile_mock.side_effect = Exception(exception_msg)
check_output_mock.return_value = '/usr/bin/xmlsec1'
logger_fixture = self.useFixture(fixtures.LoggerFixture())
self.assertRaises(exception.SAMLSigningError,
@ -4066,6 +4076,22 @@ class SAMLGenerationTests(test_v3.RestfulTestCase):
'Error when signing assertion, reason: %s\n' % exception_msg)
self.assertEqual(expected_log, logger_fixture.output)
def test_sign_assertion_logs_message_if_xmlsec1_is_not_installed(self):
with mock.patch.object(subprocess, 'check_output') as co_mock:
co_mock.side_effect = subprocess.CalledProcessError(
returncode=1, cmd=CONF.saml.xmlsec1_binary,
)
logger_fixture = self.useFixture(fixtures.LoggerFixture())
self.assertRaises(
exception.SAMLSigningError,
keystone_idp._sign_assertion,
self.signed_assertion
)
expected_log = ('Unable to locate xmlsec1 binary on the system. '
'Check to make sure it is installed.\n')
self.assertEqual(expected_log, logger_fixture.output)
class IdPMetadataGenerationTests(test_v3.RestfulTestCase):
"""A class for testing Identity Provider Metadata generation."""