Merge pull request mehcode#2 from AMeng/verify-bad-signatures

Dont raise exceptions for bad signatures

Author: mehcode

Diff for: .travis.yml

@@ -5,6 +5,7 @@ python:
   - '3.4'
 
 before_install:
+  - 'travis_retry sudo apt-get update'
   - 'travis_retry sudo apt-get install python-dev libxml2-dev libxmlsec1-dev'
   - 'travis_retry pip install Cython --use-mirrors'
 

Diff for: saml/__init__.py

@@ -9,14 +9,15 @@
 """
 # Version of the library.
 from ._version import __version__, __version_info__  # noqa
-VERSION = __version__
 
 # Version of the SAML standard supported.
 from .schema import VERSION as SAML_VERSION
 
 from .signature import sign, verify
 from . import client
 
+VERSION = __version__
+
 __all__ = [
     'VERSION',
     'SAML_VERSION',

Diff for: saml/schema/base.py

@@ -55,6 +55,10 @@ def _is_derived(cls, name, bases):
         # This is not derived at all from Resource (eg. is base).
         return False
 
+    @classmethod
+    def _get_attributes_dict(cls, obj):
+        return {n: getattr(obj, n) for n in dir(obj)}
+
     def __new__(cls, name, bases, attrs):
         # Only continue if we are dervied from declarative.
         if not cls._is_derived(name, bases):
@@ -64,20 +68,19 @@ def __new__(cls, name, bases, attrs):
         # Gather the attributes of all options classes.
         # Start with the base configuration.
         metadata = {}
-        values = lambda x: {n: getattr(x, n) for n in dir(x)}
 
         # Expand the options class with the gathered metadata.
         base_meta = []
         cls._gather_metadata(base_meta, bases)
 
         # Apply the configuration from each class in the chain.
         for meta in base_meta:
-            metadata.update(**values(meta))
+            metadata.update(**cls._get_attributes_dict(meta))
 
         # Apply the configuration from the current class.
         cur_meta = {}
         if attrs.get('Meta'):
-            cur_meta = values(attrs['Meta'])
+            cur_meta = cls._get_attributes_dict(attrs['Meta'])
             metadata.update(**cur_meta)
 
         # Gather and construct the options object.
@@ -93,8 +96,8 @@ def __new__(cls, name, bases, attrs):
                 attrs['_items'].update(values)
 
         # Collect attributes from current class.
-        test = lambda x: issubclass(type(x[1]), Component)
-        attrs_l = list(filter(test, attrs.items()))
+        attrs_l = list(filter(lambda x: issubclass(type(x[1]), Component),
+                              attrs.items()))
         attrs_l.sort(key=lambda x: x[1].creation_counter)
         for key, attr in attrs_l:
             # If name reference is null; default to camel-cased name.

Diff for: saml/signature.py

@@ -48,7 +48,7 @@ def verify(xml, stream):
     signature_node = xmlsec.tree.find_node(xml, xmlsec.Node.SIGNATURE)
     if signature_node is None:
         # No `signature` node found; we cannot verify
-        return None
+        return False
 
     # Create a digital signature context (no key manager is needed).
     ctx = xmlsec.SignatureContext()
@@ -72,4 +72,7 @@ def verify(xml, stream):
     ctx.key = key
 
     # Verify the signature.
-    return ctx.verify(signature_node)
+    try:
+        return ctx.verify(signature_node)
+    except RuntimeError:
+        return False

Diff for: tests/test_schema.py

@@ -1,4 +1,5 @@
 import saml
+import xmlsec
 from saml import schema
 from saml.schema import utils
 from datetime import datetime
@@ -279,13 +280,27 @@ def test_signed_deserialize(name):
     assert_node(expected, result)
 
 
-# NAMES = [
-#     'assertion',
-#     'response',
-#     'logout-response',
-#     'artifact-resolve',
-#     'artifact-response'
-# ]
+@mark.parametrize('name', NAMES)
+def test_generic_deserialize(name):
+    filename = path.join(BASE_DIR, '%s-simple.xml' % name)
+    parser = etree.XMLParser(
+        ns_clean=True, remove_blank_text=True, remove_comments=True)
+    target = etree.parse(filename, parser).getroot()
+
+    build_fn_name = ('build-%s-simple' % name).replace('-', '_')
+    expected = globals()[build_fn_name]().serialize()
+
+    result = schema.deserialize(target).serialize()
+
+    assert_node(expected, result)
+
+
+def test_generic_deserialize_outside_registry():
+    xml = build_authentication_request_simple().serialize()
+    xml.tag = 'BadTagName'
+    result = schema.deserialize(xml)
+
+    assert result is None
 
 
 @mark.parametrize('name', NAMES)
@@ -306,10 +321,6 @@ def test_sign(name):
         # Sign the result.
         saml.sign(result, stream)
 
-    # print()
-    # print(etree.tostring(result).decode('utf8'))
-    # print()
-
     # Compare the nodes.
     assert_node(expected, result)
 
@@ -323,3 +334,24 @@ def test_verify(name):
     # Sign the result.
     with open(path.join(BASE_DIR, 'rsapub.pem'), 'r') as stream:
         assert saml.verify(expected, stream)
+
+
+@mark.parametrize('name', NAMES)
+def test_verify_with_bad_signature_returns_False(name):
+    filename = path.join(BASE_DIR, '%s-signed.xml' % name)
+    expected = etree.parse(filename).getroot()
+
+    signature_node = xmlsec.tree.find_node(expected, xmlsec.Node.SIGNATURE)
+    signature_node.clear()
+
+    with open(path.join(BASE_DIR, 'rsapub.pem'), 'r') as stream:
+        assert saml.verify(expected, stream) is False
+
+
+@mark.parametrize('name', NAMES)
+def test_verify_with_no_signature_returns_False(name):
+    filename = path.join(BASE_DIR, '%s-simple.xml' % name)
+    expected = etree.parse(filename).getroot()
+
+    with open(path.join(BASE_DIR, 'rsapub.pem'), 'r') as stream:
+        assert saml.verify(expected, stream) is False