Merge pull request #8 from webmd-health-services/bugfix/error-handlin…

…g-fail 1.0.2
webmd-health-services · Feb 6, 2021 · 0fa35b6 · 0fa35b6
2 parents a9debc9 + 4a2bb8b
commit 0fa35b6
Show file tree

Hide file tree

Showing 5 changed files with 101 additions and 7 deletions.
diff --git a/Carbon.Cryptography/Carbon.Cryptography.psd1 b/Carbon.Cryptography/Carbon.Cryptography.psd1
@@ -17,7 +17,7 @@
     RootModule = 'Carbon.Cryptography.psm1'
 
     # Version number of this module.
-    ModuleVersion = '1.0.1'
+    ModuleVersion = '1.0.2'
 
     # ID used to uniquely identify this module
     GUID = '225b9f63-3e3e-406c-87a0-33d34f30cd8e'
@@ -134,8 +134,7 @@
 
             # ReleaseNotes of this module
             ReleaseNotes = @'
-* Fixed: `Protect-CString` incorrectly marked as a filter instead of a function.
-* Fixed: `Protect-CString` and `Unprotect-CString` failed to handle encryption exceptions.
+* Fixed: `Unprotect-CString` error handling fails when encryption fails.
 '@
         } # End of PSData hashtable
 

diff --git a/Carbon.Cryptography/Functions/Unprotect-CString.ps1 b/Carbon.Cryptography/Functions/Unprotect-CString.ps1
@@ -258,8 +258,9 @@ function Unprotect-CString
                 $isRsa = $privateKeyType.IsSubclassOf([Security.Cryptography.RSA])
                 if( -not $isRsa )
                 {
-                    $msg = "$($certDesc) is not an RSA key. Found a private key of type ""$($privateKeyType.FullName)"", but " +
-                        "expected type ""$([Security.Cryptography.RSA].FullName)"" or one of its sub-types."
+                    $msg = "$($certDesc) is not an RSA key. Found a private key of type " +
+                           """$($privateKeyType.FullName)"", but expected type " +
+                           """$([Security.Cryptography.RSA].FullName)"" or one of its sub-types."
                     Write-Error -Message $msg -ErrorAction $ErrorActionPreference
                     return
                 }
@@ -340,7 +341,7 @@ function Unprotect-CString
         }
         catch
         {
-            Write-Error -ErrorRecrd $_ -ErrorAction $ErrorActionPreference
+            Write-Error -ErrorRecord $_ -ErrorAction $ErrorActionPreference
         }
         finally
         {

diff --git a/Tests/Install-CCertificate.Tests.ps1 b/Tests/Install-CCertificate.Tests.ps1
@@ -65,7 +65,17 @@ function ThenCertificateInstalled
         $In = 'My'
     )
 
-    $cert = Get-CCertificate -Thumbprint $WithThumbprint -StoreLocation $For -StoreName $In
+    $tries = 100
+    $cert = $null
+    for( $tryNum = 0; $tryNum -lt $tries; ++$tryNum )
+    {
+        $cert = Get-CCertificate -Thumbprint $WithThumbprint -StoreLocation $For -StoreName $In
+        if( $cert )
+        {
+            break
+        }
+        Start-Sleep -Milliseconds 100
+    }
     $cert | Should -Not -BeNullOrEmpty
     $cert.Thumbprint | Should -Be $WithThumbprint
 }

diff --git a/Tests/Protect-CString.Tests.ps1 b/Tests/Protect-CString.Tests.ps1
@@ -231,3 +231,22 @@ foreach( $keySize in @( 128, 192, 256 ) )
         }
     }
 }
+
+Describe 'Protect-CString.when encryption fails' {
+    # Anyone know how to get DPAPI or AES encryption to fail?
+    Context 'RSA' {
+        It 'should fail' {
+            { 
+                $Global:Error.Clear()
+                # Definitely too big to be encrypted by RSA.
+                $plainText = 'a' * 1000
+                Protect-CString -String $plainText -PublicKeyPath $publicKeyFilePath -ErrorAction SilentlyContinue |
+                    Should -BeNullOrEmpty
+                # Different error message on different versions of .NET and different platforms
+                #                              WinPS 5.1 | PS Core 7            | Linux        | macOS
+                $Global:Error | Should -Match 'Bad Length|parameter is incorrect|data too large|message exceeds the maximum'
+            } |
+                Should -Not -Throw
+        }
+    }
+}
diff --git a/Tests/Unprotect-CString.Tests.ps1 b/Tests/Unprotect-CString.Tests.ps1
@@ -164,3 +164,68 @@ Describe 'Unprotect-String.AES' {
     }
 }
 
+if( (Test-Path -Path 'cert:') )
+{
+    $certWithEmptyPrivateKey = 
+        Get-ChildItem -Path 'cert:\*\*\*' |
+        Where-Object 'HasPrivateKey' -eq $true |
+        Where-Object 'PrivateKey' -eq $null |
+        Select-Object -First 1
+
+    if( $certWithEmptyPrivateKey )
+    {
+        Describe 'Unprotect-CString.when user does not have access to private key' {
+            It 'should fail' {
+                { Unprotect-CString -ProtectedString 'doesn''t matter' -Thumbprint $certWithEmptyPrivateKey.Thumbprint -ErrorAction Stop } |
+                    Should -Throw 'has a private key, but it is null'
+            }
+        }
+    }
+}
+
+Describe 'Unprotect-CString.when decryption fails' {
+    if( (Test-COperatingSystem -IsWindows) )
+    {
+        Context 'DPAPI' {
+            It 'should fail' {
+                { 
+                    $Global:Error.Clear()
+                    Unprotect-CString -ProtectedString 'not encrypted' -ErrorAction SilentlyContinue |
+                        Should -BeNullOrEmpty
+                    $Global:Error | Should -Match 'parameter is incorrect'
+                } |
+                    Should -Not -Throw
+            }
+        }
+    }
+    Context 'RSA' {
+        It 'should fail' {
+            { 
+                $Global:Error.Clear()
+                Unprotect-CString -ProtectedString 'not encrypted' `
+                                  -PrivateKeyPath $privateKeyPath `
+                                  -ErrorAction SilentlyContinue |
+                    Should -BeNullOrEmpty
+                # Different error message on different versions of .NET.
+                $Global:Error | Should -Match 'decoding OAEP padding|length of the data to decrypt'
+            } |
+                Should -Not -Throw
+        }
+    }
+    Context 'AES' {
+        It 'should fail' {
+            { 
+                $Global:Error.Clear()
+                $key = 'passwordpasswordpasswordpassword'
+                $fakeCipherText =
+                    "$('iv' * 8)not encrypted)" | ConvertTo-CBase64 -Encoding ([Text.Encoding]::UTF8)
+                Unprotect-CString -ProtectedString $fakeCipherText `
+                                  -Key (ConvertTo-SecureString $key -AsPlainText -Force) `
+                                  -ErrorAction SilentlyContinue |
+                    Should -BeNullOrEmpty
+                $Global:Error | Should -Match 'input data is not a complete block' #head
+            } |
+                Should -Not -Throw
+        }
+    }
+}