From 281d2e165b674b315529b60d145f1b57a7bdb08e Mon Sep 17 00:00:00 2001 From: Alex Bublichenko Date: Fri, 24 May 2019 15:29:28 -0700 Subject: Gracefully handle invalid HOK assertions --- src/saml2/response.py | 3 ++- tests/saml_hok_invalid.xml | 30 ++++++++++++++++++++++++++++++ tests/test_93_hok.py | 33 ++++++++++++++++++++++++--------- 3 files changed, 56 insertions(+), 10 deletions(-) create mode 100644 tests/saml_hok_invalid.xml diff --git a/src/saml2/response.py b/src/saml2/response.py index c16be47f..118f7fe0 100644 --- a/src/saml2/response.py +++ b/src/saml2/response.py @@ -726,7 +726,8 @@ class AuthnResponse(StatusResponse): return False has_keyinfo = False - for element in extension_elements_to_elements(data.key_info, + key_info = data.key_info or () + for element in extension_elements_to_elements(key_info, [samlp, saml, xenc, ds]): if isinstance(element, ds.KeyInfo): has_keyinfo = True diff --git a/tests/saml_hok_invalid.xml b/tests/saml_hok_invalid.xml new file mode 100644 index 00000000..53c9edb9 --- /dev/null +++ b/tests/saml_hok_invalid.xml @@ -0,0 +1,30 @@ + + + + https://idp:8443 + + + + + https://idp:8443 + + 57a0a35eefdb29ca8b4ab78d5a118117 + + + + + + + urn:oasis:names:tc:SAML:2.0:ac:classes:Password + + + + + testuser + + + + diff --git a/tests/test_93_hok.py b/tests/test_93_hok.py index df740722..dc6aac6e 100644 --- a/tests/test_93_hok.py +++ b/tests/test_93_hok.py @@ -1,23 +1,19 @@ #!/usr/bin/env python # -*- coding: utf-8 -*- -from saml2.response import authn_response +from saml2.response import authn_response, VerificationError from saml2.config import config_factory from pathutils import dotname, full_path -HOLDER_OF_KEY_RESPONSE_FILE = full_path("saml_hok.xml") +HOLDER_OF_KEY_RESPONSE_FILE = full_path("saml_hok.xml") +INVALID_HOLDER_OF_KEY_RESPONSE_FILE = full_path("saml_hok_invalid.xml") class TestHolderOfKeyResponse: - def test_hok_response_is_parsed(self): + def test_valid_hok_response_is_parsed(self): """Verifies that response with 'holder-of-key' subject confirmations is parsed successfully.""" - conf = config_factory("idp", dotname("server_conf")) - resp = authn_response(conf, "https://sp:443/.auth/saml/login", asynchop=False, allow_unsolicited=True) - with open(HOLDER_OF_KEY_RESPONSE_FILE, 'r') as fp: - authn_response_xml = fp.read() - resp.loads(authn_response_xml, False) + resp = self._get_test_response(HOLDER_OF_KEY_RESPONSE_FILE) resp.do_not_verify = True - resp.parse_assertion() assert resp.get_subject() is not None @@ -56,6 +52,25 @@ class TestHolderOfKeyResponse: certs[index] = item return certs + def test_invalid_hok_response_fails_verification(self): + """Verifies that response with invalid 'holder-of-key' subject confirmations is parsed successfully.""" + resp = self._get_test_response(INVALID_HOLDER_OF_KEY_RESPONSE_FILE) + resp.do_not_verify = True + + try: + resp.parse_assertion() + assert False, "parse_assertion() did not fail as expected" + except VerificationError as e: + assert e is not None + + def _get_test_response(self, path): + conf = config_factory("idp", dotname("server_conf")) + resp = authn_response(conf, "https://sp:443/.auth/saml/login", asynchop=False, allow_unsolicited=True) + with open(path, 'r') as fp: + authn_response_xml = fp.read() + resp.loads(authn_response_xml, False) + return resp + if __name__ == "__main__": t = TestHolderOfKeyResponse() -- cgit v1.2.1