IdentityPython / pysaml2

Python implementation of SAML2
Apache License 2.0
555 stars 422 forks source link

soap.open_soap_envelope()['body'] appears to alter contents of a SOAP message #551

Open spaetow opened 5 years ago

spaetow commented 5 years ago

Using PySAML2 v4.6.2, I am trying to build a SAML Response consumer (not necessarily a SP).

I receive a message using pyCURL (attached as xmldoc-_7bb54ec7a8a59f971b1b995206ce6a344600051f63.txt ) - I then do this to extract the SAML response from the SOAP message:

aq_response = soap.open_soap_envelope(curl_output)['body']

I then feed this into a sigver.CryptoBackendXMLSecurity() object (using pyXMLSecurity v0.18) like so:

cb_xmlsec = sigver.CryptoBackendXMLSecurity()
is_valid_signature = cb_xmlsec.validate_signature(aq_response, aq_ca_cert, 'pem', None, None, None)

The signature validation fails, and in the logger I see these messages:

DEBUG:root:key size: 4096 bits
DEBUG:root:Found signedinfo <ns2:SignedInfo xmlns:ns2="http://www.w3.org/2000/09/xmldsig#" xmlns:ns0="urn:oasis:names:tc:SAML:2.0:protocol" xmlns:ns1="urn:oasis:names:tc:SAML:2.0:assertion" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"><ns2:CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/><ns2:SignatureMethod Algorithm="http://www.w3.org/2000/09/xmldsig#rsa-sha1"/><ns2:Reference URI="#_7bb54ec7a8a59f971b1b995206ce6a344600051f63"><ns2:Transforms><ns2:Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature"/><ns2:Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/></ns2:Transforms><ns2:DigestMethod Algorithm="http://www.w3.org/2000/09/xmldsig#sha1"/><ns2:DigestValue>mTRj5GCV7+jP7HKCHYk+paTfxas=</ns2:DigestValue></ns2:Reference></ns2:SignedInfo>
DEBUG:root:Looking for #_7bb54ec7a8a59f971b1b995206ce6a344600051f63 using id attribute 'ID'
DEBUG:root:using hash algorithm sha1
DEBUG:root:computed sha1 digest tO/IPLnB5XI6tP8kJM1RhPS1L/8= for ref #_7bb54ec7a8a59f971b1b995206ce6a344600051f63
DEBUG:root:found sha1 digest mTRj5GCV7+jP7HKCHYk+paTfxas= for ref #_7bb54ec7a8a59f971b1b995206ce6a344600051f63
ERROR:root:not returning ref #_7bb54ec7a8a59f971b1b995206ce6a344600051f63 - digest mismatch

See the digest mismatch? When I manually strip the SOAP tags out and feed that into the code, I get further in the log (although is_valid_signature is still False):

