Implement SHA1 deprecation policy

kislyuk · kislyuk · commit 9c827a922bfe · 2022-12-04T07:56:32.000-08:00
diff --git a/signxml/algorithms.py b/signxml/algorithms.py
@@ -49,6 +49,9 @@ class InvalidInputErrorMixin:
     def _missing_(cls, value):
         raise InvalidInput(f"Unrecognized {cls.__name__}: {value}")
 
+    def __repr__(self):
+        return f"{self.__class__.__name__}.{self.name}"
+
 
 class DigestAlgorithm(FragmentLookupMixin, InvalidInputErrorMixin, Enum):
     """
@@ -160,7 +163,7 @@ class CanonicalizationMethod(InvalidInputErrorMixin, Enum):
     EXCLUSIVE_XML_CANONICALIZATION_1_0_WITH_COMMENTS = "http://www.w3.org/2001/10/xml-exc-c14n#WithComments"
 
     # The identifier for Canonical XML 2.0 is "http://www.w3.org/2010/xml-c14n2", but it is not a W3C standard.
-    # While it is supported by lxml, it's not in general use and not supported by SignXML as a matter of policy
+    # While it is supported by lxml, it's not in general use and not supported by SignXML
 
 
 digest_algorithm_implementations = {
diff --git a/signxml/signer.py b/signxml/signer.py
@@ -111,11 +111,17 @@ def __init__(
             self.digest_alg = DigestAlgorithm.from_fragment(digest_algorithm)
         else:
             self.digest_alg = DigestAlgorithm(digest_algorithm)
+        self.check_deprecated_methods()
         self.c14n_alg = CanonicalizationMethod(c14n_algorithm)
         self.namespaces = dict(ds=namespaces.ds)
         self._parser = None
         self.signature_annotators = [self._add_key_info]
 
+    def check_deprecated_methods(self):
+        if "SHA1" in self.sign_alg.name or "SHA1" in self.digest_alg.name:
+            msg = "SHA1-based algorithms are not supported in the default configuration because they are not secure"
+            raise InvalidInput(msg)
+
     def sign(
         self,
         data,
diff --git a/signxml/verifier.py b/signxml/verifier.py
@@ -60,13 +60,13 @@ class SignatureConfiguration:
     If set to a non-integer, any number of references is accepted (otherwise a mismatch raises an error).
     """
 
-    signature_methods: FrozenSet[SignatureMethod] = frozenset()
+    signature_methods: FrozenSet[SignatureMethod] = frozenset(sm for sm in SignatureMethod if "SHA1" not in sm.name)
     """
     Set of acceptable signature methods (signature algorithms). Any signature generated using an algorithm not listed
     here will fail verification (but if this set is left empty, then all supported algorithms are accepted).
     """
 
