Remove salt length check from Rfc2898DeriveBytes

Some standards/protocols want salts smaller than we permit, so we're really just getting in people's way.

diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Managed.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Managed.cs
index 296c7bf..9bb1a2c 100644 (file)
--- a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Managed.cs
+++ b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Managed.cs
@@ -25,8 +25,7 @@ namespace Internal.Cryptography
                 salt.ToArray(),
                 iterations,
                 hashAlgorithmName,
-                clearPassword: true,
-                requireMinimumSaltLength: false))
+                clearPassword: true))
             {
                 byte[] result = deriveBytes.GetBytes(destination.Length);
                 result.AsSpan().CopyTo(destination);

diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/Resources/Strings.resx b/src/libraries/System.Security.Cryptography.Algorithms/src/Resources/Strings.resx
index c0ee812..0aafdc0 100644 (file)
--- a/src/libraries/System.Security.Cryptography.Algorithms/src/Resources/Strings.resx
+++ b/src/libraries/System.Security.Cryptography.Algorithms/src/Resources/Strings.resx
@@ -273,9 +273,6 @@
   <data name="Cryptography_PartialBlock" xml:space="preserve">
     <value>The input data is not a complete block.</value>
   </data>
-  <data name="Cryptography_PasswordDerivedBytes_FewBytesSalt" xml:space="preserve">
-    <value>Salt is not at least eight bytes.</value>
-  </data>
   <data name="Cryptography_Pkcs8_EncryptedReadFailed" xml:space="preserve">
     <value>The EncryptedPrivateKeyInfo structure was decoded but was not successfully interpreted, the password may be incorrect.</value>
   </data>

diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.cs
index bc01e7d..d7a8168 100644 (file)
--- a/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.cs
+++ b/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.cs
@@ -14,8 +14,6 @@ namespace System.Security.Cryptography
     [UnsupportedOSPlatform("browser")]
     public partial class Rfc2898DeriveBytes : DeriveBytes
     {
-        private const int MinimumSaltSize = 8;
-
         private byte[] _salt;
         private uint _iterations;
         private HMAC _hmac;
@@ -37,7 +35,7 @@ namespace System.Security.Cryptography
         }
 
         public Rfc2898DeriveBytes(byte[] password, byte[] salt, int iterations, HashAlgorithmName hashAlgorithm)
-            :this(password, salt, iterations, hashAlgorithm, clearPassword: false, requireMinimumSaltLength: true)
+            :this(password, salt, iterations, hashAlgorithm, clearPassword: false)
         {
         }
 
@@ -52,7 +50,7 @@ namespace System.Security.Cryptography
         }
 
         public Rfc2898DeriveBytes(string password, byte[] salt, int iterations, HashAlgorithmName hashAlgorithm)
-            : this(Encoding.UTF8.GetBytes(password), salt, iterations, hashAlgorithm, clearPassword: true, requireMinimumSaltLength: true)
+            : this(Encoding.UTF8.GetBytes(password), salt, iterations, hashAlgorithm, clearPassword: true)
         {
         }
 
@@ -70,8 +68,6 @@ namespace System.Security.Cryptography
         {
             if (saltSize < 0)
                 throw new ArgumentOutOfRangeException(nameof(saltSize), SR.ArgumentOutOfRange_NeedNonNegNum);
-            if (saltSize < MinimumSaltSize)
-                throw new ArgumentException(SR.Cryptography_PasswordDerivedBytes_FewBytesSalt, nameof(saltSize));
             if (iterations <= 0)
                 throw new ArgumentOutOfRangeException(nameof(iterations), SR.ArgumentOutOfRange_NeedPosNum);
 
@@ -89,12 +85,10 @@ namespace System.Security.Cryptography
             Initialize();
         }
 
