From 1dcc87395258e46a9b139fa831ed337ae7c88133 Mon Sep 17 00:00:00 2001 From: Paul Higinbotham Date: Thu, 9 Sep 2021 10:36:05 -0700 Subject: [PATCH] Fix `ConvertTo-SecureString` with key regression due to .NET breaking change (#16068) --- .../security/SecureStringHelper.cs | 151 ++++++++---------- .../SecureString.Tests.ps1 | 15 +- 2 files changed, 79 insertions(+), 87 deletions(-) diff --git a/src/System.Management.Automation/security/SecureStringHelper.cs b/src/System.Management.Automation/security/SecureStringHelper.cs index 6d38859162..ddb94e52ee 100644 --- a/src/System.Management.Automation/security/SecureStringHelper.cs +++ b/src/System.Management.Automation/security/SecureStringHelper.cs @@ -217,8 +217,6 @@ namespace Microsoft.PowerShell /// A string (see summary). internal static EncryptionResult Encrypt(SecureString input, SecureString key) { - EncryptionResult output = null; - // // get clear text key from the SecureString key // @@ -227,14 +225,14 @@ namespace Microsoft.PowerShell // // encrypt the data // - output = Encrypt(input, keyBlob); - - // - // clear the clear text key - // - Array.Clear(keyBlob, 0, keyBlob.Length); - - return output; + try + { + return Encrypt(input, keyBlob); + } + finally + { + Array.Clear(keyBlob); + } } /// @@ -254,48 +252,46 @@ namespace Microsoft.PowerShell Utils.CheckSecureStringArg(input, "input"); Utils.CheckKeyArg(key, "key"); - byte[] encryptedData = null; - MemoryStream ms = null; - ICryptoTransform encryptor = null; - CryptoStream cs = null; - // // prepare the crypto stuff. Initialization Vector is // randomized by default. // - Aes aes = Aes.Create(); - if (iv == null) - iv = aes.IV; - - encryptor = aes.CreateEncryptor(key, iv); - ms = new MemoryStream(); - - using (cs = new CryptoStream(ms, encryptor, CryptoStreamMode.Write)) + using (Aes aes = Aes.Create()) { + if (iv is null) + { + iv = aes.IV; + } + // // get clear text data from the input SecureString // byte[] data = GetData(input); + try + { + using (ICryptoTransform encryptor = aes.CreateEncryptor(key, iv)) + using (var sourceStream = new MemoryStream(data)) + using (var encryptedStream = new MemoryStream()) + { + // + // encrypt it + // + using (var cryptoStream = new CryptoStream(encryptedStream, encryptor, CryptoStreamMode.Write)) + { + sourceStream.CopyTo(cryptoStream); + } - // - // encrypt it - // - cs.Write(data, 0, data.Length); - cs.FlushFinalBlock(); - - // - // clear the clear text data array - // - Array.Clear(data, 0, data.Length); - - // - // convert the encrypted blob to a string - // - encryptedData = ms.ToArray(); - - EncryptionResult output = new EncryptionResult(ByteArrayToString(encryptedData), Convert.ToBase64String(iv)); - - return output; + // + // return encrypted data + // + byte[] encryptedData = encryptedStream.ToArray(); + return new EncryptionResult(ByteArrayToString(encryptedData), Convert.ToBase64String(iv)); + } + } + finally + { + Array.Clear(data, 0, data.Length); + } } } @@ -311,8 +307,6 @@ namespace Microsoft.PowerShell /// SecureString . internal static SecureString Decrypt(string input, SecureString key, byte[] IV) { - SecureString output = null; - // // get clear text key from the SecureString key // @@ -321,14 +315,14 @@ namespace Microsoft.PowerShell // // decrypt the data // - output = Decrypt(input, keyBlob, IV); - - // - // clear the clear text key - // - Array.Clear(keyBlob, 0, keyBlob.Length); - - return output; + try + { + return Decrypt(input, keyBlob, IV); + } + finally + { + Array.Clear(keyBlob); + } } /// @@ -346,44 +340,33 @@ namespace Microsoft.PowerShell Utils.CheckArgForNullOrEmpty(input, "input"); Utils.CheckKeyArg(key, "key"); - byte[] decryptedData = null; - byte[] encryptedData = null; - SecureString s = null; - // // prepare the crypto stuff // - Aes aes = Aes.Create(); - encryptedData = ByteArrayFromString(input); - - var decryptor = aes.CreateDecryptor(key, IV ?? aes.IV); - - MemoryStream ms = new MemoryStream(encryptedData); - - using (CryptoStream cs = new CryptoStream(ms, decryptor, CryptoStreamMode.Read)) + using (var aes = Aes.Create()) { - byte[] tempDecryptedData = new byte[encryptedData.Length]; - - int numBytesRead = 0; - - // - // decrypt the data - // - numBytesRead = cs.Read(tempDecryptedData, 0, - tempDecryptedData.Length); - - decryptedData = new byte[numBytesRead]; - - for (int i = 0; i < numBytesRead; i++) + using (ICryptoTransform decryptor = aes.CreateDecryptor(key, IV ?? aes.IV)) + using (var encryptedStream = new MemoryStream(ByteArrayFromString(input))) + using (var targetStream = new MemoryStream()) { - decryptedData[i] = tempDecryptedData[i]; + // + // decrypt the data and return as SecureString + // + using (var sourceStream = new CryptoStream(encryptedStream, decryptor, CryptoStreamMode.Read)) + { + sourceStream.CopyTo(targetStream); + } + + byte[] decryptedData = targetStream.ToArray(); + try + { + return New(decryptedData); + } + finally + { + Array.Clear(decryptedData); + } } - - s = New(decryptedData); - Array.Clear(decryptedData, 0, decryptedData.Length); - Array.Clear(tempDecryptedData, 0, tempDecryptedData.Length); - - return s; } } diff --git a/test/powershell/Modules/Microsoft.PowerShell.Security/SecureString.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Security/SecureString.Tests.ps1 index 6b8f45d509..df1fa9294e 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Security/SecureString.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Security/SecureString.Tests.ps1 @@ -11,12 +11,12 @@ Describe "SecureString conversion tests" -Tags "CI" { $string.ToCharArray() | ForEach-Object { $securestring.AppendChar($_) } } - It "using null arguments to ConvertFrom-SecureString produces an exception" { + It "Using null arguments to ConvertFrom-SecureString produces an exception" { { ConvertFrom-SecureString -SecureString $null -Key $null } | Should -Throw -ErrorId "ParameterArgumentValidationErrorNullNotAllowed,Microsoft.PowerShell.Commands.ConvertFromSecureStringCommand" } - It "using a bad key produces an exception" { + It "Using a bad key produces an exception" { $badkey = [byte[]]@(1,2) { ConvertFrom-SecureString -SecureString $secureString -Key $badkey } | Should -Throw -ErrorId "Argument,Microsoft.PowerShell.Commands.ConvertFromSecureStringCommand" @@ -27,9 +27,18 @@ Describe "SecureString conversion tests" -Tags "CI" { $ss | Should -BeOfType SecureString } - It "can convert back from a secure string" { + It "Can convert back from a secure string" { $ss1 = ConvertTo-SecureString -AsPlainText -Force $string $ss2 = ConvertFrom-SecureString $ss1 | ConvertTo-SecureString $ss2 | ConvertFrom-SecureString -AsPlainText | Should -Be $string } + + It "Can encode secure string with key" { + $testString = '[8Chars][8Chars][Not8]' + $key = [System.Text.Encoding]::UTF8.GetBytes("1234"*8) + $ss1 = $testString | ConvertTo-SecureString -AsPlainText -Force + $encodedStr = $ss1 | ConvertFrom-SecureString -Key $key + $ss2 = $encodedStr | ConvertTo-SecureString -Key $key + $ss2 | ConvertFrom-SecureString -AsPlainText | Should -BeExactly $testString + } }