From ac3acbcf8f28008f185a5e77e4a9f17b9bc87eb3 Mon Sep 17 00:00:00 2001 From: Jan Jahoda Date: Thu, 6 Aug 2020 13:10:19 +0200 Subject: [PATCH 1/3] Fix credscan bugs --- .config/CredScanSuppressions.json | 11 +++++++++-- .../Cryptography/DSASecurityTransforms.cs | 1 + .../Cryptography/EccSecurityTransforms.cs | 2 ++ .../Cryptography/RSASecurityTransforms.cs | 1 + .../tests/ConfigurationExtensionTest.cs | 1 + .../tests/Certificates.cs | 16 ++++++++-------- .../tests/Pkcs12/Pkcs12Documents.cs | 3 ++- 7 files changed, 24 insertions(+), 11 deletions(-) diff --git a/.config/CredScanSuppressions.json b/.config/CredScanSuppressions.json index 984a86f68e69e6..5779b36160a1bb 100644 --- a/.config/CredScanSuppressions.json +++ b/.config/CredScanSuppressions.json @@ -53,7 +53,11 @@ "src/libraries/System.Net.Http/tests/UnitTests/HttpEnvironmentProxyTest.cs", "src/libraries/System.Net.Mail/tests/Functional/SmtpClientTest.cs", "src/libraries/System.Security.Cryptography.Xml/tests/SignedXmlTest.cs", - "src/libraries/System.Security.Cryptography.Xml/tests/TestHelpers.cs" + "src/libraries/System.Security.Cryptography.Xml/tests/TestHelpers.cs", + "src/libraries/System.Security.Cryptography.X509Certificates/tests/PfxTests.cs", + "src/libraries/System.Management/src/System/Management/ManagementScope.cs", + "src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898Tests.cs", + "src/libraries/Common/tests/System/Net/Http/PostScenarioTest.cs" ], "placeholder": [ "\"anotherpassword\"", @@ -63,7 +67,10 @@ "\"rightpassword\"", "\"testcertificate\"", "\"unused\"", - "\"wrongpassword\"" + "\"wrongpassword\"", + "\"NotVerySecret\"", + "\"MyPassword\"", + "\"PasswordGoesHere\"" ] } ] diff --git a/src/libraries/Common/src/System/Security/Cryptography/DSASecurityTransforms.cs b/src/libraries/Common/src/System/Security/Cryptography/DSASecurityTransforms.cs index 31c05b54e90916..e3eec5b7d5d803 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/DSASecurityTransforms.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/DSASecurityTransforms.cs @@ -85,6 +85,7 @@ public override DSAParameters ExportParameters(bool includePrivateParameters) { // Apple requires all private keys to be exported encrypted, but since we're trying to export // as parsed structures we will need to decrypt it for the user. + // [SuppressMessage("Microsoft.Security", "CS002:SecretInNextLine", Justification="Unit test password.")] const string ExportPassword = "DotnetExportPassphrase"; SecKeyPair keys = GetKeys(); diff --git a/src/libraries/Common/src/System/Security/Cryptography/EccSecurityTransforms.cs b/src/libraries/Common/src/System/Security/Cryptography/EccSecurityTransforms.cs index 71d8ce8a71f3ae..38e61a1c792f41 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/EccSecurityTransforms.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/EccSecurityTransforms.cs @@ -118,6 +118,7 @@ private void SetKey(SecKeyPair keyPair) internal static ECParameters ExportPublicParametersFromPrivateKey(SafeSecKeyRefHandle handle) { + // [SuppressMessage("Microsoft.Security", "CS002:SecretInNextLine", Justification="Unit test password.")] const string ExportPassword = "DotnetExportPassphrase"; byte[] keyBlob = Interop.AppleCrypto.SecKeyExport(handle, exportPrivate: true, password: ExportPassword); EccKeyFormatHelper.ReadEncryptedPkcs8(keyBlob, ExportPassword, out _, out ECParameters key); @@ -131,6 +132,7 @@ internal ECParameters ExportParameters(bool includePrivateParameters, int keySiz { // Apple requires all private keys to be exported encrypted, but since we're trying to export // as parsed structures we will need to decrypt it for the user. + // [SuppressMessage("Microsoft.Security", "CS002:SecretInNextLine", Justification="Unit test password.")] const string ExportPassword = "DotnetExportPassphrase"; SecKeyPair keys = GetOrGenerateKeys(keySizeInBits); diff --git a/src/libraries/Common/src/System/Security/Cryptography/RSASecurityTransforms.cs b/src/libraries/Common/src/System/Security/Cryptography/RSASecurityTransforms.cs index ce73b15310a5da..af44b303669a74 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/RSASecurityTransforms.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/RSASecurityTransforms.cs @@ -92,6 +92,7 @@ public override RSAParameters ExportParameters(bool includePrivateParameters) { // Apple requires all private keys to be exported encrypted, but since we're trying to export // as parsed structures we will need to decrypt it for the user. + // [SuppressMessage("Microsoft.Security", "CS002:SecretInNextLine", Justification="Unit test password.")] const string ExportPassword = "DotnetExportPassphrase"; SecKeyPair keys = GetKeys(); diff --git a/src/libraries/Microsoft.Extensions.Configuration.UserSecrets/tests/ConfigurationExtensionTest.cs b/src/libraries/Microsoft.Extensions.Configuration.UserSecrets/tests/ConfigurationExtensionTest.cs index 6f59604a79e19d..62e08ba44eb6fb 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.UserSecrets/tests/ConfigurationExtensionTest.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.UserSecrets/tests/ConfigurationExtensionTest.cs @@ -12,6 +12,7 @@ using Newtonsoft.Json.Linq; using Xunit; +// [SuppressMessage("Microsoft.Security", "CS002:SecretInNextLine", Justification="Context for unit tests.")] [assembly: UserSecretsId(ConfigurationExtensionTest.TestSecretsId)] namespace Microsoft.Extensions.Configuration.UserSecrets.Test diff --git a/src/libraries/System.Security.Cryptography.Pkcs/tests/Certificates.cs b/src/libraries/System.Security.Cryptography.Pkcs/tests/Certificates.cs index 1c1ca3e58f3c8a..3095dc61a050d7 100644 --- a/src/libraries/System.Security.Cryptography.Pkcs/tests/Certificates.cs +++ b/src/libraries/System.Security.Cryptography.Pkcs/tests/Certificates.cs @@ -48,7 +48,7 @@ private static class RawData + "86b73d334e8b3afa7fb8eb31483efc0c7ccb0f8c1ca94d8be4f0daade4498501d02e6f92dd7b2f4401550896eb511ef14417" + "cbb5a1b360d67998d334").HexToByteArray(); - // password = "1111" + // password is 1111 public static byte[] s_RSAKeyTransfer1Pfx = ("308205d20201033082058e06092a864886f70d010701a082057f0482057b308205773082034806092a864886f70d010701a0" + "82033904820335308203313082032d060b2a864886f70d010c0a0102a08202a6308202a2301c060a2a864886f70d010c0103" @@ -93,7 +93,7 @@ private static class RawData + "41da152963e700a6f37faf7678f084a9fb4fe88f7b2cbc6cdeb0b9fdcc6a8a16843e7bc281a71dc6eb8bbc4092d299bf7599" + "a3492c99c9a3acf41b29").HexToByteArray(); - // password = "1111" + // password is 1111 public static byte[] s_RSAKeyTransfer2Pfx = ("308205d20201033082058e06092a864886f70d010701a082057f0482057b308205773082034806092a864886f70d010701a0" + "82033904820335308203313082032d060b2a864886f70d010c0a0102a08202a6308202a2301c060a2a864886f70d010c0103" @@ -138,7 +138,7 @@ private static class RawData + "5207e7c70d843deda8754af8ef1029e0b68c35d88c30d7da2f85d1a20dd4099facf373341b50a8a213f735421062e1477459" + "6e27a32e23b3f3fcfec3").HexToByteArray(); - // password = "1111" + // password is 1111 public static byte[] s_RSAKeyTransfer3Pfx = ("308205d20201033082058e06092a864886f70d010701a082057f0482057b308205773082034806092a864886f70d010701a0" + "82033904820335308203313082032d060b2a864886f70d010c0a0102a08202a6308202a2301c060a2a864886f70d010c0103" @@ -184,7 +184,7 @@ private static class RawData + "e2fce3d019fa70d54646975b6dc2a3ba72d5a5274c1866da6d7a5df47938e034a075d11957d653b5c78e5291e4401045576f" + "6d4eda81bef3c369af56121e49a083c8d1adb09f291822e99a429646").HexToByteArray(); - // Password = "1111" + // password is 1111 // // Built by: // @@ -237,7 +237,7 @@ private static class RawData + "74227900cfefbe2fdac92b4f769cf2bf3befb485f282a85bfb09454b797ce5286de560c219fb0dd6fce0442adbfef4f767e9" + "ac81cf3e9701baf81efc73a0ed88576adff12413b827").HexToByteArray(); - // password = "1111" + // password is 1111 public static byte[] s_RSASha256KeyTransfer1Pfx = ("308205de0201033082059a06092a864886f70d010701a082058b04820587308205833082034806092a864886f70d010701a0" + "82033904820335308203313082032d060b2a864886f70d010c0a0102a08202a6308202a2301c060a2a864886f70d010c0103" @@ -283,7 +283,7 @@ private static class RawData + "90ebcdbce563b8d4209efc1b04750f46c8c6117ccb96b26b5f02b0b5f961ab01b0c3b4cdb2530cbc5dcf37786712a3476ce7" + "32c5c544c328db5ebc3a338b18fe32aedaffedd973ef").HexToByteArray(); - // password = "1111" + // password is 1111 public static byte[] s_RSASha384KeyTransfer1Pfx = ("308205de0201033082059a06092a864886f70d010701a082058b04820587308205833082034806092a864886f70d010701a0" + "82033904820335308203313082032d060b2a864886f70d010c0a0102a08202a6308202a2301c060a2a864886f70d010c0103" @@ -329,7 +329,7 @@ private static class RawData + "b966d3ea397fe25457b8a703fb43ddab1c52272d6a12476df1df1826c90fb679cebc4c04efc764fd8ce3277305c3bcdf1637" + "91784d778663194097180584e5e8ab69039908bf6f86").HexToByteArray(); - // password = "1111" + // password is 1111 public static byte[] s_RSASha512KeyTransfer1Pfx = ("308205de0201033082059a06092a864886f70d010701a082058b04820587308205833082034806092a864886f70d010701a0" + "82033904820335308203313082032d060b2a864886f70d010c0a0102a08202a6308202a2301c060a2a864886f70d010c0103" @@ -487,7 +487,7 @@ private static class RawData + "31124ABBB4C3D3D49C220CCB6F2F94176B8225A0E2ADDB0F4A72E6B021601CD297AC45A0CAB95EBAC4001C8167899868" + "3188DB9364AAD52D4E28169CC898B621FF84").HexToByteArray(); - // password = "1111" + // password is 1111 public static byte[] s_RSAKeyTransferPfx_ExplicitSki = ("308209810201033082094706092A864886F70D010701A08209380482093430820930308203E706092A864886F70D0107" + "06A08203D8308203D4020100308203CD06092A864886F70D010701301C060A2A864886F70D010C0106300E0408101C5A" diff --git a/src/libraries/System.Security.Cryptography.Pkcs/tests/Pkcs12/Pkcs12Documents.cs b/src/libraries/System.Security.Cryptography.Pkcs/tests/Pkcs12/Pkcs12Documents.cs index 51c80e09c768b3..edfb42c2fa0d5b 100644 --- a/src/libraries/System.Security.Cryptography.Pkcs/tests/Pkcs12/Pkcs12Documents.cs +++ b/src/libraries/System.Security.Cryptography.Pkcs/tests/Pkcs12/Pkcs12Documents.cs @@ -136,6 +136,7 @@ internal static class Pkcs12Documents "21300906052B0E03021A050004148B12EE39C54B03EF4C1B0C2D8A3A9624D629" + "285A0408E398C69C57E4782102020400").HexToByteArray(); + // [SuppressMessage("Microsoft.Security", "CS002:SecretInNextLine", Justification="Unit test PKCS#12.")] internal static readonly ReadOnlyMemory HighPbeIterationCount3Des = ( "308209c40201033082098906092a864886f70d010701a082097a04820976" + "308209723082043006092a864886f70d010706a08204213082041d020100" + @@ -222,7 +223,7 @@ internal static class Pkcs12Documents "2b0e03021a05000414c429b968eeca558cc2ec486f89b78c024bdecf2804" + "087cbeafa8089685a102030927c1").HexToByteArray(); - // [SuppressMessage("Microsoft.Security", "CS002:SecretInNextLine", Justification="Unit test dummy credentials.")] + // [SuppressMessage("Microsoft.Security", "CS002:SecretInNextLine", Justification="Unit test password.")] internal const string OracleWalletPassword = "123Wallet"; } } From f3c15715dbdfb3fe899b1a4599675713d38936f2 Mon Sep 17 00:00:00 2001 From: Jan Jahoda Date: Thu, 6 Aug 2020 13:32:28 +0200 Subject: [PATCH 2/3] Suppress Extension test --- .../tests/ConfigurationExtensionTest.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/Microsoft.Extensions.Configuration.UserSecrets/tests/ConfigurationExtensionTest.cs b/src/libraries/Microsoft.Extensions.Configuration.UserSecrets/tests/ConfigurationExtensionTest.cs index 62e08ba44eb6fb..17f6fb6847748b 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.UserSecrets/tests/ConfigurationExtensionTest.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.UserSecrets/tests/ConfigurationExtensionTest.cs @@ -113,6 +113,7 @@ public void AddUserSecrets_DoesNotThrowsIfOptional() public void AddUserSecrets_With_SecretsId_Passed_Explicitly() { var userSecretsId = Guid.NewGuid().ToString(); + // [SuppressMessage("Microsoft.Security", "CS002:SecretInNextLine", Justification="Unit test password.")] SetSecret(userSecretsId, "Facebook:AppSecret", "value1"); var builder = new ConfigurationBuilder().AddUserSecrets(userSecretsId); From 9e1eb446b91986a9640770cc13f86dbf1fe26ff5 Mon Sep 17 00:00:00 2001 From: Jan Jahoda Date: Fri, 7 Aug 2020 10:50:38 +0200 Subject: [PATCH 3/3] Change jsutification --- .../src/System/Security/Cryptography/DSASecurityTransforms.cs | 2 +- .../src/System/Security/Cryptography/EccSecurityTransforms.cs | 4 ++-- .../src/System/Security/Cryptography/RSASecurityTransforms.cs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libraries/Common/src/System/Security/Cryptography/DSASecurityTransforms.cs b/src/libraries/Common/src/System/Security/Cryptography/DSASecurityTransforms.cs index e3eec5b7d5d803..92fd747efb51bb 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/DSASecurityTransforms.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/DSASecurityTransforms.cs @@ -85,7 +85,7 @@ public override DSAParameters ExportParameters(bool includePrivateParameters) { // Apple requires all private keys to be exported encrypted, but since we're trying to export // as parsed structures we will need to decrypt it for the user. - // [SuppressMessage("Microsoft.Security", "CS002:SecretInNextLine", Justification="Unit test password.")] + // [SuppressMessage("Microsoft.Security", "CS002:SecretInNextLine", Justification="Password for temporary operation. See code comments for more details.")] const string ExportPassword = "DotnetExportPassphrase"; SecKeyPair keys = GetKeys(); diff --git a/src/libraries/Common/src/System/Security/Cryptography/EccSecurityTransforms.cs b/src/libraries/Common/src/System/Security/Cryptography/EccSecurityTransforms.cs index 38e61a1c792f41..da5d1710785878 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/EccSecurityTransforms.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/EccSecurityTransforms.cs @@ -118,7 +118,7 @@ private void SetKey(SecKeyPair keyPair) internal static ECParameters ExportPublicParametersFromPrivateKey(SafeSecKeyRefHandle handle) { - // [SuppressMessage("Microsoft.Security", "CS002:SecretInNextLine", Justification="Unit test password.")] + // [SuppressMessage("Microsoft.Security", "CS002:SecretInNextLine", Justification="Password for temporary operation. See code comments for more details.")] const string ExportPassword = "DotnetExportPassphrase"; byte[] keyBlob = Interop.AppleCrypto.SecKeyExport(handle, exportPrivate: true, password: ExportPassword); EccKeyFormatHelper.ReadEncryptedPkcs8(keyBlob, ExportPassword, out _, out ECParameters key); @@ -132,7 +132,7 @@ internal ECParameters ExportParameters(bool includePrivateParameters, int keySiz { // Apple requires all private keys to be exported encrypted, but since we're trying to export // as parsed structures we will need to decrypt it for the user. - // [SuppressMessage("Microsoft.Security", "CS002:SecretInNextLine", Justification="Unit test password.")] + // [SuppressMessage("Microsoft.Security", "CS002:SecretInNextLine", Justification="Password for temporary operation. See code comments for more details.")] const string ExportPassword = "DotnetExportPassphrase"; SecKeyPair keys = GetOrGenerateKeys(keySizeInBits); diff --git a/src/libraries/Common/src/System/Security/Cryptography/RSASecurityTransforms.cs b/src/libraries/Common/src/System/Security/Cryptography/RSASecurityTransforms.cs index af44b303669a74..a3de7d0324d6b4 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/RSASecurityTransforms.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/RSASecurityTransforms.cs @@ -92,7 +92,7 @@ public override RSAParameters ExportParameters(bool includePrivateParameters) { // Apple requires all private keys to be exported encrypted, but since we're trying to export // as parsed structures we will need to decrypt it for the user. - // [SuppressMessage("Microsoft.Security", "CS002:SecretInNextLine", Justification="Unit test password.")] + // [SuppressMessage("Microsoft.Security", "CS002:SecretInNextLine", Justification="Password for temporary operation. See code comments for more details.")] const string ExportPassword = "DotnetExportPassphrase"; SecKeyPair keys = GetKeys();