Skip to content

Fix credscan bugs #40454

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
aik-jahoda wants to merge 3 commits into from
Closed

Fix credscan bugs #40454

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ac3acbc
Fix credscan bugs
Aug 6, 2020
f3c1571
Suppress Extension test
Aug 6, 2020
9e1eb44
Change jsutification
Aug 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions .config/CredScanSuppressions.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Copy link
Member

@ManickaP ManickaP Aug 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file, according to the path, doesn't look like a test code. Is there a reason we cannot do inline suppression for it instead?

"src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898Tests.cs",
"src/libraries/Common/tests/System/Net/Http/PostScenarioTest.cs"
],
"placeholder": [
"\"anotherpassword\"",
Expand All @@ -63,7 +67,10 @@
"\"rightpassword\"",
"\"testcertificate\"",
"\"unused\"",
"\"wrongpassword\""
"\"wrongpassword\"",
"\"NotVerySecret\"",
"\"MyPassword\"",
"\"PasswordGoesHere\""
]
}
]
Expand Down
1 change: 1 addition & 0 deletions src/libraries/Common/src/System/Security/Cryptography/DSASecurityTransforms.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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="Password for temporary operation. See code comments for more details.")]
const string ExportPassword = "DotnetExportPassphrase";
SecKeyPair keys = GetKeys();

Expand Down
2 changes: 2 additions & 0 deletions src/libraries/Common/src/System/Security/Cryptography/EccSecurityTransforms.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ private void SetKey(SecKeyPair keyPair)

internal static ECParameters ExportPublicParametersFromPrivateKey(SafeSecKeyRefHandle handle)
{
// [SuppressMessage("Microsoft.Security", "CS002:SecretInNextLine", Justification="Password for temporary operation. See code comments for more details.")]
Copy link
Member

@ManickaP ManickaP Aug 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See code comments for more details.

There're no comments though.

const string ExportPassword = "DotnetExportPassphrase";
byte[] keyBlob = Interop.AppleCrypto.SecKeyExport(handle, exportPrivate: true, password: ExportPassword);
EccKeyFormatHelper.ReadEncryptedPkcs8(keyBlob, ExportPassword, out _, out ECParameters key);
Expand All @@ -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="Password for temporary operation. See code comments for more details.")]
const string ExportPassword = "DotnetExportPassphrase";
SecKeyPair keys = GetOrGenerateKeys(keySizeInBits);

Expand Down
1 change: 1 addition & 0 deletions src/libraries/Common/src/System/Security/Cryptography/RSASecurityTransforms.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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="Password for temporary operation. See code comments for more details.")]
const string ExportPassword = "DotnetExportPassphrase";
SecKeyPair keys = GetKeys();

Expand Down
2 changes: 2 additions & 0 deletions ...raries/Microsoft.Extensions.Configuration.UserSecrets/tests/ConfigurationExtensionTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -112,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);
Expand Down
16 changes: 8 additions & 8 deletions src/libraries/System.Security.Cryptography.Pkcs/tests/Certificates.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ private static class RawData
+ "86b73d334e8b3afa7fb8eb31483efc0c7ccb0f8c1ca94d8be4f0daade4498501d02e6f92dd7b2f4401550896eb511ef14417"
+ "cbb5a1b360d67998d334").HexToByteArray();

// password = "1111"
// password is 1111
public static byte[] s_RSAKeyTransfer1Pfx =
("308205d20201033082058e06092a864886f70d010701a082057f0482057b308205773082034806092a864886f70d010701a0"
+ "82033904820335308203313082032d060b2a864886f70d010c0a0102a08202a6308202a2301c060a2a864886f70d010c0103"
Expand Down Expand Up @@ -93,7 +93,7 @@ private static class RawData
+ "41da152963e700a6f37faf7678f084a9fb4fe88f7b2cbc6cdeb0b9fdcc6a8a16843e7bc281a71dc6eb8bbc4092d299bf7599"
+ "a3492c99c9a3acf41b29").HexToByteArray();

// password = "1111"
// password is 1111
public static byte[] s_RSAKeyTransfer2Pfx =
("308205d20201033082058e06092a864886f70d010701a082057f0482057b308205773082034806092a864886f70d010701a0"
+ "82033904820335308203313082032d060b2a864886f70d010c0a0102a08202a6308202a2301c060a2a864886f70d010c0103"
Expand Down Expand Up @@ -138,7 +138,7 @@ private static class RawData
+ "5207e7c70d843deda8754af8ef1029e0b68c35d88c30d7da2f85d1a20dd4099facf373341b50a8a213f735421062e1477459"
+ "6e27a32e23b3f3fcfec3").HexToByteArray();

