Support CFB8 for 3DES and AES on Windows 7
authorKevin Jones <kevin@vcsjones.com>
Wed, 16 Sep 2020 16:52:57 +0000 (12:52 -0400)
committerGitHub <noreply@github.com>
Wed, 16 Sep 2020 16:52:57 +0000 (09:52 -0700)
This also throws a better exception on platforms that do not support
setting the feedback size (such as Win7).

src/libraries/Common/src/Interop/Windows/BCrypt/AesBCryptModes.cs
src/libraries/Common/src/Interop/Windows/BCrypt/DESBCryptModes.cs
src/libraries/Common/src/Interop/Windows/BCrypt/TripleDesBCryptModes.cs
src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/AES/AesCipherTests.cs
src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/AES/AesContractTests.cs
src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/AES/AesModeTests.cs
src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/TripleDES/TripleDESCipherTests.cs
src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/TripleDES/TripleDESContractTests.cs
src/libraries/System.Security.Cryptography.Algorithms/src/Resources/Strings.resx
src/libraries/System.Security.Cryptography.Cng/src/Resources/Strings.resx

index ad685bc..9b16158 100644 (file)
@@ -35,9 +35,20 @@ namespace Internal.Cryptography
                 hAlg.SetCipherMode(cipherMode);
 
                 // feedback is in bytes!
-                if (feedback > 0)
+                // The default feedback size is 1 (CFB8) on Windows. Do not set the CNG property
+                // if we would be setting it to the default. Windows 7 only supports CFB8 and
+                // does not permit setting the feedback size, so we don't call the property
+                // setter at all in that case.
+                if (feedback > 0 && feedback != 1)
                 {
-                    hAlg.SetFeedbackSize(feedback);
+                    try
+                    {
+                        hAlg.SetFeedbackSize(feedback);
+                    }
+                    catch (CryptographicException ex)
+                    {
+                        throw new CryptographicException(SR.Cryptography_FeedbackSizeNotSupported, ex);
+                    }
                 }
 
                 return hAlg;
index 85f9b65..0502b44 100644 (file)
@@ -11,7 +11,7 @@ namespace Internal.Cryptography
     {
         private static readonly Lazy<SafeAlgorithmHandle> s_hAlgCbc = OpenDesAlgorithm(Cng.BCRYPT_CHAIN_MODE_CBC);
         private static readonly Lazy<SafeAlgorithmHandle> s_hAlgEcb = OpenDesAlgorithm(Cng.BCRYPT_CHAIN_MODE_ECB);
-        private static readonly Lazy<SafeAlgorithmHandle> s_hAlgCfb8 = OpenDesAlgorithm(Cng.BCRYPT_CHAIN_MODE_CFB, 1);
+        private static readonly Lazy<SafeAlgorithmHandle> s_hAlgCfb8 = OpenDesAlgorithm(Cng.BCRYPT_CHAIN_MODE_CFB);
 
         internal static SafeAlgorithmHandle GetSharedHandle(CipherMode cipherMode, int feedback) =>
             // Windows 8 added support to set the CipherMode value on a key,
@@ -24,20 +24,16 @@ namespace Internal.Cryptography
                 _ => throw new NotSupportedException(),
             };
 
-        private static Lazy<SafeAlgorithmHandle> OpenDesAlgorithm(string cipherMode, int feedback = 0)
+        private static Lazy<SafeAlgorithmHandle> OpenDesAlgorithm(string cipherMode)
         {
             return new Lazy<SafeAlgorithmHandle>(() =>
             {
-                SafeAlgorithmHandle hAlg = Cng.BCryptOpenAlgorithmProvider(Cng.BCRYPT_DES_ALGORITHM,
+                SafeAlgorithmHandle hAlg = Cng.BCryptOpenAlgorithmProvider(
+                    Cng.BCRYPT_DES_ALGORITHM,
                     null,
                     Cng.OpenAlgorithmProviderFlags.NONE);
                 hAlg.SetCipherMode(cipherMode);
 
-                if (feedback > 0)
-                {
-                    hAlg.SetFeedbackSize(feedback);
-                }
-
                 return hAlg;
             });
         }
index c20b040..8493d1d 100644 (file)
@@ -38,10 +38,20 @@ namespace Internal.Cryptography
                     Cng.OpenAlgorithmProviderFlags.NONE);
                 hAlg.SetCipherMode(cipherMode);
 
