From 4662645425b0142829e93376f11c367c7748e120 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Tue, 21 Jan 2025 15:50:07 -0500 Subject: [PATCH 1/4] Obsolete all constructors for Rfc2898DeriveBytes --- docs/project/list-of-diagnostics.md | 1 + src/libraries/Common/src/System/Obsoletions.cs | 6 ++++-- .../Cryptography/PasswordBasedEncryption.cs | 4 ++++ .../ref/System.Security.Cryptography.cs | 13 ++++++++----- .../Security/Cryptography/Rfc2898DeriveBytes.cs | 13 ++++++++----- .../tests/Rfc2898OneShotTests.cs | 4 ++++ .../tests/Rfc2898Tests.cs | 8 +++----- 7 files changed, 32 insertions(+), 17 deletions(-) diff --git a/docs/project/list-of-diagnostics.md b/docs/project/list-of-diagnostics.md index 0e3f10d2df6c3d..16cae97e66d345 100644 --- a/docs/project/list-of-diagnostics.md +++ b/docs/project/list-of-diagnostics.md @@ -114,6 +114,7 @@ The PR that reveals the implementation of the ` public HashAlgorithmName HashAlgorithm { get; } - [Obsolete(Obsoletions.Rfc2898OutdatedCtorMessage, DiagnosticId = Obsoletions.Rfc2898OutdatedCtorDiagId, UrlFormat = Obsoletions.SharedUrlFormat)] + [Obsolete(Obsoletions.Rfc2898DeriveBytesCtorMessage, DiagnosticId = Obsoletions.Rfc2898DeriveBytesCtorDiagId, UrlFormat = Obsoletions.SharedUrlFormat)] public Rfc2898DeriveBytes(byte[] password, byte[] salt, int iterations) : this(password, salt, iterations, HashAlgorithmName.SHA1) { } + [Obsolete(Obsoletions.Rfc2898DeriveBytesCtorMessage, DiagnosticId = Obsoletions.Rfc2898DeriveBytesCtorDiagId, UrlFormat = Obsoletions.SharedUrlFormat)] public Rfc2898DeriveBytes(byte[] password, byte[] salt, int iterations, HashAlgorithmName hashAlgorithm) : this(password, salt, iterations, hashAlgorithm, clearPassword: false) { } - [Obsolete(Obsoletions.Rfc2898OutdatedCtorMessage, DiagnosticId = Obsoletions.Rfc2898OutdatedCtorDiagId, UrlFormat = Obsoletions.SharedUrlFormat)] + [Obsolete(Obsoletions.Rfc2898DeriveBytesCtorMessage, DiagnosticId = Obsoletions.Rfc2898DeriveBytesCtorDiagId, UrlFormat = Obsoletions.SharedUrlFormat)] public Rfc2898DeriveBytes(string password, byte[] salt) : this(password, salt, 1000) { } - [Obsolete(Obsoletions.Rfc2898OutdatedCtorMessage, DiagnosticId = Obsoletions.Rfc2898OutdatedCtorDiagId, UrlFormat = Obsoletions.SharedUrlFormat)] + [Obsolete(Obsoletions.Rfc2898DeriveBytesCtorMessage, DiagnosticId = Obsoletions.Rfc2898DeriveBytesCtorDiagId, UrlFormat = Obsoletions.SharedUrlFormat)] public Rfc2898DeriveBytes(string password, byte[] salt, int iterations) : this(password, salt, iterations, HashAlgorithmName.SHA1) { } + [Obsolete(Obsoletions.Rfc2898DeriveBytesCtorMessage, DiagnosticId = Obsoletions.Rfc2898DeriveBytesCtorDiagId, UrlFormat = Obsoletions.SharedUrlFormat)] public Rfc2898DeriveBytes(string password, byte[] salt, int iterations, HashAlgorithmName hashAlgorithm) : this( Encoding.UTF8.GetBytes(password ?? throw new ArgumentNullException(nameof(password))), @@ -62,18 +64,19 @@ public Rfc2898DeriveBytes(string password, byte[] salt, int iterations, HashAlgo { } - [Obsolete(Obsoletions.Rfc2898OutdatedCtorMessage, DiagnosticId = Obsoletions.Rfc2898OutdatedCtorDiagId, UrlFormat = Obsoletions.SharedUrlFormat)] + [Obsolete(Obsoletions.Rfc2898DeriveBytesCtorMessage, DiagnosticId = Obsoletions.Rfc2898DeriveBytesCtorDiagId, UrlFormat = Obsoletions.SharedUrlFormat)] public Rfc2898DeriveBytes(string password, int saltSize) : this(password, saltSize, 1000) { } - [Obsolete(Obsoletions.Rfc2898OutdatedCtorMessage, DiagnosticId = Obsoletions.Rfc2898OutdatedCtorDiagId, UrlFormat = Obsoletions.SharedUrlFormat)] + [Obsolete(Obsoletions.Rfc2898DeriveBytesCtorMessage, DiagnosticId = Obsoletions.Rfc2898DeriveBytesCtorDiagId, UrlFormat = Obsoletions.SharedUrlFormat)] public Rfc2898DeriveBytes(string password, int saltSize, int iterations) : this(password, saltSize, iterations, HashAlgorithmName.SHA1) { } + [Obsolete(Obsoletions.Rfc2898DeriveBytesCtorMessage, DiagnosticId = Obsoletions.Rfc2898DeriveBytesCtorDiagId, UrlFormat = Obsoletions.SharedUrlFormat)] public Rfc2898DeriveBytes(string password, int saltSize, int iterations, HashAlgorithmName hashAlgorithm) { ArgumentOutOfRangeException.ThrowIfNegative(saltSize); diff --git a/src/libraries/System.Security.Cryptography/tests/Rfc2898OneShotTests.cs b/src/libraries/System.Security.Cryptography/tests/Rfc2898OneShotTests.cs index a5dbee9dc2259b..41f3b2444f1a33 100644 --- a/src/libraries/System.Security.Cryptography/tests/Rfc2898OneShotTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/Rfc2898OneShotTests.cs @@ -222,7 +222,9 @@ public static void Pbkdf2_PasswordBytes_Compare( HashAlgorithmName hashAlgorithmName = new HashAlgorithmName(hashAlgorithm); byte[] key1; +#pragma warning disable SYSLIB0060 // Creating instances of Rfc2898DeriveBytes is obsolete. using (Rfc2898DeriveBytes instanceKdf = new Rfc2898DeriveBytes(password, salt, iterations, hashAlgorithmName)) +#pragma warning restore SYSLIB0060 { key1 = instanceKdf.GetBytes(length); } @@ -253,7 +255,9 @@ public static void Pbkdf2_PasswordString_Compare( HashAlgorithmName hashAlgorithmName = new HashAlgorithmName(hashAlgorithm); byte[] key1; +#pragma warning disable SYSLIB0060 // Creating instances of Rfc2898DeriveBytes is obsolete. using (Rfc2898DeriveBytes instanceKdf = new Rfc2898DeriveBytes(password, salt, iterations, hashAlgorithmName)) +#pragma warning restore SYSLIB0060 { key1 = instanceKdf.GetBytes(length); } diff --git a/src/libraries/System.Security.Cryptography/tests/Rfc2898Tests.cs b/src/libraries/System.Security.Cryptography/tests/Rfc2898Tests.cs index 4342bebb14072d..e85bf8baae70a9 100644 --- a/src/libraries/System.Security.Cryptography/tests/Rfc2898Tests.cs +++ b/src/libraries/System.Security.Cryptography/tests/Rfc2898Tests.cs @@ -7,6 +7,8 @@ using Test.Cryptography; using Xunit; +#pragma warning disable SYSLIB0060 // Rfc2898DeriveBytes constructors are obsolete. + namespace System.Security.Cryptography { public class Rfc2898Tests @@ -119,9 +121,7 @@ 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); } @@ -373,13 +373,11 @@ public static void CheckHashAlgorithmValue(HashAlgorithmName hashAlgorithm) [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(() => deriveBytes.CryptDeriveKey("RC2", "SHA1", 128, new byte[8])); #pragma warning restore SYSLIB0033 -#pragma warning restore SYSLIB0041 } } From e0d99e23b4d314ee64173553eecc5cbd3a1d66ae Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Tue, 21 Jan 2025 22:04:48 -0500 Subject: [PATCH 2/4] Feedback on obsolete message. Co-authored-by: Stephen Toub --- docs/project/list-of-diagnostics.md | 2 +- src/libraries/Common/src/System/Obsoletions.cs | 2 +- .../ref/System.Security.Cryptography.cs | 16 ++++++++-------- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/docs/project/list-of-diagnostics.md b/docs/project/list-of-diagnostics.md index 16cae97e66d345..56aac0743803e5 100644 --- a/docs/project/list-of-diagnostics.md +++ b/docs/project/list-of-diagnostics.md @@ -114,7 +114,7 @@ The PR that reveals the implementation of the ` Date: Tue, 21 Jan 2025 22:07:34 -0500 Subject: [PATCH 3/4] Update comments to reflect new obsolete message. --- .../System.Security.Cryptography/tests/Rfc2898OneShotTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Security.Cryptography/tests/Rfc2898OneShotTests.cs b/src/libraries/System.Security.Cryptography/tests/Rfc2898OneShotTests.cs index 41f3b2444f1a33..3a54623527d8d6 100644 --- a/src/libraries/System.Security.Cryptography/tests/Rfc2898OneShotTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/Rfc2898OneShotTests.cs @@ -222,7 +222,7 @@ public static void Pbkdf2_PasswordBytes_Compare( HashAlgorithmName hashAlgorithmName = new HashAlgorithmName(hashAlgorithm); byte[] key1; -#pragma warning disable SYSLIB0060 // Creating instances of Rfc2898DeriveBytes is obsolete. +#pragma warning disable SYSLIB0060 // The constructors on Rfc2898DeriveBytes are obsolete. using (Rfc2898DeriveBytes instanceKdf = new Rfc2898DeriveBytes(password, salt, iterations, hashAlgorithmName)) #pragma warning restore SYSLIB0060 { @@ -255,7 +255,7 @@ public static void Pbkdf2_PasswordString_Compare( HashAlgorithmName hashAlgorithmName = new HashAlgorithmName(hashAlgorithm); byte[] key1; -#pragma warning disable SYSLIB0060 // Creating instances of Rfc2898DeriveBytes is obsolete. +#pragma warning disable SYSLIB0060 // The constructors on Rfc2898DeriveBytes are obsolete. using (Rfc2898DeriveBytes instanceKdf = new Rfc2898DeriveBytes(password, salt, iterations, hashAlgorithmName)) #pragma warning restore SYSLIB0060 { From 9b714a8618cc38aa22189870e39845a9a83a0b70 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Wed, 22 Jan 2025 23:38:36 -0500 Subject: [PATCH 4/4] Update Obsoletions.cs Co-authored-by: Stephen Toub --- src/libraries/Common/src/System/Obsoletions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Common/src/System/Obsoletions.cs b/src/libraries/Common/src/System/Obsoletions.cs index ac35f87f147d60..d79c8a8de4366e 100644 --- a/src/libraries/Common/src/System/Obsoletions.cs +++ b/src/libraries/Common/src/System/Obsoletions.cs @@ -133,7 +133,7 @@ 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"; - // SYSLIB0041 is no longer used and superseded by SYSLIB0060. + // SYSLIB0041 is no longer used and is superseded by SYSLIB0060. internal const string EccXmlExportImportMessage = "ToXmlString and FromXmlString have no implementation for ECC types, and are obsolete. Use a standard import and export format such as ExportSubjectPublicKeyInfo or ImportSubjectPublicKeyInfo for public keys and ExportPkcs8PrivateKey or ImportPkcs8PrivateKey for private keys."; internal const string EccXmlExportImportDiagId = "SYSLIB0042";