Merge pull request #439 from jkakavas/fix_sane_defaults

Ensure signature checking for SAML Responses is enabled by default

6 files changed, 41 insertions, 14 deletions

diff --git a/src/saml2/client_base.py b/src/saml2/client_base.py
index 531ddea5..f8704c20 100644
--- a/src/saml2/client_base.py
+++ b/src/saml2/client_base.py
@@ -113,17 +113,30 @@ class Base(Entity):
         else:
             self.state = state_cache
 
-        self.logout_requests_signed = False
-        self.allow_unsolicited = False
-        self.authn_requests_signed = False
-        self.want_assertions_signed = False
-        self.want_response_signed = False
-        for foo in ["allow_unsolicited", "authn_requests_signed",
-                    "logout_requests_signed", "want_assertions_signed",
-                    "want_response_signed"]:
-            v = self.config.getattr(foo, "sp")
-            if v is True or v == 'true':
-                setattr(self, foo, True)
+        attribute_defaults = {
+            "logout_requests_signed": False,
+            "allow_unsolicited": False,
+            "authn_requests_signed": False,
+            "want_assertions_signed": False,
+            "want_response_signed": True,
+        }
+
+        for attr, val_default in attribute_defaults.items():
+            val_config = self.config.getattr(attr, "sp")
+            if val_config is None:
+                val = val_default
+            else:
+                val = val_config
+
+            if val == 'true':
+                val = True
+
+            setattr(self, attr, val)
+
+        if self.entity_type == "sp" and not any([self.want_assertions_signed,
+                                                self.want_response_signed]):
+            logger.warning("The SAML service provider accepts unsigned SAML Responses " +
+                           "and Assertions. This configuration is insecure.")
 
         self.artifact2response = {}
 
diff --git a/tests/test_51_client.py b/tests/test_51_client.py
index 2bd4d7cf..d72d8895 100644
--- a/tests/test_51_client.py
+++ b/tests/test_51_client.py
@@ -405,6 +405,7 @@ class TestClient:
             destination="http://lingon.catalogix.se:8087/",
             sp_entity_id="urn:mace:example.com:saml:roland:sp",
             name_id_policy=nameid_policy,
+            sign_response=True,
             userid="foba0001@example.com",
             authn=AUTHN)
 
@@ -449,6 +450,7 @@ class TestClient:
             in_response_to="id2",
             destination="http://lingon.catalogix.se:8087/",
             sp_entity_id="urn:mace:example.com:saml:roland:sp",
+            sign_response=True,
             name_id_policy=nameid_policy,
             userid="also0001@example.com",
             authn=AUTHN)
@@ -905,7 +907,6 @@ class TestClient:
                                      node_id=assertion.id)
 
         sigass = rm_xmltag(sigass)
-
         response = sigver.response_factory(
             in_response_to="_012345",
             destination="http://lingon.catalogix.se:8087/",
@@ -928,6 +929,8 @@ class TestClient:
 
         resp_str = base64.encodestring(enctext.encode('utf-8'))
         # Now over to the client side
+        # Explicitely allow unsigned responses for this and the following 2 tests
+        self.client.want_response_signed = False
         resp = self.client.parse_authn_request_response(
             resp_str, BINDING_HTTP_POST,
             {"_012345": "http://foo.example.com/service"})
@@ -1329,6 +1332,9 @@ class TestClient:
 
     def test_signed_redirect(self):
 
+        # Revert configuration change to disallow unsinged responses
+        self.client.want_response_signed = True
+
         msg_str = "%s" % self.client.create_authn_request(
             "http://localhost:8088/sso", message_id="id1")[1]
 
@@ -1560,6 +1566,8 @@ class TestClientWithDummy():
         response = self.client.send(**http_args)
         print(response.text)
         _dic = unpack_form(response.text, "SAMLResponse")
+        # Explicitly allow unsigned responses for this test
+        self.client.want_response_signed = False
         resp = self.client.parse_authn_request_response(_dic["SAMLResponse"],
                                                         BINDING_HTTP_POST,
                                                         {sid: "/"})
diff --git a/tests/test_60_sp.py b/tests/test_60_sp.py
index 65504635..78e88400 100644
--- a/tests/test_60_sp.py
+++ b/tests/test_60_sp.py
@@ -53,6 +53,8 @@ AUTHN = {
 class TestSP():
     def setup_class(self):
         self.sp = make_plugin("rem", saml_conf="server_conf")
+        # Explicitly allow unsigned responses for this test
+        self.sp.saml_client.want_response_signed = False
         self.server = Server(config_file="idp_conf")
 
     def teardown_class(self):
diff --git a/tests/test_63_ecp.py b/tests/test_63_ecp.py
index 32a1aaed..0db36003 100644
--- a/tests/test_63_ecp.py
+++ b/tests/test_63_ecp.py
@@ -136,7 +136,8 @@ def test_complete_flow():
                 assert inst.text == "XYZ"
 
         # parse the response
-
+        # Explicitly allow unsigned responses for this test
+        sp.want_response_signed = False
         resp = sp.parse_authn_request_response(respdict["body"], None, {sid: "/"})
 
         print(resp.response)
diff --git a/tests/test_65_authn_query.py b/tests/test_65_authn_query.py
index 68af10a1..77ccaa9e 100644
--- a/tests/test_65_authn_query.py
+++ b/tests/test_65_authn_query.py
@@ -92,6 +92,8 @@ def test_flow():
         # ------- @SP ----------
 
         xmlstr = get_msg(hinfo, binding)
+        # Explicitly allow unsigned responses for this test
+        sp.want_response_signed = False
         aresp = sp.parse_authn_request_response(xmlstr, binding,
                                                 {resp.in_response_to: "/"})
 
diff --git a/tests/test_68_assertion_id.py b/tests/test_68_assertion_id.py
index 283b4da6..31b7e8e0 100644
--- a/tests/test_68_assertion_id.py
+++ b/tests/test_68_assertion_id.py
@@ -78,7 +78,8 @@ def test_basic_flow():
         # --------- @SP -------------
 
         xmlstr = get_msg(hinfo, binding)
-
+        # Explicitly allow unsigned responses for this test
+        sp.want_response_signed = False
         aresp = sp.parse_authn_request_response(xmlstr, binding,
                                                 {resp.in_response_to: "/"})