-                if (feedback > 0)
+                // The default feedback size is 1 (CFB8) on Windows. Do not set the CNG property
+                // if we would be setting it to the default. Windows 7 only supports CFB8 and
+                // does not permit setting the feedback size, so we don't call the property
+                // setter at all in that case.
+                if (feedback > 0 && feedback != 1)
                 {
-                    // feedback is in bytes!
-                    hAlg.SetFeedbackSize(feedback);
+                    try
+                    {
+                        hAlg.SetFeedbackSize(feedback);
+                    }
+                    catch (CryptographicException ex)
+                    {
+                        throw new CryptographicException(SR.Cryptography_FeedbackSizeNotSupported, ex);
+                    }
                 }
 
                 return hAlg;
index 896c3f8..8883d60 100644 (file)
@@ -406,7 +406,7 @@ namespace System.Security.Cryptography.Encryption.Aes.Tests
                 128);
         }
 
-        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))]
+        [Fact]
         public static void VerifyKnownTransform_CFB8_256_NoPadding()
         {
             TestAesTransformDirectKey(
@@ -494,7 +494,7 @@ namespace System.Security.Cryptography.Encryption.Aes.Tests
                 128);
         }
 
-        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))]
+        [Fact]
         public static void VerifyKnownTransform_CFB8_192_NoPadding()
         {
             TestAesTransformDirectKey(
@@ -626,7 +626,7 @@ namespace System.Security.Cryptography.Encryption.Aes.Tests
             }
         }
 
-        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))]
+        [Fact]
         public static void VerifyKnownTransform_CFB8_128_NoPadding_4()
         {
             // NIST CAVP AESMMT.ZIP CFB8MMT128.rsp, [ENCRYPT] COUNT=4
@@ -669,7 +669,7 @@ namespace System.Security.Cryptography.Encryption.Aes.Tests
                 feedbackSize: 128);
         }
 
-        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))]
+        [Fact]
         public static void VerifyKnownTransform_CFB8_128_PKCS7_4()
         {
             TestAesTransformDirectKey(
@@ -682,7 +682,7 @@ namespace System.Security.Cryptography.Encryption.Aes.Tests
                 feedbackSize: 8);
         }
 
-        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))]
+        [Fact]
         public static void VerifyKnownTransform_CFB8_128_NoPadding_0_Extended()
         {
             // NIST CAVP AESMMT.ZIP CFB8MMT128.rsp, [ENCRYPT] COUNT=0
@@ -698,7 +698,7 @@ namespace System.Security.Cryptography.Encryption.Aes.Tests
                 feedbackSize: 8);
         }
 
-        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))]
+        [Fact]
         public static void VerifyKnownTransform_CFB8_128_NoPadding_9_Extended()
         {
             // NIST CAVP AESMMT.ZIP CFB8MMT128.rsp, [ENCRYPT] COUNT=9
@@ -714,7 +714,7 @@ namespace System.Security.Cryptography.Encryption.Aes.Tests
                 feedbackSize: 8);
         }
 
-        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))]
+        [Fact]
         public static void VerifyKnownTransform_CFB8_192_NoPadding_0_Extended()
         {
             // NIST CAVP AESMMT.ZIP CFB8MMT192.rsp, [ENCRYPT] COUNT=0
@@ -730,7 +730,7 @@ namespace System.Security.Cryptography.Encryption.Aes.Tests
                 feedbackSize: 8);
         }
 
