cryptography icon indicating copy to clipboard operation
cryptography copied to clipboard

Fix ASN.1 issues in PKCS#7 and S/MIME signing

Open facutuesca opened this issue 1 year ago • 1 comments

This PR fixes two issues with the ASN.1 generated when signing using PKCS#7:

First issue

The current implementation defines the SMIMECapabilities attribute so that its value is a SEQUENCE of all the algorithm OIDs that are supported. This is what the ASN.1 currently looks like for a signed message using cryptography:

  U.P.SEQUENCE {
     U.P.OBJECTIDENTIFIER 1.2.840.113549.1.9.15 (S/MIME capabilities)
     U.P.SET {
        U.P.SEQUENCE {
           U.P.OBJECTIDENTIFIER 2.16.840.1.101.3.4.1.42 (aes256-CBC-PAD)
           U.P.OBJECTIDENTIFIER 2.16.840.1.101.3.4.1.22 (aes192-CBC-PAD)
           U.P.OBJECTIDENTIFIER 2.16.840.1.101.3.4.1.2 (aes128-CBC-PAD)
        }
     }
  }

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)

The spec matches what OpenSSL outputs (using openssl smime -sign):

  U.P.SEQUENCE {
     U.P.OBJECTIDENTIFIER 1.2.840.113549.1.9.15 (S/MIME capabilities)
     U.P.SET {
        U.P.SEQUENCE {
           U.P.SEQUENCE {
              U.P.OBJECTIDENTIFIER 2.16.840.1.101.3.4.1.42 (aes256-CBC-PAD)
           }
           U.P.SEQUENCE {
              U.P.OBJECTIDENTIFIER 2.16.840.1.101.3.4.1.22 (aes192-CBC-PAD)
           }
           U.P.SEQUENCE {
              U.P.OBJECTIDENTIFIER 2.16.840.1.101.3.4.1.2 (aes128-CBC-PAD)
           }
           U.P.SEQUENCE {
              U.P.OBJECTIDENTIFIER 1.2.840.113549.3.7 (id_des_EDE3_CBC)
           }
          .....

This PR changes the implementation so that each algorithm is inside its own SEQUENCE. After the changes in this PR, the ASN.1 output looks like:

  U.P.SEQUENCE {
     U.P.OBJECTIDENTIFIER 1.2.840.113549.1.9.15 (S/MIME capabilities)
     U.P.SET {
        U.P.SEQUENCE {
           U.P.SEQUENCE {
              U.P.OBJECTIDENTIFIER 2.16.840.1.101.3.4.1.42 (aes256-CBC-PAD)
           }
           U.P.SEQUENCE {
              U.P.OBJECTIDENTIFIER 2.16.840.1.101.3.4.1.22 (aes192-CBC-PAD)
           }
           U.P.SEQUENCE {
              U.P.OBJECTIDENTIFIER 2.16.840.1.101.3.4.1.2 (aes128-CBC-PAD)
           }
        }
     }
  }

Second issue

There is an issue with the RSA OID used for signing PKCS#7/SMIME, in the SignatureAlgorithmIdentifier field.

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.

facutuesca avatar Feb 09 '24 17:02 facutuesca

Haven't re-reviewed the RFC, but we should add a test for this.

alex avatar Feb 09 '24 17:02 alex

@alex I also added an item to the CHANGELOG

facutuesca avatar Feb 20 '24 06:02 facutuesca