Fix CVE-2021-21238 - SAML XML Signature wrapping

All users of pysaml2 that use the default `CryptoBackendXmlSec1` backend and need to verify signed SAML documents are impacted. `pysaml2 <= 6.4.1` does not validate the SAML document against an XML schema. This allows invalid XML documents to trick the verification process, by presenting elements with a valid signature inside elements whose content has been malformed. The verification is offloaded to `xmlsec1` and `xmlsec1` will not validate every signature in the given document, but only the first it finds in the given scope. Credits for the report: - Victor Schönfelder Garcia (isits AG International School of IT Security) - Juraj Somorovsky (Paderborn University) - Vladislav Mladenov (Ruhr University Bochum) Signed-off-by: Ivan Kanakarakis <[email protected]>
IdentityPython · Jan 15, 2021 · 3b70772 · 3b70772
1 parent b76ea40
commit 3b70772
Show file tree

Hide file tree

Showing 7 changed files with 318 additions and 0 deletions.
diff --git a/setup.cfg b/setup.cfg
@@ -54,6 +54,7 @@ install_requires =
     requests >= 1.0.0
     six
     importlib_resources
+    xmlschema
 
 
 [options.packages.find]

diff --git a/src/saml2/sigver.py b/src/saml2/sigver.py
@@ -51,6 +51,8 @@
 from saml2.xmlenc import CipherData
 from saml2.xmlenc import CipherValue
 from saml2.xmlenc import EncryptedData
+from saml2.xml.schema import node_to_schema
+from saml2.xml.schema import XMLSchemaError
 
 
 logger = logging.getLogger(__name__)
