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

Fix ASN.1 issues in PKCS#7 and S/MIME signing (#10373) #10442

Merged
merged 1 commit into from
Feb 21, 2024

Conversation

alex
Copy link
Member

@alex alex commented Feb 20, 2024

  • Fix ASN.1 for S/MIME capabilities.

The current implementation defines the SMIMECapabilities attribute so that its value is a SEQUENCE of all the algorithm OIDs that are supported.
However, the S/MIME v3 spec (RFC 2633) specifies that each algorithm should be specified in its own SEQUENCE:

SMIMECapabilities ::= SEQUENCE OF SMIMECapability

SMIMECapability ::= SEQUENCE {
capabilityID OBJECT IDENTIFIER,
parameters ANY DEFINED BY capabilityID OPTIONAL }

(RFC 2633, Appendix A)

This commit changes the implementation so that each algorithm is inside its own SEQUENCE. This also matches the OpenSSL implementation.

  • Fix the RSA OID used for signing PKCS#7/SMIME

The current implementation computes the algorithm identifier used in the digest_encryption_algorithm PKCS#7 field
(or SignatureAlgorithmIdentifier in S/MIME) based on both the algorithm used to sign (e.g. RSA) and the digest algorithm (e.g. SHA512).

This is correct for ECDSA signatures, where the OIDs used include the digest algorithm (e.g: ecdsa-with-SHA512). However, due to historical reasons, when signing with RSA the OID specified should be the one corresponding to just RSA ("1.2.840.113549.1.1.1" rsaEncryption), rather than OIDs which also include the digest algorithm (such as "1.2.840.113549.1.1.13", sha512WithRSAEncryption).

This means that the logic to compute the algorithm identifier is the same except when signing with RSA, in which case the OID will always be rsaEncryption. This is consistent with the OpenSSL implementation, and the RFCs that define PKCS#7 and S/MIME.

See RFC 3851 (section 2.2), and RFC 3370 (section 3.2) for more details.

  • Add tests for the changes in PKCS7 signing

  • PKCS7 fixes from code review

  • Update CHANGELOG

* Fix ASN.1 for S/MIME capabilities.

The current implementation defines the SMIMECapabilities attribute
so that its value is a SEQUENCE of all the algorithm OIDs that are
supported.
However, the S/MIME v3 spec (RFC 2633) specifies that each algorithm
should be specified in its own SEQUENCE:

SMIMECapabilities ::= SEQUENCE OF SMIMECapability

SMIMECapability ::= SEQUENCE {
   capabilityID OBJECT IDENTIFIER,
   parameters ANY DEFINED BY capabilityID OPTIONAL }

(RFC 2633, Appendix A)

This commit changes the implementation so that each algorithm
is inside its own SEQUENCE. This also matches the OpenSSL
implementation.

* Fix the RSA OID used for signing PKCS#7/SMIME

The current implementation computes the algorithm identifier used
in the `digest_encryption_algorithm` PKCS#7 field
(or `SignatureAlgorithmIdentifier` in S/MIME) based on both the
algorithm used to sign (e.g. RSA) and the digest algorithm (e.g. SHA512).

This is correct for ECDSA signatures, where the OIDs used include the
digest algorithm (e.g: ecdsa-with-SHA512). However, due to historical
reasons, when signing with RSA the OID specified should be the one
corresponding to just RSA ("1.2.840.113549.1.1.1" rsaEncryption),
rather than OIDs which also include the digest algorithm (such as
"1.2.840.113549.1.1.13", sha512WithRSAEncryption).

This means that the logic to compute the algorithm identifier is the
same except when signing with RSA, in which case the OID will always
be `rsaEncryption`. This is consistent with the OpenSSL implementation,
and the RFCs that define PKCS#7 and S/MIME.

See RFC 3851 (section 2.2), and RFC 3370 (section 3.2) for more details.

* Add tests for the changes in PKCS7 signing

* PKCS7 fixes from code review

* Update CHANGELOG
@alex alex marked this pull request as ready for review February 20, 2024 22:19
@reaperhulk reaperhulk merged commit aaa2dd0 into pyca:42.0.x Feb 21, 2024
58 checks passed
@alex alex deleted the backport-smime-oid branch February 21, 2024 01:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants