From 505c381a7d0fa6eb799ed28dcab93af0fbd156fa Mon Sep 17 00:00:00 2001 From: Adam Young Date: Wed, 2 Sep 2015 12:49:42 -0400 Subject: [PATCH] Reject modified PKI Tokens PKI tokens use OpenSSL to validate a CMS document. Most of that document is designed to detect changes using a hash; if either to data or the hash changes the document is invalid. However, OpenSSL does not validate the values in every portion of the document, and, it a document is modified, it will have a different hash. This effectively un-revokes the token; the hash is checked against the revocation list. Change-Id: I36682ce9b59b8d15127a59b9c4f0abf8aca252e3 --- keystoneclient/common/cms.py | 54 ++++++++++++++++++++++++++++++++++- keystoneclient/tests/unit/test_cms.py | 29 +++++++++++++++++++ requirements.txt | 2 ++ 3 files changed, 84 insertions(+), 1 deletion(-) diff --git a/keystoneclient/common/cms.py b/keystoneclient/common/cms.py index 1bd0f41..d9231df 100644 --- a/keystoneclient/common/cms.py +++ b/keystoneclient/common/cms.py @@ -29,6 +29,9 @@ import zlib from debtcollector import removals import six +from pyasn1.codec.der import decoder +from pyasn1_modules import rfc2315 + from keystoneclient import exceptions from keystoneclient.i18n import _, _LE @@ -130,10 +133,55 @@ def _encoding_for_form(inform): raise ValueError( _('"inform" must be one of: %s') % ','.join((PKI_ASN1_FORM, PKIZ_CMS_FORM))) - return encoding +def _validate_asn1_values(data): + start_delim = '-----BEGIN CMS-----' + end_delim = '-----END CMS-----\n' + substrate = data.strip() + if not data.startswith(start_delim) or not data.endswith(end_delim): + raise exceptions.CMSError('token CMS document invalid') + + substrate = substrate.replace(start_delim, '') + substrate = substrate.replace(end_delim, '') + substrate = base64.urlsafe_b64decode(substrate) + + contentInfo, rest = decoder.decode(substrate, + asn1Spec=rfc2315.ContentInfo()) + contentType = contentInfo.getComponentByName('contentType') + contentInfoMap = { + (1, 2, 840, 113549, 1, 7, 1): rfc2315.Data(), + (1, 2, 840, 113549, 1, 7, 2): rfc2315.SignedData(), + (1, 2, 840, 113549, 1, 7, 3): rfc2315.EnvelopedData(), + (1, 2, 840, 113549, 1, 7, 4): rfc2315.SignedAndEnvelopedData(), + (1, 2, 840, 113549, 1, 7, 5): rfc2315.DigestedData(), + (1, 2, 840, 113549, 1, 7, 6): rfc2315.EncryptedData() + } + + content, _ = decoder.decode( + contentInfo.getComponentByName('content'), + asn1Spec=contentInfoMap[contentType] + ) + + signerInfos = content.getComponentByName('signerInfos') + if len(signerInfos) == 0: + raise exceptions.CMSError('token CMS document invalid') + # check version + if (signerInfos[0][0]._value != 1): + raise exceptions.CMSError('token CMS document invalid') + + if (content[2][0].__str__() != "1.2.840.113549.1.7.1"): + raise exceptions.CMSError('token CMS document invalid') + + if (content[5][0][4][0].__str__() != "1.2.840.113549.1.1.1"): + raise exceptions.CMSError('token CMS document invalid') + + blank_field = content[5][0][4][1].asNumbers() + if (blank_field[0] != 5) or (blank_field[1] != 0): + raise exceptions.CMSError('token CMS document invalid') + + def cms_verify(formatted, signing_cert_file_name, ca_file_name, inform=PKI_ASN1_FORM): """Verifies the signature of the contents IAW CMS syntax. @@ -148,6 +196,7 @@ def cms_verify(formatted, signing_cert_file_name, ca_file_name, data = bytearray(formatted, _encoding_for_form(inform)) else: data = formatted + process = subprocess.Popen(['openssl', 'cms', '-verify', '-certfile', signing_cert_file_name, '-CAfile', ca_file_name, @@ -185,6 +234,9 @@ def cms_verify(formatted, signing_cert_file_name, ca_file_name, e = subprocess.CalledProcessError(retcode, 'openssl') e.output = err raise e + + _validate_asn1_values(data) + return output diff --git a/keystoneclient/tests/unit/test_cms.py b/keystoneclient/tests/unit/test_cms.py index 019730d..fc53d33 100644 --- a/keystoneclient/tests/unit/test_cms.py +++ b/keystoneclient/tests/unit/test_cms.py @@ -155,6 +155,35 @@ class CMSTest(utils.TestCase, testresources.ResourcedTestCase): # sha256 hash is 64 chars. self.assertThat(token_id, matchers.HasLength(64)) + def _assert_tamper_resistant(self, token_orig): + uncompressed = cms.pkiz_uncompress(token_orig) + cms.cms_verify(uncompressed, + self.examples.SIGNING_CERT_FILE, + self.examples.SIGNING_CA_FILE, + inform=cms.PKI_ASN1_FORM) + token_bin = uncompressed + for i in range(1, len(token_bin)): + token_mutated = (token_bin[:i] + + chr((ord(token_bin[i]) - 1) % 256) + + token_bin[i + 1:]) + try: + cms.cms_verify(token_mutated, + self.examples.SIGNING_CERT_FILE, + self.examples.SIGNING_CA_FILE, + inform=cms.PKI_ASN1_FORM) + self.fail('token modification of byte %d not detected.' % (i)) + + except exceptions.CMSError: + pass + except subprocess.CalledProcessError: + pass + + def test_token_tampering(self): + self._assert_tamper_resistant( + self.examples.SIGNED_TOKEN_SCOPED_PKIZ) + self._assert_tamper_resistant( + self.examples.SIGNED_v3_TOKEN_SCOPED_PKIZ) + def load_tests(loader, tests, pattern): return testresources.OptimisingTestSuite(tests) diff --git a/requirements.txt b/requirements.txt index b846bb3..e9a1986 100644 --- a/requirements.txt +++ b/requirements.txt @@ -5,6 +5,8 @@ pbr<2.0,>=1.6 argparse +pyasn1 +pyasn1-modules Babel>=1.3 iso8601>=0.1.9 debtcollector>=0.3.0 # Apache-2.0 -- 2.4.3