Parse assertions with Holder-of-Key profile

Problem:

Holder-of-Key assertions are used to achieve higher levels of federation security, compared to bearer assertions, by having Relying Party challenge subscriber to prove possession of the key specified in the assertion that represents subscriber in addition to verifying the assertion itself signed by Identity Provider. More information about it can be found in https://pages.nist.gov/800-63-3/sp800-63c.html

This library fails to parase SAML respones containing assertions with Holder-of-Key profile, for example:
```
<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">
        <ns2:KeyInfo>
            <ns2:X509Data>
                <ns2:X509Certificate>MIICITCCAYoCAQEwDQYJKoZIhvcNAQELBQAwWDELMAkGA1UEBhMCenoxCzAJBgNVBAgMAnp6MQ0wCwYDVQQHDAR6enp6MQ4wDAYDVQQKDAVaenp6ejEOMAwGA1UECwwFWnp6enoxDTALBgNVBAMMBHRlc3QwIBcNMTkwNDEyMTk1MDM0WhgPMzAxODA4MTMxOTUwMzRaMFgxCzAJBgNVBAYTAnp6MQswCQYDVQQIDAJ6ejENMAsGA1UEBwwEenp6ejEOMAwGA1UECgwFWnp6enoxDjAMBgNVBAsMBVp6enp6MQ0wCwYDVQQDDAR0ZXN0MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDHcj80WU/XBsd9FlyQmfjPUdfmedhCFDd6TEQmZNNqP/UG+VkGa+BXjRIHMfic/WxPTbGhCjv68ci0UDNomUXagFexLGNpkwa7+CRVtoc/1xgq+ySE6M4nhcCutScoxNvWNn5eSQ66i3U0sTv91MgsXxqEdTaiZg0BIufEc3dueQIDAQABMA0GCSqGSIb3DQEBCwUAA4GBAGUV5B+USHvaRa8kgCNJSuNpo6ARlv0ekrk8bbdNRBiEUdCMyoGJFfuM9K0zybX6Vr25wai3nvaog294Vx/jWjX2g5SDbjItH6VGy6C9GCGf1A07VxFRCfJn5tA9HuJjPKiE+g/BmrV5N4CealzFxPHWYkNOzoRU8qI7OqUai1kL</ns2:X509Certificate>
            </ns2:X509Data>
        </ns2:KeyInfo>
    </ns1:SubjectConfirmationData>
</ns1:SubjectConfirmation>
```

fails to be parsed with the following error:
```
ERROR    saml2.response:response.py:836 get subject
Traceback (most recent call last):
  File "/home/abublich/repos/abliqo-pysaml2/venv/local/lib/python2.7/site-packages/pysaml2-4.7.0-py2.7.egg/saml2/response.py", line 828, in _assertion
    self.get_subject()
  File "/home/abublich/repos/abliqo-pysaml2/venv/local/lib/python2.7/site-packages/pysaml2-4.7.0-py2.7.egg/saml2/response.py", line 753, in get_subject
    if not self._holder_of_key_confirmed(_data):
  File "/home/abublich/repos/abliqo-pysaml2/venv/local/lib/python2.7/site-packages/pysaml2-4.7.0-py2.7.egg/saml2/response.py", line 730, in _holder_of_key_confirmed
    [samlp, saml, xenc, ds]):
  File "/home/abublich/repos/abliqo-pysaml2/venv/local/lib/python2.7/site-packages/pysaml2-4.7.0-py2.7.egg/saml2/__init__.py", line 1004, in extension_elements_to_elements
    for extension_element in extension_elements:
TypeError: 'SubjectConfirmationData' object is not iterable
```

The root cause is two-fold:
1. The type SubjectConfirmationDataType_ does not declare KeyInfo as child element.
2. The bug in function _holder_of_key_confirmed: it should check KeyInfo child element of SubjectConfirmationData instead of SubjectConfirmationData itself.

Solution:
Fixed the root cause and added new unit tests that verify successful parsing of Holder-of-Key assertions.

7 files changed, 172 insertions, 18 deletions

diff --git a/.gitignore b/.gitignore
index a1e06506..7db203ec 100644
--- a/.gitignore
+++ b/.gitignore
@@ -114,6 +114,9 @@ venv.bak/
 # Rope project settings
 .ropeproject
 
