Add rejectDeprecatedAlgorithm settings. Define DEPRECATED_ALGORITHMS list on Constants. If flag enabled, reject signatures on response, logout_request and logout_response with deprecated algorithm

pitbulk · pitbulk · commit 171a89e75a6e · 2022-01-28T20:53:13.000+01:00
diff --git a/README.md b/README.md
@@ -14,6 +14,8 @@ This version supports Python3. There is a separate version that only support Pyt
 
 #### Warning ####
 
+Version 1.13.0 sets sha256 and rsa-sha256 as default algorithms
+
 Version 1.8.0 sets strict mode active by default
 
 Update ``python3-saml`` to ``1.5.0``, this version includes security improvements for preventing XEE and Xpath Injections.
@@ -485,7 +487,12 @@ In addition to the required settings data (idp, sp), extra settings can be defin
             
         // Specify if you want the SP to view assertions with duplicated Name or FriendlyName attributes to be valid
         // Defaults to false if not specified
-        'allowRepeatAttributeName': false
+        'allowRepeatAttributeName': false,
+
+        // If the toolkit receive a message signed with a
+        // deprecated algoritm (defined at the constant class)
+        // will raise an error and reject the message
+        "rejectDeprecatedAlgorithm": true
     },
 
     // Contact information template, it is recommended to suply a
diff --git a/demo-django/saml/advanced_settings.json b/demo-django/saml/advanced_settings.json
@@ -12,7 +12,8 @@
         "wantAssertionsEncrypted": false,
         "allowSingleLabelDomains": false,
         "signatureAlgorithm": "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256",
