Obsolete outdated constructors on Rfc2898DeriveBytes

docs/project/list-of-diagnostics.md

@@ -95,6 +95,7 @@ The PR that reveals the implementation of the `<IncludeInternalObsoleteAttribute
 |  __`SYSLIB0038`__ | SerializationFormat.Binary is obsolete and should not be used. See https://aka.ms/serializationformat-binary-obsolete for more information. |
 |  __`SYSLIB0039`__ | TLS versions 1.0 and 1.1 have known vulnerabilities and are not recommended. Use a newer TLS version instead, or use SslProtocols.None to defer to OS defaults. |
 |  __`SYSLIB0040`__ | EncryptionPolicy.NoEncryption and AllowEncryption significantly reduce security and should not be used in production code. |
+|  __`SYSLIB0041`__ | The default hash algorithm and iteration counts in Rfc2898DeriveBytes constructors are outdated and insecure. Use a constructor that accepts the hash algorithm and the number of iterations. |
 
 ## Analyzer Warnings
 

src/libraries/Common/src/System/Obsoletions.cs

@@ -132,5 +132,8 @@ internal static class Obsoletions
 
         internal const string EncryptionPolicyMessage = "EncryptionPolicy.NoEncryption and AllowEncryption significantly reduce security and should not be used in production code.";
         internal const string EncryptionPolicyDiagId = "SYSLIB0040";
+
+        internal const string Rfc2898OutdatedCtorMessage = "The default hash algorithm and iteration counts in Rfc2898DeriveBytes constructors are outdated and insecure. Use a constructor that accepts the hash algorithm and the number of iterations.";
+        internal const string Rfc2898OutdatedCtorDiagId = "SYSLIB0041";
     }
 }

src/libraries/System.Security.Cryptography/ref/System.Security.Cryptography.cs

@@ -1716,12 +1716,17 @@ public sealed partial class RC2CryptoServiceProvider : System.Security.Cryptogra
     [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")]
     public partial class Rfc2898DeriveBytes : System.Security.Cryptography.DeriveBytes
     {
+        [System.ObsoleteAttribute("The default hash algorithm and iteration counts in Rfc2898DeriveBytes constructors are outdated and insecure. Use a constructor that accepts the hash algorithm and the number of iterations.", DiagnosticId="SYSLIB0041", UrlFormat="https://aka.ms/dotnet-warnings/{0}")]
         public Rfc2898DeriveBytes(byte[] password, byte[] salt, int iterations) { }
         public Rfc2898DeriveBytes(byte[] password, byte[] salt, int iterations, System.Security.Cryptography.HashAlgorithmName hashAlgorithm) { }
+        [System.ObsoleteAttribute("The default hash algorithm and iteration counts in Rfc2898DeriveBytes constructors are outdated and insecure. Use a constructor that accepts the hash algorithm and the number of iterations.", DiagnosticId="SYSLIB0041", UrlFormat="https://aka.ms/dotnet-warnings/{0}")]
         public Rfc2898DeriveBytes(string password, byte[] salt) { }
+        [System.ObsoleteAttribute("The default hash algorithm and iteration counts in Rfc2898DeriveBytes constructors are outdated and insecure. Use a constructor that accepts the hash algorithm and the number of iterations.", DiagnosticId="SYSLIB0041", UrlFormat="https://aka.ms/dotnet-warnings/{0}")]
         public Rfc2898DeriveBytes(string password, byte[] salt, int iterations) { }
         public Rfc2898DeriveBytes(string password, byte[] salt, int iterations, System.Security.Cryptography.HashAlgorithmName hashAlgorithm) { }
+        [System.ObsoleteAttribute("The default hash algorithm and iteration counts in Rfc2898DeriveBytes constructors are outdated and insecure. Use a constructor that accepts the hash algorithm and the number of iterations.", DiagnosticId="SYSLIB0041", UrlFormat="https://aka.ms/dotnet-warnings/{0}")]
         public Rfc2898DeriveBytes(string password, int saltSize) { }
+        [System.ObsoleteAttribute("The default hash algorithm and iteration counts in Rfc2898DeriveBytes constructors are outdated and insecure. Use a constructor that accepts the hash algorithm and the number of iterations.", DiagnosticId="SYSLIB0041", UrlFormat="https://aka.ms/dotnet-warnings/{0}")]
         public Rfc2898DeriveBytes(string password, int saltSize, int iterations) { }
         public Rfc2898DeriveBytes(string password, int saltSize, int iterations, System.Security.Cryptography.HashAlgorithmName hashAlgorithm) { }
         public System.Security.Cryptography.HashAlgorithmName HashAlgorithm { get { throw null; } }

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/Rfc2898DeriveBytes.cs

 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System;
 using System.Buffers;
 using System.Buffers.Binary;
 using System.Diagnostics;
@@ -29,6 +30,7 @@ public partial class Rfc2898DeriveBytes : DeriveBytes
         /// </summary>
         public HashAlgorithmName HashAlgorithm { get; }
 
+        [Obsolete(Obsoletions.Rfc2898OutdatedCtorMessage, DiagnosticId = Obsoletions.Rfc2898OutdatedCtorDiagId, UrlFormat = Obsoletions.SharedUrlFormat)]
         public Rfc2898DeriveBytes(byte[] password, byte[] salt, int iterations)
             : this(password, salt, iterations, HashAlgorithmName.SHA1)
         {
@@ -39,11 +41,13 @@ public Rfc2898DeriveBytes(byte[] password, byte[] salt, int iterations, HashAlgo
         {
         }
 
+        [Obsolete(Obsoletions.Rfc2898OutdatedCtorMessage, DiagnosticId = Obsoletions.Rfc2898OutdatedCtorDiagId, UrlFormat = Obsoletions.SharedUrlFormat)]
         public Rfc2898DeriveBytes(string password, byte[] salt)
              : this(password, salt, 1000)
         {
         }
 
+        [Obsolete(Obsoletions.Rfc2898OutdatedCtorMessage, DiagnosticId = Obsoletions.Rfc2898OutdatedCtorDiagId, UrlFormat = Obsoletions.SharedUrlFormat)]
         public Rfc2898DeriveBytes(string password, byte[] salt, int iterations)
             : this(password, salt, iterations, HashAlgorithmName.SHA1)
         {
@@ -54,11 +58,13 @@ public Rfc2898DeriveBytes(string password, byte[] salt, int iterations, HashAlgo
         {
         }
 
+        [Obsolete(Obsoletions.Rfc2898OutdatedCtorMessage, DiagnosticId = Obsoletions.Rfc2898OutdatedCtorDiagId, UrlFormat = Obsoletions.SharedUrlFormat)]
         public Rfc2898DeriveBytes(string password, int saltSize)
             : this(password, saltSize, 1000)
         {
         }
 
+        [Obsolete(Obsoletions.Rfc2898OutdatedCtorMessage, DiagnosticId = Obsoletions.Rfc2898OutdatedCtorDiagId, UrlFormat = Obsoletions.SharedUrlFormat)]
         public Rfc2898DeriveBytes(string password, int saltSize, int iterations)
             : this(password, saltSize, iterations, HashAlgorithmName.SHA1)
         {

src/libraries/System.Security.Cryptography/tests/Rfc2898Tests.cs

@@ -21,41 +21,51 @@ public class Rfc2898Tests
         [Fact]
         public static void Ctor_NullPasswordBytes()
         {
-            Assert.Throws<NullReferenceException>(() => new Rfc2898DeriveBytes((byte[])null, s_testSalt, DefaultIterationCount));
+            Assert.Throws<NullReferenceException>(() =>
+                new Rfc2898DeriveBytes((byte[])null, s_testSalt, DefaultIterationCount, HashAlgorithmName.SHA1));
         }
 
         [Fact]
         public static void Ctor_NullPasswordString()
         {
-            Assert.Throws<ArgumentNullException>(() => new Rfc2898DeriveBytes((string)null, s_testSalt, DefaultIterationCount));
+            Assert.Throws<ArgumentNullException>(() =>
+                new Rfc2898DeriveBytes((string)null, s_testSalt, DefaultIterationCount, HashAlgorithmName.SHA1));
         }
 
         [Fact]
         public static void Ctor_NullSalt()
         {
-            Assert.Throws<ArgumentNullException>(() => new Rfc2898DeriveBytes(TestPassword, null, DefaultIterationCount));
+            AssertExtensions.Throws<ArgumentNullException>("salt", () =>
+                new Rfc2898DeriveBytes(TestPassword, null, DefaultIterationCount, HashAlgorithmName.SHA1));
         }
 
         [Fact]
         public static void Ctor_GenerateNegativeSalt()
         {
-            Assert.Throws<ArgumentOutOfRangeException>(() => new Rfc2898DeriveBytes(TestPassword, -1));
-            Assert.Throws<ArgumentOutOfRangeException>(() => new Rfc2898DeriveBytes(TestPassword, int.MinValue));
-            Assert.Throws<ArgumentOutOfRangeException>(() => new Rfc2898DeriveBytes(TestPassword, int.MinValue / 2));
+            AssertExtensions.Throws<ArgumentOutOfRangeException>("saltSize", () =>
+                new Rfc2898DeriveBytes(TestPassword, -1, DefaultIterationCount, HashAlgorithmName.SHA1));
+            AssertExtensions.Throws<ArgumentOutOfRangeException>("saltSize", () =>
+                new Rfc2898DeriveBytes(TestPassword, int.MinValue, DefaultIterationCount, HashAlgorithmName.SHA1));
+            AssertExtensions.Throws<ArgumentOutOfRangeException>("saltSize", () =>
+                new Rfc2898DeriveBytes(TestPassword, int.MinValue / 2, DefaultIterationCount, HashAlgorithmName.SHA1));
         }
 
         [Fact]
         public static void Ctor_TooFewIterations()
         {
-            Assert.Throws<ArgumentOutOfRangeException>(() => new Rfc2898DeriveBytes(TestPassword, s_testSalt, 0));
+            AssertExtensions.Throws<ArgumentOutOfRangeException>("iterations", () =>
+                new Rfc2898DeriveBytes(TestPassword, s_testSalt, 0, HashAlgorithmName.SHA1));
         }
 
         [Fact]
         public static void Ctor_NegativeIterations()
         {
-            Assert.Throws<ArgumentOutOfRangeException>(() => new Rfc2898DeriveBytes(TestPassword, s_testSalt, -1));
-            Assert.Throws<ArgumentOutOfRangeException>(() => new Rfc2898DeriveBytes(TestPassword, s_testSalt, int.MinValue));
-            Assert.Throws<ArgumentOutOfRangeException>(() => new Rfc2898DeriveBytes(TestPassword, s_testSalt, int.MinValue / 2));
+            AssertExtensions.Throws<ArgumentOutOfRangeException>("iterations", () =>
+                new Rfc2898DeriveBytes(TestPassword, s_testSalt, -1, HashAlgorithmName.SHA1));
+            AssertExtensions.Throws<ArgumentOutOfRangeException>("iterations", () =>
+                new Rfc2898DeriveBytes(TestPassword, s_testSalt, int.MinValue, HashAlgorithmName.SHA1));
+            AssertExtensions.Throws<ArgumentOutOfRangeException>("iterations", () =>
+                new Rfc2898DeriveBytes(TestPassword, s_testSalt, int.MinValue / 2, HashAlgorithmName.SHA1));
         }
 
         [Fact]
@@ -90,7 +100,7 @@ public static void Ctor_SaltCopied()
         {
             byte[] saltIn = (byte[])s_testSalt.Clone();
 
-            using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, saltIn, DefaultIterationCount))
+            using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, saltIn, DefaultIterationCount, HashAlgorithmName.SHA1))
             {
                 byte[] saltOut = deriveBytes.Salt;
 
@@ -110,7 +120,9 @@ public static void Ctor_SaltCopied()
         [Fact]
         public static void Ctor_DefaultIterations()
         {
+#pragma warning disable SYSLIB0041 // Rfc2898DeriveBytes insecure constructor defaults
             using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt))
+#pragma warning restore SYSLIB0041
             {
                 Assert.Equal(DefaultIterationCount, deriveBytes.IterationCount);
             }
@@ -119,7 +131,7 @@ public static void Ctor_DefaultIterations()
         [Fact]
         public static void Ctor_GenerateEmptySalt()
         {
-            using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, 0, 1))
+            using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, 0, 1, HashAlgorithmName.SHA1))
             {
                 Assert.Empty(deriveBytes.Salt);
             }
@@ -128,7 +140,7 @@ public static void Ctor_GenerateEmptySalt()
         [Fact]
         public static void Ctor_IterationsRespected()
         {
-            using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt, 1))
+            using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt, 1, HashAlgorithmName.SHA1))
             {
                 Assert.Equal(1, deriveBytes.IterationCount);
             }
@@ -140,7 +152,7 @@ public static void GetSaltCopies()
             byte[] first;
             byte[] second;
 
-            using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt, DefaultIterationCount))
+            using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt, DefaultIterationCount, HashAlgorithmName.SHA1))
             {
                 first = deriveBytes.Salt;
                 second = deriveBytes.Salt;
@@ -155,7 +167,7 @@ public static void MinimumAcceptableInputs()
         {
             byte[] output;
 
-            using (var deriveBytes = new Rfc2898DeriveBytes("", Array.Empty<byte>(), 1))
+            using (var deriveBytes = new Rfc2898DeriveBytes("", Array.Empty<byte>(), 1, HashAlgorithmName.SHA1))
             {
                 output = deriveBytes.GetBytes(1);
                 Assert.Empty(deriveBytes.Salt);
@@ -168,7 +180,7 @@ public static void MinimumAcceptableInputs()
         [Fact]
         public static void GetBytes_ZeroLength()
         {
-            using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt))
+            using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt, DefaultIterationCount, HashAlgorithmName.SHA1))
             {
                 Assert.Throws<ArgumentOutOfRangeException>(() => deriveBytes.GetBytes(0));
             }
@@ -177,7 +189,7 @@ public static void GetBytes_ZeroLength()
         [Fact]
         public static void GetBytes_NegativeLength()
         {
-            Rfc2898DeriveBytes deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt);
+            Rfc2898DeriveBytes deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt, DefaultIterationCount, HashAlgorithmName.SHA1);
             Assert.Throws<ArgumentOutOfRangeException>(() => deriveBytes.GetBytes(-1));
             Assert.Throws<ArgumentOutOfRangeException>(() => deriveBytes.GetBytes(int.MinValue));
             Assert.Throws<ArgumentOutOfRangeException>(() => deriveBytes.GetBytes(int.MinValue / 2));
@@ -189,7 +201,7 @@ public static void GetBytes_NotIdempotent()
             byte[] first;
             byte[] second;
 
-            using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt))
+            using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt, DefaultIterationCount, HashAlgorithmName.SHA1))
             {
                 first = deriveBytes.GetBytes(32);
                 second = deriveBytes.GetBytes(32);
@@ -212,7 +224,7 @@ public static void GetBytes_StreamLike(int size)
         {
             byte[] first;
 
-            using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt))
+            using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt, DefaultIterationCount, HashAlgorithmName.SHA1))
             {
                 first = deriveBytes.GetBytes(size);
             }
@@ -220,7 +232,7 @@ public static void GetBytes_StreamLike(int size)
             byte[] second = new byte[first.Length];
 
             // Reset
-            using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt))
+            using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt, DefaultIterationCount, HashAlgorithmName.SHA1))
             {
                 byte[] secondFirstHalf = deriveBytes.GetBytes(first.Length / 2);
                 byte[] secondSecondHalf = deriveBytes.GetBytes(first.Length - secondFirstHalf.Length);
@@ -246,7 +258,7 @@ public static void GetBytes_StreamLike_OneAtATime(int size)
         {
             byte[] first;
 
-            using (var deriveBytes = new Rfc2898DeriveBytes(TestPasswordB, s_testSaltB))
+            using (var deriveBytes = new Rfc2898DeriveBytes(TestPasswordB, s_testSaltB, DefaultIterationCount, HashAlgorithmName.SHA1))
             {
                 first = deriveBytes.GetBytes(size);
             }
@@ -254,7 +266,7 @@ public static void GetBytes_StreamLike_OneAtATime(int size)
             byte[] second = new byte[first.Length];
 
             // Reset
-            using (var deriveBytes = new Rfc2898DeriveBytes(TestPasswordB, s_testSaltB))
+            using (var deriveBytes = new Rfc2898DeriveBytes(TestPasswordB, s_testSaltB, DefaultIterationCount, HashAlgorithmName.SHA1))
             {
                 for (int i = 0; i < second.Length; i++)
                 {
@@ -367,11 +379,13 @@ public static void CheckHashAlgorithmValue(string hashAlgorithmName)
         [Fact]
         public static void CryptDeriveKey_NotSupported()
         {
+#pragma warning disable SYSLIB0041 // Rfc2898DeriveBytes insecure constructor defaults
+#pragma warning disable SYSLIB0033 // Rfc2898DeriveBytes.CryptDeriveKey is obsolete
             using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt))
             {
-#pragma warning disable SYSLIB0033 // Rfc2898DeriveBytes.CryptDeriveKey is obsolete
                 Assert.Throws<PlatformNotSupportedException>(() => deriveBytes.CryptDeriveKey("RC2", "SHA1", 128, new byte[8]));
 #pragma warning restore SYSLIB0033
+#pragma warning restore SYSLIB0041
             }
         }
 
@@ -381,7 +395,7 @@ public static void GetBytes_ExceedCounterLimit()
             FieldInfo blockField = typeof(Rfc2898DeriveBytes).GetField("_block", BindingFlags.NonPublic | BindingFlags.Instance);
             Assert.NotNull(blockField);
 
-            using (Rfc2898DeriveBytes deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt))
+            using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt, 1, HashAlgorithmName.SHA1))
             {
                 // Set the internal block counter to be on the last possible block. This should succeed.
                 blockField.SetValue(deriveBytes, uint.MaxValue - 1);
@@ -398,12 +412,12 @@ public static void Ctor_PasswordMutatedAfterCreate()
             byte[] passwordBytes = Encoding.UTF8.GetBytes(TestPassword);
             byte[] derived;
 
-            using (Rfc2898DeriveBytes deriveBytes = new Rfc2898DeriveBytes(passwordBytes, s_testSaltB, DefaultIterationCount))
+            using (var deriveBytes = new Rfc2898DeriveBytes(passwordBytes, s_testSaltB, DefaultIterationCount, HashAlgorithmName.SHA1))
             {
                 derived = deriveBytes.GetBytes(64);
             }
 
-            using (Rfc2898DeriveBytes deriveBytes = new Rfc2898DeriveBytes(passwordBytes, s_testSaltB, DefaultIterationCount))
+            using (var deriveBytes = new Rfc2898DeriveBytes(passwordBytes, s_testSaltB, DefaultIterationCount, HashAlgorithmName.SHA1))
             {
                 passwordBytes[0] ^= 0xFF; // Flipping a byte after the object is constructed should not be observed.
 
@@ -418,7 +432,7 @@ public static void Ctor_PasswordBytes_NotCleared()
             byte[] passwordBytes = Encoding.UTF8.GetBytes(TestPassword);
             byte[] passwordBytesOriginal = passwordBytes.AsSpan().ToArray();
 
-            using (Rfc2898DeriveBytes deriveBytes = new Rfc2898DeriveBytes(passwordBytes, s_testSaltB, DefaultIterationCount))
+            using (var deriveBytes = new Rfc2898DeriveBytes(passwordBytes, s_testSaltB, DefaultIterationCount, HashAlgorithmName.SHA1))
             {
                 Assert.Equal(passwordBytesOriginal, passwordBytes);
             }
@@ -428,7 +442,7 @@ private static void TestKnownValue(string password, byte[] salt, int iterationCo
         {
             byte[] output;
 
-            using (var deriveBytes = new Rfc2898DeriveBytes(password, salt, iterationCount))
+            using (var deriveBytes = new Rfc2898DeriveBytes(password, salt, iterationCount, HashAlgorithmName.SHA1))
             {
                 output = deriveBytes.GetBytes(expected.Length);
             }