Fix signature verification for the redirect binding

- When an AuthnRequest is received by the Server
- When a LogoutRequest is received by the client or the server

Author: c00kiemon5ter

.gitignore

@@ -123,6 +123,7 @@ venv.bak/
 
 # PyCharm project files
 .idea/
+*.iml
 
 # mkdocs documentation
 /site

src/saml2/client.py

@@ -630,7 +630,9 @@ def handle_logout_request(
         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 @@ def handle_logout_request(
         :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 @@ def handle_logout_request(
         """
         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:

src/saml2/entity.py

@@ -1024,7 +1024,16 @@ def srv2typ(service):
                 else:
                     return typ
 
-    def _parse_request(self, enc_request, request_cls, service, binding):
+    def _parse_request(
+        self,
+        enc_request,
+        request_cls,
+        service,
+        binding,
+        relay_state=None,
+        sigalg=None,
+        signature=None,
+    ):
         """Parse a Request
 
         :param enc_request: The request in its transport format
@@ -1039,8 +1048,7 @@ def _parse_request(self, enc_request, request_cls, service, binding):
         _log_debug = logger.debug
 
         # The addresses I should receive messages like this on
-        receiver_addresses = self.config.endpoint(service, binding,
-                                                  self.entity_type)
+        receiver_addresses = self.config.endpoint(service, binding, self.entity_type)
         if not receiver_addresses and self.entity_type == "idp":
             for typ in ["aa", "aq", "pdp"]:
                 receiver_addresses = self.config.endpoint(service, binding, typ)
@@ -1070,7 +1078,9 @@ def _parse_request(self, enc_request, request_cls, service, binding):
         if only_valid_cert:
             must = True
         _request = _request.loads(xmlstr, binding, origdoc=enc_request,
-                                  must=must, only_valid_cert=only_valid_cert)
+                                  must=must, only_valid_cert=only_valid_cert,
+                                  relay_state=relay_state, sigalg=sigalg,
+                                  signature=signature)
 
         _log_debug("Loaded request")
 
@@ -1574,7 +1584,14 @@ def parse_logout_request_response(self, xmlstr, binding=BINDING_SOAP):
 
     # ------------------------------------------------------------------------
 
-    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
@@ -1584,8 +1601,15 @@ def parse_logout_request(self, xmlstr, binding=BINDING_SOAP):
             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):
         """

src/saml2/request.py

@@ -1,12 +1,15 @@
 import logging
 
+from saml2 import time_util
+from saml2 import BINDING_HTTP_REDIRECT
+from saml2 import BINDING_HTTP_POST
 from saml2.attribute_converter import to_local
-from saml2 import time_util, BINDING_HTTP_REDIRECT
 from saml2.s_utils import OtherError
 
 from saml2.validate import valid_instance
 from saml2.validate import NotValid
 from saml2.response import IncorrectlySigned
+from saml2.sigver import verify_redirect_signature
 
 logger = logging.getLogger(__name__)
 
@@ -36,38 +39,84 @@ def _clear(self):
         self.message = None
         self.not_on_or_after = 0
 
-    def _loads(self, xmldata, binding=None, origdoc=None, must=None,
-               only_valid_cert=False):
-        if binding == BINDING_HTTP_REDIRECT:
-            pass
-
+    def _loads(
+        self,
+        xmldata,
+        binding=None,
+        origdoc=None,
+        must=None,
+        only_valid_cert=False,
+        relay_state=None,
+        sigalg=None,
+        signature=None,
+    ):
         # own copy
         self.xmlstr = xmldata[:]
-        logger.debug("xmlstr: %s", self.xmlstr)
+        logger.debug("xmlstr: %s, relay_state: %s, sigalg: %s, signature: %s",
+                     self.xmlstr, relay_state, sigalg, signature)
+
+        signed_post = must and binding == BINDING_HTTP_POST
+        signed_redirect = must and binding == BINDING_HTTP_REDIRECT
+        incorrectly_signed = IncorrectlySigned("Request was not signed correctly")
+
         try:
-            self.message = self.signature_check(xmldata, origdoc=origdoc,
-                                                must=must,
-                                                only_valid_cert=only_valid_cert)
-        except TypeError:
-            raise
-        except Exception as excp:
-            logger.info("EXCEPTION: %s", excp)
+            self.message = self.signature_check(
+                xmldata,
+                origdoc=origdoc,
+                must=signed_post,
+                only_valid_cert=only_valid_cert,
+            )
+        except Exception as e:
+            self.message = None
+            raise incorrectly_signed from e
+
+        if signed_redirect:
+            if sigalg is None or signature is None:
+                raise incorrectly_signed
+
+            _saml_msg = {
+                "SAMLRequest": origdoc,
+                "Signature": signature,
+                "SigAlg": sigalg,
+            }
+            if relay_state is not None:
+                _saml_msg["RelayState"] = relay_state
+            try:
+                sig_verified = self._do_redirect_sig_check(_saml_msg)
+            except Exception as e:
+                self.message = None
+                raise incorrectly_signed from e
+            else:
+                if not sig_verified:
+                    self.message = None
+                    raise incorrectly_signed
 
         if not self.message:
-            logger.error("Response was not correctly signed")
-            logger.info("Response: %s", xmldata)
-            raise IncorrectlySigned()
+            logger.error("Request was not signed correctly")
+            logger.info("Request data: %s", xmldata)
+            raise incorrectly_signed
 
-        logger.info("request: %s", self.message)
+        logger.info("Request message: %s", self.message)
 
         try:
             valid_instance(self.message)
         except NotValid as exc:
-            logger.error("Not valid request: %s", exc.args[0])
+            logger.error("Request not valid: %s", exc.args[0])
             raise
 
         return self
 
+    def _do_redirect_sig_check(self, _saml_msg):
+        issuer = self.message.issuer.text.strip()
+        certs = self.sec.metadata.certs(issuer, "any", "signing")
+        logger.debug("Certs to verify request sig: %s, _saml_msg: %s", certs, _saml_msg)
+        verified = any(
+            verify_redirect_signature(_saml_msg, self.sec.sec_backend, cert)
+            for cert_name, cert in certs
+        )
+        logger.info("Redirect request signature check: %s", verified)
+        return verified
+
     def issue_instant_ok(self):
         """ Check that the request was issued at a reasonable time """
         upper = time_util.shift_time(time_util.time_in_a_while(days=1),
@@ -97,9 +146,10 @@ def _verify(self):
         return valid
 
     def loads(self, xmldata, binding, origdoc=None, must=None,
-              only_valid_cert=False):
+              only_valid_cert=False, relay_state=None, sigalg=None, signature=None):
         return self._loads(xmldata, binding, origdoc, must,
-                           only_valid_cert=only_valid_cert)
+                           only_valid_cert=only_valid_cert, relay_state=relay_state,
+                           sigalg=sigalg, signature=signature)
 
     def verify(self):
         try:

src/saml2/server.py

@@ -226,17 +226,23 @@ def verify_assertion_consumer_service(self, request):
 
     # -------------------------------------------------------------------------
 
-    def parse_authn_request(self, enc_request, binding=BINDING_HTTP_REDIRECT):
+    def parse_authn_request(self, enc_request, binding=BINDING_HTTP_REDIRECT,
+                            relay_state=None, sigalg=None, signature=None):
         """Parse a Authentication Request
 
         :param enc_request: The request in its transport format
         :param binding: Which binding that was used to transport the message
+        :param relay_state: RelayState, when binding=redirect
+        :param sigalg: Signature Algorithm, when binding=redirect
+        :param signature: Signature, when binding=redirect
             to this entity.
         :return: A request instance
         """
 
         return self._parse_request(enc_request, AuthnRequest,
-                                   "single_sign_on_service", binding)
+                                   "single_sign_on_service", binding,
+                                   relay_state=relay_state, sigalg=sigalg,
+                                   signature=signature)
 
     def parse_attribute_query(self, xml_string, binding):
         """ Parse an attribute query