SignedCms add test and reduce allocations

Author: PranavSenthilnathan

src/libraries/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/SignerInfo.cs

@@ -493,7 +493,10 @@ private bool CheckHash(bool compatMode)
             Debug.Assert(_signatureAlgorithm == Oids.NoSignature);
 
             // The signature is a hash of the message or signed attributes.
-            return VerifyHashedMessage(compatMode, contentToVerify => _signature.Span.SequenceEqual(contentToVerify));
+            return VerifyHashedMessage(
+                compatMode,
+                _signature,
+                static (signature, contentToVerify) => signature.Span.SequenceEqual(contentToVerify));
         }
 
         private X509Certificate2? FindSignerCertificate()
@@ -604,12 +607,13 @@ private CmsHash GetContentHash(ReadOnlyMemory<byte> content, ReadOnlyMemory<byte
             return hasher;
         }
 
-        private static bool VerifyAttributes(
+        private static bool VerifyAttributes<TState>(
             ReadOnlySpan<byte> digest,
             AttributeAsn[] signedAttributes,
             bool compatMode,
             bool needsContentAttr,
-            VerifyCallback verify)
+            TState state,
+            VerifyCallback<TState> verify)
         {
             bool hasMatchingDigestAttr = false;
             bool hasContentAttr = false;
@@ -663,23 +667,40 @@ private static bool VerifyAttributes(
 
             if (compatMode)
             {
-                byte[] encoded = writer.Encode();
-                encoded[0] = 0x31;
-                return verify(encoded);
+                int encodedLength = writer.GetEncodedLength();
+                byte[]? rented = null;
+                Span<byte> encoded = encodedLength <= 256
+                    ? stackalloc byte[256]
+                    : (rented = CryptoPool.Rent(encodedLength));
+
+                try
+                {
+                    encoded = encoded.Slice(0, encodedLength);
+                    writer.Encode(encoded);
+                    encoded[0] = 0x31;
+                    return verify(state, encoded);
+                }
+                finally
+                {
+                    if (rented != null)
+                    {
+                        CryptoPool.Return(rented);
+                    }
+                }
             }
             else
             {
 #if NET9_0_OR_GREATER
-                return writer.Encode(verify, static (verify, encoded) => verify(encoded));
+                return writer.Encode((state, verify), static (state, encoded) => state.verify(state.state, encoded));
 #else
-                return verify(writer.Encode());
+                return verify(state, writer.Encode());
 #endif
             }
         }
 
-        private delegate bool VerifyCallback(ReadOnlySpan<byte> contentToVerify);
+        private delegate bool VerifyCallback<TState>(TState state, ReadOnlySpan<byte> contentToVerify);
 
-        private bool VerifyHashedMessage(bool compatMode, VerifyCallback verify)
+        private bool VerifyHashedMessage<TState>(bool compatMode, TState state, VerifyCallback<TState> verify)
         {
             ReadOnlyMemory<byte> content = GetContentForVerification(out ReadOnlyMemory<byte>? additionalContent);
 
@@ -710,7 +731,7 @@ private bool VerifyHashedMessage(bool compatMode, VerifyCallback verify)
                         throw new CryptographicException(SR.Cryptography_Cms_MissingAuthenticatedAttribute);
                     }
 
-                    return verify(contentHash);
+                    return verify(state, contentHash);
                 }
 
                 // Since there are signed attributes, we need to verify those instead.
@@ -720,8 +741,10 @@ private bool VerifyHashedMessage(bool compatMode, VerifyCallback verify)
                         _signedAttributes,
                         compatMode,
                         needsContentAttr: false,
-                        span =>
+                        (state, hasher, verify),
+                        static (state, span) =>
                         {
+                            CmsHash hasher = state.hasher;
                             hasher.AppendData(span);
 
 #if NET || NETSTANDARD2_1
@@ -740,12 +763,12 @@ private bool VerifyHashedMessage(bool compatMode, VerifyCallback verify)
                             byte[] attrHash = hasher.GetHashAndReset();
 #endif
 
-                            return verify(attrHash);
+                            return state.verify(state.state, attrHash);
                         });
             }
         }
 