DEBUG:root:key size: 4096 bits
DEBUG:root:Found signedinfo <ds:SignedInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#" xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion"><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="#_7bb54ec7a8a59f971b1b995206ce6a344600051f63"><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>mTRj5GCV7+jP7HKCHYk+paTfxas=</ds:DigestValue></ds:Reference></ds:SignedInfo>
DEBUG:root:Looking for #_7bb54ec7a8a59f971b1b995206ce6a344600051f63 using id attribute 'ID'
DEBUG:root:using hash algorithm sha1
DEBUG:root:computed sha1 digest mTRj5GCV7+jP7HKCHYk+paTfxas= for ref #_7bb54ec7a8a59f971b1b995206ce6a344600051f63
DEBUG:root:found sha1 digest mTRj5GCV7+jP7HKCHYk+paTfxas= for ref #_7bb54ec7a8a59f971b1b995206ce6a344600051f63
DEBUG:root:transform http://www.w3.org/2001/10/xml-exc-c14n# on <ds:SignedInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#" xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion"><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="#_7bb54ec7a8a59f971b1b995206ce6a344600051f63"><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>mTRj5GCV7+jP7HKCHYk+paTfxas=</ds:DigestValue></ds:Reference></ds:SignedInfo>
DEBUG:root:SignedInfo C14N: <ds:SignedInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#"><ds:CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"></ds:CanonicalizationMethod><ds:SignatureMethod Algorithm="http://www.w3.org/2000/09/xmldsig#rsa-sha1"></ds:SignatureMethod><ds:Reference URI="#_7bb54ec7a8a59f971b1b995206ce6a344600051f63"><ds:Transforms><ds:Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature"></ds:Transform><ds:Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"></ds:Transform></ds:Transforms><ds:DigestMethod Algorithm="http://www.w3.org/2000/09/xmldsig#sha1"></ds:DigestMethod><ds:DigestValue>mTRj5GCV7+jP7HKCHYk+paTfxas=</ds:DigestValue></ds:Reference></ds:SignedInfo>
DEBUG:root:SignedInfo digest: c94rVY2u5+eFvrZEXJnxZQzKHQw=
ERROR:root:Failed to validate <ds:Signature xmlns:ds="http://www.w3.org/2000/09/xmldsig#" xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion"><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="#_7bb54ec7a8a59f971b1b995206ce6a344600051f63"><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>mTRj5GCV7+jP7HKCHYk+paTfxas=</ds:DigestValue></ds:Reference></ds:SignedInfo><ds:SignatureValue>dLtV7jF5PgeGjfMTbnshfzD1g2qPdKPZNzfCEzlfPrMx8ZZsqUQJNIVoqKx6Vqi8iJIdoezJwtz8XOmXO5vkZe1aNVVC3hptPGbNJbUwU8Q8exhHwlc9w6u+nHqeh9EtQgMThv3c1PxxBpZ9o2HsqU/PFADuP/JrwdLdAS3BSeGNp9YpUu0FswjpFQn5t2pwNMvkZzhMdzsa/KV0JtAuBlUo8dseFU0hgKnPlXn821C7cS0Lc28DU5H4Xjgwyfd7ymOswwbm5IteZyGlq5gUP3YHODxl3RAEp0cKL7DPhmDe1BUAD68WhVq5bUs1mQdYwKogSLP0aHmuGGaHjDPt185PVcjRnKbMyplIdFZzE3c+ttSSYOP1/PApt0Y3uJS85rn53Kipk/z07ZvYe8sE4+OdtTeWNVtvAR41tc7p56Csc5wa1wUhARnSv6NwVhwM1aySkZu11BGtqSy7lEvHQsHBb8U3VTacDogD9CtyGsiQi+qLqVSTrFT/tKf697PKsKWOJkSrcP51v/OjB/XqEN6fN45YMuIK8P8T8iBlOtcfV4Hlq0lST4gGSw4OP/ds2xCXBsBYJLkMoIIwzX9tgVLhFmqDWCvzqZ/6/WTvYwI6p6HhmM0+xpou/OmBqHDlZ5kUk3qthjmhWXEPdXSknZ7W6zMDpN2tpxDC3eJLVSM=</ds:SignatureValue><ds:KeyInfo><ds:X509Data><ds:X509Certificate>MIIFLzCCAxegAwIBAgIJAMszSeMjDJ4CMA0GCSqGSIb3DQEBCwUAMC4xLDAqBgNVBAMMI2NvbWFuYWdlLmVkdXRlYW1zLXBsYXlwZW4udGkuamEubmV0MB4XDTE4MDUxNTE1NDc1NFoXDTI4MDUxNDE1NDc1NFowLjEsMCoGA1UEAwwjY29tYW5hZ2UuZWR1dGVhbXMtcGxheXBlbi50aS5qYS5uZXQwggIiMA0GCSqGSIb3DQEBAQUAA4ICDwAwggIKAoICAQDIDKmsP1QAs4EcySN9CFEC3PkzXcIF6wUNsZ6IBpAGQzGdwmabEI53P6cnoAlcVQN4PCY1O15V1W7dKrEDBMchEksss1LjzrajhBreIVJRDxzPs4L4RQTC83z2tvjLlA15MH0rqOtOfv9Ql2KOeNuShDXS4fZl4mI2lOWIVn9FI/H0rpBnY/IupwLMsp0yEDyppU3EGa9xDigS9KP6e51BjVOk/s9GJz4TW/zUukLW4jR5AEY2Yitxi2h8HMArMj0HWIfdIptXWrVfv63FMJonaZ9LQr7Ye6mEKGlQTYLD+pjEoJPEwbqmLwg8Tn5tjAfs7m1KgTd6TAPh6QxpYtVsTXvHVfP3qie+je4oNDzhkzjdhJr7eutkgBJiorkuSGjwvsaywMS27EHdCfI57HsuOuTsmCKJaI3FOLyqXDLwq8LO20YdVGUTvcs7rhW/oWQYiNWCk/EhW5iNbs0XbwyMpuPVH5TDOPNHeIUdQE2v+cfU1Fs+P5cHz1PhOD117k5EzEKM6ZZXfrZI71qnEDe0mWbyIY13OjDNS9T0YNMG+l1zsukG8TTiad0pI0X9OaFzZuvsN51nZWuTvoI3JUJMckF3f2fnlWm55uLiLKf7KtJkO2anSyCcHtzEDuFInV22x/P3HVKQhDqqrs2HX+Z3z996R117nai/cPkcQAJ/gwIDAQABo1AwTjAdBgNVHQ4EFgQUpWMBNlgCeXz0mJSu+ee0VgRavjYwHwYDVR0jBBgwFoAUpWMBNlgCeXz0mJSu+ee0VgRavjYwDAYDVR0TBAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAgEAouf3R/1z7mk2tnhkBsb5vv3tKt3XXJcGjKPMK/gaY1YTNj61pVudPswXBWLoNTn6yKBBy5uhm7mRX5rcuq+gfpdXOqZXxZoNXGx5qkd5A7zJkbLfW5TgD+sp9Ln+5KgtjN3wt6JkgZlKspd0jcsuH0og4BXlAEHtLIG6IOC2Arby+8vy8mfYB2JMtcHnwcn2WaixzuBmuGxovpzwN+xE9Yt6rFnizYWtQs40vubxpvJRToymUEd26jWvLhpG9B6U/oeFZtOHUX/ebhsBnkouPOvPTJm8cM6hiIb2ExeNh4IfP3KT7zIeOTWR/aQ4z1v6Jcn763Xj5EHYpRTiyWOLvslmAgr2k0HraZDWwfDX+Lifxz5iuXGZs1Jmxu6NCiTkqOIioIKZLA9R8KJ/r2Oc5j//lAXWWcpaqtogn0ObAmfr4g1rT/Da8YGsuou821i5Wa7IlkuSP+hLqve0XxyDRi2YH39yp0x28hisvrq4kAD343dN7QPemJ5VJXJ6pMTNza6wXe8L1VWDqYzDowSrmG/lmhn2Z3bTx3Xfwbw/c1ynRfrbBtdo9MKyGfQnn4xT8VQn4lp1i9uyJoURoLSazCN5y21qYuGInSxE/Irz36HaZXKiwD2snH+fDPJGIq057VayDKtgOoaKKXYiVNUSxu7qtMlPUacUoeoAZwRQv1g=</ds:X509Certificate></ds:X509Data></ds:KeyInfo></ds:Signature> using sig digest sha1 and cm http://www.w3.org/2001/10/xml-exc-c14n#