-        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))]
+        [Fact]
         public static void VerifyKnownTransform_CFB8_192_NoPadding_9_Extended()
         {
             // NIST CAVP AESMMT.ZIP CFB8MMT192.rsp, [ENCRYPT] COUNT=9
@@ -746,7 +746,7 @@ namespace System.Security.Cryptography.Encryption.Aes.Tests
                 feedbackSize: 8);
         }
 
-        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))]
+        [Fact]
         public static void VerifyKnownTransform_CFB8_256_NoPadding_0_Extended()
         {
             // NIST CAVP AESMMT.ZIP CFB8MMT256.rsp, [ENCRYPT] COUNT=0
@@ -762,7 +762,7 @@ namespace System.Security.Cryptography.Encryption.Aes.Tests
                 feedbackSize: 8);
         }
 
-        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))]
+        [Fact]
         public static void VerifyKnownTransform_CFB8_256_NoPadding_9_Extended()
         {
             // NIST CAVP AESMMT.ZIP CFB8MMT256.rsp, [ENCRYPT] COUNT=9
index 49d7665..8fe23f2 100644 (file)
@@ -94,7 +94,7 @@ namespace System.Security.Cryptography.Encryption.Aes.Tests
             }
         }
 
-        [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))]
+        [Theory]
         [InlineData(0, true)]
         [InlineData(1, true)]
         [InlineData(7, true)]
@@ -134,11 +134,17 @@ namespace System.Security.Cryptography.Encryption.Aes.Tests
             }
         }
 
-        [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))]
+        [Theory]
         [InlineData(8)]
         [InlineData(128)]
         public static void ValidCFBFeedbackSizes(int feedbackSize)
         {
+            // Windows 7 only supports CFB8.
+            if (feedbackSize != 8 && PlatformDetection.IsWindows7)
+            {
+                return;
+            }
+
             using (Aes aes = AesFactory.Create())
             {
                 aes.GenerateKey();
index f36e0f0..6be8a5f 100644 (file)
@@ -22,16 +22,22 @@ namespace System.Security.Cryptography.Encryption.Aes.Tests
             SupportsMode(CipherMode.ECB);
         }
 
+        [Fact]
+        public static void SupportsCFB8()
+        {
+            SupportsMode(CipherMode.CFB, feedbackSize: 8);
+        }
+
         [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))]
-        public static void SupportsCFB()
+        public static void SupportsCFB128()
         {
-            SupportsMode(CipherMode.CFB);
+            SupportsMode(CipherMode.CFB, feedbackSize: 128);
         }
 
         [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows7))]
-        public static void Windows7DoesNotSupportCFB()
+        public static void Windows7DoesNotSupportCFB128()
         {
-            DoesNotSupportMode(CipherMode.CFB);
+            DoesNotSupportMode(CipherMode.CFB, feedbackSize: 128);
         }
 
         [Fact]
@@ -40,13 +46,18 @@ namespace System.Security.Cryptography.Encryption.Aes.Tests
             DoesNotSupportMode(CipherMode.CTS);
         }
 
-        private static void SupportsMode(CipherMode mode)
+        private static void SupportsMode(CipherMode mode, int? feedbackSize = null)
         {
             using (Aes aes = AesFactory.Create())
             {
                 aes.Mode = mode;
                 Assert.Equal(mode, aes.Mode);
 
+                if (feedbackSize.HasValue)
+                {
+                    aes.FeedbackSize = feedbackSize.Value;
+                }
+
                 using (ICryptoTransform transform = aes.CreateEncryptor())
                 {
                     transform.TransformFinalBlock(Array.Empty<byte>(), 0, 0);
@@ -54,7 +65,7 @@ namespace System.Security.Cryptography.Encryption.Aes.Tests
             }
         }
 
-        private static void DoesNotSupportMode(CipherMode mode)
+        private static void DoesNotSupportMode(CipherMode mode, int? feedbackSize = null)
         {
             using (Aes aes = AesFactory.Create())
             {
@@ -68,6 +79,11 @@ namespace System.Security.Cryptography.Encryption.Aes.Tests
                     {
                         aes.Mode = mode;
 
+                        if (feedbackSize.HasValue)
+                        {
+                            aes.FeedbackSize = feedbackSize.Value;
+                        }
+
                         // If assigning the Mode property did not fail, then it should reflect what we asked for.
                         Assert.Equal(mode, aes.Mode);
 
index f63b0fb..dcab872 100644 (file)
@@ -42,7 +42,7 @@ namespace System.Security.Cryptography.Encryption.TripleDes.Tests
             }
         }
 
-        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))]
+        [Fact]
         public static void VerifyKnownTransform_CFB8_NoPadding_0()
         {
             // NIST CAVS TDESMMT.ZIP TCFB8MMT2.rsp, [DECRYPT] COUNT=0
@@ -57,7 +57,7 @@ namespace System.Security.Cryptography.Encryption.TripleDes.Tests
             );
         }
 
