System.Security.Cryptography.Pkcs behaviour fixes (#76)

Downstream https://github.com/dotnet/corefx/pull/30046

* Return same error code for invalid encoded messages in ContentInfo.GetContentType on Linux/Mac as on Windows.

* Fix compilation of CmsSignature.DSA in non-netcoreapp scenario in the same way as RSA and ECDsa.

* Handle invalid value when CmsSigner.SignerIdentifierType is being assigned in the same was as on .NET Framework.

* Throw correct exception (as documented) for incorrect index values.

7 files changed, 102 insertions, 8 deletions

diff --git a/src/System.Security.Cryptography.Pkcs/src/Internal/Cryptography/Pal/AnyOS/ManagedPal.Asn.cs b/src/System.Security.Cryptography.Pkcs/src/Internal/Cryptography/Pal/AnyOS/ManagedPal.Asn.cs
index 361f55ac75..bd7463c3b1 100644
--- a/src/System.Security.Cryptography.Pkcs/src/Internal/Cryptography/Pal/AnyOS/ManagedPal.Asn.cs
+++ b/src/System.Security.Cryptography.Pkcs/src/Internal/Cryptography/Pal/AnyOS/ManagedPal.Asn.cs
@@ -143,7 +143,18 @@ namespace Internal.Cryptography.Pal.AnyOS
                 reader.GetEncodedValue(),
                 AsnEncodingRules.BER);
 
-            return new Oid(contentInfo.ContentType);
+            switch (contentInfo.ContentType)
+            {
+                case Oids.Pkcs7Data:
+                case Oids.Pkcs7Signed:
+                case Oids.Pkcs7Enveloped:
+                case Oids.Pkcs7SignedEnveloped:
+                case Oids.Pkcs7Hashed:
+                case Oids.Pkcs7Encrypted:
+                    return new Oid(contentInfo.ContentType);
+            }
+
+            throw new CryptographicException(SR.Cryptography_Cms_InvalidMessageType);
         }
     }
 }
diff --git a/src/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/CmsSignature.DSA.cs b/src/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/CmsSignature.DSA.cs
index e85cfbd3fc..53e21182c7 100644
--- a/src/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/CmsSignature.DSA.cs
+++ b/src/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/CmsSignature.DSA.cs
@@ -32,8 +32,13 @@ namespace System.Security.Cryptography.Pkcs
             }
 
             internal override bool VerifySignature(
+#if netcoreapp
                 ReadOnlySpan<byte> valueHash,
                 ReadOnlyMemory<byte> signature,
+#else
+                byte[] valueHash,
+                byte[] signature,
+#endif                
                 string digestAlgorithmOid,
                 HashAlgorithmName digestAlgorithmName,
                 ReadOnlyMemory<byte>? signatureParameters,
@@ -58,28 +63,38 @@ namespace System.Security.Cryptography.Pkcs
                 DSAParameters dsaParameters = dsa.ExportParameters(false);
                 int bufSize = 2 * dsaParameters.Q.Length;
 
+#if netcoreapp
                 ArrayPool<byte> pool = ArrayPool<byte>.Shared;
                 byte[] rented = pool.Rent(bufSize);
                 Span<byte> ieee = new Span<byte>(rented, 0, bufSize);
 
                 try
                 {
+#else
+                byte[] ieee = new byte[bufSize];
+#endif
                     if (!DsaDerToIeee(signature, ieee))
                     {
                         return false;
                     }
 
                     return dsa.VerifySignature(valueHash, ieee);
+#if netcoreapp
                 }
                 finally
                 {
                     ieee.Clear();
                     pool.Return(rented);
                 }
+#endif
             }
 
             protected override bool Sign(
+#if netcoreapp
                 ReadOnlySpan<byte> dataHash,
+#else
+                byte[] dataHash,
+#endif
                 HashAlgorithmName hashAlgorithmName,
                 X509Certificate2 certificate,
                 bool silent,
@@ -114,6 +129,7 @@ namespace System.Security.Cryptography.Pkcs
 
                 signatureAlgorithm = new Oid(oidValue, oidValue);
 
+#if netcoreapp
                 ArrayPool<byte> pool = ArrayPool<byte>.Shared;
                 // The Q size cannot be bigger than the KeySize.
                 byte[] rented = pool.Rent(dsa.KeySize / 8);
@@ -135,6 +151,11 @@ namespace System.Security.Cryptography.Pkcs
 
                 signatureValue = null;
                 return false;
+#else
+                byte[] signature = dsa.CreateSignature(dataHash);
+                signatureValue = DsaIeeeToDer(new ReadOnlySpan<byte>(signature));
+                return true;
+#endif                
             }
         }
     }