So, something open_soap_envelope() does breaks the contents in a way that causes the digest to be calculated incorrectly. That said, I have an issue open with the pyXMLSecurity people to check why the signature calculation fails.

Can someone let me know if I'm doing this wrong? If I am, what should I be doing?

c00kiemon5ter commented 5 years ago

hello, I am referencing IdentityPython/pyXMLSecurity#54 and IdentityPython/pyXMLSecurity#55 to have them link here, as the issues are closely connected.

This is the relevant code I tested with in a python shell:

from lxml import etree
import saml2.entity
import saml2.soap
import xmlsec

# soap_data is the xml string of the soap-envelop
with open('test-data/xmldoc-_7bb54ec7a8a59f971b1b995206ce6a344600051f63.txt') as f:
    soap_data = f.read()

# response_data is the xml string of the saml-response
# generated by stripping the soap tags from the soap_data
response_data = saml2.soap.parse_soap_enveloped_saml_authn_response(soap_data)

# response_tree is the lxml/etree representation of the saml-response document
response_tree = etree.fromstring(response_data)

# soap_obj is an instance of the saml2.samlp.Response class
soap_obj = saml2.entity.Entity.parse_soap_message(soap_data)['body']

# keyspec holds the key
keyspec = soap_obj.signature.key_info.x509_data[0].x509_certificate.text

# having collected all the info we need, we call verify
xmlsec.verify(response_tree, keyspec)

verify() will fail with:

(1) ERROR:root:not returning ref #_7bb54ec7a8a59f971b1b995206ce6a344600051f63 - digest mismatch (2) XMLSigException: No valid ds:Signature elements found

(1) is the real reason validation failed; the generated digest did not match. (2) is actually a lie; the signature was found but did not pass validation.


The parsed document by pysaml2 is:

