From da6521450e49e9de2d97fbd5b8cbf72c6de217a4 Mon Sep 17 00:00:00 2001 From: Egor Panfilov Date: Mon, 19 Nov 2018 18:47:07 +0300 Subject: [PATCH 1/2] Fix many issues with parse_ecp_authn_response Because ElementTree generates custom prefixes for namespaces, in some cases xmlsec couldn't verify responses that was unpacked from Envelope, because of changed prefixes. In this change we store old prefixes and add register them for future conversions. Also, previous code of parse_ecp_authn_response passed response to parse_authn_request_response method as object, while it can handle only strings. --- src/saml2/__init__.py | 27 +++++++++++++++++++++++++++ src/saml2/client_base.py | 16 ++++++++-------- src/saml2/soap.py | 25 +++++++++++++++++++------ 3 files changed, 54 insertions(+), 14 deletions(-) diff --git a/src/saml2/__init__.py b/src/saml2/__init__.py index cc3c06f7d..6d549ff1e 100644 --- a/src/saml2/__init__.py +++ b/src/saml2/__init__.py @@ -130,6 +130,31 @@ class members. return None +def parse_xml_and_get_ns(file_like): + """Behaves like ElementTree.fromstring, but also retrieves namespaces as + separate parameter. + + :param file_like: File-like object, that implements read() method. + :return: ElementTree.Element and ns (as dict) + """ + events = "start", "start-ns" + root = None + ns = {} + for event, elem in defusedxml.ElementTree.iterparse(file_like, events): + if event == "start-ns": + # if elem[0] in ns and ns[elem[0]] != elem[1]: + # # NOTE: It is perfectly valid to have the same prefix refer + # # to different URI namespaces in different parts of the + # # document. This exception serves as a reminder that this + # # solution is not robust. Use at your own peril. + # raise KeyError("Duplicate prefix with different URI found.") + ns[elem[0]] = elem[1] + elif event == "start": + if root is None: + root = elem + return ElementTree.ElementTree(root).getroot(), ns + + class Error(Exception): """Exception class thrown by this module.""" pass @@ -640,6 +665,8 @@ def get_xml_string_with_self_contained_assertion_within_encrypted_assertion( def set_prefixes(self, elem, prefix_map): + self.register_prefix(prefix_map) + # check if this is a tree wrapper if not ElementTree.iselement(elem): elem = elem.getroot() diff --git a/src/saml2/client_base.py b/src/saml2/client_base.py index d0a8e82c8..fcbc63857 100644 --- a/src/saml2/client_base.py +++ b/src/saml2/client_base.py @@ -842,19 +842,19 @@ def create_ecp_authn_request(self, entityid=None, relay_state="", return req_id, "%s" % soap_envelope def parse_ecp_authn_response(self, txt, outstanding=None): - rdict = soap.class_instances_from_soap_enveloped_saml_thingies(txt, - [paos, - ecp, - samlp]) + rdict = soap.class_instances_from_soap_enveloped_saml_thingies( + txt, [paos, ecp, samlp]) _relay_state = None for item in rdict["header"]: - if item.c_tag == "RelayState" and \ - item.c_namespace == ecp.NAMESPACE: + if (item.c_tag == "RelayState" and + item.c_namespace == ecp.NAMESPACE): _relay_state = item - response = self.parse_authn_request_response(rdict["body"], - BINDING_PAOS, outstanding) + xmlstr = rdict["body"].to_string(rdict['ns']) + + response = self.parse_authn_request_response(xmlstr, BINDING_PAOS, + outstanding) return response, _relay_state diff --git a/src/saml2/soap.py b/src/saml2/soap.py index 055c690aa..05dd01bcd 100644 --- a/src/saml2/soap.py +++ b/src/saml2/soap.py @@ -6,8 +6,13 @@ Suppport for the client part of the SAML2.0 SOAP binding. """ import logging +from io import BytesIO +import re + +from six import StringIO -from saml2 import create_class_from_element_tree +import six +from saml2 import create_class_from_element_tree, parse_xml_and_get_ns from saml2.samlp import NAMESPACE as SAMLP_NAMESPACE from saml2.schema import soapenv @@ -126,6 +131,7 @@ def parse_soap_enveloped_saml_authn_response(text): # expected_tag = '{%s}LogoutResponse' % SAMLP_NAMESPACE # return parse_soap_enveloped_saml_thingy(text, [expected_tag]) + def parse_soap_enveloped_saml_thingy(text, expected_tags): """Parses a SOAP enveloped SAML thing and returns the thing as a string. @@ -157,7 +163,6 @@ def parse_soap_enveloped_saml_thingy(text, expected_tags): raise WrongMessageType("Was '%s' expected one of %s" % (saml_part.tag, expected_tags)) -import re NS_AND_TAG = re.compile("\{([^}]+)\}(.*)") @@ -184,13 +189,17 @@ def class_instances_from_soap_enveloped_saml_thingies(text, modules): :return: The body and headers as class instances """ try: - envelope = defusedxml.ElementTree.fromstring(text) + if isinstance(text, six.string_types): + io_wrapper = StringIO + else: + io_wrapper = BytesIO + envelope, ns = parse_xml_and_get_ns(io_wrapper(text)) except Exception as exc: raise XmlParseError("%s" % exc) assert envelope.tag == '{%s}Envelope' % soapenv.NAMESPACE assert len(envelope) >= 1 - env = {"header": [], "body": None} + env = {"header": [], "body": None, "ns": ns} for part in envelope: if part.tag == '{%s}Body' % soapenv.NAMESPACE: @@ -210,13 +219,17 @@ def open_soap_envelope(text): :return: dictionary with two keys "body"/"header" """ try: - envelope = defusedxml.ElementTree.fromstring(text) + if isinstance(text, six.string_types): + io_wrapper = StringIO + else: + io_wrapper = BytesIO + envelope, ns = parse_xml_and_get_ns(io_wrapper(text)) except Exception as exc: raise XmlParseError("%s" % exc) assert envelope.tag == '{%s}Envelope' % soapenv.NAMESPACE assert len(envelope) >= 1 - content = {"header": [], "body": None} + content = {"header": [], "body": None, "ns": ns} for part in envelope: if part.tag == '{%s}Body' % soapenv.NAMESPACE: From 428b85b6ada5f4d966acc0d27bada62a18ccd3fa Mon Sep 17 00:00:00 2001 From: Egor Panfilov Date: Mon, 19 Nov 2018 18:47:53 +0300 Subject: [PATCH 2/2] Add BINDING_PAOS handling to unravel method --- src/saml2/entity.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/saml2/entity.py b/src/saml2/entity.py index e53804c3b..090d4a1f5 100644 --- a/src/saml2/entity.py +++ b/src/saml2/entity.py @@ -377,7 +377,7 @@ def unravel(txt, binding, msgtype="response"): # logger.debug("unravel '%s'", txt) if binding not in [BINDING_HTTP_REDIRECT, BINDING_HTTP_POST, BINDING_SOAP, BINDING_URI, BINDING_HTTP_ARTIFACT, - None]: + BINDING_PAOS, None]: raise UnknownBinding("Don't know how to handle '%s'" % binding) else: try: @@ -389,6 +389,8 @@ def unravel(txt, binding, msgtype="response"): func = getattr(soap, "parse_soap_enveloped_saml_%s" % msgtype) xmlstr = func(txt) + elif binding == BINDING_PAOS: + xmlstr = txt elif binding == BINDING_HTTP_ARTIFACT: xmlstr = base64.b64decode(txt) else: