From 16b612a72db771a921259b5aba7f27563b2ccd35 Mon Sep 17 00:00:00 2001 From: Jeff Handley Date: Thu, 6 May 2021 02:03:55 -0700 Subject: [PATCH 1/5] Mark RNGCryptoServiceProvider as Obsolete --- docs/project/list-of-diagnostics.md | 1 + src/libraries/Common/src/System/Obsoletions.cs | 3 +++ src/libraries/Directory.Build.targets | 8 ++++++-- .../NamedPipeTest.CurrentUserOnly.Windows.cs | 2 +- .../ref/System.Security.Cryptography.Csp.cs | 1 + .../src/System.Security.Cryptography.Csp.csproj | 2 ++ .../Security/Cryptography/RNGCryptoServiceProvider.cs | 1 + .../tests/RNGCryptoServiceProviderTests.cs | 4 ++++ .../tests/WindowsIdentityImpersonatedTests.netcoreapp.cs | 2 +- 9 files changed, 20 insertions(+), 4 deletions(-) diff --git a/docs/project/list-of-diagnostics.md b/docs/project/list-of-diagnostics.md index f12b4ef549192a..246d178d0be7c9 100644 --- a/docs/project/list-of-diagnostics.md +++ b/docs/project/list-of-diagnostics.md @@ -70,6 +70,7 @@ The PR that reveals the implementation of the `$(NoWarn);nullable $(NoWarn);nullable;CA1052 - $(NoWarn);SYSLIB0003;SYSLIB0004;SYSLIB0015;SYSLIB0017 + SYSLIB0003: Code Access Security (CAS). + SYSLIB0004: Constrained Execution Region (CER). + SYSLIB0017: Strong name signing. + SYSLIB0023: RNGCryptoServiceProvider. + --> + $(NoWarn);SYSLIB0003;SYSLIB0004;SYSLIB0015;SYSLIB0017;SYSLIB0023 diff --git a/src/libraries/System.IO.Pipes/tests/NamedPipeTests/NamedPipeTest.CurrentUserOnly.Windows.cs b/src/libraries/System.IO.Pipes/tests/NamedPipeTests/NamedPipeTest.CurrentUserOnly.Windows.cs index 08c45dee85cef0..a95bc722603ba2 100644 --- a/src/libraries/System.IO.Pipes/tests/NamedPipeTests/NamedPipeTest.CurrentUserOnly.Windows.cs +++ b/src/libraries/System.IO.Pipes/tests/NamedPipeTests/NamedPipeTest.CurrentUserOnly.Windows.cs @@ -22,7 +22,7 @@ public class TestAccountImpersonator : IDisposable public TestAccountImpersonator() { string testAccountPassword; - using (RandomNumberGenerator rng = new RNGCryptoServiceProvider()) + using (RandomNumberGenerator rng = RandomNumberGenerator.Create()) { var randomBytes = new byte[33]; rng.GetBytes(randomBytes); diff --git a/src/libraries/System.Security.Cryptography.Csp/ref/System.Security.Cryptography.Csp.cs b/src/libraries/System.Security.Cryptography.Csp/ref/System.Security.Cryptography.Csp.cs index a992ffcf0392fb..0a889696cf46ab 100644 --- a/src/libraries/System.Security.Cryptography.Csp/ref/System.Security.Cryptography.Csp.cs +++ b/src/libraries/System.Security.Cryptography.Csp/ref/System.Security.Cryptography.Csp.cs @@ -175,6 +175,7 @@ public RC2CryptoServiceProvider() { } public override void GenerateIV() { } public override void GenerateKey() { } } + [System.ObsoleteAttribute("RNGCryptoServiceProvider is obsolete. Use RandomNumberGenerator.Create() instead.", DiagnosticId = "SYSLIB0023", UrlFormat = "https://aka.ms/dotnet-warnings/{0}")] [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Never)] public sealed partial class RNGCryptoServiceProvider : System.Security.Cryptography.RandomNumberGenerator { diff --git a/src/libraries/System.Security.Cryptography.Csp/src/System.Security.Cryptography.Csp.csproj b/src/libraries/System.Security.Cryptography.Csp/src/System.Security.Cryptography.Csp.csproj index da68c42443e032..2e8119ae25704b 100644 --- a/src/libraries/System.Security.Cryptography.Csp/src/System.Security.Cryptography.Csp.csproj +++ b/src/libraries/System.Security.Cryptography.Csp/src/System.Security.Cryptography.Csp.csproj @@ -27,6 +27,8 @@ + @@ -120,3 +122,5 @@ public static void VerifyCtors() } } } + +#pragma warning restore SYSLIB0023 diff --git a/src/libraries/System.Security.Principal.Windows/tests/WindowsIdentityImpersonatedTests.netcoreapp.cs b/src/libraries/System.Security.Principal.Windows/tests/WindowsIdentityImpersonatedTests.netcoreapp.cs index 02846c7bffab77..868afcc8c1a4f5 100644 --- a/src/libraries/System.Security.Principal.Windows/tests/WindowsIdentityImpersonatedTests.netcoreapp.cs +++ b/src/libraries/System.Security.Principal.Windows/tests/WindowsIdentityImpersonatedTests.netcoreapp.cs @@ -128,7 +128,7 @@ public WindowsTestAccount(string userName) private void CreateUser() { string testAccountPassword; - using (RandomNumberGenerator rng = new RNGCryptoServiceProvider()) + using (RandomNumberGenerator rng = RandomNumberGenerator.Create()) { byte[] randomBytes = new byte[33]; rng.GetBytes(randomBytes); From 62521ee6836dff965cf4f507942c84aa037a6767 Mon Sep 17 00:00:00 2001 From: Jeff Handley Date: Thu, 6 May 2021 18:43:19 -0700 Subject: [PATCH 2/5] Use the static methods on RandomNumberGenerator instead of Create() --- docs/project/list-of-diagnostics.md | 2 +- src/libraries/Common/src/System/Obsoletions.cs | 2 +- .../NamedPipeTest.CurrentUserOnly.Windows.cs | 10 +++------- .../ref/System.Security.Cryptography.Csp.cs | 2 +- 4 files changed, 6 insertions(+), 10 deletions(-) diff --git a/docs/project/list-of-diagnostics.md b/docs/project/list-of-diagnostics.md index dbec31f9e7335b..78509d235f7bc8 100644 --- a/docs/project/list-of-diagnostics.md +++ b/docs/project/list-of-diagnostics.md @@ -76,7 +76,7 @@ The PR that reveals the implementation of the ` Date: Thu, 6 May 2021 18:58:31 -0700 Subject: [PATCH 3/5] Use RandomNumberGenerator.GetBytes() instead of RandomNumberGenerator.Create().GetBytes() --- ...owsIdentityImpersonatedTests.netcoreapp.cs | 89 +++++++++---------- 1 file changed, 42 insertions(+), 47 deletions(-) diff --git a/src/libraries/System.Security.Principal.Windows/tests/WindowsIdentityImpersonatedTests.netcoreapp.cs b/src/libraries/System.Security.Principal.Windows/tests/WindowsIdentityImpersonatedTests.netcoreapp.cs index 868afcc8c1a4f5..b47921dfa84976 100644 --- a/src/libraries/System.Security.Principal.Windows/tests/WindowsIdentityImpersonatedTests.netcoreapp.cs +++ b/src/libraries/System.Security.Principal.Windows/tests/WindowsIdentityImpersonatedTests.netcoreapp.cs @@ -127,62 +127,57 @@ public WindowsTestAccount(string userName) private void CreateUser() { - string testAccountPassword; - using (RandomNumberGenerator rng = RandomNumberGenerator.Create()) - { - byte[] randomBytes = new byte[33]; - rng.GetBytes(randomBytes); + byte[] randomBytes = RandomNumberGenerator.GetBytes(33); - // Add special chars to ensure it satisfies password requirements. - testAccountPassword = Convert.ToBase64String(randomBytes) + "_-As@!%*(1)4#2"; + // Add special chars to ensure it satisfies password requirements. + string testAccountPassword = Convert.ToBase64String(randomBytes) + "_-As@!%*(1)4#2"; - USER_INFO_1 userInfo = new USER_INFO_1 - { - usri1_name = _userName, - usri1_password = testAccountPassword, - usri1_priv = 1 - }; + USER_INFO_1 userInfo = new USER_INFO_1 + { + usri1_name = _userName, + usri1_password = testAccountPassword, + usri1_priv = 1 + }; - // Create user and remove/create if already exists - uint result = NetUserAdd(null, 1, ref userInfo, out uint param_err); + // Create user and remove/create if already exists + uint result = NetUserAdd(null, 1, ref userInfo, out uint param_err); - // error codes https://docs.microsoft.com/en-us/windows/desktop/netmgmt/network-management-error-codes - // 0 == NERR_Success - if (result == 2224) // NERR_UserExists + // error codes https://docs.microsoft.com/en-us/windows/desktop/netmgmt/network-management-error-codes + // 0 == NERR_Success + if (result == 2224) // NERR_UserExists + { + result = NetUserDel(null, userInfo.usri1_name); + if (result != 0) { - result = NetUserDel(null, userInfo.usri1_name); - if (result != 0) - { - throw new Win32Exception((int)result); - } - result = NetUserAdd(null, 1, ref userInfo, out param_err); - if (result != 0) - { - throw new Win32Exception((int)result); - } + throw new Win32Exception((int)result); } - - const int LOGON32_PROVIDER_DEFAULT = 0; - const int LOGON32_LOGON_INTERACTIVE = 2; - - if (!LogonUser(_userName, ".", testAccountPassword, LOGON32_LOGON_INTERACTIVE, LOGON32_PROVIDER_DEFAULT, out _accountTokenHandle)) + result = NetUserAdd(null, 1, ref userInfo, out param_err); + if (result != 0) { - _accountTokenHandle = null; - throw new Exception($"Failed to get SafeAccessTokenHandle for test account {_userName}", new Win32Exception()); + throw new Win32Exception((int)result); } + } - bool gotRef = false; - try - { - _accountTokenHandle.DangerousAddRef(ref gotRef); - IntPtr logonToken = _accountTokenHandle.DangerousGetHandle(); - AccountName = new WindowsIdentity(logonToken).Name; - } - finally - { - if (gotRef) - _accountTokenHandle.DangerousRelease(); - } + const int LOGON32_PROVIDER_DEFAULT = 0; + const int LOGON32_LOGON_INTERACTIVE = 2; + + if (!LogonUser(_userName, ".", testAccountPassword, LOGON32_LOGON_INTERACTIVE, LOGON32_PROVIDER_DEFAULT, out _accountTokenHandle)) + { + _accountTokenHandle = null; + throw new Exception($"Failed to get SafeAccessTokenHandle for test account {_userName}", new Win32Exception()); + } + + bool gotRef = false; + try + { + _accountTokenHandle.DangerousAddRef(ref gotRef); + IntPtr logonToken = _accountTokenHandle.DangerousGetHandle(); + AccountName = new WindowsIdentity(logonToken).Name; + } + finally + { + if (gotRef) + _accountTokenHandle.DangerousRelease(); } } From 2665a3ac2f923bd0b122b9f75de42e392066b8b9 Mon Sep 17 00:00:00 2001 From: Jeff Handley Date: Thu, 6 May 2021 19:28:08 -0700 Subject: [PATCH 4/5] Revert "Use RandomNumberGenerator.GetBytes() instead of RandomNumberGenerator.Create().GetBytes()" This reverts commit 447f8485a08f831f34d00597a30800ac36f034ef. --- ...owsIdentityImpersonatedTests.netcoreapp.cs | 89 ++++++++++--------- 1 file changed, 47 insertions(+), 42 deletions(-) diff --git a/src/libraries/System.Security.Principal.Windows/tests/WindowsIdentityImpersonatedTests.netcoreapp.cs b/src/libraries/System.Security.Principal.Windows/tests/WindowsIdentityImpersonatedTests.netcoreapp.cs index b47921dfa84976..868afcc8c1a4f5 100644 --- a/src/libraries/System.Security.Principal.Windows/tests/WindowsIdentityImpersonatedTests.netcoreapp.cs +++ b/src/libraries/System.Security.Principal.Windows/tests/WindowsIdentityImpersonatedTests.netcoreapp.cs @@ -127,57 +127,62 @@ public WindowsTestAccount(string userName) private void CreateUser() { - byte[] randomBytes = RandomNumberGenerator.GetBytes(33); - - // Add special chars to ensure it satisfies password requirements. - string testAccountPassword = Convert.ToBase64String(randomBytes) + "_-As@!%*(1)4#2"; - - USER_INFO_1 userInfo = new USER_INFO_1 + string testAccountPassword; + using (RandomNumberGenerator rng = RandomNumberGenerator.Create()) { - usri1_name = _userName, - usri1_password = testAccountPassword, - usri1_priv = 1 - }; + byte[] randomBytes = new byte[33]; + rng.GetBytes(randomBytes); - // Create user and remove/create if already exists - uint result = NetUserAdd(null, 1, ref userInfo, out uint param_err); + // Add special chars to ensure it satisfies password requirements. + testAccountPassword = Convert.ToBase64String(randomBytes) + "_-As@!%*(1)4#2"; - // error codes https://docs.microsoft.com/en-us/windows/desktop/netmgmt/network-management-error-codes - // 0 == NERR_Success - if (result == 2224) // NERR_UserExists - { - result = NetUserDel(null, userInfo.usri1_name); - if (result != 0) + USER_INFO_1 userInfo = new USER_INFO_1 { - throw new Win32Exception((int)result); - } - result = NetUserAdd(null, 1, ref userInfo, out param_err); - if (result != 0) + usri1_name = _userName, + usri1_password = testAccountPassword, + usri1_priv = 1 + }; + + // Create user and remove/create if already exists + uint result = NetUserAdd(null, 1, ref userInfo, out uint param_err); + + // error codes https://docs.microsoft.com/en-us/windows/desktop/netmgmt/network-management-error-codes + // 0 == NERR_Success + if (result == 2224) // NERR_UserExists { - throw new Win32Exception((int)result); + result = NetUserDel(null, userInfo.usri1_name); + if (result != 0) + { + throw new Win32Exception((int)result); + } + result = NetUserAdd(null, 1, ref userInfo, out param_err); + if (result != 0) + { + throw new Win32Exception((int)result); + } } - } - const int LOGON32_PROVIDER_DEFAULT = 0; - const int LOGON32_LOGON_INTERACTIVE = 2; + const int LOGON32_PROVIDER_DEFAULT = 0; + const int LOGON32_LOGON_INTERACTIVE = 2; - if (!LogonUser(_userName, ".", testAccountPassword, LOGON32_LOGON_INTERACTIVE, LOGON32_PROVIDER_DEFAULT, out _accountTokenHandle)) - { - _accountTokenHandle = null; - throw new Exception($"Failed to get SafeAccessTokenHandle for test account {_userName}", new Win32Exception()); - } + if (!LogonUser(_userName, ".", testAccountPassword, LOGON32_LOGON_INTERACTIVE, LOGON32_PROVIDER_DEFAULT, out _accountTokenHandle)) + { + _accountTokenHandle = null; + throw new Exception($"Failed to get SafeAccessTokenHandle for test account {_userName}", new Win32Exception()); + } - bool gotRef = false; - try - { - _accountTokenHandle.DangerousAddRef(ref gotRef); - IntPtr logonToken = _accountTokenHandle.DangerousGetHandle(); - AccountName = new WindowsIdentity(logonToken).Name; - } - finally - { - if (gotRef) - _accountTokenHandle.DangerousRelease(); + bool gotRef = false; + try + { + _accountTokenHandle.DangerousAddRef(ref gotRef); + IntPtr logonToken = _accountTokenHandle.DangerousGetHandle(); + AccountName = new WindowsIdentity(logonToken).Name; + } + finally + { + if (gotRef) + _accountTokenHandle.DangerousRelease(); + } } } From df830c2957eff1aed5cd8d0cd1b4a4e6b7e4accb Mon Sep 17 00:00:00 2001 From: Jeremy Barton Date: Fri, 7 May 2021 08:53:44 -0700 Subject: [PATCH 5/5] Update src/libraries/System.IO.Pipes/tests/NamedPipeTests/NamedPipeTest.CurrentUserOnly.Windows.cs --- .../NamedPipeTests/NamedPipeTest.CurrentUserOnly.Windows.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.IO.Pipes/tests/NamedPipeTests/NamedPipeTest.CurrentUserOnly.Windows.cs b/src/libraries/System.IO.Pipes/tests/NamedPipeTests/NamedPipeTest.CurrentUserOnly.Windows.cs index deed0e9d069b15..439454502655d8 100644 --- a/src/libraries/System.IO.Pipes/tests/NamedPipeTests/NamedPipeTest.CurrentUserOnly.Windows.cs +++ b/src/libraries/System.IO.Pipes/tests/NamedPipeTests/NamedPipeTest.CurrentUserOnly.Windows.cs @@ -22,7 +22,7 @@ public class TestAccountImpersonator : IDisposable public TestAccountImpersonator() { string testAccountPassword; - var randomBytes = RandomNumberGenerator.GetBytes(33); + byte[] randomBytes = RandomNumberGenerator.GetBytes(33); // Add special chars to ensure it satisfies password requirements. testAccountPassword = Convert.ToBase64String(randomBytes) + "_-As@!%*(1)4#2";