From 88ced31d9decc1d190ff32b42c775f7ca8ab294e Mon Sep 17 00:00:00 2001
From: Nicolas Bachschmidt <nicolas@nunk.net>
Date: Thu, 16 Jan 2025 17:41:23 +0100
Subject: [PATCH] Use proper hash when computing message digest in signed
 attributes (#219)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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 <lukasa@apple.com>
---
 .../CMSOperations.swift                       | 19 ++++---------------
 Sources/X509/Digests.swift                    | 16 ++++++++++++++++
 Tests/X509Tests/CMSTests.swift                | 19 +++++++++++++++++++
 3 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/Sources/X509/CryptographicMessageSyntax/CMSOperations.swift b/Sources/X509/CryptographicMessageSyntax/CMSOperations.swift
index be2315c..34af492 100644
--- 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
index 9c84064..40fea28 100644
--- 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
index 702f121..a0c8f31 100644
--- 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(