-        internal Rfc2898DeriveBytes(byte[] password, byte[] salt, int iterations, HashAlgorithmName hashAlgorithm, bool clearPassword, bool requireMinimumSaltLength)
+        internal Rfc2898DeriveBytes(byte[] password, byte[] salt, int iterations, HashAlgorithmName hashAlgorithm, bool clearPassword)
         {
             if (salt is null)
                 throw new ArgumentNullException(nameof(salt));
-            if (requireMinimumSaltLength && salt.Length < MinimumSaltSize)
-                throw new ArgumentException(SR.Cryptography_PasswordDerivedBytes_FewBytesSalt, nameof(salt));
             if (iterations <= 0)
                 throw new ArgumentOutOfRangeException(nameof(iterations), SR.ArgumentOutOfRange_NeedPosNum);
             if (password is null)
@@ -143,8 +137,6 @@ namespace System.Security.Cryptography
             {
                 if (value == null)
                     throw new ArgumentNullException(nameof(value));
-                if (value.Length < MinimumSaltSize)
-                    throw new ArgumentException(SR.Cryptography_PasswordDerivedBytes_FewBytesSalt);
 
                 _salt = new byte[value.Length + sizeof(uint)];
                 value.AsSpan().CopyTo(_salt);

diff --git a/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898Tests.cs b/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898Tests.cs
index d733c75..5e06494 100644 (file)
--- a/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898Tests.cs
+++ b/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898Tests.cs
@@ -12,7 +12,6 @@ namespace System.Security.Cryptography.DeriveBytesTests
     [SkipOnPlatform(TestPlatforms.Browser, "Not supported on Browser")]
     public class Rfc2898Tests
     {
-        // 8 bytes is the minimum accepted value, by using it we've already assured that the minimum is acceptable.
         private static readonly byte[] s_testSalt = new byte[] { 9, 5, 5, 5, 1, 2, 1, 2 };
         private static readonly byte[] s_testSaltB = new byte[] { 0, 4, 0, 4, 1, 9, 7, 5 };
         private const string TestPassword = "PasswordGoesHere";
@@ -38,24 +37,6 @@ namespace System.Security.Cryptography.DeriveBytesTests
         }
 
         [Fact]
-        public static void Ctor_EmptySalt()
-        {
-            AssertExtensions.Throws<ArgumentException>("salt", null, () => new Rfc2898DeriveBytes(TestPassword, Array.Empty<byte>(), DefaultIterationCount));
-        }
-
-        [Fact]
-        public static void Ctor_DiminishedSalt()
-        {
-            AssertExtensions.Throws<ArgumentException>("salt", null, () => new Rfc2898DeriveBytes(TestPassword, new byte[7], DefaultIterationCount));
-        }
-
-        [Fact]
-        public static void Ctor_GenerateZeroSalt()
-        {
-            AssertExtensions.Throws<ArgumentException>("saltSize", null, () => new Rfc2898DeriveBytes(TestPassword, 0));
-        }
-
-        [Fact]
         public static void Ctor_GenerateNegativeSalt()
         {
             Assert.Throws<ArgumentOutOfRangeException>(() => new Rfc2898DeriveBytes(TestPassword, -1));
@@ -64,12 +45,6 @@ namespace System.Security.Cryptography.DeriveBytesTests
         }
 
         [Fact]
-        public static void Ctor_GenerateDiminishedSalt()
-        {
-            AssertExtensions.Throws<ArgumentException>("saltSize", null, () => new Rfc2898DeriveBytes(TestPassword, 7));
-        }
-
-        [Fact]
         public static void Ctor_TooFewIterations()
         {
             Assert.Throws<ArgumentOutOfRangeException>(() => new Rfc2898DeriveBytes(TestPassword, s_testSalt, 0));
@@ -83,7 +58,6 @@ namespace System.Security.Cryptography.DeriveBytesTests
             Assert.Throws<ArgumentOutOfRangeException>(() => new Rfc2898DeriveBytes(TestPassword, s_testSalt, int.MinValue / 2));
         }
 
-#if NETCOREAPP
         [Fact]
         public static void Ctor_EmptyAlgorithm()
         {
@@ -110,7 +84,6 @@ namespace System.Security.Cryptography.DeriveBytesTests
             Assert.Throws<CryptographicException>(
                 () => new Rfc2898DeriveBytes(TestPassword, s_testSalt, DefaultIterationCount, new HashAlgorithmName("PotatoLemming")));
         }
-#endif
 
         [Fact]
         public static void Ctor_SaltCopied()
@@ -144,6 +117,15 @@ namespace System.Security.Cryptography.DeriveBytesTests
         }
 
         [Fact]
+        public static void Ctor_GenerateEmptySalt()
+        {
+            using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, 0, 1))
+            {
+                Assert.Empty(deriveBytes.Salt);
+            }
+        }
+
+        [Fact]
         public static void Ctor_IterationsRespected()
         {
             using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt, 1))
@@ -173,13 +155,14 @@ namespace System.Security.Cryptography.DeriveBytesTests
         {
             byte[] output;
 
-            using (var deriveBytes = new Rfc2898DeriveBytes("", new byte[8], 1))
+            using (var deriveBytes = new Rfc2898DeriveBytes("", Array.Empty<byte>(), 1))
             {
                 output = deriveBytes.GetBytes(1);
+                Assert.Empty(deriveBytes.Salt);
             }
 
             Assert.Equal(1, output.Length);
-            Assert.Equal(0xA6, output[0]);
+            Assert.Equal(0x1E, output[0]);
         }
 
         [Fact]
@@ -346,7 +329,6 @@ namespace System.Security.Cryptography.DeriveBytesTests
                 });
         }
 
-#if NETCOREAPP
         [Theory]
         [MemberData(nameof(KnownValuesTestCases))]
         public static void GetBytes_KnownValues_WithAlgorithm(KnownValuesTestCase testCase)
@@ -381,7 +363,6 @@ namespace System.Security.Cryptography.DeriveBytesTests
                 Assert.Equal(hashAlgorithm, pbkdf2.HashAlgorithm);
             }
         }
-#endif
 
         [Fact]
         public static void CryptDeriveKey_NotSupported()
@@ -495,6 +476,16 @@ namespace System.Security.Cryptography.DeriveBytesTests
 
             yield return new KnownValuesTestCase
             {
+                CaseName = "RFC 6070 Case 1",
+                HashAlgorithmName = "SHA1",
+                Password = "password",
+                Salt = ascii.GetBytes("salt"),
+                IterationCount = 1,
+                AnswerHex="0C60C80F961F0E71F3A9B524AF6012062FE037A6",
+            };
+
+            yield return new KnownValuesTestCase
+            {
                 CaseName = "RFC 6070 Case 5",
                 HashAlgorithmName = "SHA1",
                 Password = "passwordPASSWORDpassword",
@@ -590,6 +581,17 @@ namespace System.Security.Cryptography.DeriveBytesTests
                     "078100F813C1F8388EC233C1397D5E18C6509B5483141EF836C15A34D6DC67" +
                     "A3C46A45798A2839CFD239749219E9F2EDAD3249EC8221AFB17C0028A4A0A5"),
             };
+
+            // Taken from the OneShot tests, which is implemented using native platform APIs.
+            yield return new KnownValuesTestCase
+            {
+                CaseName = "SHA1 empty",
+                HashAlgorithmName = "SHA1",
+                Password = "",
+                Salt = Array.Empty<byte>(),
+                IterationCount = 1,
+                AnswerHex = "1E437A1C79D75BE61E91141DAE20",
+            };
         }
 
         public class KnownValuesTestCase