-    digest_algorithms: FrozenSet[DigestAlgorithm] = frozenset()
+    digest_algorithms: FrozenSet[DigestAlgorithm] = frozenset(da for da in DigestAlgorithm if "SHA1" not in da.name)
     """
     Set of acceptable digest algorithms. Any signature or reference transform generated using an algorithm not listed
     here will cause verification to fail (but if this set is left empty, then all supported algorithms are accepted).
@@ -384,6 +384,7 @@ def verify(
 
             if signature_alg.name.startswith("ECDSA"):
                 raw_signature = self._encode_dss_signature(raw_signature, signing_cert.get_pubkey().bits())
+
             try:
                 digest_alg_name = str(digest_algorithm_implementations[signature_alg].name)
                 openssl_verify(signing_cert, raw_signature, signed_info_c14n, digest_alg_name)
diff --git a/test/test.py b/test/test.py
@@ -35,13 +35,25 @@
 )
 from signxml.xades import (  # noqa:E402
     XAdESDataObjectFormat,
+    XAdESSignatureConfiguration,
     XAdESSignaturePolicy,
     XAdESSigner,
     XAdESVerifier,
     XAdESVerifyResult,
 )
 
 
+class XMLSignerWithSHA1(XMLSigner):
+    def check_deprecated_methods(self):
+        pass
+
+
+sha1_ok = SignatureConfiguration(signature_methods=list(SignatureMethod), digest_algorithms=list(DigestAlgorithm))
+xades_sha1_ok = XAdESSignatureConfiguration(
+    signature_methods=list(SignatureMethod), digest_algorithms=list(DigestAlgorithm)
+)
+
+
 def reset_tree(t, method):
     if not isinstance(t, str):
         for s in t.findall(".//ds:Signature", namespaces=namespaces):
@@ -101,7 +113,7 @@ def test_basic_signxml_statements(self):
         self.assertEqual(SignatureMethod.RSA_SHA256.value, "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256")
         self.assertEqual(SignatureConstructionMethod.enveloped, methods.enveloped)
         with self.assertRaisesRegex(InvalidInput, "Unknown signature construction method"):
-            signer = XMLSigner(method=None)
+            signer = XMLSignerWithSHA1(method=None)
 
         with self.assertRaisesRegex(InvalidInput, "must be an XML element"):
             XMLSigner(signature_algorithm="hmac-sha256").sign("x", key=b"abc")
@@ -124,7 +136,7 @@ def test_basic_signxml_statements(self):
                     continue
                 print(digest_alg.name, sig_alg.name, c14n_alg, method, type(d))
                 reset_tree(d, method)
-                signer = XMLSigner(
+                signer = XMLSignerWithSHA1(
                     method=method,
                     signature_algorithm=sig_alg,
                     digest_algorithm=digest_alg,
@@ -136,7 +148,7 @@ def test_basic_signxml_statements(self):
                 )
                 # print(etree.tostring(signed))
                 hmac_key = self.keys["hmac"] if sig_alg_type == "hmac" else None
-                verify_kwargs = dict(require_x509=False, hmac_key=hmac_key, validate_schema=True)
+                verify_kwargs = dict(require_x509=False, hmac_key=hmac_key, validate_schema=True, expect_config=sha1_ok)
 
                 if method == methods.detached:
 
@@ -169,7 +181,12 @@ def resolver(uri):
                         XMLVerifier().verify(signed_data, id_attribute="X", **verify_kwargs)
 
                 with self.assertRaisesRegex(InvalidInput, "Expected a X.509 certificate based signature"):
-                    XMLVerifier().verify(signed_data, hmac_key=hmac_key, uri_resolver=verify_kwargs.get("uri_resolver"))
+                    XMLVerifier().verify(
+                        signed_data,
+                        hmac_key=hmac_key,
+                        uri_resolver=verify_kwargs.get("uri_resolver"),
+                        expect_config=sha1_ok,
+                    )
 
                 if method != methods.detached:
                     with self.assertRaisesRegex(InvalidSignature, "Digest mismatch"):
@@ -205,36 +222,34 @@ def test_x509_certs(self):
         tree = etree.parse(self.example_xml_files[0])
         ca_pem_file = os.path.join(os.path.dirname(__file__), "example-ca.pem").encode("utf-8")
         crt, key = self.load_example_keys()
-        for hash_alg in "sha1", "sha256":
-            for method in methods.enveloped, methods.enveloping:
-                print(hash_alg, method)
-                data = tree.getroot()
-                reset_tree(data, method)
-                signer = XMLSigner(method=method, signature_algorithm="rsa-" + hash_alg)
-                signed = signer.sign(data, key=key, cert=crt)
-                signed_data = etree.tostring(signed)
-                XMLVerifier().verify(signed_data, ca_pem_file=ca_pem_file)
-                XMLVerifier().verify(signed_data, x509_cert=crt)
-                XMLVerifier().verify(signed_data, x509_cert=load_certificate(FILETYPE_PEM, crt))
-                XMLVerifier().verify(signed_data, x509_cert=crt, cert_subject_name="*.example.com")
-
-                with self.assertRaises(OpenSSLCryptoError):
-                    XMLVerifier().verify(signed_data, x509_cert=crt[::-1])
-
-                with self.assertRaises(InvalidSignature):
-                    XMLVerifier().verify(signed_data, x509_cert=crt, cert_subject_name="test")
-
-                with self.assertRaisesRegex(InvalidCertificate, "unable to get local issuer certificate"):
-                    XMLVerifier().verify(signed_data)
-                # TODO: negative: verify with wrong cert, wrong CA
+        for method in methods.enveloped, methods.enveloping:
+            data = tree.getroot()
+            reset_tree(data, method)
+            signer = XMLSigner(method=method, signature_algorithm=SignatureMethod.RSA_SHA256)
+            signed = signer.sign(data, key=key, cert=crt)
+            signed_data = etree.tostring(signed)
+            XMLVerifier().verify(signed_data, ca_pem_file=ca_pem_file)
+            XMLVerifier().verify(signed_data, x509_cert=crt)
+            XMLVerifier().verify(signed_data, x509_cert=load_certificate(FILETYPE_PEM, crt))
+            XMLVerifier().verify(signed_data, x509_cert=crt, cert_subject_name="*.example.com")
+
+            with self.assertRaises(OpenSSLCryptoError):
+                XMLVerifier().verify(signed_data, x509_cert=crt[::-1])
+
+            with self.assertRaises(InvalidSignature):
+                XMLVerifier().verify(signed_data, x509_cert=crt, cert_subject_name="test")
+
+            with self.assertRaisesRegex(InvalidCertificate, "unable to get local issuer certificate"):
+                XMLVerifier().verify(signed_data)
+            # TODO: negative: verify with wrong cert, wrong CA
 
     def test_xmldsig_interop_examples(self):
         ca_pem_file = os.path.join(os.path.dirname(__file__), "interop", "cacert.pem").encode("utf-8")
         for signature_file in glob(os.path.join(os.path.dirname(__file__), "interop", "*.xml")):
             print("Verifying", signature_file)
             with open(signature_file, "rb") as fh:
                 with self.assertRaisesRegex(InvalidCertificate, "certificate has expired"):
-                    XMLVerifier().verify(fh.read(), ca_pem_file=ca_pem_file)
+                    XMLVerifier().verify(fh.read(), ca_pem_file=ca_pem_file, expect_config=sha1_ok)
 
     def test_xmldsig_interop_TR2012(self):
         def get_x509_cert(**kwargs):
@@ -256,6 +271,7 @@ def get_x509_cert(**kwargs):
                         hmac_key="testkey",
                         validate_schema=True,
                         cert_resolver=get_x509_cert if "x509digest" in signature_file else None,
+                        expect_config=sha1_ok,
                     )
                     sig.decode("utf-8")
                 except Exception as e:
@@ -325,6 +341,7 @@ def cert_resolver(x509_issuer_name, x509_serial_number, x509_digest):
                         x509_cert=get_x509_cert(signature_file),
                         cert_resolver=cert_resolver if "issuer-serial" in signature_file else None,
                         ca_pem_file=get_ca_pem_file(signature_file),
+                        expect_config=sha1_ok,
                     )
                     decoded_sig = sig.decode("utf-8")
                     if "HMACOutputLength" in decoded_sig or "bad" in signature_file or "expired" in signature_file:
@@ -543,13 +560,13 @@ def test_ws_security(self):
         with open(os.path.join(wsse_dir, "examples", "server_public.pem"), "rb") as fh:
             crt = fh.read()
         data = etree.parse(os.path.join(wsse_dir, "test", "unit", "client", "files", "valid wss resp.xml"))
-        XMLVerifier().verify(data, x509_cert=crt, validate_schema=False, expect_references=2)
+        XMLVerifier().verify(data, x509_cert=crt, validate_schema=False, expect_references=2, expect_config=sha1_ok)
 
         data = etree.parse(
             os.path.join(wsse_dir, "test", "unit", "client", "files", "invalid wss resp - changed content.xml")
         )
         with self.assertRaisesRegex(InvalidDigest, "Digest mismatch for reference 0"):
-            XMLVerifier().verify(data, x509_cert=crt, validate_schema=False, expect_references=2)
+            XMLVerifier().verify(data, x509_cert=crt, validate_schema=False, expect_references=2, expect_config=sha1_ok)
 
     def test_psha1(self):
         from signxml.util import p_sha1
@@ -620,6 +637,18 @@ def test_verify_config(self):
         with self.assertRaisesRegex(InvalidInput, "Digest algorithm SHA256 forbidden by configuration"):
             verifier.verify(signed, x509_cert=cert, expect_config=config)
 
+    def test_sha1_policy(self):
+        data = etree.parse(self.example_xml_files[0]).getroot()
+        cert, key = self.load_example_keys()
+        with self.assertRaisesRegex(InvalidInput, "SHA1-based algorithms are not supported"):
+            XMLSigner(signature_algorithm=SignatureMethod.RSA_SHA1)
+        signer = XMLSignerWithSHA1(signature_algorithm=SignatureMethod.RSA_SHA1, digest_algorithm=DigestAlgorithm.SHA1)
+        signed = signer.sign(data, cert=cert, key=key)
+        verifier = XMLVerifier()
+        with self.assertRaisesRegex(InvalidInput, "Signature method RSA_SHA1 forbidden by configuration"):
+            verifier.verify(signed, x509_cert=cert)
+        verifier.verify(signed, x509_cert=cert, expect_config=sha1_ok)
+
 
 class TestXAdES(unittest.TestCase, LoadExampleKeys):
     expect_references = {
@@ -681,7 +710,11 @@ def test_xades_interop_examples(self):
             with open(sig_file, "rb") as fh:
                 doc = etree.parse(fh)
             cert = doc.find("//{http://www.w3.org/2000/09/xmldsig#}X509Certificate").text
-            kwargs = dict(x509_cert=cert, expect_references=self.expect_references.get(os.path.basename(sig_file), 2))
+            kwargs = dict(
+                x509_cert=cert,
+                expect_references=self.expect_references.get(os.path.basename(sig_file), 2),
+                expect_config=xades_sha1_ok,
+            )
             if "nonconformant" in sig_file:
                 kwargs.update(validate_schema=False)
             if "sigPolStore" in sig_file:
