Skip to content

Commit

Permalink
Introduce xml_safe as a module to control xml opearations
Browse files Browse the repository at this point in the history
This is related to CVE-2017-11427[0] and VU#475445[1]
Related issues: IdentityPython#496 IdentityPython#497
Reported by duo[2] through this blog post[3]

pysaml2 is not affected, as, by default, the xml.etree.ElementTree and
xml.etree.cElementTree parsers ignore comments. However, this commit
makes sure that the ElementTree being used is set correctly through
defusexml lib and centralizes the control of which functions are exposed
and available for usage in the code. Any code that needs a function to
parse, modify or serialize XML should be obtain through the xml_safe
module. The new module asks defusexml to provide the function and if it
is not available it will fallback to the one provided by
xml.etree.cElementTree. This is a guarantee that functions like parse,
fromstring et al are provided by defusexml lib.

  [0]: https://cve.mitre.org/cgi-bin/cvename.cgi?name=2017-11427
  [1]: https://www.kb.cert.org/vuls/id/475445
  [2]: https://duo.com
  [3]: https://duo.com/blog/duo-finds-saml-vulnerabilities-affecting-multiple-implementations
  • Loading branch information
c00kiemon5ter committed Mar 2, 2018
1 parent febbf22 commit 88d3a90
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 54 deletions.
29 changes: 6 additions & 23 deletions src/saml2/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,23 +20,12 @@
__version__ = "4.5.0"

import logging
import six

import saml2.xml_safe as ElementTree
from saml2.validate import valid_instance

try:
from xml.etree import cElementTree as ElementTree
import six

if ElementTree.VERSION < '1.3.0':
# cElementTree has no support for register_namespace
# neither _namespace_map, thus we sacrify performance
# for correctness
from xml.etree import ElementTree
except ImportError:
try:
import cElementTree as ElementTree
except ImportError:
from elementtree import ElementTree
import defusedxml.ElementTree

root_logger = logging.getLogger(__name__)
root_logger.level = logging.NOTSET
Expand Down Expand Up @@ -88,7 +77,7 @@ def create_class_from_xml_string(target_class, xml_string):
"""
if not isinstance(xml_string, six.binary_type):
xml_string = xml_string.encode('utf-8')
tree = defusedxml.ElementTree.fromstring(xml_string)
tree = ElementTree.fromstring(xml_string)
return create_class_from_element_tree(target_class, tree)


Expand Down Expand Up @@ -270,7 +259,7 @@ def loadd(self, ava):


def extension_element_from_string(xml_string):
element_tree = defusedxml.ElementTree.fromstring(xml_string)
element_tree = ElementTree.fromstring(xml_string)
return _extension_element_from_element_tree(element_tree)


Expand Down Expand Up @@ -556,13 +545,7 @@ def register_prefix(self, nspair):
:return:
"""
for prefix, uri in nspair.items():
try:
ElementTree.register_namespace(prefix, uri)
except AttributeError:
# Backwards compatibility with ET < 1.3
ElementTree._namespace_map[uri] = prefix
except ValueError:
pass
ElementTree.register_namespace(prefix, uri)

def get_ns_map_attribute(self, attributes, uri_set):
for attribute in attributes:
Expand Down
18 changes: 2 additions & 16 deletions src/saml2/pack.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,24 +21,10 @@
from saml2.sigver import SIGNER_ALGS
import six
from saml2.xmldsig import SIG_ALLOWED_ALG
import saml2.xml_safe as ElementTree

logger = logging.getLogger(__name__)

try:
from xml.etree import cElementTree as ElementTree

if ElementTree.VERSION < '1.3.0':
# cElementTree has no support for register_namespace
# neither _namespace_map, thus we sacrify performance
# for correctness
from xml.etree import ElementTree
except ImportError:
try:
import cElementTree as ElementTree
except ImportError:
from elementtree import ElementTree
import defusedxml.ElementTree

NAMESPACE = "http://schemas.xmlsoap.org/soap/envelope/"

FORM_SPEC = """\
Expand Down Expand Up @@ -257,7 +243,7 @@ def parse_soap_enveloped_saml(text, body_class, header_class=None):
:param text: The SOAP object as XML
:return: header parts and body as saml.samlbase instances
"""
envelope = defusedxml.ElementTree.fromstring(text)
envelope = ElementTree.fromstring(text)
assert envelope.tag == '{%s}Envelope' % NAMESPACE

# print(len(envelope))
Expand Down
4 changes: 2 additions & 2 deletions src/saml2/sigver.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
from subprocess import Popen
from subprocess import PIPE

import saml2.xml_safe as ElementTree
from saml2 import samlp
from saml2 import SamlBase
from saml2 import SAMLError
Expand Down Expand Up @@ -1034,11 +1035,10 @@ def sign_statement(self, statement, node_name, key_file, node_id,
:returns: Signed XML as string
"""
import xmlsec
import lxml.etree

xml = xmlsec.parse_xml(statement)
signed = xmlsec.sign(xml, key_file)
return lxml.etree.tostring(signed, xml_declaration=True)
return ElementTree.tostring(signed, xml_declaration=True)

def validate_signature(self, signedtext, cert_file, cert_type, node_name,
node_id, id_attr):
Expand Down
17 changes: 4 additions & 13 deletions src/saml2/soap.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,11 @@
"""
import logging

import saml2.xml_safe as ElementTree
from saml2 import create_class_from_element_tree
from saml2.samlp import NAMESPACE as SAMLP_NAMESPACE
from saml2.schema import soapenv

try:
from xml.etree import cElementTree as ElementTree
except ImportError:
try:
import cElementTree as ElementTree
except ImportError:
#noinspection PyUnresolvedReferences
from elementtree import ElementTree
import defusedxml.ElementTree


logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -134,7 +125,7 @@ def parse_soap_enveloped_saml_thingy(text, expected_tags):
:param expected_tags: What the tag of the SAML thingy is expected to be.
:return: SAML thingy as a string
"""
envelope = defusedxml.ElementTree.fromstring(text)
envelope = ElementTree.fromstring(text)

# Make sure it's a SOAP message
assert envelope.tag == '{%s}Envelope' % soapenv.NAMESPACE
Expand Down Expand Up @@ -184,7 +175,7 @@ def class_instances_from_soap_enveloped_saml_thingies(text, modules):
:return: The body and headers as class instances
"""
try:
envelope = defusedxml.ElementTree.fromstring(text)
envelope = ElementTree.fromstring(text)
except Exception as exc:
raise XmlParseError("%s" % exc)

Expand All @@ -210,7 +201,7 @@ def open_soap_envelope(text):
:return: dictionary with two keys "body"/"header"
"""
try:
envelope = defusedxml.ElementTree.fromstring(text)
envelope = ElementTree.fromstring(text)
except Exception as exc:
raise XmlParseError("%s" % exc)

Expand Down
10 changes: 10 additions & 0 deletions src/saml2/xml_safe.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
from xml.etree.cElementTree import * # noqa

import defusedxml.cElementTree as defusedElementTree
from defusedxml.cElementTree import * # noqa


assert all( # noqa
globals().get(attr_str) is getattr(defusedElementTree, attr_str)
for attr_str in defusedElementTree.__all__), (
"defusedxml not loaded correctly")

0 comments on commit 88d3a90

Please sign in to comment.