add certreq basic error handling (#231)

* add cert req basic error handling
dsccommunity · May 22, 2020 · e40acce · e40acce
1 parent 593f2c9
commit e40acce
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 22 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+- Added a catch for certreq generic errors which fixes [Issue #224](https://github.com/dsccommunity/CertificateDsc/issues/224)
+
 ### Changed
 
 - Corrected incorrectly located entries in `CHANGELOG.MD`.

diff --git a/source/DSCResources/DSC_CertReq/DSC_CertReq.psm1 b/source/DSCResources/DSC_CertReq/DSC_CertReq.psm1
@@ -197,16 +197,16 @@ function Get-TargetResource
         $Subject = "CN=$Subject"
     } # if
 
-    $cert = Get-Childitem -Path Cert:\LocalMachine\My |
+    $cert = Get-ChildItem -Path Cert:\LocalMachine\My |
         Where-Object -FilterScript {
             $_.Subject -eq $Subject -and `
             (Compare-CertificateIssuer -Issuer $_.Issuer -CARootName $CARootName)
-    }
+        }
 
     # If multiple certs have the same subject and were issued by the CA, return the newest
     $cert = $cert |
         Sort-Object -Property NotBefore -Descending |
-        Select-Object -First 1
+            Select-Object -First 1
 
     if ($cert)
     {
@@ -231,7 +231,7 @@ function Get-TargetResource
     }
     else
     {
-        $returnValue = @{}
+        $returnValue = @{ }
     }
 
     $returnValue
@@ -419,17 +419,17 @@ function Set-TargetResource
     # If we should look for renewals, check for existing certs
     if ($AutoRenew)
     {
-        $certs = Get-Childitem -Path Cert:\LocalMachine\My |
+        $certs = Get-ChildItem -Path Cert:\LocalMachine\My |
             Where-Object -FilterScript {
                 $_.Subject -eq $Subject -and `
                 (Compare-CertificateIssuer -Issuer $_.Issuer -CARootName $CARootName) -and `
-                $_.NotAfter -lt (Get-Date).AddDays(30)
-        }
+                    $_.NotAfter -lt (Get-Date).AddDays(30)
+            }
 
         # If multiple certs have the same subject and were issued by the CA and are 30 days from expiration, return the newest
         $firstCert = $certs |
             Sort-Object -Property NotBefore -Descending |
-            Select-Object -First 1
+                Select-Object -First 1
         $thumbprint = $firstCert |
             ForEach-Object -Process { $_.Thumbprint }
     } # if
@@ -669,10 +669,17 @@ CertificateTemplate = "$CertificateTemplate"
 
         $acceptRequest = & certreq.exe @('-accept', '-machine', '-q', $cerPath)
 
-        Write-Verbose -Message ( @(
-                "$($MyInvocation.MyCommand): "
-                $($script:localizedData.AcceptingRequestResultCertificateMessage -f ($acceptRequest | Out-String))
-            ) -join '' )
+        if ($acceptRequest -match '0x')
+        {
+            New-InvalidOperationException -Message ($script:localizedData.GenericErrorThrown -f ($acceptRequest | Out-String))
+        }
+        else
+        {
+            Write-Verbose -Message ( @(
+                    "$($MyInvocation.MyCommand): "
+                    $($script:localizedData.AcceptingRequestResultCertificateMessage -f ($acceptRequest | Out-String))
+                ) -join '' )
+        }
     }
     else
     {
@@ -867,26 +874,26 @@ function Test-TargetResource
             $($script:localizedData.TestingCertReqStatusMessage -f $Subject, $ca)
         ) -join '' )
 
-    $certificate = Get-Childitem -Path Cert:\LocalMachine\My |
+    $certificate = Get-ChildItem -Path Cert:\LocalMachine\My |
         Where-Object -FilterScript {
             (Compare-CertificateSubject -ReferenceSubject $_.Subject -DifferenceSubject $Subject) -and `
             (Compare-CertificateIssuer -Issuer $_.Issuer -CARootName $CARootName)
-    }
+        }
 
     # Exception for standard template DomainControllerAuthentication
     if ($CertificateTemplate -eq 'DomainControllerAuthentication')
     {
-        $certificate = Get-Childitem -Path Cert:\LocalMachine\My |
+        $certificate = Get-ChildItem -Path Cert:\LocalMachine\My |
             Where-Object -FilterScript {
                 (Get-CertificateTemplateName -Certificate $PSItem) -eq $CertificateTemplate -and `
                 (Compare-CertificateIssuer -Issuer $_.Issuer -CARootName $CARootName)
-        }
+            }
     }
 
     # If multiple certs have the same subject and were issued by the CA, return the newest
     $certificate = $certificate |
         Sort-Object -Property NotBefore -Descending |
-        Select-Object -First 1
+            Select-Object -First 1
 
     if ($certificate)
     {
@@ -1027,9 +1034,9 @@ function Assert-ResourceProperty
     )
 
     if ((($KeyType -eq 'RSA') -and ($KeyLength -notin '1024', '2048', '4096', '8192')) -or `
-    (($KeyType -eq 'ECDH') -and ($KeyLength -notin '192', '224', '256', '384', '521')))
+        (($KeyType -eq 'ECDH') -and ($KeyLength -notin '192', '224', '256', '384', '521')))
     {
-        New-InvalidArgumentException -Message (($script:localizedData.InvalidKeySize) -f $KeyLength,$KeyType) -ArgumentName 'KeyLength'
+        New-InvalidArgumentException -Message (($script:localizedData.InvalidKeySize) -f $KeyLength, $KeyType) -ArgumentName 'KeyLength'
     }
 }# end function Assert-ResourceProperty
 
