Merge pull request #487 from MatthiasValvekens/feature/strict-flag-po…

…st-signing

Fixes #466

pyhanko/sign/signers/pdf_signer.py

@@ -838,6 +838,9 @@ async def async_timestamp_pdf(
                 credential = pdf_out.security_handler.extract_credential()
                 if credential:
                     credential_ser = credential.serialise()
+            strict = True
+            if isinstance(pdf_out, IncrementalPdfFileWriter):
+                strict = pdf_out.prev.strict
             validation.DocumentSecurityStore.add_dss(
                 output_stream=res_output,
                 sig_contents=sig_contents,
@@ -846,6 +849,7 @@ async def async_timestamp_pdf(
                 force_write=not dss_settings.skip_if_unneeded,
                 embed_roots=embed_roots,
                 file_credential=credential_ser,
+                strict=strict,
             )
 
         return misc.finalise_output(output, res_output)
@@ -2308,6 +2312,12 @@ def prepare_tbs_document(
                 credential = security_handler.extract_credential()
                 if credential is not None:
                     credential_ser = credential.serialise()
+            # Preserve strict mode setting if we're doing
+            # an incremental signature
+            strict = True
+            if isinstance(self.pdf_out, IncrementalPdfFileWriter):
+                strict = self.pdf_out.prev.strict
+
             post_signing_instr = PostSignInstructions(
                 validation_info=validation_info,
                 # use the same algorithm
@@ -2322,6 +2332,7 @@ def prepare_tbs_document(
                 tight_size_estimates=signature_meta.tight_size_estimates,
                 embed_roots=embed_roots,
                 file_credential=credential_ser,
+                strict=strict,
             )
         return PdfTBSDocument(
             cms_writer=self.cms_writer,
@@ -2413,6 +2424,14 @@ class PostSignInstructions:
     Serialised file credential, to update encrypted files.
     """
 
+    strict: bool = True
+    """
+    .. versionadded:: 0.25.2
+
+    Controls whether to open the signed document in strict mode before applying
+    post-signing instructions.
+    """
+
 
 class PdfMacAttrProvider(attributes.CMSAttributeProvider):
     attribute_type = 'pdf_mac_data'
@@ -2851,10 +2870,11 @@ async def post_signature_processing(
                 output_stream=output,
                 **dss_op_kwargs,
                 file_credential=instr.file_credential,
+                strict=instr.strict,
             )
         if timestamper is not None:
             # append a document timestamp after the DSS update
-            w = IncrementalPdfFileWriter(output)
+            w = IncrementalPdfFileWriter(output, strict=instr.strict)
             if (
                 w.security_handler is not None
                 and instr.file_credential is not None

pyhanko/sign/validation/dss.py

@@ -490,6 +490,7 @@ def add_dss(
         force_write: bool = False,
         embed_roots: bool = True,
         file_credential: Optional[SerialisedCredential] = None,
+        strict: bool = True,
     ):
         """
         Wrapper around :meth:`supply_dss_in_writer`.
@@ -535,8 +536,11 @@ def add_dss(
             .. versionadded:: 0.13.0
 
             Serialised file credential, to update encrypted files.
+        :param strict:
+            If ``True``, enforce strict validation of the input stream.
+            Default is ``True``.
         """
-        pdf_out = IncrementalPdfFileWriter(output_stream)
+        pdf_out = IncrementalPdfFileWriter(output_stream, strict=strict)
         if pdf_out.security_handler is not None and file_credential is not None:
             pdf_out.security_handler.authenticate(file_credential)
         dss = cls.supply_dss_in_writer(

pyhanko_tests/cli_tests/conftest.py

@@ -14,6 +14,7 @@
 from freezegun import freeze_time
 from pyhanko_certvalidator import ValidationContext
 
+from pyhanko.pdf_utils.misc import PdfStrictReadError
 from pyhanko.pdf_utils.reader import PdfFileReader
 from pyhanko.sign.validation import (
     RevocationInfoValidationType,
@@ -131,12 +132,12 @@ def _write_config(config: dict, fname: str = 'pyhanko.yml'):
 logger = logging.getLogger(__name__)
 
 
-def _validate_last_sig_in(arch: PKIArchitecture, pdf_file):
+def _validate_last_sig_in(arch: PKIArchitecture, pdf_file, *, strict):
     vc_kwargs = dict(trust_roots=[arch.get_cert(CertLabel('root'))])
     vc = ValidationContext(**vc_kwargs, allow_fetching=True)
     with open(pdf_file, 'rb') as result:
         logger.info(f"Validating last signature in {pdf_file}...")
-        r = PdfFileReader(result)
+        r = PdfFileReader(result, strict=strict)
         # Little hack for the tests with encrypted files
         if r.security_handler is not None:
             r.decrypt("ownersecret")
@@ -163,5 +164,20 @@ def _validate_last_sig_in(arch: PKIArchitecture, pdf_file):
 @pytest.fixture
 def post_validate(pki_arch):
     yield
+    input_passes_strict = True
+    if os.path.isfile(INPUT_PATH):
+        try:
+            with open(INPUT_PATH, 'rb') as inf:
+                PdfFileReader(inf)
+        except PdfStrictReadError:
+            logger.info(
+                f"Input file {INPUT_PATH} can't be opened in strict mode, "
+                f"will validate output {SIGNED_OUTPUT_PATH} in "
+                f"nonstrict mode as well"
+            )
+            input_passes_strict = False
+
     if os.path.isfile(SIGNED_OUTPUT_PATH):
-        _validate_last_sig_in(pki_arch, SIGNED_OUTPUT_PATH)
+        _validate_last_sig_in(
+            pki_arch, SIGNED_OUTPUT_PATH, strict=input_passes_strict
+        )

pyhanko_tests/cli_tests/test_cli_signing.py

@@ -740,6 +740,50 @@ def test_cli_pades_lta(
     assert not result.exception, result.output
 
 
+def test_cli_pades_lta_nonstrict(
+    pki_arch_name, timestamp_url, cli_runner, root_cert, p12_keys
+):
+    if pki_arch_name == 'ed448':
+        # FIXME deal with this bug on the Certomancer end
+        pytest.skip("ed448 timestamping in Certomancer doesn't work")
+    cfg = {
+        'pkcs12-setups': {
+            'test': {'pfx-file': p12_keys, 'pfx-passphrase': DUMMY_PASSPHRASE}
+        },
+        'validation-contexts': {
+            'test': {
+                'trust': root_cert,
+            }
+        },
+    }
+
+    _write_config(cfg)
+    with open(INPUT_PATH, 'wb') as inf:
+        inf.write(MINIMAL_SLIGHTLY_BROKEN)
+    result = cli_runner.invoke(
+        cli_root,
+        [
+            'sign',
+            'addsig',
+            '--field',
+            'Sig1',
+            '--no-strict-syntax',
+            '--validation-context',
+            'test',
+            '--with-validation-info',
+            '--use-pades-lta',
+            '--timestamp-url',
+            timestamp_url,
+            'pkcs12',
+            '--p12-setup',
+            'test',
+            INPUT_PATH,
+            SIGNED_OUTPUT_PATH,
+        ],
+    )
+    assert not result.exception, result.output
+
+
 def test_cli_addsig_pemder_encrypted_file(
     cli_runner, cert_chain, user_key, monkeypatch
 ):

pyhanko_tests/test_pades.py

@@ -89,12 +89,15 @@
     SIMPLE_ECC_V_CONTEXT,
     SIMPLE_V_CONTEXT,
     TRUST_ROOTS,
+    async_val_trusted,
     dummy_ocsp_vc,
     live_ac_vcs,
     live_testing_vc,
     val_trusted,
 )
 
+from .samples import MINIMAL_SLIGHTLY_BROKEN
+
 
 def ts_response_callback(request, _context):
     req = tsp.TimeStampReq.load(request.body)
@@ -2380,3 +2383,37 @@ async def signed_attrs(self, *args, **kwargs):
             # validate
             ac_validation_context=(main_vc if pass_ac_vc else None),
         )
+
+
+@freeze_time('2020-11-01')
+@pytest.mark.asyncio
+async def test_interrupted_nonstrict_with_psi():
+    w = IncrementalPdfFileWriter(BytesIO(MINIMAL_SLIGHTLY_BROKEN), strict=False)
+    pdf_signer = signers.PdfSigner(
+        signers.PdfSignatureMetadata(
+            field_name='SigNew',
+            subfilter=PADES,
+            embed_validation_info=True,
+            validation_context=SIMPLE_V_CONTEXT(),
+        ),
+        signer=FROM_CA,
+        timestamper=DUMMY_TS,
+    )
+    prep_digest, tbs_document, output = (
+        await pdf_signer.async_digest_doc_for_signing(w)
+    )
+    md_algorithm = tbs_document.md_algorithm
+    assert tbs_document.post_sign_instructions is not None
+
+    await PdfTBSDocument.async_finish_signing(
+        output,
+        prep_digest,
+        await FROM_CA.async_sign(
+            prep_digest.document_digest,
+            digest_algorithm=md_algorithm,
+        ),
+        post_sign_instr=tbs_document.post_sign_instructions,
+    )
+
+    r = PdfFileReader(output, strict=False)
+    await async_val_trusted(r.embedded_signatures[0], extd=True)