Merge branch 'bugfix/correct-ed448-shake'

pyhanko/sign/attributes.py

@@ -130,10 +130,19 @@ def __init__(
     async def build_attr_value(
         self, dry_run=False
     ) -> cms.CMSAlgorithmProtection:
+        digest_algorithm_args = {'algorithm': self.digest_algo}
+        if self.digest_algo == 'shake256':
+            # RFC 8419 requirement
+            mech = self.signature_algo
+            if mech['algorithm'].native == 'ed448':
+                digest_algorithm_args = {
+                    'algorithm': 'shake256_len',
+                    'parameters': core.Integer(512),
+                }
         return cms.CMSAlgorithmProtection(
             {
                 'digest_algorithm': algos.DigestAlgorithm(
-                    {'algorithm': self.digest_algo}
+                    digest_algorithm_args
                 ),
                 'signature_algorithm': self.signature_algo,
             }

pyhanko/sign/general.py

@@ -348,8 +348,9 @@ class UnacceptableSignerError(SigningError):
 
 
 def get_pyca_cryptography_hash(algorithm) -> Union[hashes.HashAlgorithm]:
-    if algorithm.lower() == 'shake256':
-        # force the output length to 64 bytes = 512 bits
+    if algorithm.lower() in ('shake256', 'shake256_len'):
+        # force the output length to 64 bytes = 512 bits. We don't
+        # support any other lengths because those can't be valid in CMS
         return hashes.SHAKE256(digest_size=64)
     else:
         return getattr(hashes, algorithm.upper())()

pyhanko/sign/signers/pdf_cms.py

@@ -589,9 +589,17 @@ def signer_info(self, digest_algorithm: str, signed_attrs, signature):
         :return:
             An :class:`.asn1crypto.cms.SignerInfo` object.
         """
-        digest_algorithm_obj = algos.DigestAlgorithm(
-            {'algorithm': digest_algorithm}
-        )
+
+        digest_algorithm_args = {'algorithm': digest_algorithm}
+        if digest_algorithm == 'shake256':
+            # RFC 8419 requirement
+            mech = self.get_signature_mechanism_for_digest('shake256')
+            if mech['algorithm'].native == 'ed448':
+                digest_algorithm_args = {
+                    'algorithm': 'shake256_len',
+                    'parameters': core.Integer(512),
+                }
+        digest_algorithm_obj = algos.DigestAlgorithm(digest_algorithm_args)
 
         signing_cert = self.signing_cert
         if signing_cert is None:
@@ -635,10 +643,8 @@ def _package_signature(
         encap_content_info,
     ) -> cms.ContentInfo:
         encap_content_info = encap_content_info or {'content_type': 'data'}
-        digest_algorithm_obj = algos.DigestAlgorithm(
-            {'algorithm': digest_algorithm}
-        )
         sig_info = self.signer_info(digest_algorithm, signed_attrs, signature)
+        digest_algorithm_obj = sig_info['digest_algorithm']
 
         if unsigned_attrs is not None:
             sig_info['unsigned_attrs'] = unsigned_attrs

pyhanko/sign/validation/utils.py

@@ -2,7 +2,7 @@
 from datetime import datetime
 from typing import Optional
 
-from asn1crypto import algos, cms, keys, x509
+from asn1crypto import algos, cms, core, keys, x509
 from cryptography.hazmat.primitives import serialization
 from cryptography.hazmat.primitives.asymmetric import padding
 from cryptography.hazmat.primitives.asymmetric.dsa import DSAPublicKey
@@ -33,6 +33,21 @@ def _ensure_digest_match(
     signature_algo: algos.SignedDigestAlgorithm,
     message_digest_algo: algos.DigestAlgorithm,
 ) -> AlgorithmUsageConstraint:
+    if signature_algo['algorithm'].native == 'ed448':
+        # be a bit more tolerant here, also don't check parameters because
+        # we only support one length anyway
+        algo = message_digest_algo['algorithm'].native
+        if algo in ('shake256', 'shake256_len'):
+            return AlgorithmUsageConstraint(allowed=True)
+        else:
+            return AlgorithmUsageConstraint(
+                allowed=False,
+                failure_reason=(
+                    f"Digest algorithm {algo} "
+                    f"does not match value implied by signature algorithm ed448"
+                ),
+            )
+
     try:
         sig_hash_algo_obj = algos.DigestAlgorithm(
             {'algorithm': signature_algo.hash_algo}
@@ -128,7 +143,6 @@ def signature_algorithm_allowed(
 
 DEFAULT_WEAK_HASH_ALGORITHMS = frozenset({'sha1', 'md5', 'md2'})
 
-
 DEFAULT_ALGORITHM_USAGE_POLICY = CMSAlgorithmUsagePolicy.lift_policy(
     DisallowWeakAlgorithmsPolicy(DEFAULT_WEAK_HASH_ALGORITHMS)
 )

pyhanko_tests/test_cms.py

@@ -2168,3 +2168,31 @@ async def test_tolerate_der_deviations_in_pdf():
     # now we run the actual validation after reopening the file
     r = PdfFileReader(output)
     await async_val_trusted(r.embedded_signatures[0])
+
+
+@freeze_time('2020-11-01')
+def test_ed448_no_length():
+    # verify that we still accept ed448 with id-shake256 without paramaters
+
+    fname = os.path.join(PDF_DATA_DIR, 'ed448-shake256-nolen.pdf')
+    with open(fname, 'rb') as inf:
+        r = PdfFileReader(inf)
+        s = r.embedded_signatures[0]
+        status = val_untrusted(s)
+    assert status.md_algorithm == 'shake256'
+
+    assert len(s.external_digest) == 64
+
+
+@freeze_time('2020-11-01')
+def test_ed448_invalid_hash_algo_validation():
+    w = IncrementalPdfFileWriter(BytesIO(MINIMAL))
+
+    with pytest.raises(
+        DisallowedAlgorithmError, match='algorithm.*does not match'
+    ):
+        fname = os.path.join(PDF_DATA_DIR, 'ed448-disallowed-hash.pdf')
+        with open(fname, 'rb') as inf:
+            r = PdfFileReader(inf)
+            s = r.embedded_signatures[0]
+            val_untrusted(s)

pyhanko_tests/test_signing.py

@@ -1512,7 +1512,7 @@ def test_ed448():
     r = PdfFileReader(out)
     s = r.embedded_signatures[0]
     status = val_untrusted(s)
-    assert status.md_algorithm == 'shake256'
+    assert status.md_algorithm == 'shake256_len'
 
     assert len(s.external_digest) == 64