<?xml version="1.0" encoding="UTF-8"?>
<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" ID="_7bb54ec7a8a59f971b1b995206ce6a344600051f63" InResponseTo="f9f6a703-aa82-4a93-a235-3a08ec62518e" IssueInstant="2018-10-11T12:57:44Z" Version="2.0"><ns1:Issuer>https://comanage.eduteams-playpen.ti.ja.net/ss</ns1:Issuer><ns2:Signature>
  <ns2:SignedInfo><ns2:CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#" />
    <ns2:SignatureMethod Algorithm="http://www.w3.org/2000/09/xmldsig#rsa-sha1" />
  <ns2:Reference URI="#_7bb54ec7a8a59f971b1b995206ce6a344600051f63"><ns2:Transforms><ns2:Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature" /><ns2:Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#" /></ns2:Transforms><ns2:DigestMethod Algorithm="http://www.w3.org/2000/09/xmldsig#sha1" /><ns2:DigestValue>mTRj5GCV7+jP7HKCHYk+paTfxas=</ns2:DigestValue></ns2:Reference></ns2:SignedInfo><ns2:SignatureValue>dLtV7jF5PgeGjfMTbnshfzD1g2qPdKPZNzfCEzlfPrMx8ZZsqUQJNIVoqKx6Vqi8iJIdoezJwtz8XOmXO5vkZe1aNVVC3hptPGbNJbUwU8Q8exhHwlc9w6u+nHqeh9EtQgMThv3c1PxxBpZ9o2HsqU/PFADuP/JrwdLdAS3BSeGNp9YpUu0FswjpFQn5t2pwNMvkZzhMdzsa/KV0JtAuBlUo8dseFU0hgKnPlXn821C7cS0Lc28DU5H4Xjgwyfd7ymOswwbm5IteZyGlq5gUP3YHODxl3RAEp0cKL7DPhmDe1BUAD68WhVq5bUs1mQdYwKogSLP0aHmuGGaHjDPt185PVcjRnKbMyplIdFZzE3c+ttSSYOP1/PApt0Y3uJS85rn53Kipk/z07ZvYe8sE4+OdtTeWNVtvAR41tc7p56Csc5wa1wUhARnSv6NwVhwM1aySkZu11BGtqSy7lEvHQsHBb8U3VTacDogD9CtyGsiQi+qLqVSTrFT/tKf697PKsKWOJkSrcP51v/OjB/XqEN6fN45YMuIK8P8T8iBlOtcfV4Hlq0lST4gGSw4OP/ds2xCXBsBYJLkMoIIwzX9tgVLhFmqDWCvzqZ/6/WTvYwI6p6HhmM0+xpou/OmBqHDlZ5kUk3qthjmhWXEPdXSknZ7W6zMDpN2tpxDC3eJLVSM=</ns2:SignatureValue>
<ns2:KeyInfo><ns2:X509Data><ns2:X509Certificate>MIIFLzCCAxegAwIBAgIJAMszSeMjDJ4CMA0GCSqGSIb3DQEBCwUAMC4xLDAqBgNVBAMMI2NvbWFuYWdlLmVkdXRlYW1zLXBsYXlwZW4udGkuamEubmV0MB4XDTE4MDUxNTE1NDc1NFoXDTI4MDUxNDE1NDc1NFowLjEsMCoGA1UEAwwjY29tYW5hZ2UuZWR1dGVhbXMtcGxheXBlbi50aS5qYS5uZXQwggIiMA0GCSqGSIb3DQEBAQUAA4ICDwAwggIKAoICAQDIDKmsP1QAs4EcySN9CFEC3PkzXcIF6wUNsZ6IBpAGQzGdwmabEI53P6cnoAlcVQN4PCY1O15V1W7dKrEDBMchEksss1LjzrajhBreIVJRDxzPs4L4RQTC83z2tvjLlA15MH0rqOtOfv9Ql2KOeNuShDXS4fZl4mI2lOWIVn9FI/H0rpBnY/IupwLMsp0yEDyppU3EGa9xDigS9KP6e51BjVOk/s9GJz4TW/zUukLW4jR5AEY2Yitxi2h8HMArMj0HWIfdIptXWrVfv63FMJonaZ9LQr7Ye6mEKGlQTYLD+pjEoJPEwbqmLwg8Tn5tjAfs7m1KgTd6TAPh6QxpYtVsTXvHVfP3qie+je4oNDzhkzjdhJr7eutkgBJiorkuSGjwvsaywMS27EHdCfI57HsuOuTsmCKJaI3FOLyqXDLwq8LO20YdVGUTvcs7rhW/oWQYiNWCk/EhW5iNbs0XbwyMpuPVH5TDOPNHeIUdQE2v+cfU1Fs+P5cHz1PhOD117k5EzEKM6ZZXfrZI71qnEDe0mWbyIY13OjDNS9T0YNMG+l1zsukG8TTiad0pI0X9OaFzZuvsN51nZWuTvoI3JUJMckF3f2fnlWm55uLiLKf7KtJkO2anSyCcHtzEDuFInV22x/P3HVKQhDqqrs2HX+Z3z996R117nai/cPkcQAJ/gwIDAQABo1AwTjAdBgNVHQ4EFgQUpWMBNlgCeXz0mJSu+ee0VgRavjYwHwYDVR0jBBgwFoAUpWMBNlgCeXz0mJSu+ee0VgRavjYwDAYDVR0TBAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAgEAouf3R/1z7mk2tnhkBsb5vv3tKt3XXJcGjKPMK/gaY1YTNj61pVudPswXBWLoNTn6yKBBy5uhm7mRX5rcuq+gfpdXOqZXxZoNXGx5qkd5A7zJkbLfW5TgD+sp9Ln+5KgtjN3wt6JkgZlKspd0jcsuH0og4BXlAEHtLIG6IOC2Arby+8vy8mfYB2JMtcHnwcn2WaixzuBmuGxovpzwN+xE9Yt6rFnizYWtQs40vubxpvJRToymUEd26jWvLhpG9B6U/oeFZtOHUX/ebhsBnkouPOvPTJm8cM6hiIb2ExeNh4IfP3KT7zIeOTWR/aQ4z1v6Jcn763Xj5EHYpRTiyWOLvslmAgr2k0HraZDWwfDX+Lifxz5iuXGZs1Jmxu6NCiTkqOIioIKZLA9R8KJ/r2Oc5j//lAXWWcpaqtogn0ObAmfr4g1rT/Da8YGsuou821i5Wa7IlkuSP+hLqve0XxyDRi2YH39yp0x28hisvrq4kAD343dN7QPemJ5VJXJ6pMTNza6wXe8L1VWDqYzDowSrmG/lmhn2Z3bTx3Xfwbw/c1ynRfrbBtdo9MKyGfQnn4xT8VQn4lp1i9uyJoURoLSazCN5y21qYuGInSxE/Irz36HaZXKiwD2snH+fDPJGIq057VayDKtgOoaKKXYiVNUSxu7qtMlPUacUoeoAZwRQv1g=</ns2:X509Certificate></ns2:X509Data></ns2:KeyInfo></ns2:Signature><ns0:Status><ns0:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:Success" /></ns0:Status><ns1:Assertion ID="_3c03d55f796d5cc9269a32ef61d726478050f1e8e6" IssueInstant="2018-10-11T12:57:44Z" Version="2.0"><ns1:Issuer>https://comanage.eduteams-playpen.ti.ja.net/ss</ns1:Issuer><ns1:Subject><ns1:NameID>test@eduteams-playpen.ti.ja.net</ns1:NameID><ns1:SubjectConfirmation Method="urn:oasis:names:tc:SAML:2.0:cm:bearer"><ns1:SubjectConfirmationData InResponseTo="f9f6a703-aa82-4a93-a235-3a08ec62518e" NotBefore="2018-10-11T12:57:44Z" NotOnOrAfter="2018-10-11T13:02:44Z" /></ns1:SubjectConfirmation></ns1:Subject><ns1:Conditions NotBefore="2018-10-11T12:57:44Z" NotOnOrAfter="2018-10-11T13:02:44Z"><ns1:AudienceRestriction><ns1:Audience>https://sp3.eduteams-playpen.ti.ja.net/shibboleth</ns1:Audience></ns1:AudienceRestriction></ns1:Conditions><ns1:AttributeStatement><ns1:Attribute Name="urn:oid:1.3.6.1.4.1.5923.1.1.1.7" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri"><ns1:AttributeValue xsi:type="xs:string">some:entitlement:prefix:hexaa:8:member</ns1:AttributeValue><ns1:AttributeValue xsi:type="xs:string">urn:janet:eduteams.org:2:moonshotco:moonshotwebauthn</ns1:AttributeValue></ns1:Attribute></ns1:AttributeStatement></ns1:Assertion></ns0:Response>