-        private bool VerifyPureMessage(bool compatMode, VerifyCallback verify)
+        private bool VerifyPureMessage<TState>(bool compatMode, TState state, VerifyCallback<TState> verify)
         {
             ReadOnlyMemory<byte> content = GetContentForVerification(out ReadOnlyMemory<byte>? additionalContent);
 
@@ -760,7 +783,7 @@ private bool VerifyPureMessage(bool compatMode, VerifyCallback verify)
 
                 if (!additionalContent.HasValue)
                 {
-                    return verify(content.Span);
+                    return verify(state, content.Span);
                 }
 
                 // If there are multiple pieces of content, concatenate them and verify.
@@ -770,7 +793,7 @@ private bool VerifyPureMessage(bool compatMode, VerifyCallback verify)
                 {
                     additionalContent.Value.Span.CopyTo(rented);
                     content.Span.CopyTo(rented.AsSpan(additionalContent.Value.Length));
-                    return verify(rented.AsSpan(0, contentToVerifyLength));
+                    return verify(state, rented.AsSpan(0, contentToVerifyLength));
                 }
                 finally
                 {
@@ -806,7 +829,8 @@ private bool VerifyPureMessage(bool compatMode, VerifyCallback verify)
                         // it is invalid for countersigners. We'll just ignore it for now, but if we decide to
                         // allow countersigners to omit the content type, we should check that `this` is a countersigner.
                         needsContentAttr: false,
-                        verify);
+                        (state, verify),
+                        static (state, span) => state.verify(state.state, span));
             }
         }
 
@@ -896,19 +920,42 @@ private bool VerifySignature(
                 return false;
             }
 
-            Func<bool, VerifyCallback, bool> verifier = signatureProcessor.NeedsHashedMessage ? VerifyHashedMessage : VerifyPureMessage;
-            return verifier(compatMode, contentToVerify =>
-                signatureProcessor.VerifySignature(
+            if (signatureProcessor.NeedsHashedMessage)
+            {
+                return VerifyHashedMessage(
+                    compatMode,
+                    (@this: this, signatureProcessor, certificate),
+                    static (state, contentToVerify) =>
+                        state.signatureProcessor.VerifySignature(
+#if NET || NETSTANDARD2_1
+                            contentToVerify,
+                            state.@this._signature,
+#else
+                            contentToVerify.ToArray(),
+                            state.@this._signature.ToArray(),
+#endif
+                            state.@this.DigestAlgorithm.Value,
+                            state.@this._signatureAlgorithmParameters,
+                            state.certificate));
+            }
+            else
+            {
+                return VerifyPureMessage(
+                    compatMode,
+                    (@this: this, signatureProcessor, certificate),
+                    static (state, contentToVerify) =>
+                        state.signatureProcessor.VerifySignature(
 #if NET || NETSTANDARD2_1
-                    contentToVerify,
-                    _signature,
+                            contentToVerify,
+                            state.@this._signature,
 #else
-                    contentToVerify.ToArray(),
-                    _signature.ToArray(),
+                            contentToVerify.ToArray(),
+                            state.@this._signature.ToArray(),
 #endif
-                    DigestAlgorithm.Value,
-                    _signatureAlgorithmParameters,
-                    certificate));
+                            state.@this.DigestAlgorithm.Value,
+                            state.@this._signatureAlgorithmParameters,
+                            state.certificate));
+            }
         }
 
         private static int FindAttributeIndexByOid(AttributeAsn[] attributes, Oid oid, int startIndex = 0)

src/libraries/System.Security.Cryptography.Pkcs/tests/SignedCms/SignedCmsTests.cs

@@ -1702,6 +1702,69 @@ public static void ComputeSignature_SlhDsa_NoSignature()
             }
         }
 
