Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/strict flag post signing #487

Merged
merged 2 commits into from
Nov 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion pyhanko/sign/signers/pdf_signer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion pyhanko/sign/validation/dss.py
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down Expand Up @@ -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(
Expand Down
22 changes: 19 additions & 3 deletions pyhanko_tests/cli_tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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")
Expand All @@ -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
)
44 changes: 44 additions & 0 deletions pyhanko_tests/cli_tests/test_cli_signing.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
):
Expand Down
37 changes: 37 additions & 0 deletions pyhanko_tests/test_pades.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Loading