MatthiasValvekens / pyHanko

pyHanko: sign and stamp PDF files
MIT License
483 stars 71 forks source link

[pyhanko-certvalidator] PEM certificate not getting extracted due to incorrect Content-Type header #319

Closed sagarvora closed 11 months ago

sagarvora commented 11 months ago

Issue

Cert URL: https://www.certificate.digital/repository/CapricornCA2022.cer

The content of this certificate is of type PEM, but the Content-Type header in CA's response is application/x-x509-ca-cert. This leads to the following traceback:

Traceback (most recent call last):
  File "pyhanko_certvalidator/fetchers/requests_fetchers/cert_fetch_client.py", line 53, in task
    results = await self._grab_certs(
  File "pyhanko_certvalidator/fetchers/requests_fetchers/cert_fetch_client.py", line 131, in _grab_certs
    return list(certs)
  File "pyhanko_certvalidator/fetchers/common_utils.py", line 120, in unpack_cert_content
    raise ValueError(
ValueError: Failed to extract certs from application/x-x509-ca-cert payload. Source URL: http://www.certificate.digital/repository/CapricornCA2022.cer.

Proposed Solution

In case is_pem is True (based on asn1crypto.pem.detect), we could ignore the Content-Type entirely. This should be fine since PEM is the last thing we accept anyway (DER gets handled in earlier code).

If this sounds acceptable, I can raise a pull request for the same.

MatthiasValvekens commented 11 months ago

Are you using the latest version? I made some tweaks to make this check more lenient in the last release, but I don't know offhand if this would've covered it.

I'm OK with your proposed solution, by the way. Clearly, there are way too many CAs out there that just do whatever and disregard the applicable standards... ;) Actually, the is_pem fallback already implies that we're in out-of-spec territory anyway---you're supposed to serve either an X.509 cert or a cert-only PKCS#7 bundle in DER---so ignoring the content type in those cases is probably acceptable.

Thanks!

sagarvora commented 11 months ago

Yes, I'm using the latest version.

Looking at the comments in the code, I immediately realised it might be against the spec to serve PEM certificates for validation. Nonetheless, I'll work on that Pull Request.

Thanks for your time & this fantastic project!

(I am working on a web-based FOSS solution for signing documents on the server-side using client tokens)

MatthiasValvekens commented 11 months ago

Thanks! I merged your changes, will cut a new release ASAP.