diff --git a/src/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/CmsSigner.cs b/src/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/CmsSigner.cs
index 6099fcfe2c..30f49b6570 100644
--- a/src/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/CmsSigner.cs
+++ b/src/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/CmsSigner.cs
@@ -19,14 +19,26 @@ namespace System.Security.Cryptography.Pkcs
         private static readonly Oid s_defaultAlgorithm = Oid.FromOidValue(Oids.Sha256, OidGroup.HashAlgorithm);
 #endif
 
+        private SubjectIdentifierType _signerIdentifierType;
+
         public X509Certificate2 Certificate { get; set; }
         public X509Certificate2Collection Certificates { get; set; } = new X509Certificate2Collection();
         public Oid DigestAlgorithm { get; set; }
         public X509IncludeOption IncludeOption { get; set; }
         public CryptographicAttributeObjectCollection SignedAttributes { get; set; } = new CryptographicAttributeObjectCollection();
-        public SubjectIdentifierType SignerIdentifierType { get; set; }
         public CryptographicAttributeObjectCollection UnsignedAttributes { get; set; } = new CryptographicAttributeObjectCollection();
 
+        public SubjectIdentifierType SignerIdentifierType
+        {
+            get { return _signerIdentifierType; }
+            set
+            {
+                if (value < SubjectIdentifierType.IssuerAndSerialNumber || value > SubjectIdentifierType.NoSignature)
+                    throw new ArgumentException(SR.Format(SR.Cryptography_Cms_Invalid_Subject_Identifier_Type, value));
+                _signerIdentifierType = value;
+            }
+        }
+
         public CmsSigner()
             : this(SubjectIdentifierType.IssuerAndSerialNumber, null)
         {
@@ -58,23 +70,23 @@ namespace System.Security.Cryptography.Pkcs
             switch (signerIdentifierType)
             {
                 case SubjectIdentifierType.Unknown:
-                    SignerIdentifierType = SubjectIdentifierType.IssuerAndSerialNumber;
+                    _signerIdentifierType = SubjectIdentifierType.IssuerAndSerialNumber;
                     IncludeOption = X509IncludeOption.ExcludeRoot;
                     break;
                 case SubjectIdentifierType.IssuerAndSerialNumber:
-                    SignerIdentifierType = signerIdentifierType;
+                    _signerIdentifierType = signerIdentifierType;
                     IncludeOption = X509IncludeOption.ExcludeRoot;
                     break;
                 case SubjectIdentifierType.SubjectKeyIdentifier:
-                    SignerIdentifierType = signerIdentifierType;
+                    _signerIdentifierType = signerIdentifierType;
                     IncludeOption = X509IncludeOption.ExcludeRoot;
                     break;
                 case SubjectIdentifierType.NoSignature:
-                    SignerIdentifierType = signerIdentifierType;
+                    _signerIdentifierType = signerIdentifierType;
                     IncludeOption = X509IncludeOption.None;
                     break;
                 default:
-                    SignerIdentifierType = SubjectIdentifierType.IssuerAndSerialNumber;
+                    _signerIdentifierType = SubjectIdentifierType.IssuerAndSerialNumber;
                     IncludeOption = X509IncludeOption.ExcludeRoot;
                     break;
             }
diff --git a/src/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/SignerInfoCollection.cs b/src/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/SignerInfoCollection.cs
index b221876e8b..37102bde1f 100644
--- a/src/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/SignerInfoCollection.cs
+++ b/src/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/SignerInfoCollection.cs
@@ -37,7 +37,15 @@ namespace System.Security.Cryptography.Pkcs
             }
         }
 