Manually stripping the SOAP tags from the original document:

<?xml version="1.0" encoding="UTF-8"?>
<samlp:Response xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" ID="_7bb54ec7a8a59f971b1b995206ce6a344600051f63" Version="2.0" IssueInstant="2018-10-11T12:57:44Z" InResponseTo="f9f6a703-aa82-4a93-a235-3a08ec62518e"><saml:Issuer>https://comanage.eduteams-playpen.ti.ja.net/ss</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="#_7bb54ec7a8a59f971b1b995206ce6a344600051f63"><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>mTRj5GCV7+jP7HKCHYk+paTfxas=</ds:DigestValue></ds:Reference></ds:SignedInfo><ds:SignatureValue>dLtV7jF5PgeGjfMTbnshfzD1g2qPdKPZNzfCEzlfPrMx8ZZsqUQJNIVoqKx6Vqi8iJIdoezJwtz8XOmXO5vkZe1aNVVC3hptPGbNJbUwU8Q8exhHwlc9w6u+nHqeh9EtQgMThv3c1PxxBpZ9o2HsqU/PFADuP/JrwdLdAS3BSeGNp9YpUu0FswjpFQn5t2pwNMvkZzhMdzsa/KV0JtAuBlUo8dseFU0hgKnPlXn821C7cS0Lc28DU5H4Xjgwyfd7ymOswwbm5IteZyGlq5gUP3YHODxl3RAEp0cKL7DPhmDe1BUAD68WhVq5bUs1mQdYwKogSLP0aHmuGGaHjDPt185PVcjRnKbMyplIdFZzE3c+ttSSYOP1/PApt0Y3uJS85rn53Kipk/z07ZvYe8sE4+OdtTeWNVtvAR41tc7p56Csc5wa1wUhARnSv6NwVhwM1aySkZu11BGtqSy7lEvHQsHBb8U3VTacDogD9CtyGsiQi+qLqVSTrFT/tKf697PKsKWOJkSrcP51v/OjB/XqEN6fN45YMuIK8P8T8iBlOtcfV4Hlq0lST4gGSw4OP/ds2xCXBsBYJLkMoIIwzX9tgVLhFmqDWCvzqZ/6/WTvYwI6p6HhmM0+xpou/OmBqHDlZ5kUk3qthjmhWXEPdXSknZ7W6zMDpN2tpxDC3eJLVSM=</ds:SignatureValue>
<ds:KeyInfo><ds:X509Data><ds:X509Certificate>MIIFLzCCAxegAwIBAgIJAMszSeMjDJ4CMA0GCSqGSIb3DQEBCwUAMC4xLDAqBgNVBAMMI2NvbWFuYWdlLmVkdXRlYW1zLXBsYXlwZW4udGkuamEubmV0MB4XDTE4MDUxNTE1NDc1NFoXDTI4MDUxNDE1NDc1NFowLjEsMCoGA1UEAwwjY29tYW5hZ2UuZWR1dGVhbXMtcGxheXBlbi50aS5qYS5uZXQwggIiMA0GCSqGSIb3DQEBAQUAA4ICDwAwggIKAoICAQDIDKmsP1QAs4EcySN9CFEC3PkzXcIF6wUNsZ6IBpAGQzGdwmabEI53P6cnoAlcVQN4PCY1O15V1W7dKrEDBMchEksss1LjzrajhBreIVJRDxzPs4L4RQTC83z2tvjLlA15MH0rqOtOfv9Ql2KOeNuShDXS4fZl4mI2lOWIVn9FI/H0rpBnY/IupwLMsp0yEDyppU3EGa9xDigS9KP6e51BjVOk/s9GJz4TW/zUukLW4jR5AEY2Yitxi2h8HMArMj0HWIfdIptXWrVfv63FMJonaZ9LQr7Ye6mEKGlQTYLD+pjEoJPEwbqmLwg8Tn5tjAfs7m1KgTd6TAPh6QxpYtVsTXvHVfP3qie+je4oNDzhkzjdhJr7eutkgBJiorkuSGjwvsaywMS27EHdCfI57HsuOuTsmCKJaI3FOLyqXDLwq8LO20YdVGUTvcs7rhW/oWQYiNWCk/EhW5iNbs0XbwyMpuPVH5TDOPNHeIUdQE2v+cfU1Fs+P5cHz1PhOD117k5EzEKM6ZZXfrZI71qnEDe0mWbyIY13OjDNS9T0YNMG+l1zsukG8TTiad0pI0X9OaFzZuvsN51nZWuTvoI3JUJMckF3f2fnlWm55uLiLKf7KtJkO2anSyCcHtzEDuFInV22x/P3HVKQhDqqrs2HX+Z3z996R117nai/cPkcQAJ/gwIDAQABo1AwTjAdBgNVHQ4EFgQUpWMBNlgCeXz0mJSu+ee0VgRavjYwHwYDVR0jBBgwFoAUpWMBNlgCeXz0mJSu+ee0VgRavjYwDAYDVR0TBAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAgEAouf3R/1z7mk2tnhkBsb5vv3tKt3XXJcGjKPMK/gaY1YTNj61pVudPswXBWLoNTn6yKBBy5uhm7mRX5rcuq+gfpdXOqZXxZoNXGx5qkd5A7zJkbLfW5TgD+sp9Ln+5KgtjN3wt6JkgZlKspd0jcsuH0og4BXlAEHtLIG6IOC2Arby+8vy8mfYB2JMtcHnwcn2WaixzuBmuGxovpzwN+xE9Yt6rFnizYWtQs40vubxpvJRToymUEd26jWvLhpG9B6U/oeFZtOHUX/ebhsBnkouPOvPTJm8cM6hiIb2ExeNh4IfP3KT7zIeOTWR/aQ4z1v6Jcn763Xj5EHYpRTiyWOLvslmAgr2k0HraZDWwfDX+Lifxz5iuXGZs1Jmxu6NCiTkqOIioIKZLA9R8KJ/r2Oc5j//lAXWWcpaqtogn0ObAmfr4g1rT/Da8YGsuou821i5Wa7IlkuSP+hLqve0XxyDRi2YH39yp0x28hisvrq4kAD343dN7QPemJ5VJXJ6pMTNza6wXe8L1VWDqYzDowSrmG/lmhn2Z3bTx3Xfwbw/c1ynRfrbBtdo9MKyGfQnn4xT8VQn4lp1i9uyJoURoLSazCN5y21qYuGInSxE/Irz36HaZXKiwD2snH+fDPJGIq057VayDKtgOoaKKXYiVNUSxu7qtMlPUacUoeoAZwRQv1g=</ds:X509Certificate></ds:X509Data></ds:KeyInfo></ds:Signature><samlp:Status><samlp:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:Success"/></samlp:Status><saml:Assertion xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xs="http://www.w3.org/2001/XMLSchema" ID="_3c03d55f796d5cc9269a32ef61d726478050f1e8e6" Version="2.0" IssueInstant="2018-10-11T12:57:44Z"><saml:Issuer>https://comanage.eduteams-playpen.ti.ja.net/ss</saml:Issuer><saml:Subject><saml:NameID>test@eduteams-playpen.ti.ja.net</saml:NameID><saml:SubjectConfirmation Method="urn:oasis:names:tc:SAML:2.0:cm:bearer"><saml:SubjectConfirmationData NotBefore="2018-10-11T12:57:44Z" NotOnOrAfter="2018-10-11T13:02:44Z" InResponseTo="f9f6a703-aa82-4a93-a235-3a08ec62518e"/></saml:SubjectConfirmation></saml:Subject><saml:Conditions NotBefore="2018-10-11T12:57:44Z" NotOnOrAfter="2018-10-11T13:02:44Z"><saml:AudienceRestriction><saml:Audience>https://sp3.eduteams-playpen.ti.ja.net/shibboleth</saml:Audience></saml:AudienceRestriction></saml:Conditions><saml:AttributeStatement><saml:Attribute Name="urn:oid:1.3.6.1.4.1.5923.1.1.1.7" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri"><saml:AttributeValue xsi:type="xs:string">some:entitlement:prefix:hexaa:8:member</saml:AttributeValue><saml:AttributeValue xsi:type="xs:string">urn:janet:eduteams.org:2:moonshotco:moonshotwebauthn</saml:AttributeValue></saml:Attribute></saml:AttributeStatement></saml:Assertion></samlp:Response>