-        "digestAlgorithm": "http://www.w3.org/2001/04/xmlenc#sha256"
+        "digestAlgorithm": "http://www.w3.org/2001/04/xmlenc#sha256",
+        "rejectDeprecatedAlgorithm": true
     },
     "contactPerson": {
         "technical": {
diff --git a/demo-flask/saml/advanced_settings.json b/demo-flask/saml/advanced_settings.json
@@ -12,7 +12,8 @@
         "wantAssertionsEncrypted": false,
         "allowSingleLabelDomains": false,
         "signatureAlgorithm": "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256",
-        "digestAlgorithm": "http://www.w3.org/2001/04/xmlenc#sha256"
+        "digestAlgorithm": "http://www.w3.org/2001/04/xmlenc#sha256",
+        "rejectDeprecatedAlgorithm": true
     },
     "contactPerson": {
         "technical": {
diff --git a/demo-tornado/saml/advanced_settings.json b/demo-tornado/saml/advanced_settings.json
@@ -12,7 +12,8 @@
         "wantAssertionsEncrypted": false,
         "allowSingleLabelDomains": false,
         "signatureAlgorithm": "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256",
-        "digestAlgorithm": "http://www.w3.org/2001/04/xmlenc#sha256"
+        "digestAlgorithm": "http://www.w3.org/2001/04/xmlenc#sha256",
+        "rejectDeprecatedAlgorithm": true
     },
     "contactPerson": {
         "technical": {
diff --git a/demo_pyramid/demo_pyramid/saml/advanced_settings.json b/demo_pyramid/demo_pyramid/saml/advanced_settings.json
@@ -12,7 +12,8 @@
         "wantAssertionsEncrypted": false,
         "allowSingleLabelDomains": false,
         "signatureAlgorithm": "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256",
-        "digestAlgorithm": "http://www.w3.org/2001/04/xmlenc#sha256"
+        "digestAlgorithm": "http://www.w3.org/2001/04/xmlenc#sha256",
+        "rejectDeprecatedAlgorithm": true
     },
     "contactPerson": {
         "technical": {
diff --git a/src/onelogin/saml2/auth.py b/src/onelogin/saml2/auth.py
@@ -167,7 +167,6 @@ def process_slo(self, keep_local_session=False, request_id=None, delete_session_
                 self._errors.append('Signature validation failed. Logout Response rejected')
             elif not logout_response.is_valid(self._request_data, request_id):
                 self._errors.append('invalid_logout_response')
-                self._error_reason = logout_response.get_error()
             elif logout_response.get_status() != OneLogin_Saml2_Constants.STATUS_SUCCESS:
                 self._errors.append('logout_not_success')
             else:
@@ -183,7 +182,6 @@ def process_slo(self, keep_local_session=False, request_id=None, delete_session_
                 self._errors.append('Signature validation failed. Logout Request rejected')
             elif not logout_request.is_valid(self._request_data):
                 self._errors.append('invalid_logout_request')
-                self._error_reason = logout_request.get_error()
             else:
                 if not keep_local_session:
                     OneLogin_Saml2_Utils.delete_local_session(delete_session_cb)
@@ -694,6 +692,15 @@ def _validate_signature(self, data, saml_type, raise_exceptions=False):
             if isinstance(sign_alg, bytes):
                 sign_alg = sign_alg.decode('utf8')
 
+            security = self._settings.get_security_data()
+            reject_deprecated_alg = security.get('rejectDeprecatedAlgorithm', False)
+            if reject_deprecated_alg:
+                if sign_alg in OneLogin_Saml2_Constants.DEPRECATED_ALGORITHMS:
+                    raise OneLogin_Saml2_ValidationError(
+                        'Deprecated signature algorithm found: %s' % sign_alg,
+                        OneLogin_Saml2_ValidationError.DEPRECATED_SIGNATURE_METHOD
+                    )
+
             query_string = self._request_data.get('query_string')
             if query_string and self._request_data.get('validate_signature_from_qs'):
                 signed_query = self._build_sign_query_from_qs(query_string, saml_type)
diff --git a/src/onelogin/saml2/constants.py b/src/onelogin/saml2/constants.py
@@ -114,3 +114,6 @@ class OneLogin_Saml2_Constants(object):
     AES256_CBC = 'http://www.w3.org/2001/04/xmlenc#aes256-cbc'
     RSA_1_5 = 'http://www.w3.org/2001/04/xmlenc#rsa-1_5'
     RSA_OAEP_MGF1P = 'http://www.w3.org/2001/04/xmlenc#rsa-oaep-mgf1p'
+
+    # Define here the deprecated algorithms
+    DEPRECATED_ALGORITHMS = [DSA_SHA1, RSA_SHA1, SHA1]
diff --git a/src/onelogin/saml2/errors.py b/src/onelogin/saml2/errors.py
@@ -110,6 +110,8 @@ class OneLogin_Saml2_ValidationError(Exception):
     WRONG_NUMBER_OF_SIGNATURES = 43
     RESPONSE_EXPIRED = 44
     AUTHN_CONTEXT_MISMATCH = 45
+    DEPRECATED_SIGNATURE_METHOD = 46
+    DEPRECATED_DIGEST_METHOD = 47
 
     def __init__(self, message, code=0, errors=None):
         """
diff --git a/src/onelogin/saml2/response.py b/src/onelogin/saml2/response.py
@@ -653,6 +653,9 @@ def process_signed_elements(self):
         response_tag = '{%s}Response' % OneLogin_Saml2_Constants.NS_SAMLP
         assertion_tag = '{%s}Assertion' % OneLogin_Saml2_Constants.NS_SAML
 
+        security = self._settings.get_security_data()
+        reject_deprecated_alg = security.get('rejectDeprecatedAlgorithm', False)
+
         for sign_node in sign_nodes:
             signed_element = sign_node.getparent().tag
             if signed_element != response_tag and signed_element != assertion_tag:
@@ -695,6 +698,26 @@ def process_signed_elements(self):
                         )
                     verified_seis.append(sei)
 
+            # Check the signature and digest algorithm
+            if reject_deprecated_alg:
+                sig_method_node = OneLogin_Saml2_XML.query(sign_node, './/ds:SignatureMethod')
+                if sig_method_node:
+                    sig_method = sig_method_node[0].get("Algorithm")
+                    if sig_method in OneLogin_Saml2_Constants.DEPRECATED_ALGORITHMS:
+                        raise OneLogin_Saml2_ValidationError(
+                            'Deprecated signature algorithm found: %s' % sig_method,
+                            OneLogin_Saml2_ValidationError.DEPRECATED_SIGNATURE_METHOD
+                        )
+
+                dig_method_node = OneLogin_Saml2_XML.query(sign_node, './/ds:DigestMethod')
+                if dig_method_node:
+                    dig_method = dig_method_node[0].get("Algorithm")
+                    if dig_method in OneLogin_Saml2_Constants.DEPRECATED_ALGORITHMS:
+                        raise OneLogin_Saml2_ValidationError(
+                            'Deprecated digest algorithm found: %s' % dig_method,
+                            OneLogin_Saml2_ValidationError.DEPRECATED_DIGEST_METHOD
+                        )
+
             signed_elements.append(signed_element)
 
         if signed_elements:
diff --git a/src/onelogin/saml2/settings.py b/src/onelogin/saml2/settings.py
@@ -312,6 +312,9 @@ def _add_default_values(self):
         # Digest Algorithm
         self._security.setdefault('digestAlgorithm', OneLogin_Saml2_Constants.SHA256)
 
+        # Reject Deprecated Algorithms
+        self._security.setdefault('rejectDeprecatedAlgorithm', False)
+
         # AttributeStatement required by default
         self._security.setdefault('wantAttributeStatement', True)
 
diff --git a/tests/src/OneLogin/saml2_tests/auth_test.py b/tests/src/OneLogin/saml2_tests/auth_test.py
@@ -1215,9 +1215,31 @@ def testIsInValidLogoutResponseSign(self):
         auth.process_slo()
         self.assertIn('Signature validation failed. Logout Response rejected', auth.get_errors())
 
+    def testIsInValidLogoutResponseSignatureRejectingDeprecatedAlgorithm(self):
+        """
+        Tests the process_slo method of the OneLogin_Saml2_Auth
+        """
+        request_data = {
+            'http_host': 'example.com',
+            'script_name': 'index.html',
+            'get_data': {
+                'SAMLResponse': 'fZHbasJAEIZfJey9ZrNZc1gSodRSBKtQxYveyGQz1kCyu2Q24OM3jS21UHo3p++f4Z+CoGud2th3O/hXJGcNYXDtWkNqapVs6I2yQA0pAx2S8lrtH142Ssy5cr31VtuW3SH/E0CEvW+sYcF6VbLTIktFLMWZgxQR8DSP85wDB4GJGMOqShYVaoBUsOCIPY1kyUahEScacG3Ig/FjiUdyxuOZ4IcoUVGq4vSNBSsk3xjwE3Xx3qkwJD+cz3NtuxBN7WxjPN1F1NLcXdwob77tONiS7bZPm93zenvCqopxgVJmuU50jREsZF4noKWAOuNZJbNznnBky+LTDDVd2S+/dje1m+MVOtfidEER3g8Vt2fsPfiBfmePtsbgCO2A/9tL07TaD1ojEQuXtw0/ouFfD19+AA==',
+                'RelayState': 'http://stuff.com/endpoints/endpoints/index.php',
+                'SigAlg': 'http://www.w3.org/2000/09/xmldsig#rsa-sha1',
+                'Signature': 'OV9c4R0COSjN69fAKCpV7Uj/yx6/KFxvbluVCzdK3UuortpNMpgHFF2wYNlMSG9GcYGk6p3I8nB7Z+1TQchMWZOlO/StjAqgtZhtpiwPcWryNuq8vm/6hnJ3zMDhHTS7F8KG4qkCXmJ9sQD3Y31UNcuygBwIbNakvhDT5Qo9Nsw='
+            }
+        }
+        settings_info = self.loadSettingsJSON('settings8.json')
+        settings_info['security']['rejectDeprecatedAlgorithm'] = True
+        settings = OneLogin_Saml2_Settings(settings_info)
+        auth = OneLogin_Saml2_Auth(request_data, old_settings=settings)
+        auth.process_slo()
+        self.assertIn('Signature validation failed. Logout Response rejected', auth.get_errors())
+        self.assertEqual('Deprecated signature algorithm found: http://www.w3.org/2000/09/xmldsig#rsa-sha1', auth.get_last_error_reason())
+
     def testIsValidLogoutRequestSign(self):
         """
-        Tests the is_valid method of the OneLogin_Saml2_LogoutRequest
+        Tests the process_slo method of the OneLogin_Saml2_Auth
         """
         request_data = {
             'http_host': 'example.com',
@@ -1303,6 +1325,29 @@ def testIsValidLogoutRequestSign(self):
         auth.process_slo()
         self.assertIn('Signature validation failed. Logout Request rejected', auth.get_errors())
 
+    def testIsInValidLogoutRequestSignatureRejectingDeprecatedAlgorithm(self):
+        """
+        Tests the process_slo method of the OneLogin_Saml2_Auth
+        """
+        request_data = {
+            'http_host': 'example.com',
+            'script_name': 'index.html',
+            'get_data': {
+                'SAMLRequest': 'fZJNa+MwEIb/itHdiTz6sC0SQyEsBPoB27KHXoIsj7cGW3IlGfLzV7G7kN1DL2KYmeedmRcdgp7GWT26326JP/FzwRCz6zTaoNbKkSzeKqfDEJTVEwYVjXp9eHpUsKNq9i4640Zyh3xP6BDQx8FZkp1PR3KpqexAl72QmpUCS8SW01IiZz2TVVGD4X1VQYlAsl/oQyKPJAklPIQFzzZEbWNK0YLnlOVA3wqpQCoB7yQ7pWsGq+NKfcQ4q/0+xKXvd8ZNe7Td7AYbw10UxrCbP2aSPbv4Yl/8Qx/R3+SB5bTOoXiDQvFNvjnc7lXrIr75kh+6eYdXPc0jrkMO+/umjXhOtpxP2Q/nJx2/9+uWGbq8X1tV9NqGAW0kzaVvoe1AAJeCSWqYaUVRM2SilKKuqDTpFSlszdcK29RthVm9YriZebYdXpsLdhVAB7VJzif3haYMqqTVcl0JMBR4y+s2zak3sf/4v8l/vlHzBw==',
+                'RelayState': '_1037fbc88ec82ce8e770b2bed1119747bb812a07e6',
+                'SigAlg': 'http://www.w3.org/2000/09/xmldsig#rsa-sha1',
+                'Signature': 'Ouxo9BV6zmq4yrgamT9EbSKy/UmvSxGS8z26lIMgKOEP4LFR/N23RftdANmo4HafrzSfA0YTXwhKDqbOByS0j+Ql8OdQOes7vGioSjo5qq/Bi+5i6jXwQfphnfcHAQiJL4gYVIifkhhHRWpvYeiysF1Y9J02me0izwazFmoRXr4='
+            }
+        }
+        settings_info = self.loadSettingsJSON('settings8.json')
+        settings_info = self.loadSettingsJSON('settings8.json')
+        settings_info['security']['rejectDeprecatedAlgorithm'] = True
+        settings = OneLogin_Saml2_Settings(settings_info)
+        auth = OneLogin_Saml2_Auth(request_data, old_settings=settings)
+        auth.process_slo()
+        self.assertIn('Signature validation failed. Logout Request rejected', auth.get_errors())
+        self.assertEqual('Deprecated signature algorithm found: http://www.w3.org/2000/09/xmldsig#rsa-sha1', auth.get_last_error_reason())
+
     def testGetLastRequestID(self):
         settings_info = self.loadSettingsJSON()
         request_data = self.get_request()
diff --git a/tests/src/OneLogin/saml2_tests/response_test.py b/tests/src/OneLogin/saml2_tests/response_test.py
@@ -1030,6 +1030,19 @@ def testIsInValidNoKey(self):
         with self.assertRaisesRegex(Exception, 'Signature validation failed. SAML Response rejected'):
             response.is_valid(self.get_request_data(), raise_exceptions=True)
 
+    def testIsInValidDeprecatedAlgorithm(self):
+        """
+        Tests the is_valid method of the OneLogin_Saml2_Response
+        Case Deprecated algorithm used
+        """
+        settings_dict = self.loadSettingsJSON()
+        settings_dict['security']['rejectDeprecatedAlgorithm'] = True
+        settings = OneLogin_Saml2_Settings(settings_dict)
+        xml = self.file_contents(join(self.data_path, 'responses', 'valid_response.xml.base64'))
+        response = OneLogin_Saml2_Response(settings, xml)
+        with self.assertRaisesRegex(Exception, 'Deprecated signature algorithm found: http://www.w3.org/2000/09/xmldsig#rsa-sha1'):
+            response.is_valid(self.get_request_data(), raise_exceptions=True)
+
     def testIsInValidMultipleAssertions(self):
         """
         Tests the is_valid method of the OneLogin_Saml2_Response
