Add support for verifying S/MIME messages
As promised in #11555, I'm opening this PR with an initial implementation of S/MIME verification, in order to better discuss the API design, and to start the reviews while I finish some other features.
Namely, the new pkcs7_verify functions do not handle the certificate verification feature as of now. It as similar to a openssl smime -verify with the -noverify flag, to verify the signature but not the certificates (similar to what #12116 needs). Can you point me towards some existing code verifying X.509 certificates, if some exists?
Also, I have one question about the certificate parameter in the functions: should we verify against one certificates? Multiple ones? All the ones that are stored in the signature (if any)?
My essential thoughts for testing were to do the round-trip: signature using the PKCS7SignatureBuilder and verifying using the pkcs_decrypt functions. For now, I've not replaced the test_support.pkcs7_verify function, but I'm planning to do so as soon as the certificate verification feature is developed.
I'm still new to rust, so please let me know if you see some issues in variable lifetime, or some unnecessary copying between Python & Rust.
cc @alex
The existing apis for cert verification are in cryptography.x509.verification
On Fri, Jan 10, 2025, 9:32 AM Quentin Retourne @.***> wrote:
As promised in #11555 https://github.com/pyca/cryptography/pull/11555, I'm opening this PR with an initial implementation of S/MIME verification, in order to better discuss the API design, and to start the reviews while I finish some other features.
Namely, the new pkcs7_verify functions do not handle the certificate verification feature as of now. It as similar to a openssl smime -verify with the -noverify flag, to verify the signature but not the certificates (similar to what #12116 https://github.com/pyca/cryptography/pull/12116 needs). Can you point me towards some existing code verifying X.509 certificates, if some exists?
Also, I have one question about the certificate parameter in the functions: should we verify against one certificates? Multiple ones? All the ones that are stored in the signature (if any)?
My essential thoughts for testing were to do the round-trip: signature using the PKCS7SignatureBuilder and verifying using the pkcs_decrypt functions. For now, I've not replaced the test_support.pkcs7_verify function, but I'm planning to do so as soon as the certificate verification feature is developed.
I'm still new to rust, so please let me know if you see some issues in variable lifetime, or some unnecessary copying between Python & Rust.
cc @alex https://github.com/alex
You can view, comment on, or merge this pull request online at:
https://github.com/pyca/cryptography/pull/12267 Commit Summary
- 794db04 https://github.com/pyca/cryptography/pull/12267/commits/794db04975549545c0cf3f776c2d473461a772bc PKCS7 signing (first version)
File Changes
(9 files https://github.com/pyca/cryptography/pull/12267/files)
- M CHANGELOG.rst https://github.com/pyca/cryptography/pull/12267/files#diff-2c623f3c6a917be56c59d43279244996836262cb1e12d9d0786c9c49eef6b43c (4)
- M docs/development/test-vectors.rst https://github.com/pyca/cryptography/pull/12267/files#diff-abc63e746ffc66f83cffdf369028671e53de77b100eb528f517f16f41201f3f7 (3)
- M docs/hazmat/primitives/asymmetric/serialization.rst https://github.com/pyca/cryptography/pull/12267/files#diff-7c2790036c595946b546ceec3cb5dc5125b4f6223761a6684f577e70fb49d458 (148)
- M src/cryptography/hazmat/bindings/_rust/pkcs7.pyi https://github.com/pyca/cryptography/pull/12267/files#diff-836c9b03108b81b3659a23a89d2c5fdddab94ca204686c4e71afcb7e433a7f87 (28)
- M src/cryptography/hazmat/primitives/serialization/pkcs7.py https://github.com/pyca/cryptography/pull/12267/files#diff-0075be2b09cefb4b36a4646255d3e1bea05c99b6e3ffd141a08aa528de9741c3 (34)
- M src/rust/src/pkcs7.rs https://github.com/pyca/cryptography/pull/12267/files#diff-8ca3ed0edb153cd77f373e5e5856e6f1fc559cddcc5ebf91cc690a7aed7f0a3a (167)
- M src/rust/src/types.rs https://github.com/pyca/cryptography/pull/12267/files#diff-4eb6bcd40dbc27bf39e7b1c1e694d48c28fec6579d4ab1cb0a7b3ad65f9d2228 (5)
- M tests/hazmat/primitives/test_pkcs7.py https://github.com/pyca/cryptography/pull/12267/files#diff-ee3cc87634fba3d98c17ee2dbcb575f63f33603a5ab7ac9fa7bcc3b17bced6a3 (169)
- A vectors/cryptography_vectors/pkcs7/signed-opaque.msg https://github.com/pyca/cryptography/pull/12267/files#diff-566d9755c6ec5055a0a4ca69a7a0b94f91d23d42491be4d6a591e88fad57f0f4 (21)
Patch Links:
- https://github.com/pyca/cryptography/pull/12267.patch
- https://github.com/pyca/cryptography/pull/12267.diff
— Reply to this email directly, view it on GitHub https://github.com/pyca/cryptography/pull/12267, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAAGBF2ULFD3GXMBFEJ3432J7KZJAVCNFSM6AAAAABU6QAS7KVHI2DSMVQWIX3LMV43ASLTON2WKOZSG44DAMZSG42DQNY . You are receiving this because you were mentioned.Message ID: @.***>
@alex, for now I'm trying the following code:
def _verify_pkcs7_certificates(certificates: list[x509.Certificate]) -> None:
builder = PolicyBuilder().store(Store(certificates))
verifier = builder.build_client_verifier()
verifier.verify(certificates[0], certificates[1:])
However, I'm getting the following error:
E cryptography.hazmat.bindings._rust.x509.VerificationError: validation failed: basicConstraints.cA must not be asserted in an EE certificate (encountered processing <Certificate(subject=<Name(C=US,CN=cryptography CA)>, ...)>)
To be honest, I'm not clear as to what we are supposed to do in certificate verification. I've not found anything specific in the RFC, but I'll keep looking. If you can guide me through this, I'd love some help 🛩️
Basically certificates used as end entities (i.e., the cert used to sign a PKCS#7/SMIME message) should not hvae ca=true in their basic constraints extension. I can't remember which spec that comes from, but its a general best practice in any event /cc @woodruffw
Yep, that comes from CABF -- we have a test and cite for it here: https://x509-limbo.com/testcases/webpki/#webpkica-as-leaf.
(RFC 5280 doesn't impose the requirement that the EE not be a CA, but firmly agreed about it being best practice.)
Basically certificates used as end entities (i.e., the cert used to sign a PKCS#7/SMIME message) should not hvae ca=true in their basic constraints extension. I can't remember which spec that comes from, but its a general best practice in any event /cc @woodruffw
OK, that raises two things for me:
- That means we will have to create a specific certificate for verifying (and signing ?) PKCS#7 files, either statically or dynamically. I'll look into it.
- This raises the question of
test_support.pkcs7_verifyfunction. It does not check this, as we can see in test_support.rs:93, by just passing an empty stack of certificates.
let certs = openssl::stack::Stack::new()?;
p7.verify(
&certs,
&store,
msg.as_ref().map(|m| m.as_bytes()),
None,
flags,
)?;
@alex this should be ready for review, the tests are passing. A few considerations to keep in mind:
- What should the priority be for user-input parameters vs. what's found in the PKCS7 structure (content, certificate)?
- Handling PKCS#7 message with multiple signers: trying to verify the first signer? All of them?
- Missing the documentation for the new pkcs#7 test vectors; still WIP.
Keep in mind that for now, I've used a certificate chain from endesive to make the tests pass, but I'll work on creating new cryptography certificates in the following days.
Some more things about the certs:
- the certificate used for testing PKCS#7 signing cannot be verified properly (due to some EE error above)
- Verifying the signed messages, when using
openssl, only works when some other certificates are passed in the-CAfileparameter. Otherwise we are getting aunable to get local issuer certificateerror. This is not necessary (for some reason that I do not understand) in the new functions.
CC @reaperhulk @facutuesca if you miraculously have time for a review 😄
Stumbled on #12104 and realized we might have the same issue with the certificate verification code!
Sorry I've been slow here, it's on my TODO list for this week.
On Mon, Jan 20, 2025 at 10:13 AM Quentin Retourne @.***> wrote:
@nitneuqr https://github.com/nitneuqr requested your review on: #12267 https://github.com/pyca/cryptography/pull/12267 Add support for verifying S/MIME messages.
— Reply to this email directly, view it on GitHub https://github.com/pyca/cryptography/pull/12267#event-16000417253, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAAGBBDZHEQAET2H44MH7L2LUHAXAVCNFSM6AAAAABU6QAS7KVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJWGAYDANBRG4ZDKMY . You are receiving this because your review was requested.Message ID: @.***>
-- All that is necessary for evil to succeed is for good people to do nothing.
No worries! Just checking in in case you forgot about it 😊
Stumbled on #12104 and realized we might have the same issue with the certificate verification code!
Not exactly sure, but first huge thanks for the work: After reading your PR, your Usecase seems to be "I want to verify a received SMIME-Message" (along what I paraphased in #12104 as verify_message-method of a Verifier to be obtained using build_smime_verifier). My usecase is more "I want to verify a Certificate to use it for sending a SMIME-Encrypted message" (described as verify_certificate).
However, my Usecase should be a subtask of your Usecase: To verify a received SMIME-Message you probably need to verify the used Certificate. Looks like you are doing this in _verify_pkcs7_certificate, which is using build_client_verifier - however from my knowledge, this requires EKU_CLIENT_AUTH_OID to be set, but SMIME-Certificates can be fine without client auth (and require EKU_EMAIL_PROTECTION_OID instead.
So in my (really basic) knowledge, your current code would give invalid validation results for Certificates which are only used for SMIME-Signing (and not client authentication). However, once you include a verifier for it, please make it publicly accessible to use it without a signed Message :)
Looking at your questions:
What should the priority be for user-input parameters vs. what's found in the PKCS7 structure (content, certificate)?
What if we start with the most restrictive implementation: it is an error for both to be present?
Handling PKCS#7 message with multiple signers: trying to verify the first signer? All of them?
I think for an MVP we should start with an "any" policy: if any of the signers matches what we want, then it's ok. (In the future maybe there's a use case for an "all" policy? I'm not sure)
Waiting for #12360 before going any further on this :)
Hello, I opened #12406 wondering about this implementation.
Would a PKCS7 verification be compatible with CMS? Such as the CMS_verify used in openssl-cms ?
According to OpenSSL they are very similar, but I couldn't pinpoint the edge cases
FYI, https://github.com/pyca/cryptography/pull/12360 is merged now.
FYI, #12360 is merged now.
Will try to work on this the following weeks. I just read https://github.com/nitneuqr/cryptography/blob/pkcs7-verify/docs/x509/verification.rst#L284; as far as I understand, it seems we need to create a dedicated S/MIME extension policy and verify based on this policy?
Do you know where you can find information on the required information for S/MIME certificates?
Update: just re-read all the comments and it seems at least part of the info is there 😄
As far as I understand it now, it seems that we'll need first to create a S/MIME extension policy (probably in another PR), test it with some certificates, and use the same certificates to test S/MIME verification. Does that make sense @alex?
Update: opened #12465 for that matter.
That sounds like a good plan.
On Sat, Feb 15, 2025 at 5:10 AM Quentin Retourne @.***> wrote:
As far as I understand it now, it seems that we'll need first to create a S/MIME extension policy (probably in another PR), test it with some certificates, and use the same certificates to test S/MIME verification. Does that make sense @alex https://github.com/alex?
— Reply to this email directly, view it on GitHub https://github.com/pyca/cryptography/pull/12267#issuecomment-2660873742, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAAGBH4KMGYVMIYZAS5U332P4OAVAVCNFSM6AAAAABU6QAS7KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMNRQHA3TGNZUGI . You are receiving this because you were mentioned.Message ID: @.***> [image: nitneuqr]nitneuqr left a comment (pyca/cryptography#12267) https://github.com/pyca/cryptography/pull/12267#issuecomment-2660873742
As far as I understand it now, it seems that we'll need first to create a S/MIME extension policy (probably in another PR), test it with some certificates, and use the same certificates to test S/MIME verification. Does that make sense @alex https://github.com/alex?
— Reply to this email directly, view it on GitHub https://github.com/pyca/cryptography/pull/12267#issuecomment-2660873742, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAAGBH4KMGYVMIYZAS5U332P4OAVAVCNFSM6AAAAABU6QAS7KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMNRQHA3TGNZUGI . You are receiving this because you were mentioned.Message ID: @.***>
-- All that is necessary for evil to succeed is for good people to do nothing.
@alex this should be ready again for review as soon as #12465 is integrated :)