The two documents are structurally equivalent. What seems to be the problem is that pysaml2 will rename the namespace prefixes:

Replacing the namespace prefixes with the original names, allows verify() to succeed.

I am looking at the specification trying to understand whether this is how it is supposed to work.

spaetow commented 5 years ago

Hi @c00kiemon5ter, thanks for looking into this. I'll manually strip the SOAP envelope off for the time being. 👍

spaetow commented 5 years ago

Ahhh, @c00kiemon5ter, I think I know why pySAML2's SOAP method does something silly... Look at open_soap_envelope(): https://github.com/IdentityPython/pysaml2/blob/6e09a25d9b4b7aa7a506853210a9a14100b8bc9b/src/saml2/soap.py#L206-L215 It uses the defusedxml.ElementTree class there. When I duplicate the code in soap.open_soap_envelope() and use the lxml.ElementTree instead, it does everything correctly (and leaves the name spaces alone).

c00kiemon5ter commented 5 years ago

To my surprise, the spec makes no mention of namespace-prefixes canonicalization. It only mentions namespaces to set the rules that create the list of namespaces that are in use by the subset of the document that will be signed, and to provide a mechanism (namely InclusiveNamespaces PrefixList) to add prefixes in the signed document that are needed but are not visibly utilized. Thus, prefixes are part of the calculated signature.