-        public SignerInfo this[int index] => _signerInfos[index];
+        public SignerInfo this[int index]
+        {
+            get
+            {
+                if (index < 0 || index >= _signerInfos.Length)
+                    throw new ArgumentOutOfRangeException("index");
+                return _signerInfos[index];
+            }
+        }
 
         public int Count => _signerInfos.Length;
 
diff --git a/src/System.Security.Cryptography.Pkcs/tests/EnvelopedCms/EdgeCasesTests.cs b/src/System.Security.Cryptography.Pkcs/tests/EnvelopedCms/EdgeCasesTests.cs
index 38b03e90e2..54f06d6b2f 100644
--- a/src/System.Security.Cryptography.Pkcs/tests/EnvelopedCms/EdgeCasesTests.cs
+++ b/src/System.Security.Cryptography.Pkcs/tests/EnvelopedCms/EdgeCasesTests.cs
@@ -481,6 +481,14 @@ namespace System.Security.Cryptography.Pkcs.EnvelopedCmsTests.Tests
         }
 
         [Fact]
+        public static void ContentInfoGetContentTypeUnknown()
+        {
+            byte[] encodedMessage =
+                 ("301A06092A864886F70D010700A00D040B48656C6C6F202E4E455421").HexToByteArray();
+            Assert.ThrowsAny<CryptographicException>(() => ContentInfo.GetContentType(encodedMessage));
+        }
+
+        [Fact]
         public static void CryptographicAttributeObjectOidCtor()
         {
             Oid oid = new Oid(Oids.DocumentDescription);
diff --git a/src/System.Security.Cryptography.Pkcs/tests/SignedCms/CmsSignerTests.cs b/src/System.Security.Cryptography.Pkcs/tests/SignedCms/CmsSignerTests.cs
new file mode 100644
index 0000000000..142449e5d9
--- /dev/null
+++ b/src/System.Security.Cryptography.Pkcs/tests/SignedCms/CmsSignerTests.cs
@@ -0,0 +1,24 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System.Linq;
+using System.Security.Cryptography.X509Certificates;
+using System.Security.Cryptography.Xml;
+using Test.Cryptography;
+using Xunit;
+
+namespace System.Security.Cryptography.Pkcs.Tests
+{
+	public static partial class CmsSignerTests
+	{
+		[Fact]
+		public void SignerIdentifierType_InvalidValues()
+		{
+			CmsSigner signer = new CmsSigner();
+			Assert.ThrowsAny<CryptographicException>(() => signer.SignerIdentifierType = SubjectIdentifierType.Unknown);
+			Assert.ThrowsAny<CryptographicException>(() => signer.SignerIdentifierType = (SubjectIdentifierType)4);
+			Assert.ThrowsAny<CryptographicException>(() => signer.SignerIdentifierType = (SubjectIdentifierType)-1);
+		}
+	}
+}
diff --git a/src/System.Security.Cryptography.Pkcs/tests/SignedCms/SignedCmsTests.cs b/src/System.Security.Cryptography.Pkcs/tests/SignedCms/SignedCmsTests.cs
index 22e84fcd5f..22641599a6 100644
--- a/src/System.Security.Cryptography.Pkcs/tests/SignedCms/SignedCmsTests.cs
+++ b/src/System.Security.Cryptography.Pkcs/tests/SignedCms/SignedCmsTests.cs
@@ -890,5 +890,15 @@ namespace System.Security.Cryptography.Pkcs.Tests
             // Assert.NoThrow
             cms.CheckSignature(true);
         }
+
+        [Fact]
+        public static void SignerInfoCollection_Indexer_MinusOne ()
+        {
+            SignedCms cms = new SignedCms();
+            cms.Decode(SignedDocuments.RsaPkcs1OneSignerIssuerAndSerialNumber);
+
+            Assert.Throws<ArgumentOutOfRangeException>(() => cms.SignerInfos[-1]);
+            Assert.Throws<ArgumentOutOfRangeException>(() => cms.SignerInfos[1]);
+        }
     }
 }