-        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))]
+        [Fact]
         public static void VerifyKnownTransform_CFB8_NoPadding_1()
         {
             // NIST CAVS TDESMMT.ZIP TCFB8MMT2.rsp, [DECRYPT] COUNT=1
@@ -72,7 +72,7 @@ namespace System.Security.Cryptography.Encryption.TripleDes.Tests
             );
         }
 
-        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))]
+        [Fact]
         public static void VerifyKnownTransform_CFB8_NoPadding_2()
         {
             // NIST CAVS TDESMMT.ZIP TCFB8MMT2.rsp, [DECRYPT] COUNT=2
@@ -87,7 +87,7 @@ namespace System.Security.Cryptography.Encryption.TripleDes.Tests
             );
         }
 
-        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))]
+        [Fact]
         public static void VerifyKnownTransform_CFB8_PKCS7_2()
         {
             // NIST CAVS TDESMMT.ZIP TCFB8MMT2.rsp, [DECRYPT] COUNT=2
@@ -117,7 +117,7 @@ namespace System.Security.Cryptography.Encryption.TripleDes.Tests
             );
         }
 
-        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))]
+        [Fact]
         public static void VerifyKnownTransform_CFB8_NoPadding_3()
         {
             // NIST CAVS TDESMMT.ZIP TCFB8MMT2.rsp, [DECRYPT] COUNT=3
@@ -132,7 +132,7 @@ namespace System.Security.Cryptography.Encryption.TripleDes.Tests
             );
         }
 
-        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))]
+        [Fact]
         public static void VerifyKnownTransform_CFB8_NoPadding_4()
         {
             // NIST CAVS TDESMMT.ZIP TCFB8MMT2.rsp, [DECRYPT] COUNT=4
@@ -147,7 +147,7 @@ namespace System.Security.Cryptography.Encryption.TripleDes.Tests
             );
         }
 
-        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))]
+        [Fact]
         public static void VerifyKnownTransform_CFB8_NoPadding_5()
         {
             // NIST CAVS TDESMMT.ZIP TCFB8MMT2.rsp, [DECRYPT] COUNT=5
@@ -162,7 +162,7 @@ namespace System.Security.Cryptography.Encryption.TripleDes.Tests
             );
         }
 
-        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))]
+        [Fact]
         public static void VerifyKnownTransform_CFB8_NoPadding_6()
         {
             // NIST CAVS TDESMMT.ZIP TCFB8MMT2.rsp, [DECRYPT] COUNT=6
@@ -177,7 +177,7 @@ namespace System.Security.Cryptography.Encryption.TripleDes.Tests
             );
         }
 
-        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))]
+        [Fact]
         public static void VerifyKnownTransform_CFB8_NoPadding_7()
         {
             // NIST CAVS TDESMMT.ZIP TCFB8MMT2.rsp, [DECRYPT] COUNT=7
@@ -192,7 +192,7 @@ namespace System.Security.Cryptography.Encryption.TripleDes.Tests
             );
         }
 
