Use proper hash when computing message digest in signed attributes (#219

) Per [RFC 5652 § 11.2](https://datatracker.ietf.org/doc/html/rfc5652#section-11.2): "For signed-data, the message digest is computed using the signer's message digest algorithm." Currently, when computing the signed attributes' message digest, `CMS.sign` assumes the digest algorithm is SHA256, which is incorrect. The behavior of `CMS.isValid` is already correct and doesn't need updating. I've made `Digest` conform to `Sequence` to facilitate the conversion/comparison of the message digest. --------- Co-authored-by: Cory Benfield <[email protected]>
apple · Jan 16, 2025 · 88ced31 · 88ced31
1 parent 8210859
commit 88ced31
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 15 deletions.
diff --git a/Sources/X509/CryptographicMessageSyntax/CMSOperations.swift b/Sources/X509/CryptographicMessageSyntax/CMSOperations.swift
@@ -71,8 +71,9 @@ public enum CMS {
         let contentTypeAttribute = CMSAttribute(attrType: .contentType, attrValues: [contentTypeVal])
         signedAttrs.append(contentTypeAttribute)
 
-        // add message-digest sha256 of provided content bytes
-        let computedDigest = SHA256.hash(data: bytes)
+        // add message-digest of provided content bytes
+        let digestAlgorithm = try AlgorithmIdentifier(digestAlgorithmFor: signatureAlgorithm)
+        let computedDigest = try Digest.computeDigest(for: bytes, using: digestAlgorithm)
         let messageDigest = ASN1OctetString(contentBytes: ArraySlice(computedDigest))
         let messageDigestVal = try ASN1Any(erasing: messageDigest)
         let messageDigestAttr = CMSAttribute(attrType: .messageDigest, attrValues: [messageDigestVal])
@@ -358,19 +359,7 @@ public enum CMS {
                 let digestAlgorithm = try AlgorithmIdentifier(digestAlgorithmFor: signatureAlgorithm)
                 let actualDigest = try Digest.computeDigest(for: dataBytes, using: digestAlgorithm)
 
-                let actualDigestSequence: any Sequence<UInt8>
-                switch actualDigest {
-                case .insecureSHA1(let sha1):
-                    actualDigestSequence = sha1
-                case .sha256(let sha256):
-                    actualDigestSequence = sha256
-                case .sha384(let sha384):
-                    actualDigestSequence = sha384
-                case .sha512(let sha512):
-                    actualDigestSequence = sha512
-                }
-
-                guard actualDigestSequence.elementsEqual(messageDigest) else {
+                guard actualDigest.elementsEqual(messageDigest) else {
                     return .failure(.init(invalidCMSBlockReason: "Message digest mismatch"))
                 }
 

diff --git a/Sources/X509/Digests.swift b/Sources/X509/Digests.swift
@@ -43,6 +43,22 @@ enum Digest {
     }
 }
 
+extension Digest: Sequence {
+    @usableFromInline
+    func makeIterator() -> some IteratorProtocol<UInt8> {
+        switch self {
+        case .insecureSHA1(let sha1):
+            return sha1.makeIterator()
+        case .sha256(let sha256):
+            return sha256.makeIterator()
+        case .sha384(let sha384):
+            return sha384.makeIterator()
+        case .sha512(let sha512):
+            return sha512.makeIterator()
+        }
+    }
+}
+
 // MARK: Public key operations
 
 extension P256.Signing.PublicKey {

diff --git a/Tests/X509Tests/CMSTests.swift b/Tests/X509Tests/CMSTests.swift
@@ -1052,6 +1052,25 @@ final class CMSTests: XCTestCase {
         XCTAssertValidSignature(isValidSignature)
     }
 
+    func testSigningWithSigningTimeSignedAttrAndSHA512() async throws {
+        let data: [UInt8] = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]
+        let signature = try CMS.sign(
+            data,
+            signatureAlgorithm: .ecdsaWithSHA512,
+            certificate: Self.leaf1Cert,
+            privateKey: Self.leaf1Key,
+            signingTime: Date()
+        )
+        let isValidSignature = await CMS.isValidSignature(
+            dataBytes: data,
+            signatureBytes: signature,
+            trustRoots: CertificateStore([Self.rootCert])
+        ) {
+            Self.defaultPolicies
+        }
+        XCTAssertValidSignature(isValidSignature)
+    }
+
     func testSigningAttachedWithSigningTimeSignedAttr() async throws {
         let data: [UInt8] = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]
         let signature = try CMS.sign(