XML Namespaces definition emphasizes that prefixes are not (semantically) important, and two documents that have chosen different prefixes (that point to the same ns-uri) should be considered the same. Unfortunately, C14N does not take that into consideration. This means that any operation that might change a prefix that is visibly utilized or provided through the InclusiveNamespaces PrefixList will invalidate the document signature.

Applications should avoid attaching any significance to the choice of prefix, but the information is provided because it can be helpful to human readers. [Element or Attribute] Names are considered equal if the namespace URI and local name match.

Note that the prefix functions only as a placeholder for a namespace name. Applications SHOULD use the namespace name, not the prefix, in constructing names whose scope extends beyond the containing document.

Looking into this I found the following paper (a good read): The Curse of Namespaces in the Domain of XML Signature. It goes through different attacks on xml-sig and finally introduces a new canonicalization method -namely PFC14N- which handles namespace-prefixes correctly (it removes them where possible and canonicalizes the ones that can't be removed).

c00kiemon5ter commented 5 years ago

It uses the defusedxml.ElementTree class there. When I duplicate the code in soap.open_soap_envelope() and use the lxml.ElementTree instead, it does everything correctly (and leaves the name spaces alone).

pysaml2 doesn't have a dependency on lxml. It uses the builtin xml parser as provided by python, but does so through defusedxml that modifies the default settings to create a more secure parser. I would like to switch to lxml at some point for other reasons.

The problem here is that the default parser modifies the namespace prefixes.

from xml.etree import ElementTree as etree

xml_data = '<my:Element xmlns:my="http://example.me">text</my:Element>'

tree = etree.fromstring(xml_data)
output = etree.tostring(tree)
print(output)

yields:

<ns0:Element xmlns:ns0="http://example.me">text</ns0:Element>
spaetow commented 5 years ago

Ahh, ok. So is this something that should be flagged with Python as a bug then? I mean... if the spec doesn't require namespaces to be preserved, then technically the parser doesn't have a bug, but if C14N does require the namespaces to be preserved, then the parser should have at least a flag to preserve them (i.e. etree.fromstring(xml_data, PreserveNamespaces=True)) to avoid the problem we're seeing?

As far as my specific case is concerned, I've duplicated the SOAP code with lxml.ElementTree for the signature validation, and using standard PySAML2 for the rest (because once the signature is validated, I don't care much for the namespaces).

c00kiemon5ter commented 5 years ago

I think there is a way to preserve namespaces, but it needs manual work - a customized parser that listens for ns-start events. It could be provided by the builtin parser, but technically not doing so is not a bug or incorrect behaviour. Per the xml namespace specification the applications should not care about the prefixes. This is incorrect handling of prefixes by the canonicalization specification rather than any parser - it is more of a "bug" with C14N.

I will try to preserve namespaces using the default parser. If that does not work out well (remember we have another layer: defusedxml), I will try to prioritise the switch to lxml.

spaetow commented 5 years ago

Fair enough! Thank you! 👍

erakli commented 5 years ago

Take a look at PR #564

macmenco commented 5 years ago

Maybe a quick work-around might be to validate the signature before removing the soap envelope?

peppelinux commented 4 years ago

@c00kiemon5ter is there any update, clue or roadmap linked to this or we can consider this thread closed?