From c8c63d868293e6775386980c98910a8e3dc479dc Mon Sep 17 00:00:00 2001 From: AMeng Date: Sat, 14 Mar 2015 23:40:47 -0400 Subject: [PATCH 1/5] Do not raise exception for bad/no signatures. Instead return False. --- saml/signature.py | 9 +++++++-- tests/test_schema.py | 47 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/saml/signature.py b/saml/signature.py index b8d7065..daca5f6 100644 --- a/saml/signature.py +++ b/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,9 @@ def verify(xml, stream): ctx.key = key # Verify the signature. - return ctx.verify(signature_node) + try: + ctx.verify(signature_node) + except RuntimeError: + return False + else: + return True diff --git a/tests/test_schema.py b/tests/test_schema.py index 9aee480..5a970fc 100644 --- a/tests/test_schema.py +++ b/tests/test_schema.py @@ -1,10 +1,11 @@ import saml +import xmlsec from saml import schema from saml.schema import utils from datetime import datetime from lxml import etree from os import path -from pytest import mark +from pytest import mark, raises BASE_DIR = path.abspath(path.dirname(__file__)) @@ -279,6 +280,29 @@ def test_signed_deserialize(name): assert_node(expected, result) +@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 + + # NAMES = [ # 'assertion', # 'response', @@ -323,3 +347,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) == 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) == False From d021902e0ddf75e7ad63d502e47f720b919eb5f2 Mon Sep 17 00:00:00 2001 From: AMeng Date: Sat, 14 Mar 2015 23:47:43 -0400 Subject: [PATCH 2/5] Remove comments and fix PEP8 issues --- saml/__init__.py | 3 ++- saml/schema/base.py | 13 ++++++++----- tests/test_schema.py | 17 ++--------------- 3 files changed, 12 insertions(+), 21 deletions(-) diff --git a/saml/__init__.py b/saml/__init__.py index 57e11d5..c15af75 100644 --- a/saml/__init__.py +++ b/saml/__init__.py @@ -9,7 +9,6 @@ """ # 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 @@ -17,6 +16,8 @@ from .signature import sign, verify from . import client +VERSION = __version__ + __all__ = [ 'VERSION', 'SAML_VERSION', diff --git a/saml/schema/base.py b/saml/schema/base.py index bd214f9..f429eef 100644 --- a/saml/schema/base.py +++ b/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,7 +68,6 @@ 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 = [] @@ -72,12 +75,12 @@ def __new__(cls, name, bases, attrs): # 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 --git a/tests/test_schema.py b/tests/test_schema.py index 5a970fc..0493c05 100644 --- a/tests/test_schema.py +++ b/tests/test_schema.py @@ -303,15 +303,6 @@ def test_generic_deserialize_outside_registry(): assert result is None -# NAMES = [ -# 'assertion', -# 'response', -# 'logout-response', -# 'artifact-resolve', -# 'artifact-response' -# ] - - @mark.parametrize('name', NAMES) def test_sign(name): # Load the expected result. @@ -330,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) @@ -358,7 +345,7 @@ def test_verify_with_bad_signature_returns_False(name): signature_node.clear() with open(path.join(BASE_DIR, 'rsapub.pem'), 'r') as stream: - assert saml.verify(expected, stream) == False + assert saml.verify(expected, stream) is False @mark.parametrize('name', NAMES) @@ -367,4 +354,4 @@ def test_verify_with_no_signature_returns_False(name): expected = etree.parse(filename).getroot() with open(path.join(BASE_DIR, 'rsapub.pem'), 'r') as stream: - assert saml.verify(expected, stream) == False + assert saml.verify(expected, stream) is False From 3b1323f78b43e5b7e2710b414be60f28dc4ff0f7 Mon Sep 17 00:00:00 2001 From: AMeng Date: Sat, 14 Mar 2015 23:57:21 -0400 Subject: [PATCH 3/5] Return boolean value from xmlsec signature validation --- saml/signature.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/saml/signature.py b/saml/signature.py index daca5f6..3eb8a8c 100644 --- a/saml/signature.py +++ b/saml/signature.py @@ -73,8 +73,6 @@ def verify(xml, stream): # Verify the signature. try: - ctx.verify(signature_node) + return ctx.verify(signature_node) except RuntimeError: return False - else: - return True From 766212e2bdab2e476055b6b50c98e29ccca6f611 Mon Sep 17 00:00:00 2001 From: AMeng Date: Sun, 15 Mar 2015 00:08:03 -0400 Subject: [PATCH 4/5] Update apt-get for travis builds. Fixes apt 404 issues. --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 170fcf3..3940138 100644 --- a/.travis.yml +++ b/.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' From fb4a2340eb879a6d5cc8a39691ec3d9eefb1fb33 Mon Sep 17 00:00:00 2001 From: AMeng Date: Sun, 15 Mar 2015 00:29:42 -0400 Subject: [PATCH 5/5] Removed an unused import --- tests/test_schema.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_schema.py b/tests/test_schema.py index 0493c05..3518a8a 100644 --- a/tests/test_schema.py +++ b/tests/test_schema.py @@ -5,7 +5,7 @@ from datetime import datetime from lxml import etree from os import path -from pytest import mark, raises +from pytest import mark BASE_DIR = path.abspath(path.dirname(__file__))