// password = "1111"
// password is 1111
public static byte[] s_RSAKeyTransfer3Pfx =
("308205d20201033082058e06092a864886f70d010701a082057f0482057b308205773082034806092a864886f70d010701a0"
+ "82033904820335308203313082032d060b2a864886f70d010c0a0102a08202a6308202a2301c060a2a864886f70d010c0103"
Expand Down Expand Up @@ -184,7 +184,7 @@ private static class RawData
+ "e2fce3d019fa70d54646975b6dc2a3ba72d5a5274c1866da6d7a5df47938e034a075d11957d653b5c78e5291e4401045576f"
+ "6d4eda81bef3c369af56121e49a083c8d1adb09f291822e99a429646").HexToByteArray();

// Password = "1111"
// password is 1111
//
// Built by:
//
Expand Down Expand Up @@ -237,7 +237,7 @@ private static class RawData
+ "74227900cfefbe2fdac92b4f769cf2bf3befb485f282a85bfb09454b797ce5286de560c219fb0dd6fce0442adbfef4f767e9"
+ "ac81cf3e9701baf81efc73a0ed88576adff12413b827").HexToByteArray();

// password = "1111"
// password is 1111
public static byte[] s_RSASha256KeyTransfer1Pfx =
("308205de0201033082059a06092a864886f70d010701a082058b04820587308205833082034806092a864886f70d010701a0"
+ "82033904820335308203313082032d060b2a864886f70d010c0a0102a08202a6308202a2301c060a2a864886f70d010c0103"
Expand Down Expand Up @@ -283,7 +283,7 @@ private static class RawData
+ "90ebcdbce563b8d4209efc1b04750f46c8c6117ccb96b26b5f02b0b5f961ab01b0c3b4cdb2530cbc5dcf37786712a3476ce7"
+ "32c5c544c328db5ebc3a338b18fe32aedaffedd973ef").HexToByteArray();

// password = "1111"
// password is 1111
public static byte[] s_RSASha384KeyTransfer1Pfx =
("308205de0201033082059a06092a864886f70d010701a082058b04820587308205833082034806092a864886f70d010701a0"
+ "82033904820335308203313082032d060b2a864886f70d010c0a0102a08202a6308202a2301c060a2a864886f70d010c0103"
Expand Down Expand Up @@ -329,7 +329,7 @@ private static class RawData
+ "b966d3ea397fe25457b8a703fb43ddab1c52272d6a12476df1df1826c90fb679cebc4c04efc764fd8ce3277305c3bcdf1637"
+ "91784d778663194097180584e5e8ab69039908bf6f86").HexToByteArray();

// password = "1111"
// password is 1111
public static byte[] s_RSASha512KeyTransfer1Pfx =
("308205de0201033082059a06092a864886f70d010701a082058b04820587308205833082034806092a864886f70d010701a0"
+ "82033904820335308203313082032d060b2a864886f70d010c0a0102a08202a6308202a2301c060a2a864886f70d010c0103"
Expand Down Expand Up @@ -487,7 +487,7 @@ private static class RawData
+ "31124ABBB4C3D3D49C220CCB6F2F94176B8225A0E2ADDB0F4A72E6B021601CD297AC45A0CAB95EBAC4001C8167899868"
+ "3188DB9364AAD52D4E28169CC898B621FF84").HexToByteArray();

// password = "1111"
// password is 1111
public static byte[] s_RSAKeyTransferPfx_ExplicitSki =
("308209810201033082094706092A864886F70D010701A08209380482093430820930308203E706092A864886F70D0107"
+ "06A08203D8308203D4020100308203CD06092A864886F70D010701301C060A2A864886F70D010C0106300E0408101C5A"
Expand Down
3 changes: 2 additions & 1 deletion src/libraries/System.Security.Cryptography.Pkcs/tests/Pkcs12/Pkcs12Documents.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<byte> HighPbeIterationCount3Des = (
"308209c40201033082098906092a864886f70d010701a082097a04820976" +
"308209723082043006092a864886f70d010706a08204213082041d020100" +
Expand Down Expand Up @@ -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";
}
}