+        // Ed25519 certificate from https://datatracker.ietf.org/doc/html/rfc8410#section-10.2
+        private const string UnknownAlgorithmCert =
+            """
+            MIIBLDCB36ADAgECAghWAUdKKo3DMDAFBgMrZXAwGTEXMBUGA1UEAwwOSUVURiBUZX
+            N0IERlbW8wHhcNMTYwODAxMTIxOTI0WhcNNDAxMjMxMjM1OTU5WjAZMRcwFQYDVQQD
+            DA5JRVRGIFRlc3QgRGVtbzAqMAUGAytlbgMhAIUg8AmJMKdUdIt93LQ+91oNvzoNJj
+            ga9OukqY6qm05qo0UwQzAPBgNVHRMBAf8EBTADAQEAMA4GA1UdDwEBAAQEAwIDCDAg
+            BgNVHQ4BAQAEFgQUmx9e7e0EM4Xk97xiPFl1uQvIuzswBQYDK2VwA0EAryMB/t3J5v
+            /BzKc9dNZIpDmAgs3babFOTQbs+BolzlDUwsPrdGxO3YNGhW7Ibz3OGhhlxXrCe1Cg
+            w1AH9efZBw==
+            """;
+
+        [Fact]
+        public static void ComputeSignature_UnknownAlgorithm_NoSignature()
+        {
+            using X509Certificate2 cert = X509CertificateLoader.LoadCertificate(Convert.FromBase64String(UnknownAlgorithmCert));
+
+            byte[] message = "Hello World!"u8.ToArray();
+            SignedCms cms = new SignedCms(new ContentInfo(message));
+            CmsSigner cmsSigner = new CmsSigner(SubjectIdentifierType.NoSignature, cert)
+            {
+                DigestAlgorithm = new Oid(Oids.Sha256, Oids.Sha256)
+            };
+
+            cms.ComputeSignature(cmsSigner);
+
+            // "NoSignature" OID
+            Assert.Equal("1.3.6.1.5.5.7.6.2", cms.SignerInfos[0].SignatureAlgorithm.Value);
+
+            byte[] messageHash = Convert.FromBase64String("f4OxZX/x/FO5LcGBSKHWXfwtSx+j1ncoSt3SABJtkGk=");
+            byte[] signature = cms.SignerInfos[0].GetSignature();
+
+            Assert.Equal(messageHash, signature);
+
+            SignedCms cmsNoCert = new SignedCms(new ContentInfo(message));
+            cmsNoCert.Decode(cms.Encode());
+            cmsNoCert.SignerInfos[0].CheckHash();
+        }
+
+        [Fact]
+        public static void ComputeSignature_NoSignature_DefaultDigest()
+        {
+            // A certificate shouldn't really be required here, but on .NET Framework
+            // it will encounter throw a NullReferenceException.
+            using X509Certificate2 cert = Certificates.RSAKeyTransferCapi1.GetCertificate();
+
+            byte[] message = "Hello World!"u8.ToArray();
+            SignedCms cms = new SignedCms(new ContentInfo(message));
+
+            // Use default value for DigestAlgorithm
+            CmsSigner signer = new CmsSigner(SubjectIdentifierType.NoSignature, cert);
+
+            cms.ComputeSignature(signer);
+
+            byte[] defaultDigestSignature = cms.SignerInfos[0].GetSignature();
+            byte[] messageHash = Convert.FromBase64String(
+                PlatformDetection.IsNetFramework
+                    ? "Lve95gjOVATpfV8EL5X4nxwjKHE="                    // Sha1
+                    : "f4OxZX/x/FO5LcGBSKHWXfwtSx+j1ncoSt3SABJtkGk=");  // Sha256
+
+            Assert.Equal(messageHash, defaultDigestSignature);
+        }
+
         private static void CheckNoSignature(byte[] encoded, bool badOid=false)
         {
             SignedCms cms = new SignedCms();