@@ -1128,7 +1135,7 @@ function ConvertTo-StringEnclosedInDoubleQuotes
     {
         $Value = '"{0}' -f $Value
     }
-    if ($Value[$Value.Length-1] -ne '"')
+    if ($Value[$Value.Length - 1] -ne '"')
     {
         $Value = '{0}"' -f $Value
     }
@@ -1156,6 +1163,6 @@ function Get-CertificateCommonName
     )
 
     return ($DistinguishedName.split(',') |
-        ForEach-Object -Process { $_.TrimStart(' ') } |
-        Where-Object -FilterScript { $_ -match 'CN=' }).replace('CN=', '')
+            ForEach-Object -Process { $_.TrimStart(' ') } |
+                Where-Object -FilterScript { $_ -match 'CN=' }).replace('CN=', '')
 }
diff --git a/source/DSCResources/DSC_CertReq/en-US/DSC_CertReq.strings.psd1 b/source/DSCResources/DSC_CertReq/en-US/DSC_CertReq.strings.psd1
@@ -21,4 +21,5 @@ ConvertFrom-StringData @'
     CertTemplateMismatch = The certificate with subject '{0}' issued by '{1}' with thumbprint {2} has the wrong template {3}.
     CertFriendlyNameMismatch = The certificate with subject '{0}' issued by '{1}' with thumbprint {2} has the wrong friendly name '{3}'.
     InvalidKeySize = The key length '{0}' specified is invalid for '{1}' key types.
+    GenericErrorThrown = A Generic Error was thrown when accepting a Certificate. It threw the following Error message: {0}
 '@
diff --git a/tests/Unit/DSC_CertReq.Tests.ps1 b/tests/Unit/DSC_CertReq.Tests.ps1
@@ -1430,6 +1430,38 @@ OID = $oid
                     Assert-MockCalled -CommandName Find-CertificateAuthority -Exactly -Times 1
                 }
             }
+
+            Context 'When CertReq.exe throws a random error' {
+
+                 Mock -CommandName Set-Content -ParameterFilter {
+                    $Path -eq 'CertReq-Test.inf'
+                }
+
+                Mock -CommandName Find-CertificateAuthority -MockWith {
+                    return New-Object -TypeName psobject -Property @{
+                        CARootName   = "ContosoCA"
+                        CAServerFQDN = "ContosoVm.contoso.com"
+                    }
+                }
+
+                Mock -CommandName Get-ChildItem
+
+                Mock -CommandName Get-Content -Mockwith { 'Output' } `
+                    -ParameterFilter $pathCertReqTestOut_parameterFilter
+
+                Mock -CommandName Remove-Item `
+                    -ParameterFilter $pathCertReqTestOut_parameterFilter
+
+                Mock -CommandName CertReq.exe -ParameterFilter {@('-accept', '-m', '-q', $pathCertReqTestCer_parameterFilter)} -MockWith {'0x'}
+
+                $errorRecord = Get-InvalidOperationRecord `
+                    -Message ($LocalizedData.GenericErrorThrown -f '0x')
+
+                It 'Should Throw A New-InvalidOperationException' {
+                    { Set-TargetResource @paramsAutoDiscovery -Verbose } | Should -Throw $errorRecord
+                }
+
+            }
         }
 
         Describe 'DSC_CertReq\Test-TargetResource' -Tag 'Test' {