Skip to content

Commit e961b10

Browse files
authored
Obsolete outdated constructors on Rfc2898DeriveBytes
1 parent 0ceb116 commit e961b10

File tree

5 files changed

+57
-28
lines changed

5 files changed

+57
-28
lines changed

docs/project/list-of-diagnostics.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ The PR that reveals the implementation of the `<IncludeInternalObsoleteAttribute
9595
| __`SYSLIB0038`__ | SerializationFormat.Binary is obsolete and should not be used. See https://aka.ms/serializationformat-binary-obsolete for more information. |
9696
| __`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. |
9797
| __`SYSLIB0040`__ | EncryptionPolicy.NoEncryption and AllowEncryption significantly reduce security and should not be used in production code. |
98+
| __`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. |
9899

99100
## Analyzer Warnings
100101

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,5 +132,8 @@ internal static class Obsoletions
132132

133133
internal const string EncryptionPolicyMessage = "EncryptionPolicy.NoEncryption and AllowEncryption significantly reduce security and should not be used in production code.";
134134
internal const string EncryptionPolicyDiagId = "SYSLIB0040";
135+
136+
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.";
137+
internal const string Rfc2898OutdatedCtorDiagId = "SYSLIB0041";
135138
}
136139
}

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1716,12 +1716,17 @@ public override void GenerateKey() { }
17161716
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")]
17171717
public partial class Rfc2898DeriveBytes : System.Security.Cryptography.DeriveBytes
17181718
{
1719+
[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}")]
17191720
public Rfc2898DeriveBytes(byte[] password, byte[] salt, int iterations) { }
17201721
public Rfc2898DeriveBytes(byte[] password, byte[] salt, int iterations, System.Security.Cryptography.HashAlgorithmName hashAlgorithm) { }
1722+
[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}")]
17211723
public Rfc2898DeriveBytes(string password, byte[] salt) { }
1724+
[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}")]
17221725
public Rfc2898DeriveBytes(string password, byte[] salt, int iterations) { }
17231726
public Rfc2898DeriveBytes(string password, byte[] salt, int iterations, System.Security.Cryptography.HashAlgorithmName hashAlgorithm) { }
1727+
[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}")]
17241728
public Rfc2898DeriveBytes(string password, int saltSize) { }
1729+
[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}")]
17251730
public Rfc2898DeriveBytes(string password, int saltSize, int iterations) { }
17261731
public Rfc2898DeriveBytes(string password, int saltSize, int iterations, System.Security.Cryptography.HashAlgorithmName hashAlgorithm) { }
17271732
public System.Security.Cryptography.HashAlgorithmName HashAlgorithm { get { throw null; } }

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using System;
45
using System.Buffers;
56
using System.Buffers.Binary;
67
using System.Diagnostics;
@@ -29,6 +30,7 @@ public partial class Rfc2898DeriveBytes : DeriveBytes
2930
/// </summary>
3031
public HashAlgorithmName HashAlgorithm { get; }
3132

33+
[Obsolete(Obsoletions.Rfc2898OutdatedCtorMessage, DiagnosticId = Obsoletions.Rfc2898OutdatedCtorDiagId, UrlFormat = Obsoletions.SharedUrlFormat)]
3234
public Rfc2898DeriveBytes(byte[] password, byte[] salt, int iterations)
3335
: this(password, salt, iterations, HashAlgorithmName.SHA1)
3436
{
@@ -39,11 +41,13 @@ public Rfc2898DeriveBytes(byte[] password, byte[] salt, int iterations, HashAlgo
3941
{
4042
}
4143

44+
[Obsolete(Obsoletions.Rfc2898OutdatedCtorMessage, DiagnosticId = Obsoletions.Rfc2898OutdatedCtorDiagId, UrlFormat = Obsoletions.SharedUrlFormat)]
4245
public Rfc2898DeriveBytes(string password, byte[] salt)
4346
: this(password, salt, 1000)
4447
{
4548
}
4649

50+
[Obsolete(Obsoletions.Rfc2898OutdatedCtorMessage, DiagnosticId = Obsoletions.Rfc2898OutdatedCtorDiagId, UrlFormat = Obsoletions.SharedUrlFormat)]
4751
public Rfc2898DeriveBytes(string password, byte[] salt, int iterations)
4852
: this(password, salt, iterations, HashAlgorithmName.SHA1)
4953
{
@@ -54,11 +58,13 @@ public Rfc2898DeriveBytes(string password, byte[] salt, int iterations, HashAlgo
5458
{
5559
}
5660

61+
[Obsolete(Obsoletions.Rfc2898OutdatedCtorMessage, DiagnosticId = Obsoletions.Rfc2898OutdatedCtorDiagId, UrlFormat = Obsoletions.SharedUrlFormat)]
5762
public Rfc2898DeriveBytes(string password, int saltSize)
5863
: this(password, saltSize, 1000)
5964
{
6065
}
6166

67+
[Obsolete(Obsoletions.Rfc2898OutdatedCtorMessage, DiagnosticId = Obsoletions.Rfc2898OutdatedCtorDiagId, UrlFormat = Obsoletions.SharedUrlFormat)]
6268
public Rfc2898DeriveBytes(string password, int saltSize, int iterations)
6369
: this(password, saltSize, iterations, HashAlgorithmName.SHA1)
6470
{

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

Lines changed: 42 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -21,41 +21,51 @@ public class Rfc2898Tests
2121
[Fact]
2222
public static void Ctor_NullPasswordBytes()
2323
{
24-
Assert.Throws<NullReferenceException>(() => new Rfc2898DeriveBytes((byte[])null, s_testSalt, DefaultIterationCount));
24+
Assert.Throws<NullReferenceException>(() =>
25+
new Rfc2898DeriveBytes((byte[])null, s_testSalt, DefaultIterationCount, HashAlgorithmName.SHA1));
2526
}
2627

2728
[Fact]
2829
public static void Ctor_NullPasswordString()
2930
{
30-
Assert.Throws<ArgumentNullException>(() => new Rfc2898DeriveBytes((string)null, s_testSalt, DefaultIterationCount));
31+
Assert.Throws<ArgumentNullException>(() =>
32+
new Rfc2898DeriveBytes((string)null, s_testSalt, DefaultIterationCount, HashAlgorithmName.SHA1));
3133
}
3234

3335
[Fact]
3436
public static void Ctor_NullSalt()
3537
{
36-
Assert.Throws<ArgumentNullException>(() => new Rfc2898DeriveBytes(TestPassword, null, DefaultIterationCount));
38+
AssertExtensions.Throws<ArgumentNullException>("salt", () =>
39+
new Rfc2898DeriveBytes(TestPassword, null, DefaultIterationCount, HashAlgorithmName.SHA1));
3740
}
3841

3942
[Fact]
4043
public static void Ctor_GenerateNegativeSalt()
4144
{
42-
Assert.Throws<ArgumentOutOfRangeException>(() => new Rfc2898DeriveBytes(TestPassword, -1));
43-
Assert.Throws<ArgumentOutOfRangeException>(() => new Rfc2898DeriveBytes(TestPassword, int.MinValue));
44-
Assert.Throws<ArgumentOutOfRangeException>(() => new Rfc2898DeriveBytes(TestPassword, int.MinValue / 2));
45+
AssertExtensions.Throws<ArgumentOutOfRangeException>("saltSize", () =>
46+
new Rfc2898DeriveBytes(TestPassword, -1, DefaultIterationCount, HashAlgorithmName.SHA1));
47+
AssertExtensions.Throws<ArgumentOutOfRangeException>("saltSize", () =>
48+
new Rfc2898DeriveBytes(TestPassword, int.MinValue, DefaultIterationCount, HashAlgorithmName.SHA1));
49+
AssertExtensions.Throws<ArgumentOutOfRangeException>("saltSize", () =>
50+
new Rfc2898DeriveBytes(TestPassword, int.MinValue / 2, DefaultIterationCount, HashAlgorithmName.SHA1));
4551
}
4652

4753
[Fact]
4854
public static void Ctor_TooFewIterations()
4955
{
50-
Assert.Throws<ArgumentOutOfRangeException>(() => new Rfc2898DeriveBytes(TestPassword, s_testSalt, 0));
56+
AssertExtensions.Throws<ArgumentOutOfRangeException>("iterations", () =>
57+
new Rfc2898DeriveBytes(TestPassword, s_testSalt, 0, HashAlgorithmName.SHA1));
5158
}
5259

5360
[Fact]
5461
public static void Ctor_NegativeIterations()
5562
{
56-
Assert.Throws<ArgumentOutOfRangeException>(() => new Rfc2898DeriveBytes(TestPassword, s_testSalt, -1));
57-
Assert.Throws<ArgumentOutOfRangeException>(() => new Rfc2898DeriveBytes(TestPassword, s_testSalt, int.MinValue));
58-
Assert.Throws<ArgumentOutOfRangeException>(() => new Rfc2898DeriveBytes(TestPassword, s_testSalt, int.MinValue / 2));
63+
AssertExtensions.Throws<ArgumentOutOfRangeException>("iterations", () =>
64+
new Rfc2898DeriveBytes(TestPassword, s_testSalt, -1, HashAlgorithmName.SHA1));
65+
AssertExtensions.Throws<ArgumentOutOfRangeException>("iterations", () =>
66+
new Rfc2898DeriveBytes(TestPassword, s_testSalt, int.MinValue, HashAlgorithmName.SHA1));
67+
AssertExtensions.Throws<ArgumentOutOfRangeException>("iterations", () =>
68+
new Rfc2898DeriveBytes(TestPassword, s_testSalt, int.MinValue / 2, HashAlgorithmName.SHA1));
5969
}
6070

6171
[Fact]
@@ -90,7 +100,7 @@ public static void Ctor_SaltCopied()
90100
{
91101
byte[] saltIn = (byte[])s_testSalt.Clone();
92102

93-
using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, saltIn, DefaultIterationCount))
103+
using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, saltIn, DefaultIterationCount, HashAlgorithmName.SHA1))
94104
{
95105
byte[] saltOut = deriveBytes.Salt;
96106

@@ -110,7 +120,9 @@ public static void Ctor_SaltCopied()
110120
[Fact]
111121
public static void Ctor_DefaultIterations()
112122
{
123+
#pragma warning disable SYSLIB0041 // Rfc2898DeriveBytes insecure constructor defaults
113124
using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt))
125+
#pragma warning restore SYSLIB0041
114126
{
115127
Assert.Equal(DefaultIterationCount, deriveBytes.IterationCount);
116128
}
@@ -119,7 +131,7 @@ public static void Ctor_DefaultIterations()
119131
[Fact]
120132
public static void Ctor_GenerateEmptySalt()
121133
{
122-
using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, 0, 1))
134+
using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, 0, 1, HashAlgorithmName.SHA1))
123135
{
124136
Assert.Empty(deriveBytes.Salt);
125137
}
@@ -128,7 +140,7 @@ public static void Ctor_GenerateEmptySalt()
128140
[Fact]
129141
public static void Ctor_IterationsRespected()
130142
{
131-
using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt, 1))
143+
using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt, 1, HashAlgorithmName.SHA1))
132144
{
133145
Assert.Equal(1, deriveBytes.IterationCount);
134146
}
@@ -140,7 +152,7 @@ public static void GetSaltCopies()
140152
byte[] first;
141153
byte[] second;
142154

143-
using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt, DefaultIterationCount))
155+
using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt, DefaultIterationCount, HashAlgorithmName.SHA1))
144156
{
145157
first = deriveBytes.Salt;
146158
second = deriveBytes.Salt;
@@ -155,7 +167,7 @@ public static void MinimumAcceptableInputs()
155167
{
156168
byte[] output;
157169

158-
using (var deriveBytes = new Rfc2898DeriveBytes("", Array.Empty<byte>(), 1))
170+
using (var deriveBytes = new Rfc2898DeriveBytes("", Array.Empty<byte>(), 1, HashAlgorithmName.SHA1))
159171
{
160172
output = deriveBytes.GetBytes(1);
161173
Assert.Empty(deriveBytes.Salt);
@@ -168,7 +180,7 @@ public static void MinimumAcceptableInputs()
168180
[Fact]
169181
public static void GetBytes_ZeroLength()
170182
{
171-
using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt))
183+
using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt, DefaultIterationCount, HashAlgorithmName.SHA1))
172184
{
173185
Assert.Throws<ArgumentOutOfRangeException>(() => deriveBytes.GetBytes(0));
174186
}
@@ -177,7 +189,7 @@ public static void GetBytes_ZeroLength()
177189
[Fact]
178190
public static void GetBytes_NegativeLength()
179191
{
180-
Rfc2898DeriveBytes deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt);
192+
Rfc2898DeriveBytes deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt, DefaultIterationCount, HashAlgorithmName.SHA1);
181193
Assert.Throws<ArgumentOutOfRangeException>(() => deriveBytes.GetBytes(-1));
182194
Assert.Throws<ArgumentOutOfRangeException>(() => deriveBytes.GetBytes(int.MinValue));
183195
Assert.Throws<ArgumentOutOfRangeException>(() => deriveBytes.GetBytes(int.MinValue / 2));
@@ -189,7 +201,7 @@ public static void GetBytes_NotIdempotent()
189201
byte[] first;
190202
byte[] second;
191203

192-
using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt))
204+
using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt, DefaultIterationCount, HashAlgorithmName.SHA1))
193205
{
194206
first = deriveBytes.GetBytes(32);
195207
second = deriveBytes.GetBytes(32);
@@ -212,15 +224,15 @@ public static void GetBytes_StreamLike(int size)
212224
{
213225
byte[] first;
214226

215-
using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt))
227+
using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt, DefaultIterationCount, HashAlgorithmName.SHA1))
216228
{
217229
first = deriveBytes.GetBytes(size);
218230
}
219231

220232
byte[] second = new byte[first.Length];
221233

222234
// Reset
223-
using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt))
235+
using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt, DefaultIterationCount, HashAlgorithmName.SHA1))
224236
{
225237
byte[] secondFirstHalf = deriveBytes.GetBytes(first.Length / 2);
226238
byte[] secondSecondHalf = deriveBytes.GetBytes(first.Length - secondFirstHalf.Length);
@@ -246,15 +258,15 @@ public static void GetBytes_StreamLike_OneAtATime(int size)
246258
{
247259
byte[] first;
248260

249-
using (var deriveBytes = new Rfc2898DeriveBytes(TestPasswordB, s_testSaltB))
261+
using (var deriveBytes = new Rfc2898DeriveBytes(TestPasswordB, s_testSaltB, DefaultIterationCount, HashAlgorithmName.SHA1))
250262
{
251263
first = deriveBytes.GetBytes(size);
252264
}
253265

254266
byte[] second = new byte[first.Length];
255267

256268
// Reset
257-
using (var deriveBytes = new Rfc2898DeriveBytes(TestPasswordB, s_testSaltB))
269+
using (var deriveBytes = new Rfc2898DeriveBytes(TestPasswordB, s_testSaltB, DefaultIterationCount, HashAlgorithmName.SHA1))
258270
{
259271
for (int i = 0; i < second.Length; i++)
260272
{
@@ -367,11 +379,13 @@ public static void CheckHashAlgorithmValue(string hashAlgorithmName)
367379
[Fact]
368380
public static void CryptDeriveKey_NotSupported()
369381
{
382+
#pragma warning disable SYSLIB0041 // Rfc2898DeriveBytes insecure constructor defaults
383+
#pragma warning disable SYSLIB0033 // Rfc2898DeriveBytes.CryptDeriveKey is obsolete
370384
using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt))
371385
{
372-
#pragma warning disable SYSLIB0033 // Rfc2898DeriveBytes.CryptDeriveKey is obsolete
373386
Assert.Throws<PlatformNotSupportedException>(() => deriveBytes.CryptDeriveKey("RC2", "SHA1", 128, new byte[8]));
374387
#pragma warning restore SYSLIB0033
388+
#pragma warning restore SYSLIB0041
375389
}
376390
}
377391

@@ -381,7 +395,7 @@ public static void GetBytes_ExceedCounterLimit()
381395
FieldInfo blockField = typeof(Rfc2898DeriveBytes).GetField("_block", BindingFlags.NonPublic | BindingFlags.Instance);
382396
Assert.NotNull(blockField);
383397

384-
using (Rfc2898DeriveBytes deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt))
398+
using (var deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt, 1, HashAlgorithmName.SHA1))
385399
{
386400
// Set the internal block counter to be on the last possible block. This should succeed.
387401
blockField.SetValue(deriveBytes, uint.MaxValue - 1);
@@ -398,12 +412,12 @@ public static void Ctor_PasswordMutatedAfterCreate()
398412
byte[] passwordBytes = Encoding.UTF8.GetBytes(TestPassword);
399413
byte[] derived;
400414

401-
using (Rfc2898DeriveBytes deriveBytes = new Rfc2898DeriveBytes(passwordBytes, s_testSaltB, DefaultIterationCount))
415+
using (var deriveBytes = new Rfc2898DeriveBytes(passwordBytes, s_testSaltB, DefaultIterationCount, HashAlgorithmName.SHA1))
402416
{
403417
derived = deriveBytes.GetBytes(64);
404418
}
405419

406-
using (Rfc2898DeriveBytes deriveBytes = new Rfc2898DeriveBytes(passwordBytes, s_testSaltB, DefaultIterationCount))
420+
using (var deriveBytes = new Rfc2898DeriveBytes(passwordBytes, s_testSaltB, DefaultIterationCount, HashAlgorithmName.SHA1))
407421
{
408422
passwordBytes[0] ^= 0xFF; // Flipping a byte after the object is constructed should not be observed.
409423

@@ -418,7 +432,7 @@ public static void Ctor_PasswordBytes_NotCleared()
418432
byte[] passwordBytes = Encoding.UTF8.GetBytes(TestPassword);
419433
byte[] passwordBytesOriginal = passwordBytes.AsSpan().ToArray();
420434

421-
using (Rfc2898DeriveBytes deriveBytes = new Rfc2898DeriveBytes(passwordBytes, s_testSaltB, DefaultIterationCount))
435+
using (var deriveBytes = new Rfc2898DeriveBytes(passwordBytes, s_testSaltB, DefaultIterationCount, HashAlgorithmName.SHA1))
422436
{
423437
Assert.Equal(passwordBytesOriginal, passwordBytes);
424438
}
@@ -428,7 +442,7 @@ private static void TestKnownValue(string password, byte[] salt, int iterationCo
428442
{
429443
byte[] output;
430444

431-
using (var deriveBytes = new Rfc2898DeriveBytes(password, salt, iterationCount))
445+
using (var deriveBytes = new Rfc2898DeriveBytes(password, salt, iterationCount, HashAlgorithmName.SHA1))
432446
{
433447
output = deriveBytes.GetBytes(expected.Length);
434448
}

0 commit comments

Comments
 (0)