Merge pull request #834 from mheuwes/soap-fixes

Fix AttributeError and signature mangling during construction of SOAP request

Author: c00kiemon5ter

src/saml2/client.py

@@ -152,8 +152,8 @@ def prepare_for_negotiated_authenticate(
             # XXX   ^through self.create_authn_request(...)
             # XXX - sign_redirect will add the signature to the query params
             # XXX   ^through self.apply_binding(...)
-            sign_post = False if binding == BINDING_HTTP_REDIRECT else sign
-            sign_redirect = False if binding == BINDING_HTTP_POST and sign else sign
+            sign_redirect = sign and binding == BINDING_HTTP_REDIRECT
+            sign_post = sign and not sign_redirect
 
             reqid, request = self.create_authn_request(
                 destination=destination,
@@ -318,10 +318,8 @@ def do_logout(
                 session_indexes = None
 
             sign = sign if sign is not None else self.logout_requests_signed
-            sign_post = sign and (
-                binding == BINDING_HTTP_POST or binding == BINDING_SOAP
-            )
             sign_redirect = sign and binding == BINDING_HTTP_REDIRECT
+            sign_post = sign and not sign_redirect
 
             log_report = {
                 "message": "Invoking SLO on entity",

src/saml2/httpbase.py

@@ -315,7 +315,7 @@ def use_soap(self, request, destination="", soap_headers=None, sign=False,
 
         if sign and self.sec:
             _signed = self.sec.sign_statement(soap_message,
-                                              class_name=class_name(request),
+                                              node_name=class_name(request),
                                               node_id=request.id)
             soap_message = _signed
 

src/saml2/pack.py

@@ -240,7 +240,7 @@ def make_soap_enveloped_saml_thingy(thingy, header_parts=None):
         if thingy[0:5].lower() == '<?xml':
             logger.debug("thingy0: %s", thingy)
             _part = thingy.split("\n")
-            thingy = "".join(_part[1:])
+            thingy = "\n".join(_part[1:])
         thingy = thingy.replace(PREFIX, "")
         logger.debug("thingy: %s", thingy)
         _child = ElementTree.Element('')

src/saml2/request.py

@@ -2,7 +2,6 @@
 
 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.s_utils import OtherError
 
@@ -55,22 +54,22 @@ def _loads(
         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
+        sign_redirect = must and binding == BINDING_HTTP_REDIRECT
+        sign_post = must and not sign_redirect
         incorrectly_signed = IncorrectlySigned("Request was not signed correctly")
 
         try:
             self.message = self.signature_check(
                 xmldata,
                 origdoc=origdoc,
-                must=signed_post,
+                must=sign_post,
                 only_valid_cert=only_valid_cert,
             )
         except Exception as e:
             self.message = None
             raise incorrectly_signed from e
 
-        if signed_redirect:
+        if sign_redirect:
             if sigalg is None or signature is None:
                 raise incorrectly_signed
 

tests/test_50_server.py

@@ -12,6 +12,7 @@
 from saml2.sigver import make_temp, DecryptError, EncryptError, CertificateError
 from saml2.assertion import Policy
 from saml2.authn_context import INTERNETPROTOCOLPASSWORD
+from saml2.response import IncorrectlySigned
 from saml2.saml import NameID, NAMEID_FORMAT_TRANSIENT
 from saml2.samlp import response_from_string
 
@@ -32,6 +33,7 @@
 from saml2.soap import make_soap_enveloped_saml_thingy
 from saml2 import BINDING_HTTP_POST
 from saml2 import BINDING_HTTP_REDIRECT
+from saml2 import BINDING_SOAP
 from saml2.time_util import instant
 
 from pytest import raises
@@ -2245,10 +2247,65 @@ def test_slo_soap(self):
         self.server.ident.close()
 
         with closing(Server("idp_soap_conf")) as idp:
+            request = idp.parse_logout_request(saml_soap)
+            assert request
+
+            idp.config.setattr("idp", "want_authn_requests_signed", True)
+            assert idp.config.getattr("want_authn_requests_signed", "idp")
+
+            with raises(IncorrectlySigned):
+                # check unsigned requests over SOAP to fail
+                request = idp.parse_logout_request(saml_soap)
+                assert not request
+
+            idp.ident.close()
+
+    def test_slo_soap_signed(self):
+        soon = time_util.in_a_while(days=1)
+        sinfo = {
+            "name_id": nid,
+            "issuer": "urn:mace:example.com:saml:roland:idp",
+            "not_on_or_after": soon,
+            "user": {
+                "givenName": "Leo",
+                "sn": "Laport",
+            }
+        }
+
+        sp = client.Saml2Client(config_file="server_conf")
+        sp.users.add_information_about_person(sinfo)
+
+        req_id, logout_request = sp.create_logout_request(
+            name_id=nid, destination="http://localhost:8088/slo",
+            issuer_entity_id="urn:mace:example.com:saml:roland:idp",
+            reason="I'm tired of this", sign=True, sign_alg=ds.SIG_RSA_SHA512,
+            digest_alg=ds.DIGEST_SHA512,
+        )
+
+        saml_soap = sp.apply_binding(BINDING_SOAP, logout_request, sign=False)
+        saml_soap = saml_soap["data"]
+        self.server.ident.close()
+
+        with closing(Server("idp_soap_conf")) as idp:
+            idp.config.setattr("idp", "want_authn_requests_signed", True)
+            assert idp.config.getattr("want_authn_requests_signed", "idp")
+
+            with raises(IncorrectlySigned):
+                # idp_soap_conf has invalid certificate for sp
+                request = idp.parse_logout_request(saml_soap)
+                assert not request
+
+            idp.ident.close()
+
+        with closing(Server("idp_conf_verify_cert")) as idp:
+            idp.config.setattr("idp", "want_authn_requests_signed", True)
+            assert idp.config.getattr("want_authn_requests_signed", "idp")
+
             request = idp.parse_logout_request(saml_soap)
             idp.ident.close()
             assert request
 
+
 # ------------------------------------------------------------------------