Gracefully handle invalid HOK assertions

3 files changed, 56 insertions, 10 deletions

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 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<!-- SAML response with invalid 'holder-of-key' SubjectConfirmation: missing KeyInfo element. -->
+<ns0:Response xmlns:ns0="urn:oasis:names:tc:SAML:2.0:protocol" 
+    xmlns:ns1="urn:oasis:names:tc:SAML:2.0:assertion" 
+    xmlns:ns2="http://www.w3.org/2000/09/xmldsig#" 
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" Destination="https://sp:443/.auth/saml/login" ID="_df9a1eadc90519252694519504a13dfb8dd67a1bb4" InResponseTo="id-KHlas49TtW2VdC8WN" IssueInstant="2019-05-14T20:35:13Z" Version="2.0">
+    <ns1:Issuer>https://idp:8443</ns1:Issuer>
+    <ns0:Status>
+        <ns0:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:Success" />
+    </ns0:Status>
+    <ns1:Assertion ID="_12d211a5015f71eba8f837d2aa8b95b28bbdc4599b" IssueInstant="2019-05-14T20:35:13Z" Version="2.0">
+        <ns1:Issuer>https://idp:8443</ns1:Issuer>
+        <ns1:Subject>
+            <ns1:NameID Format="urn:oasis:names:tc:SAML:2.0:nameid-format:persistent">57a0a35eefdb29ca8b4ab78d5a118117</ns1:NameID>
+            <ns1:SubjectConfirmation Method="urn:oasis:names:tc:SAML:2.0:cm:holder-of-key">
+                <ns1:SubjectConfirmationData InResponseTo="id-KHlas49TtW2VdC8WN" NotOnOrAfter="2019-05-14T20:36:13Z" Recipient="https://sp:443/.auth/saml/login" />
+            </ns1:SubjectConfirmation>
+        </ns1:Subject>
+        <ns1:AuthnStatement AuthnInstant="2019-05-14T20:35:13Z" SessionIndex="1">
+            <ns1:AuthnContext>
+                <ns1:AuthnContextClassRef>urn:oasis:names:tc:SAML:2.0:ac:classes:Password</ns1:AuthnContextClassRef>
+            </ns1:AuthnContext>
+        </ns1:AuthnStatement>
+        <ns1:AttributeStatement>
+            <ns1:Attribute Name="uid" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri">
+                <ns1:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xsi:type="xs:string">testuser</ns1:AttributeValue>
+            </ns1:Attribute>
+        </ns1:AttributeStatement>
+    </ns1:Assertion>
+</ns0:Response>
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()