@@ -1460,6 +1462,30 @@ def _check_signature(self, decoded_xml, item, node_name=NODE_NAME, origdoc=None,
         if not certs:
             raise MissingKey(_issuer)
 
+        # validate XML with the appropriate schema
+        try:
+            _schema = node_to_schema[node_name]
+        except KeyError as e:
+            error_context = {
+                "message": "Signature verification failed. Unknown node type.",
+                "issuer": _issuer,
+                "type": node_name,
+                "document": decoded_xml,
+            }
+            raise SignatureError(error_context) from e
+
+        try:
+            _schema.validate(str(item))
+        except XMLSchemaError as e:
+            error_context = {
+                "message": "Signature verification failed. Invalid document format.",
+                "ID": item.id,
+                "issuer": _issuer,
+                "type": node_name,
+                "document": decoded_xml,
+            }
+            raise SignatureError(error_context) from e
+
         # saml-core section "5.4 XML Signature Profile" defines constrains on the
         # xmldsig-core facilities. It explicitly dictates that enveloped signatures
         # are the only signatures allowed. This means that:

diff --git a/src/saml2/xml/__init__.py b/src/saml2/xml/__init__.py
diff --git a/src/saml2/xml/schema/__init__.py b/src/saml2/xml/schema/__init__.py
@@ -0,0 +1,74 @@
+from importlib_resources import path as _resource_path
+
+from xmlschema import XMLSchema as _XMLSchema
+from xmlschema.exceptions import XMLSchemaException as XMLSchemaError
+
+import saml2.data.schemas as _data_schemas
+
+
+def _create_xml_schema_validator(source, **kwargs):
+    kwargs = {
+        **kwargs,
+        "validation": "strict",
+        "locations": _locations,
+        "base_url": source,
+        "allow": "sandbox",
+        "use_fallback": False,
+    }
+    return _XMLSchema(source, **kwargs)
+
+
+with _resource_path(_data_schemas, "xml.xsd") as fp:
+    _path_schema_xml = str(fp)
+with _resource_path(_data_schemas, "envelope.xsd") as fp:
+    _path_schema_envelope = str(fp)
+with _resource_path(_data_schemas, "xenc-schema.xsd") as fp:
+    _path_schema_xenc = str(fp)
+with _resource_path(_data_schemas, "xmldsig-core-schema.xsd") as fp:
+    _path_schema_xmldsig_core = str(fp)
+with _resource_path(_data_schemas, "saml-schema-assertion-2.0.xsd") as fp:
+    _path_schema_saml_assertion = str(fp)
+with _resource_path(_data_schemas, "saml-schema-metadata-2.0.xsd") as fp:
+    _path_schema_saml_metadata = str(fp)
+with _resource_path(_data_schemas, "saml-schema-protocol-2.0.xsd") as fp:
+    _path_schema_saml_protocol = str(fp)
+
+_locations = {
+    "http://www.w3.org/XML/1998/namespace": _path_schema_xml,
+    "http://schemas.xmlsoap.org/soap/envelope/": _path_schema_envelope,
+    "http://www.w3.org/2001/04/xmlenc#": _path_schema_xenc,
+    "http://www.w3.org/2000/09/xmldsig#": _path_schema_xmldsig_core,
+    "urn:oasis:names:tc:SAML:2.0:assertion": _path_schema_saml_assertion,
+    "urn:oasis:names:tc:SAML:2.0:protocol": _path_schema_saml_protocol,
+}
+
+with _resource_path(_data_schemas, "saml-schema-assertion-2.0.xsd") as fp:
+    schema_saml_assertion = _create_xml_schema_validator(str(fp))
+with _resource_path(_data_schemas, "saml-schema-metadata-2.0.xsd") as fp:
+    schema_saml_metadata = _create_xml_schema_validator(str(fp))
+with _resource_path(_data_schemas, "saml-schema-protocol-2.0.xsd") as fp:
+    schema_saml_protocol = _create_xml_schema_validator(str(fp))
+
+
+node_to_schema = {
+    # AssertionType
+    "urn:oasis:names:tc:SAML:2.0:assertion:Assertion": schema_saml_assertion,
+    # EntitiesDescriptorType
+    "urn:oasis:names:tc:SAML:2.0:metadata:EntitiesDescriptor": schema_saml_metadata,
+    # EntityDescriptorType
+    "urn:oasis:names:tc:SAML:2.0:metadata:EntityDescriptor": schema_saml_metadata,
+    # RequestAbstractType
+    "urn:oasis:names:tc:SAML:2.0:protocol:AssertionIDRequest": schema_saml_protocol,
+    "urn:oasis:names:tc:SAML:2.0:protocol:SubjectQuery": schema_saml_protocol,
+    "urn:oasis:names:tc:SAML:2.0:protocol:AuthnRequest": schema_saml_protocol,
+    "urn:oasis:names:tc:SAML:2.0:protocol:ArtifactResolve": schema_saml_protocol,
+    "urn:oasis:names:tc:SAML:2.0:protocol:ManageNameIDRequest": schema_saml_protocol,
+    "urn:oasis:names:tc:SAML:2.0:protocol:LogoutRequest": schema_saml_protocol,
+    "urn:oasis:names:tc:SAML:2.0:protocol:NameIDMappingRequest": schema_saml_protocol,
+    # StatusResponseType
+    "urn:oasis:names:tc:SAML:2.0:protocol:Response": schema_saml_protocol,
+    "urn:oasis:names:tc:SAML:2.0:protocol:ArtifactResponse": schema_saml_protocol,
+    "urn:oasis:names:tc:SAML:2.0:protocol:ManageNameIDResponse": schema_saml_protocol,
+    "urn:oasis:names:tc:SAML:2.0:protocol:LogoutResponse": schema_saml_protocol,
+    "urn:oasis:names:tc:SAML:2.0:protocol:NameIDMappingResponse": schema_saml_protocol,
+}
diff --git a/tests/test_xsw.py b/tests/test_xsw.py
@@ -18,6 +18,8 @@
 SIGNED_XSW_ASSERTION_EXTENSIONS = full_path("xsw/signed-xsw-assertion-extensions.xml")
 SIGNED_XSW_ASSERTION_ASSERTION = full_path("xsw/signed-xsw-assertion-assertion.xml")
 
+SIGNED_ASSERTION_FIRST_SIG = full_path("xsw/signed-xsw-assertion-in-assertion-first-sig.xml")
+SIGNED_REPONSE_FIRST_SIG = full_path("xsw/signed-xsw-response-in-response-first-sig.xml")
 
 
 class TestXSW:
@@ -87,3 +89,42 @@ def test_signed_xsw_assertion_assertion_should_fail(self, mock_validate_on_or_af
 
         assert self.ar.ava is None
         assert self.ar.name_id is None
+
+
+class TestInvalidDepthFirstSig:
+    def setup_class(self):
+        self.conf = config_factory("sp", dotname("server_conf"))
+        self.ar = authn_response(self.conf, return_addrs="https://example.org/acs/post")
+
+    @patch('saml2.response.validate_on_or_after', return_value=True)
+    def test_signed_assertion_first_sig_should_fail(self, mock_validate_on_or_after):
+        self.ar.issue_instant_ok = Mock(return_value=True)
+
+        with open(SIGNED_ASSERTION_FIRST_SIG) as fp:
+            xml_response = fp.read()
+
+        self.ar.outstanding_queries = {"id-abc": "http://localhost:8088/sso"}
+        self.ar.timeslack = 10000
+        self.ar.loads(xml_response, decode=False)
+
+        assert self.ar.came_from == 'http://localhost:8088/sso'
+        assert self.ar.session_id() == "id-abc"
+        assert self.ar.issuer() == 'urn:mace:example.com:saml:roland:idp'
+
+        with raises(SignatureError):
+            self.ar.verify()
+
+        assert self.ar.ava is None
+        assert self.ar.name_id is None
+
+    @patch('saml2.response.validate_on_or_after', return_value=True)
+    def test_signed_response_first_sig_should_fail(self, mock_validate_on_or_after):
+        self.ar.issue_instant_ok = Mock(return_value=True)
+
+        with open(SIGNED_REPONSE_FIRST_SIG) as fp:
+            xml_response = fp.read()
+
+        self.ar.outstanding_queries = {"id-abc": "http://localhost:8088/sso"}
+        self.ar.timeslack = 10000
+        with raises(SignatureError):
+            self.ar.loads(xml_response, decode=False)
diff --git a/tests/xsw/signed-xsw-assertion-in-assertion-first-sig.xml b/tests/xsw/signed-xsw-assertion-in-assertion-first-sig.xml
@@ -0,0 +1,85 @@
+<?xml version="1.0"?>
+<samlp:Response xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" ID="the-response-id" InResponseTo="id-abc" Version="2.0" IssueInstant="2020-09-14T22:37:32Z" Destination="https://example.org/acs/post">
+	<saml:Issuer xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion">urn:mace:example.com:saml:roland:idp</saml:Issuer>
+	<samlp:Status xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol">
+		<samlp:StatusCode xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" Value="urn:oasis:names:tc:SAML:2.0:status:Success"/>
+	</samlp:Status>
+	<saml:Assertion xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" xmlns:ds="http://www.w3.org/2000/09/xmldsig#" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" ID="attack-assertion-id" IssueInstant="2020-09-14T22:37:32Z" Version="2.0">
+	<saml:Assertion xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" xmlns:ds="http://www.w3.org/2000/09/xmldsig#" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" ID="the-assertion-id" IssueInstant="2020-09-14T22:37:32Z" Version="2.0">
+		<saml:Issuer>urn:mace:example.com:saml:roland:idp</saml:Issuer>
+		<ds:Signature xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
+			<ds:SignedInfo>
+				<ds:CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/>
+				<ds:SignatureMethod Algorithm="http://www.w3.org/2000/09/xmldsig#rsa-sha1"/>
+				<ds:Reference URI="#the-assertion-id">
+					<ds:Transforms>
+						<ds:Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature"/>
+						<ds:Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/>
+					</ds:Transforms>
+					<ds:DigestMethod Algorithm="http://www.w3.org/2000/09/xmldsig#sha1"/>
+					<ds:DigestValue>iLDF5/5VJs4sb3TasVTvFCsIi0k=</ds:DigestValue>
+				</ds:Reference>
+			</ds:SignedInfo>
+			<ds:SignatureValue>Ked5gvNcRhHCivVN9y9+5LDAZLqLhRg3Sw2xlRR4HP2am1mFoBDdUx4khEWdcC2dknbzfo2AC1AtcbHTogDLOSLzYX9sT/gj995qotu4fUFQPMiocbCZRpbXTI6iDRiytwYtAkw28yQ4FVCe99GUThbV9tpLIoqMPZYNJ3TmL/I=</ds:SignatureValue>
+			<ds:KeyInfo>
+				<ds:X509Data>
+					<ds:X509Certificate>MIICsDCCAhmgAwIBAgIJAJrzqSSwmDY9MA0GCSqGSIb3DQEBBQUAMEUxCzAJBgNVBAYTAkFVMRMwEQYDVQQIEwpTb21lLVN0YXRlMSEwHwYDVQQKExhJbnRlcm5ldCBXaWRnaXRzIFB0eSBMdGQwHhcNMDkxMDA2MTk0OTQxWhcNMDkxMTA1MTk0OTQxWjBFMQswCQYDVQQGEwJBVTETMBEGA1UECBMKU29tZS1TdGF0ZTEhMB8GA1UEChMYSW50ZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDJg2cms7MqjniT8Fi/XkNHZNPbNVQyMUMXE9tXOdqwYCA1cc8vQdzkihscQMXy3iPw2cMggBu6gjMTOSOxECkuvX5ZCclKr8pXAJM5cY6gVOaVO2PdTZcvDBKGbiaNefiEw5hnoZomqZGp8wHNLAUkwtH9vjqqvxyS/vclc6k2ewIDAQABo4GnMIGkMB0GA1UdDgQWBBRePsKHKYJsiojE78ZWXccK9K4aJTB1BgNVHSMEbjBsgBRePsKHKYJsiojE78ZWXccK9K4aJaFJpEcwRTELMAkGA1UEBhMCQVUxEzARBgNVBAgTClNvbWUtU3RhdGUxITAfBgNVBAoTGEludGVybmV0IFdpZGdpdHMgUHR5IEx0ZIIJAJrzqSSwmDY9MAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQEFBQADgYEAJSrKOEzHO7TL5cy6h3qh+3+JAk8HbGBW+cbX6KBCAw/mzU8flK25vnWwXS3dv2FF3Aod0/S7AWNfKib5U/SA9nJaz/mWeF9S0farz9AQFc8/NSzAzaVq7YbM4F6f6N2FRl7GikdXRCed45j6mrPzGzk3ECbupFnqyREH3+ZPSdk=</ds:X509Certificate>
+				</ds:X509Data>
+			</ds:KeyInfo>
+		</ds:Signature>
+		<saml:Subject>
+			<saml:NameID Format="urn:oasis:names:tc:SAML:2.0:nameid-format:persistent">the-name-id</saml:NameID>
+			<saml:SubjectConfirmation Method="urn:oasis:names:tc:SAML:2.0:cm:bearer">
+				<saml:SubjectConfirmationData InResponseTo="id-abc" NotOnOrAfter="2020-09-14T22:47:32Z" Recipient="https://example.org/acs/post"/>
+			</saml:SubjectConfirmation>
+		</saml:Subject>
+		<saml:Conditions NotBefore="2020-09-14T22:27:32Z" NotOnOrAfter="2020-09-14T22:47:32Z">
+			<saml:AudienceRestriction>
+				<saml:Audience>urn:mace:example.com:saml:roland:sp</saml:Audience>
+			</saml:AudienceRestriction>
+		</saml:Conditions>
+		<saml:AuthnStatement AuthnInstant="2020-09-14T22:37:32Z" SessionIndex="id-sessidx">
+			<saml:AuthnContext>
+				<saml:AuthnContextClassRef>urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport</saml:AuthnContextClassRef>
+			</saml:AuthnContext>
+		</saml:AuthnStatement>
+	</saml:Assertion>
+		<saml:Issuer>urn:mace:example.com:saml:roland:idp</saml:Issuer>
+		<ds:Signature xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
+			<ds:SignedInfo>
+				<ds:CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/>
+				<ds:SignatureMethod Algorithm="http://www.w3.org/2000/09/xmldsig#rsa-sha1"/>
+				<ds:Reference URI="#attack-assertion-id">
+					<ds:Transforms>
+						<ds:Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature"/>
+						<ds:Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/>
+					</ds:Transforms>
+					<ds:DigestMethod Algorithm="http://www.w3.org/2000/09/xmldsig#sha1"/>
+					<ds:DigestValue>dGhpcyBpcyBza2lwcGVkOyBvbmx5IHRoZSBmaXJzdCBzaWduYXR1cmUgaXMgcHJvY2Vzc2VkCg==</ds:DigestValue>
+				</ds:Reference>
+			</ds:SignedInfo>
+			<ds:SignatureValue>dGhpcyBpcyBza2lwcGVkOyBvbmx5IHRoZSBmaXJzdCBzaWduYXR1cmUgaXMgcHJvY2Vzc2VkCg==</ds:SignatureValue>
+			<ds:KeyInfo>
+				<ds:X509Data>
+					<ds:X509Certificate>MIICsDCCAhmgAwIBAgIJAJrzqSSwmDY9MA0GCSqGSIb3DQEBBQUAMEUxCzAJBgNVBAYTAkFVMRMwEQYDVQQIEwpTb21lLVN0YXRlMSEwHwYDVQQKExhJbnRlcm5ldCBXaWRnaXRzIFB0eSBMdGQwHhcNMDkxMDA2MTk0OTQxWhcNMDkxMTA1MTk0OTQxWjBFMQswCQYDVQQGEwJBVTETMBEGA1UECBMKU29tZS1TdGF0ZTEhMB8GA1UEChMYSW50ZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDJg2cms7MqjniT8Fi/XkNHZNPbNVQyMUMXE9tXOdqwYCA1cc8vQdzkihscQMXy3iPw2cMggBu6gjMTOSOxECkuvX5ZCclKr8pXAJM5cY6gVOaVO2PdTZcvDBKGbiaNefiEw5hnoZomqZGp8wHNLAUkwtH9vjqqvxyS/vclc6k2ewIDAQABo4GnMIGkMB0GA1UdDgQWBBRePsKHKYJsiojE78ZWXccK9K4aJTB1BgNVHSMEbjBsgBRePsKHKYJsiojE78ZWXccK9K4aJaFJpEcwRTELMAkGA1UEBhMCQVUxEzARBgNVBAgTClNvbWUtU3RhdGUxITAfBgNVBAoTGEludGVybmV0IFdpZGdpdHMgUHR5IEx0ZIIJAJrzqSSwmDY9MAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQEFBQADgYEAJSrKOEzHO7TL5cy6h3qh+3+JAk8HbGBW+cbX6KBCAw/mzU8flK25vnWwXS3dv2FF3Aod0/S7AWNfKib5U/SA9nJaz/mWeF9S0farz9AQFc8/NSzAzaVq7YbM4F6f6N2FRl7GikdXRCed45j6mrPzGzk3ECbupFnqyREH3+ZPSdk=</ds:X509Certificate>
+				</ds:X509Data>
+			</ds:KeyInfo>
+		</ds:Signature>
+		<saml:Subject>
+			<saml:NameID Format="urn:oasis:names:tc:SAML:2.0:nameid-format:persistent">attack-name-id</saml:NameID>
+			<saml:SubjectConfirmation Method="urn:oasis:names:tc:SAML:2.0:cm:bearer">
+				<saml:SubjectConfirmationData InResponseTo="id-abc" NotOnOrAfter="2020-09-14T22:47:32Z" Recipient="https://example.org/acs/post"/>
+			</saml:SubjectConfirmation>
+		</saml:Subject>
+		<saml:Conditions NotBefore="2020-09-14T22:27:32Z" NotOnOrAfter="2020-09-14T22:47:32Z">
+			<saml:AudienceRestriction>
+				<saml:Audience>urn:mace:example.com:saml:roland:sp</saml:Audience>
+			</saml:AudienceRestriction>
+		</saml:Conditions>
+		<saml:AuthnStatement AuthnInstant="2020-09-14T22:37:32Z" SessionIndex="id-sessidx">
+			<saml:AuthnContext>
+				<saml:AuthnContextClassRef>urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport</saml:AuthnContextClassRef>
+			</saml:AuthnContext>
+		</saml:AuthnStatement>
+	</saml:Assertion>
+</samlp:Response>