+# Visual Studio Code files
+.vscode/
+
 # mkdocs documentation
 /site
 
diff --git a/src/saml2/response.py b/src/saml2/response.py
index 2660e738..c16be47f 100644
--- a/src/saml2/response.py
+++ b/src/saml2/response.py
@@ -722,11 +722,11 @@ class AuthnResponse(StatusResponse):
         return True
 
     def _holder_of_key_confirmed(self, data):
-        if not data:
+        if not data or not data.key_info:
             return False
 
         has_keyinfo = False
-        for element in extension_elements_to_elements(data,
+        for element in extension_elements_to_elements(data.key_info,
                                                       [samlp, saml, xenc, ds]):
             if isinstance(element, ds.KeyInfo):
                 has_keyinfo = True
diff --git a/src/saml2/saml.py b/src/saml2/saml.py
index bdb1ec60..0d6728e5 100644
--- a/src/saml2/saml.py
+++ b/src/saml2/saml.py
@@ -482,8 +482,12 @@ class SubjectConfirmationDataType_(SamlBase):
     c_any = {"namespace": "##any", "processContents": "lax", "minOccurs": "0",
              "maxOccurs": "unbounded"}
     c_any_attribute = {"namespace": "##other", "processContents": "lax"}
+    c_children['{http://www.w3.org/2000/09/xmldsig#}KeyInfo'] = ('key_info',
+                                                                 [ds.KeyInfo])
+    c_cardinality['key_info'] = {"min": 0, "max": 1}
 
     def __init__(self,
+                 key_info=None,
                  not_before=None,
                  not_on_or_after=None,
                  recipient=None,
@@ -496,6 +500,7 @@ class SubjectConfirmationDataType_(SamlBase):
                           text=text,
                           extension_elements=extension_elements,
                           extension_attributes=extension_attributes)
+        self.key_info = key_info
         self.not_before = not_before
         self.not_on_or_after = not_on_or_after
         self.recipient = recipient
diff --git a/tests/saml2_data.py b/tests/saml2_data.py
index fe650e7b..f81f2ce5 100644
--- a/tests/saml2_data.py
+++ b/tests/saml2_data.py
@@ -123,6 +123,36 @@ TEST_SUBJECT_CONFIRMATION = """<?xml version="1.0" encoding="utf-8"?>
 </SubjectConfirmation>
 """
 
+TEST_HOLDER_OF_KEY_SUBJECT_CONFIRMATION = """<?xml version="1.0" encoding="utf-8"?>
+<SubjectConfirmation
+  Method="urn:oasis:names:tc:SAML:2.0:cm:holder-of-key"
+  xmlns="urn:oasis:names:tc:SAML:2.0:assertion">
+  <SubjectConfirmationData
+    InResponseTo="responseID"
+    NotOnOrAfter="2007-09-14T01:05:02Z"
+    Recipient="recipient">
+    <KeyInfo xmlns="http://www.w3.org/2000/09/xmldsig#">
+      <X509Data xmlns="http://www.w3.org/2000/09/xmldsig#">
+        <X509Certificate xmlns="http://www.w3.org/2000/09/xmldsig#">
+MIICITCCAYoCAQEwDQYJKoZIhvcNAQELBQAwWDELMAkGA1UEBhMCenoxCzAJBgNV
+BAgMAnp6MQ0wCwYDVQQHDAR6enp6MQ4wDAYDVQQKDAVaenp6ejEOMAwGA1UECwwF
+Wnp6enoxDTALBgNVBAMMBHRlc3QwIBcNMTkwNDEyMTk1MDM0WhgPMzAxODA4MTMx
+OTUwMzRaMFgxCzAJBgNVBAYTAnp6MQswCQYDVQQIDAJ6ejENMAsGA1UEBwwEenp6
+ejEOMAwGA1UECgwFWnp6enoxDjAMBgNVBAsMBVp6enp6MQ0wCwYDVQQDDAR0ZXN0
+MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDHcj80WU/XBsd9FlyQmfjPUdfm
+edhCFDd6TEQmZNNqP/UG+VkGa+BXjRIHMfic/WxPTbGhCjv68ci0UDNomUXagFex
+LGNpkwa7+CRVtoc/1xgq+ySE6M4nhcCutScoxNvWNn5eSQ66i3U0sTv91MgsXxqE
+dTaiZg0BIufEc3dueQIDAQABMA0GCSqGSIb3DQEBCwUAA4GBAGUV5B+USHvaRa8k
+gCNJSuNpo6ARlv0ekrk8bbdNRBiEUdCMyoGJFfuM9K0zybX6Vr25wai3nvaog294
+Vx/jWjX2g5SDbjItH6VGy6C9GCGf1A07VxFRCfJn5tA9HuJjPKiE+g/BmrV5N4Ce
+alzFxPHWYkNOzoRU8qI7OqUai1kL
+        </X509Certificate>
+      </X509Data>
+    </KeyInfo>
+  </SubjectConfirmationData>
+</SubjectConfirmation>
+"""
+
 TEST_SUBJECT = """<?xml version="1.0" encoding="utf-8"?>
 <Subject xmlns="urn:oasis:names:tc:SAML:2.0:assertion">
   <NameID Format="urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress"
diff --git a/tests/saml_hok.xml b/tests/saml_hok.xml
new file mode 100644
index 00000000..6aad625a
--- /dev/null
+++ b/tests/saml_hok.xml
@@ -0,0 +1,45 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<!-- SAML response with multiple 'holder-of-key' subject confirmations. -->
+<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">
+                    <ns2:KeyInfo>
+                        <ns2:X509Data>
+                            <ns2:X509Certificate>MIICITCCAYoCAQEwDQYJKoZIhvcNAQELBQAwWDELMAkGA1UEBhMCenoxCzAJBgNVBAgMAnp6MQ0wCwYDVQQHDAR6enp6MQ4wDAYDVQQKDAVaenp6ejEOMAwGA1UECwwFWnp6enoxDTALBgNVBAMMBHRlc3QwIBcNMTkwNDEyMTk1MDM0WhgPMzAxODA4MTMxOTUwMzRaMFgxCzAJBgNVBAYTAnp6MQswCQYDVQQIDAJ6ejENMAsGA1UEBwwEenp6ejEOMAwGA1UECgwFWnp6enoxDjAMBgNVBAsMBVp6enp6MQ0wCwYDVQQDDAR0ZXN0MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDHcj80WU/XBsd9FlyQmfjPUdfmedhCFDd6TEQmZNNqP/UG+VkGa+BXjRIHMfic/WxPTbGhCjv68ci0UDNomUXagFexLGNpkwa7+CRVtoc/1xgq+ySE6M4nhcCutScoxNvWNn5eSQ66i3U0sTv91MgsXxqEdTaiZg0BIufEc3dueQIDAQABMA0GCSqGSIb3DQEBCwUAA4GBAGUV5B+USHvaRa8kgCNJSuNpo6ARlv0ekrk8bbdNRBiEUdCMyoGJFfuM9K0zybX6Vr25wai3nvaog294Vx/jWjX2g5SDbjItH6VGy6C9GCGf1A07VxFRCfJn5tA9HuJjPKiE+g/BmrV5N4CealzFxPHWYkNOzoRU8qI7OqUai1kL</ns2:X509Certificate>
+                        </ns2:X509Data>
+                    </ns2:KeyInfo>
+                </ns1:SubjectConfirmationData>
+            </ns1:SubjectConfirmation>
+            <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">
+                    <ns2:KeyInfo>
+                        <ns2:X509Data>
+                            <ns2:X509Certificate>MIICITCCAYoCAQEwDQYJKoZIhvcNAQELBQAwWDELMAkGA1UEBhMCenoxCzAJBgNVBAgMAnp6MQ0wCwYDVQQHDAR6enp6MQ4wDAYDVQQKDAVaenp6ejEOMAwGA1UECwwFWnp6enoxDTALBgNVBAMMBHRlc3QwIBcNMTkwNDEyMTk1MDM0WhgPMzAxODA4MTMxOTUwMzRaMFgxCzAJBgNVBAYTAnp6MQswCQYDVQQIDAJ6ejENMAsGA1UEBwwEenp6ejEOMAwGA1UECgwFWnp6enoxDjAMBgNVBAsMBVp6enp6MQ0wCwYDVQQDDAR0ZXN0MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDjW0kJM+4baWKtvO24ZsGXNvNKKkwTMz7OW5Z6BRqhSOq2WA0c5NCpMk6rD8Z2OTFEolPojEjf8dVyd/Ds/hrjFKQv8wQgbdXLN51YTIsgd6h+hBJO+vzhl0PT4aT7M0JKo5ALtS6qk4tsworW2BnwyvsGSAinwfeWt4t/b1J3kwIDAQABMA0GCSqGSIb3DQEBCwUAA4GBAFtj7WArQQBugmh/KQjjlfTQ5A052QeXfgTyO9vv1S6MRIi7qgiaEv49cGXnJv/TWbySkMKObPMUApjg6z8PqcxuShew5FCTkNvwhABFPiyu0fUj3e2FEPHfsBu76jz4ugtmhUqjqhzwFY9ctnWRkkl6J0AjM3LnHOSgjNIclDZG</ns2:X509Certificate>
+                        </ns2:X509Data>
+                    </ns2:KeyInfo>
+                </ns1:SubjectConfirmationData>
+            </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_02_saml.py b/tests/test_02_saml.py
index 7ff64885..2cdb5064 100644
--- a/tests/test_02_saml.py
+++ b/tests/test_02_saml.py
@@ -867,35 +867,53 @@ class TestSubjectConfirmation:
         self.sc.subject_confirmation_data = saml.subject_confirmation_data_from_string(
             saml2_data.TEST_SUBJECT_CONFIRMATION_DATA)
         new_sc = saml.subject_confirmation_from_string(self.sc.to_string())
-        assert new_sc.name_id.sp_provided_id == "sp provided id"
-        assert new_sc.method == saml.SCM_BEARER
-        assert new_sc.subject_confirmation_data.not_before == \
-               "2007-08-31T01:05:02Z"
-        assert new_sc.subject_confirmation_data.not_on_or_after == \
-               "2007-09-14T01:05:02Z"
-        assert new_sc.subject_confirmation_data.recipient == "recipient"
-        assert new_sc.subject_confirmation_data.in_response_to == "responseID"
-        assert new_sc.subject_confirmation_data.address == "127.0.0.1"
-
-    def testUsingTestData(self):
-        """Test subject_confirmation_from_string() using test data"""
+        self._assertBearer(new_sc)
 
+    def testBearerUsingTestData(self):
+        """Test subject_confirmation_from_string() using test data for 'bearer' SubjectConfirmation"""
         sc = saml.subject_confirmation_from_string(
             saml2_data.TEST_SUBJECT_CONFIRMATION)
+        assert sc.verify()
+        self._assertBearer(sc)
+
+    def _assertBearer(self, sc):
+        """Asserts SubjectConfirmation that has method 'bearer'"""
         assert sc.name_id.sp_provided_id == "sp provided id"
         assert sc.method == saml.SCM_BEARER
+        assert sc.subject_confirmation_data is not None
         assert sc.subject_confirmation_data.not_before == "2007-08-31T01:05:02Z"
         assert sc.subject_confirmation_data.not_on_or_after == "2007-09-14T01:05:02Z"
         assert sc.subject_confirmation_data.recipient == "recipient"
         assert sc.subject_confirmation_data.in_response_to == "responseID"
         assert sc.subject_confirmation_data.address == "127.0.0.1"
+        assert sc.subject_confirmation_data.key_info is None
 
-    def testVerify(self):
-        """Test SubjectConfirmation verify"""
-
+    def testHolderOfKeyUsingTestData(self):
+        """Test subject_confirmation_from_string() using test data for 'holder-of-key' SubjectConfirmation"""
         sc = saml.subject_confirmation_from_string(
-            saml2_data.TEST_SUBJECT_CONFIRMATION)
+            saml2_data.TEST_HOLDER_OF_KEY_SUBJECT_CONFIRMATION)
         assert sc.verify()
+        assert sc.method == saml.SCM_HOLDER_OF_KEY
+        assert sc.subject_confirmation_data is not None
+        assert sc.subject_confirmation_data.not_on_or_after == "2007-09-14T01:05:02Z"
+        assert sc.subject_confirmation_data.recipient == "recipient"
+        assert sc.subject_confirmation_data.in_response_to == "responseID"
+        key_info = sc.subject_confirmation_data.key_info
+        assert len(key_info) == 1
+        assert len(key_info[0].x509_data) == 1
+        assert key_info[0].x509_data[0].x509_certificate.text.strip() == """
+MIICITCCAYoCAQEwDQYJKoZIhvcNAQELBQAwWDELMAkGA1UEBhMCenoxCzAJBgNV
+BAgMAnp6MQ0wCwYDVQQHDAR6enp6MQ4wDAYDVQQKDAVaenp6ejEOMAwGA1UECwwF
+Wnp6enoxDTALBgNVBAMMBHRlc3QwIBcNMTkwNDEyMTk1MDM0WhgPMzAxODA4MTMx
+OTUwMzRaMFgxCzAJBgNVBAYTAnp6MQswCQYDVQQIDAJ6ejENMAsGA1UEBwwEenp6
+ejEOMAwGA1UECgwFWnp6enoxDjAMBgNVBAsMBVp6enp6MQ0wCwYDVQQDDAR0ZXN0
+MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDHcj80WU/XBsd9FlyQmfjPUdfm
+edhCFDd6TEQmZNNqP/UG+VkGa+BXjRIHMfic/WxPTbGhCjv68ci0UDNomUXagFex
+LGNpkwa7+CRVtoc/1xgq+ySE6M4nhcCutScoxNvWNn5eSQ66i3U0sTv91MgsXxqE
+dTaiZg0BIufEc3dueQIDAQABMA0GCSqGSIb3DQEBCwUAA4GBAGUV5B+USHvaRa8k
+gCNJSuNpo6ARlv0ekrk8bbdNRBiEUdCMyoGJFfuM9K0zybX6Vr25wai3nvaog294
+Vx/jWjX2g5SDbjItH6VGy6C9GCGf1A07VxFRCfJn5tA9HuJjPKiE+g/BmrV5N4Ce
+alzFxPHWYkNOzoRU8qI7OqUai1kL""".strip()
 
 
 class TestSubject:
diff --git a/tests/test_93_hok.py b/tests/test_93_hok.py
new file mode 100644
index 00000000..085c930d
--- /dev/null
+++ b/tests/test_93_hok.py
@@ -0,0 +1,53 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+from contextlib import closing
+from datetime import datetime
+from dateutil import parser
+from string import translate, whitespace
+from saml2.authn_context import INTERNETPROTOCOLPASSWORD
+
+from saml2.server import Server
+from saml2.response import authn_response
+from saml2.config import config_factory
+
+from pathutils import dotname, full_path
+
+# Example SAML response iwth 'holder-of-key' subject confirmtaions
+# containing DER-base64 copies (without PEM enclosure) of test_1.crt and test_2.crt 
+HOLDER_OF_KEY_RESPONSE_FILE = full_path("saml_hok.xml") 
+
+TEST_CERT_1 = full_path("test_1.crt")
+TEST_CERT_2 = full_path("test_2.crt")
+
+
+class TestHolderOfKeyResponse:
+    def test_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.do_not_verify = True
+
+        resp.parse_assertion()
+
+        assert resp.get_subject() is not None
+        assert len(resp.assertion.subject.subject_confirmation) == 2
+        actual_certs = [sc.subject_confirmation_data.key_info[0].x509_data[0].x509_certificate.text.strip() 
+                            for sc in resp.assertion.subject.subject_confirmation]
+        expected_certs = [self._read_cert_without_pem_enclosure(TEST_CERT_1),
+                          self._read_cert_without_pem_enclosure(TEST_CERT_2)]
+        assert actual_certs == expected_certs
+
+    def _read_cert_without_pem_enclosure(self, path):
+        with open(path, 'r') as fp:
+            lines = fp.readlines()
+        lines_without_enclosure = lines[1:-1]
+        return ''.join(lines_without_enclosure).translate(None, whitespace)
+
+
+if __name__ == "__main__":
+    t = TestHolderOfKeyResponse()
+    t.setup_class()
+    t.test_hok_response_is_parsed()