diff --git a/pyhanko/sign/signers/pdf_signer.py b/pyhanko/sign/signers/pdf_signer.py index cb81750e..2ef19c64 100644 --- a/pyhanko/sign/signers/pdf_signer.py +++ b/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 diff --git a/pyhanko/sign/validation/dss.py b/pyhanko/sign/validation/dss.py index d33eae1a..b566bed8 100644 --- a/pyhanko/sign/validation/dss.py +++ b/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( diff --git a/pyhanko_tests/cli_tests/conftest.py b/pyhanko_tests/cli_tests/conftest.py index 40d78891..6ec39154 100644 --- a/pyhanko_tests/cli_tests/conftest.py +++ b/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 + ) diff --git a/pyhanko_tests/cli_tests/test_cli_signing.py b/pyhanko_tests/cli_tests/test_cli_signing.py index 8299c212..3de02468 100644 --- a/pyhanko_tests/cli_tests/test_cli_signing.py +++ b/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 ): diff --git a/pyhanko_tests/test_pades.py b/pyhanko_tests/test_pades.py index adf570cd..66bcc134 100644 --- a/pyhanko_tests/test_pades.py +++ b/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)