From 24a170095cfff54142dc22a924e4e263b3f1f71e Mon Sep 17 00:00:00 2001 From: Christopher Perrin Date: Wed, 25 Jan 2017 17:38:55 +0100 Subject: [PATCH 1/2] Fixed bug with formated cert fingerprints - Added Test - Fixed typo in README.md --- README.md | 4 +- src/onelogin/saml2/response.py | 2 + tests/settings/settings6.json | 48 ++++++++++++++++ .../src/OneLogin/saml2_tests/response_test.py | 57 +++++++++++++++++++ 4 files changed, 109 insertions(+), 2 deletions(-) create mode 100644 tests/settings/settings6.json diff --git a/README.md b/README.md index d6c1e9c2..4bb3bf2e 100644 --- a/README.md +++ b/README.md @@ -289,8 +289,8 @@ This is the settings.json file: * Notice that if you want to validate any SAML Message sent by the HTTP-Redirect binding, you * will need to provide the whole x509cert. */ - // "certFingerprint" => "", - // "certFingerprintAlgorithm" => "sha1", + // "certFingerprint": "", + // "certFingerprintAlgorithm": "sha1", } } ``` diff --git a/src/onelogin/saml2/response.py b/src/onelogin/saml2/response.py index ba2b3545..faf303ce 100644 --- a/src/onelogin/saml2/response.py +++ b/src/onelogin/saml2/response.py @@ -274,6 +274,8 @@ def is_valid(self, request_data, request_id=None, raise_exceptions=False): else: cert = idp_data.get('x509cert', None) fingerprint = idp_data.get('certFingerprint', None) + if fingerprint: + fingerprint = OneLogin_Saml2_Utils.format_finger_print(fingerprint) fingerprintalg = idp_data.get('certFingerprintAlgorithm', None) # If find a Signature on the Response, validates it checking the original response diff --git a/tests/settings/settings6.json b/tests/settings/settings6.json new file mode 100644 index 00000000..5730925d --- /dev/null +++ b/tests/settings/settings6.json @@ -0,0 +1,48 @@ +{ + "strict": false, + "debug": false, + "custom_base_path": "../../../tests/data/customPath/", + "sp": { + "entityId": "http://stuff.com/endpoints/metadata.php", + "assertionConsumerService": { + "url": "http://stuff.com/endpoints/endpoints/acs.php" + }, + "singleLogoutService": { + "url": "http://stuff.com/endpoints/endpoints/sls.php" + }, + "NameIDFormat": "urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified" + }, + "idp": { + "entityId": "http://idp.example.com/", + "singleSignOnService": { + "url": "http://idp.example.com/SSOService.php" + }, + "singleLogoutService": { + "url": "http://idp.example.com/SingleLogoutService.php" + }, + "certFingerprint": "AF:E7:1C:28:EF:74:0B:C8:74:25:BE:13:A2:26:3D:37:97:1D:A1:F9", + "certFingerprintAlgorithm": "sha1" + }, + "security": { + "authnRequestsSigned": true, + "wantAssertionsSigned": false, + "signMetadata": false + }, + "contactPerson": { + "technical": { + "givenName": "technical_name", + "emailAddress": "technical@example.com" + }, + "support": { + "givenName": "support_name", + "emailAddress": "support@example.com" + } + }, + "organization": { + "en-US": { + "name": "sp_test", + "displayname": "SP test", + "url": "http://sp.example.com" + } + } +} diff --git a/tests/src/OneLogin/saml2_tests/response_test.py b/tests/src/OneLogin/saml2_tests/response_test.py index 46e9c8a1..28b34f86 100644 --- a/tests/src/OneLogin/saml2_tests/response_test.py +++ b/tests/src/OneLogin/saml2_tests/response_test.py @@ -1238,6 +1238,63 @@ def testIsValidSign(self): # Modified message self.assertFalse(response_9.is_valid(self.get_request_data())) + def testIsValidSignFingerprint(self): + """ + Tests the is_valid method of the OneLogin_Saml2_Response + Case valid sign response / sign assertion / both signed + + Strict mode will always fail due destination problem, if we manipulate + it the sign will fail. + """ + settings = OneLogin_Saml2_Settings(self.loadSettingsJSON("settings6.json")) + + # expired cert + xml = self.file_contents(join(self.data_path, 'responses', 'signed_message_response.xml.base64')) + response = OneLogin_Saml2_Response(settings, xml) + self.assertTrue(response.is_valid(self.get_request_data())) + + xml_2 = self.file_contents(join(self.data_path, 'responses', 'signed_assertion_response.xml.base64')) + response_2 = OneLogin_Saml2_Response(settings, xml_2) + self.assertTrue(response_2.is_valid(self.get_request_data())) + + xml_3 = self.file_contents(join(self.data_path, 'responses', 'double_signed_response.xml.base64')) + response_3 = OneLogin_Saml2_Response(settings, xml_3) + self.assertTrue(response_3.is_valid(self.get_request_data())) + + settings_2 = OneLogin_Saml2_Settings(self.loadSettingsJSON('settings2.json')) + xml_4 = self.file_contents(join(self.data_path, 'responses', 'signed_message_response2.xml.base64')) + response_4 = OneLogin_Saml2_Response(settings_2, xml_4) + self.assertTrue(response_4.is_valid(self.get_request_data())) + + xml_5 = self.file_contents(join(self.data_path, 'responses', 'signed_assertion_response2.xml.base64')) + response_5 = OneLogin_Saml2_Response(settings_2, xml_5) + self.assertTrue(response_5.is_valid(self.get_request_data())) + + xml_6 = self.file_contents(join(self.data_path, 'responses', 'double_signed_response2.xml.base64')) + response_6 = OneLogin_Saml2_Response(settings_2, xml_6) + self.assertTrue(response_6.is_valid(self.get_request_data())) + + dom = parseString(b64decode(xml_4)) + dom.firstChild.firstChild.firstChild.nodeValue = 'https://example.com/other-idp' + xml_7 = OneLogin_Saml2_Utils.b64encode(dom.toxml()) + response_7 = OneLogin_Saml2_Response(settings, xml_7) + # Modified message + self.assertFalse(response_7.is_valid(self.get_request_data())) + + dom_2 = parseString(OneLogin_Saml2_Utils.b64decode(xml_5)) + dom_2.firstChild.firstChild.firstChild.nodeValue = 'https://example.com/other-idp' + xml_8 = OneLogin_Saml2_Utils.b64encode(dom_2.toxml()) + response_8 = OneLogin_Saml2_Response(settings, xml_8) + # Modified message + self.assertFalse(response_8.is_valid(self.get_request_data())) + + dom_3 = parseString(OneLogin_Saml2_Utils.b64decode(xml_6)) + dom_3.firstChild.firstChild.firstChild.nodeValue = 'https://example.com/other-idp' + xml_9 = OneLogin_Saml2_Utils.b64encode(dom_3.toxml()) + response_9 = OneLogin_Saml2_Response(settings, xml_9) + # Modified message + self.assertFalse(response_9.is_valid(self.get_request_data())) + def testIsValidSignWithEmptyReferenceURI(self): settings_info = self.loadSettingsJSON() del settings_info['idp']['x509cert'] From 5abf90836f4086369afaedc33207988e16176b19 Mon Sep 17 00:00:00 2001 From: Christopher Perrin Date: Wed, 25 Jan 2017 17:44:41 +0100 Subject: [PATCH 2/2] Clarify the testcase comment --- tests/src/OneLogin/saml2_tests/response_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/src/OneLogin/saml2_tests/response_test.py b/tests/src/OneLogin/saml2_tests/response_test.py index 28b34f86..36fd2184 100644 --- a/tests/src/OneLogin/saml2_tests/response_test.py +++ b/tests/src/OneLogin/saml2_tests/response_test.py @@ -1241,7 +1241,7 @@ def testIsValidSign(self): def testIsValidSignFingerprint(self): """ Tests the is_valid method of the OneLogin_Saml2_Response - Case valid sign response / sign assertion / both signed + Case valid sign response / sign assertion / both signed with fingerprint Strict mode will always fail due destination problem, if we manipulate it the sign will fail.