-        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))]
+        [Fact]
         public static void VerifyKnownTransform_CFB8_NoPadding_8()
         {
             // NIST CAVS TDESMMT.ZIP TCFB8MMT2.rsp, [DECRYPT] COUNT=8
@@ -207,7 +207,7 @@ namespace System.Security.Cryptography.Encryption.TripleDes.Tests
             );
         }
 
-        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))]
+        [Fact]
         public static void VerifyKnownTransform_CFB8_NoPadding_9()
         {
             // NIST CAVS TDESMMT.ZIP TCFB8MMT2.rsp, [DECRYPT] COUNT=9
index a36167b..43c6de5 100644 (file)
@@ -11,55 +11,21 @@ namespace System.Security.Cryptography.Encryption.TripleDes.Tests
     public static class TripleDESContractTests
     {
 
-        [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows7))]
-        [InlineData(0, true)]
-        [InlineData(1, true)]
-        [InlineData(7, true)]
-        [InlineData(9, true)]
-        [InlineData(-1, true)]
-        [InlineData(int.MaxValue, true)]
-        [InlineData(int.MinValue, true)]
-        [InlineData(8, false)]
-        [InlineData(64, false)]
-        [InlineData(256, true)]
-        [InlineData(128, true)]
-        [InlineData(127, true)]
-        public static void Windows7DoesNotSupportCFB(int feedbackSize, bool discoverableInSetter)
+        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows7))]
+        public static void Windows7DoesNotSupportCFB64()
         {
             using (TripleDES tdes = TripleDESFactory.Create())
             {
                 tdes.GenerateKey();
                 tdes.Mode = CipherMode.CFB;
+                tdes.FeedbackSize = 64;
 
-                if (discoverableInSetter)
-                {
-                    // there are some key sizes that are invalid for any of the modes,
-                    // so the exception is thrown in the setter
-                    Assert.ThrowsAny<CryptographicException>(() =>
-                    {
-                        tdes.FeedbackSize = feedbackSize;
-                    });
-                }
-                else
-                {
-                    tdes.FeedbackSize = feedbackSize;
-
-                    // however, for CFB only few sizes are valid. Those should throw in the
-                    // actual AES instantiation.
-
-                    Assert.ThrowsAny<CryptographicException>(() =>
-                    {
-                        return tdes.CreateDecryptor();
-                    });
-                    Assert.ThrowsAny<CryptographicException>(() =>
-                    {
-                        return tdes.CreateEncryptor();
-                    });
-                }
+                Assert.ThrowsAny<CryptographicException>(() => tdes.CreateDecryptor());
+                Assert.ThrowsAny<CryptographicException>(() => tdes.CreateEncryptor());
             }
         }
 
-        [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))]
+        [Theory]
         [InlineData(0, true)]
         [InlineData(1, true)]
         [InlineData(7, true)]
@@ -104,6 +70,12 @@ namespace System.Security.Cryptography.Encryption.TripleDes.Tests
         [InlineData(64)]
         public static void ValidCFBFeedbackSizes(int feedbackSize)
         {
+            // Windows 7 only supports CFB8.
+            if (feedbackSize != 8 && PlatformDetection.IsWindows7)
+            {
+                return;
+            }
+
             using (TripleDES tdes = TripleDESFactory.Create())
             {
                 tdes.GenerateKey();
index 162f1ab..a0008c8 100644 (file)
   <data name="Cryptography_ExceedKdfExtractLimit" xml:space="preserve">
     <value>The total number of bytes extracted cannot exceed UInt32.MaxValue * hash length.</value>
   </data>
+  <data name="Cryptography_FeedbackSizeNotSupported" xml:space="preserve">
+    <value>The current platform does not support the specified feedback size.</value>
+  </data>
 </root>
index 567cfde..bc28c38 100644 (file)
   <data name="Argument_DestinationTooShort" xml:space="preserve">
     <value>Destination is too short.</value>
   </data>
+  <data name="Cryptography_FeedbackSizeNotSupported" xml:space="preserve">
+    <value>The current platform does not support the specified feedback size.</value>
+  </data>
 </root>