Merge pull request #439 from jkakavas/fix_sane_defaults

Ensure signature checking for SAML Responses is enabled by default

Author: rohe

src/saml2/client_base.py

@@ -113,17 +113,30 @@ def __init__(self, config=None, identity_cache=None, state_cache=None,
         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 = {}
 

tests/test_51_client.py

@@ -405,6 +405,7 @@ def test_response_1(self):
             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="[email protected]",
             authn=AUTHN)
 
@@ -449,6 +450,7 @@ def test_response_1(self):
             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="[email protected]",
             authn=AUTHN)
@@ -905,7 +907,6 @@ def test_sign_then_encrypt_assertion2(self):
                                      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 @@ def test_sign_then_encrypt_assertion2(self):
 
         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 @@ def test_sign_then_encrypt_assertion_advice_2(self):
 
     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 @@ def test_post_sso(self):
         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: "/"})

tests/test_60_sp.py

@@ -53,6 +53,8 @@
 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):

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)

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: "/"})
 

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: "/"})