-
Notifications
You must be signed in to change notification settings - Fork 1.6k
add test vectors for pkcs#7 verification policies #13376
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
Conversation
docs/development/test-vectors.rst
Outdated
@@ -1002,6 +1002,12 @@ Custom PKCS7 Test Vectors | |||
* ``pkcs7/enveloped-no-content.der``- A DER encoded PKCS7 file with | |||
enveloped data, without encrypted content, with key encrypted under the | |||
public key of ``x509/custom/ca/rsa_ca.pem``. | |||
* ``pkcs7/ca.pem`` - A certificate adapted for S/MIME signature & verification. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These shouldn't be named "ca" -- that'd be used for certificate authorities that are used to issue other cers, not for stand-alone certs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What should they be named?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After reading https://github.com/pyca/cryptography/pull/12465/files#diff-ee3cc87634fba3d98c17ee2dbcb575f63f33603a5ab7ac9fa7bcc3b17bced6a3R154 i recon only the description is missleading: The ca.pem
is used to issue certificates used during testing. Maybe the description should be replaced with A certificate adapted to sign certificates used for S/MIME signature & verification
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, that's a bit surprising to me since it has a Subject Alt Names extension which generally implies its an end-entity certificate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the code it is only used as a CA, so maybe the SAN is there by accident? @nitneuqr can you help out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I might have a better understanding now: While ca.pem
actually is a CA-certificate, the test code will simply copy the SAN and AKI-Extensions from the CA-Cert to the (temporary generated) EE-certificate. Otherwise, it basically is x509/custom/ca/ca.pem
(even sharing ca_key.pem
). The basic question is: What should it be like? I think ca.pem
could be replaced if the test code is changed to add the SAN and AKI-extensions in code instead of reading them from file. Would it be worth to propose such a change? Or should we change documentation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, it was quite simple to change the test and avoid shipping a custom ca.pem
and ca_key.pem
. Will push the change here and submit a patch to #12465
Thanks to @briantist in #12465 I separated the vectors needed to test PKCS#7 verification. The files were created by @nitneuqr over in #12465