From a3b26f3b6d3ea8122c69f7172cafc250c74f1481 Mon Sep 17 00:00:00 2001 From: Ivan Kanakarakis Date: Tue, 16 Nov 2021 13:17:28 +0200 Subject: Verify signed logout requests with the redirect binding Signed-off-by: Ivan Kanakarakis --- src/saml2/client.py | 18 +++++++++-- src/saml2/entity.py | 20 ++++++++++-- tests/test_51_client.py | 82 +++++++++++++++++++++++++++++++++++++++++-------- 3 files changed, 102 insertions(+), 18 deletions(-) diff --git a/src/saml2/client.py b/src/saml2/client.py index aa0bd0c9..a7469d4f 100644 --- a/src/saml2/client.py +++ b/src/saml2/client.py @@ -630,7 +630,9 @@ class Saml2Client(Base): sign=None, sign_alg=None, digest_alg=None, - relay_state="", + relay_state=None, + sigalg=None, + signature=None, ): """ Deal with a LogoutRequest @@ -639,6 +641,11 @@ class Saml2Client(Base): :param name_id: The id of the current user :param binding: Which binding the message came in over :param sign: Whether the response will be signed or not + :param sign_alg: The signing algorithm for the response + :param digest_alg: The digest algorithm for the the response + :param relay_state: The relay state of the request + :param sigalg: The SigAlg query param of the request + :param signature: The Signature query param of the request :return: Keyword arguments which can be used to send the response what's returned follow different patterns for different bindings. If the binding is BINDIND_SOAP, what is returned looks like this:: @@ -652,8 +659,13 @@ class Saml2Client(Base): """ logger.info("logout request: %s", request) - _req = self._parse_request(request, LogoutRequest, - "single_logout_service", binding) + _req = self.parse_logout_request( + xmlstr=request, + binding=binding, + relay_state=relay_state, + sigalg=sigalg, + signature=signature, + ) if _req.message.name_id == name_id: try: diff --git a/src/saml2/entity.py b/src/saml2/entity.py index e926d2c5..f818b702 100644 --- a/src/saml2/entity.py +++ b/src/saml2/entity.py @@ -1584,7 +1584,14 @@ class Entity(HTTPBase): # ------------------------------------------------------------------------ - def parse_logout_request(self, xmlstr, binding=BINDING_SOAP): + def parse_logout_request( + self, + xmlstr, + binding=BINDING_SOAP, + relay_state=None, + sigalg=None, + signature=None, + ): """ Deal with a LogoutRequest :param xmlstr: The response as a xml string @@ -1594,8 +1601,15 @@ class Entity(HTTPBase): was not. """ - return self._parse_request(xmlstr, saml_request.LogoutRequest, - "single_logout_service", binding) + return self._parse_request( + enc_request=xmlstr, + request_cls=saml_request.LogoutRequest, + service="single_logout_service", + binding=binding, + relay_state=relay_state, + sigalg=sigalg, + signature=signature, + ) def use_artifact(self, message, endpoint_index=0): """ diff --git a/tests/test_51_client.py b/tests/test_51_client.py index 095d80ab..a323de79 100644 --- a/tests/test_51_client.py +++ b/tests/test_51_client.py @@ -1644,14 +1644,68 @@ class TestClient: loc = info["headers"][0][1] _, _, _, _, qs, _ = parse.urlparse(loc) qs = parse.parse_qs(qs) - assert _leq(qs.keys(), - ['SigAlg', 'SAMLRequest', 'RelayState', 'Signature']) + assert _leq(qs.keys(), ['SigAlg', 'SAMLRequest', 'RelayState', 'Signature']) - assert verify_redirect_signature(list_values2simpletons(qs), - client.sec.sec_backend) + qs_simple = list_values2simpletons(qs) + assert verify_redirect_signature(qs_simple, client.sec.sec_backend) - res = self.server.parse_logout_request(qs["SAMLRequest"][0], - BINDING_HTTP_REDIRECT) + res = self.server.parse_logout_request( + qs_simple["SAMLRequest"], + BINDING_HTTP_REDIRECT, + relay_state=qs_simple['RelayState'], + sigalg=qs_simple['SigAlg'], + signature=qs_simple['Signature'], + ) + + def test_do_logout_signed_redirect_invalid(self): + conf = config.SPConfig() + conf.load_file("sp_slo_redirect_conf") + client = Saml2Client(conf) + + session_info = { + "name_id": nid, + "issuer": "urn:mace:example.com:saml:roland:idp", + "not_on_or_after": in_a_while(minutes=15), + "ava": { + "givenName": "Anders", + "sn": "Andersson", + "mail": "anders.andersson@example.com" + } + } + client.users.add_information_about_person(session_info) + entity_ids = client.users.issuers_of_info(nid) + + resp = client.do_logout( + nid, + entity_ids, + "Tired", + in_a_while(minutes=5), + sign=True, + expected_binding=BINDING_HTTP_REDIRECT, + ) + + binding, info = resp[entity_ids[0]] + loc = info["headers"][0][1] + _, _, _, _, qs, _ = parse.urlparse(loc) + qs = parse.parse_qs(qs) + qs_simple = list_values2simpletons(qs) + + invalid_signature = 'ZEdMZUQ3SjBjQ2ozWmlGaHhyV3JZSzNkTWhQWU02bjA0dzVNeUd1UWgrVDhnYm1oc1R1TTFjPQo=' + qs_simple_invalid = { + **qs_simple, + 'Signature': invalid_signature, + } + assert not verify_redirect_signature(qs_simple_invalid, client.sec.sec_backend) + + self.server.config.setattr("idp", "want_authn_requests_signed", True) + with raises(IncorrectlySigned): + res = self.server.parse_logout_request( + qs_simple["SAMLRequest"], + BINDING_HTTP_REDIRECT, + relay_state=qs_simple['RelayState'], + sigalg=qs_simple['SigAlg'], + signature=invalid_signature, + ) def test_do_logout_post(self): # information about the user from an IdP @@ -3245,14 +3299,18 @@ class TestClientNonAsciiAva: loc = info["headers"][0][1] _, _, _, _, qs, _ = parse.urlparse(loc) qs = parse.parse_qs(qs) - assert _leq(qs.keys(), - ['SigAlg', 'SAMLRequest', 'RelayState', 'Signature']) + assert _leq(qs.keys(), ['SigAlg', 'SAMLRequest', 'RelayState', 'Signature']) - assert verify_redirect_signature(list_values2simpletons(qs), - client.sec.sec_backend) + qs_simple = list_values2simpletons(qs) + assert verify_redirect_signature(qs_simple, client.sec.sec_backend) - res = self.server.parse_logout_request(qs["SAMLRequest"][0], - BINDING_HTTP_REDIRECT) + res = self.server.parse_logout_request( + qs_simple["SAMLRequest"], + BINDING_HTTP_REDIRECT, + relay_state=qs_simple['RelayState'], + sigalg=qs_simple['SigAlg'], + signature=qs_simple['Signature'], + ) def test_do_logout_post(self): # information about the user from an IdP -- cgit v1.2.1