Bugfix: Remove assert
from Certificate.from_p12
#274
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PKCS#12 containers can be valid even if they do not contain any certificate entries in them. Furthermore,
cryptography
's built-inpkcs12.load_key_and_certificates
, which is used byCertificate.from_p12
, has a return typeOptional[x509.Certificate]
for the returned certificate.The factory method however expected that the given PKCS#12 container always contains the certificate, which is a false assumption, and did a mere
assert x509_certificate is not None
. This can lead to unexpected results:AssertionError
is thrown in case assertions are enabled.To overcome this, check that the certificate acquired from the container is usable, and raise a descriptive
ValueError
otherwise.