BREAKING CHANGE: CertReq: Improve certificate match selectivity (#271)

dsccommunity · Dec 23, 2022 · 8d5f01f · 8d5f01f
1 parent a541b45
commit 8d5f01f
Show file tree

Hide file tree

Showing 6 changed files with 373 additions and 98 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- CertReq:
+  - BREAKING CHANGE: Made Certificate FriendlyName a mandatory parameter - Fixes [Issue #269](https://github.com/dsccommunity/CertificateDsc/issues/269).
+  - Consider FriendlyName + Template when getting existing certs - Fixes [Issue #121](https://github.com/dsccommunity/CertificateDsc/issues/121).
+
+### Changed
+
 - Added support for publishing code coverage to `CodeCov.io` and
   Azure Pipelines - Fixes [Issue #255](https://github.com/dsccommunity/CertificateDsc/issues/255).
 - Updated build to use `Sampler.GitHubTasks` - Fixes [Issue #254](https://github.com/dsccommunity/CertificateDsc/issues/254).

diff --git a/azure-pipelines.yml b/azure-pipelines.yml
@@ -24,7 +24,8 @@ stages:
       - job: Package_Module
         displayName: 'Package Module'
         pool:
-          vmImage: 'ubuntu-latest'
+          # Fails on 'ubuntu-latest' as libmi shared object library not available
+          vmImage: 'windows-latest'
         steps:
           - pwsh: |
               dotnet tool install --global GitVersion.Tool

diff --git a/source/DSCResources/DSC_CertReq/DSC_CertReq.psm1 b/source/DSCResources/DSC_CertReq/DSC_CertReq.psm1
@@ -158,7 +158,7 @@ function Get-TargetResource
         [System.Boolean]
         $UseMachineContext,
 
-        [Parameter()]
+        [Parameter(Mandatory = $true)]
         [ValidateNotNullOrEmpty()]
         [System.String]
         $FriendlyName,
@@ -188,7 +188,7 @@ function Get-TargetResource
 
     Write-Verbose -Message ( @(
             "$($MyInvocation.MyCommand): "
-            $($script:localizedData.GettingCertReqStatusMessage -f $Subject, $CA)
+            $($script:localizedData.GettingCertReqStatusMessage -f $Subject, $CA, $FriendlyName, $CertificateTemplate)
         ) -join '' )
 
     # If the Subject does not contain a full X500 path, construct just the CN
@@ -199,11 +199,13 @@ function Get-TargetResource
 
     $cert = Get-ChildItem -Path Cert:\LocalMachine\My |
         Where-Object -FilterScript {
-            $_.Subject -eq $Subject -and `
-            (Compare-CertificateIssuer -Issuer $_.Issuer -CARootName $CARootName)
+            (Compare-CertificateSubject -ReferenceSubject $_.Subject -DifferenceSubject $Subject) -and `
+            (Compare-CertificateIssuer -Issuer $_.Issuer -CARootName $CARootName) -and `
+            (Get-CertificateTemplateName -Certificate $PSItem) -eq $CertificateTemplate -and `
+            $_.FriendlyName -eq $FriendlyName
         }
 
-    # If multiple certs have the same subject and were issued by the CA, return the newest
+    # If multiple certs have the same subject, issuer, friendly name and template, return the newest
     $cert = $cert |
         Sort-Object -Property NotBefore -Descending |
             Select-Object -First 1
@@ -212,7 +214,7 @@ function Get-TargetResource
     {
         Write-Verbose -Message ( @(
                 "$($MyInvocation.MyCommand): "
-                $($script:localizedData.CertificateExistsMessage -f $Subject, $ca, $cert.Thumbprint)
+                $($script:localizedData.CertificateExistsMessage -f $Subject, $ca, $FriendlyName, $CertificateTemplate, $certificate.Thumbprint)
             ) -join '' )
 
         $returnValue = @{
@@ -377,7 +379,7 @@ function Set-TargetResource
         [System.Boolean]
         $UseMachineContext,
 
-        [Parameter()]
+        [Parameter(Mandatory = $true)]
         [ValidateNotNullOrEmpty()]
         [System.String]
         $FriendlyName,
@@ -407,7 +409,7 @@ function Set-TargetResource
 
     Write-Verbose -Message ( @(
             "$($MyInvocation.MyCommand): "
-            $($script:localizedData.StartingCertReqMessage -f $Subject, $ca)
+            $($script:localizedData.StartingCertReqMessage -f $Subject, $ca, $FriendlyName, $CertificateTemplate)
         ) -join '' )
 
     # If the Subject does not contain a full X500 path, construct just the CN
@@ -671,7 +673,7 @@ CertificateTemplate = "$CertificateTemplate"
 
         if ($acceptRequest -match '0x')
         {
-            New-InvalidOperationException -Message ($script:localizedData.GenericErrorThrown -f ($acceptRequest | Out-String))
+            New-InvalidOperationException -Message ($script:localizedData.GenericError -f ($acceptRequest | Out-String))
         }
         else
         {
@@ -835,7 +837,7 @@ function Test-TargetResource
         [System.Boolean]
         $UseMachineContext,
 
-        [Parameter()]
+        [Parameter(Mandatory = $true)]
         [ValidateNotNullOrEmpty()]
         [System.String]
         $FriendlyName,
@@ -871,25 +873,28 @@ function Test-TargetResource
 
     Write-Verbose -Message ( @(
             "$($MyInvocation.MyCommand): "
-            $($script:localizedData.TestingCertReqStatusMessage -f $Subject, $ca)
+            $($script:localizedData.TestingCertReqStatusMessage -f $Subject, $ca, $FriendlyName, $CertificateTemplate)
         ) -join '' )
 
-    $certificate = Get-ChildItem -Path Cert:\LocalMachine\My |
-        Where-Object -FilterScript {
-            (Compare-CertificateSubject -ReferenceSubject $_.Subject -DifferenceSubject $Subject) -and `
-            (Compare-CertificateIssuer -Issuer $_.Issuer -CARootName $CARootName)
-        }
+    $filterScript = {
+        (Compare-CertificateSubject -ReferenceSubject $_.Subject -DifferenceSubject $Subject) -and `
+        (Compare-CertificateIssuer -Issuer $_.Issuer -CARootName $CARootName) -and `
+        (Get-CertificateTemplateName -Certificate $PSItem) -eq $CertificateTemplate -and `
+        $_.FriendlyName -eq $FriendlyName
+    }
 
     # Exception for standard template DomainControllerAuthentication
     if ($CertificateTemplate -eq 'DomainControllerAuthentication')
     {
-        $certificate = Get-ChildItem -Path Cert:\LocalMachine\My |
-            Where-Object -FilterScript {
-                (Get-CertificateTemplateName -Certificate $PSItem) -eq $CertificateTemplate -and `
-                (Compare-CertificateIssuer -Issuer $_.Issuer -CARootName $CARootName)
-            }
+        $filterScript = {
+            (Get-CertificateTemplateName -Certificate $PSItem) -eq $CertificateTemplate -and `
+            (Compare-CertificateIssuer -Issuer $_.Issuer -CARootName $CARootName)
+        }
     }
 
+    $certificate = Get-ChildItem -Path Cert:\LocalMachine\My |
+        Where-Object -FilterScript $filterScript
+
     # If multiple certs have the same subject and were issued by the CA, return the newest
     $certificate = $certificate |
         Sort-Object -Property NotBefore -Descending |
@@ -899,7 +904,7 @@ function Test-TargetResource
     {
         Write-Verbose -Message ( @(
                 "$($MyInvocation.MyCommand): "
-                $($script:localizedData.CertificateExistsMessage -f $Subject, $ca, $certificate.Thumbprint)
+                $($script:localizedData.CertificateExistsMessage -f $Subject, $ca, $FriendlyName, $CertificateTemplate, $certificate.Thumbprint)
             ) -join '' )
 
         if ($AutoRenew)
@@ -909,7 +914,7 @@ function Test-TargetResource
                 # The certificate was found but it is expiring within 30 days or has expired
                 Write-Verbose -Message ( @(
                         "$($MyInvocation.MyCommand): "
-                        $($script:localizedData.ExpiringCertificateMessage -f $Subject, $ca, $certificate.Thumbprint)
+                        $($script:localizedData.ExpiringCertificateMessage -f $Subject, $ca, $FriendlyName, $CertificateTemplate ,$certificate.Thumbprint)
                     ) -join '' )
                 return $false
             } # if
@@ -921,7 +926,7 @@ function Test-TargetResource
                 # The certificate has expired
                 Write-Verbose -Message ( @(
                         "$($MyInvocation.MyCommand): "
-                        $($script:localizedData.ExpiredCertificateMessage -f $Subject, $ca, $certificate.Thumbprint)
+                        $($script:localizedData.ExpiredCertificateMessage -f $Subject, $ca, $FriendlyName, $CertificateTemplate ,$certificate.Thumbprint)
                     ) -join '' )
                 return $false
             } # if
@@ -943,10 +948,10 @@ function Test-TargetResource
             }
 
             # Find out what SANs are on the current cert
+            $currentDns = @()
             if ($certificate.Extensions.Count -gt 0)
             {
                 $currentSanList = Get-CertificateSubjectAlternativeNameList -Certificate $certificate
-                $currentDns = @()
 
                 foreach ($san in $currentSanList)
                 {
@@ -956,53 +961,53 @@ function Test-TargetResource
                         $currentDns += $san.split('=')[1]
                     }
                 }
+            }
 
-                # Do the cert's DNS SANs and the desired DNS SANs match?
-                if (@(Compare-Object -ReferenceObject $currentDns -DifferenceObject $correctDns).Count -gt 0)
-                {
-                    return $false
-                }
+            if ($currentDns.Count -eq 0)
+            {
+                # There are no current DNS SANs and there should be
+                Write-Verbose -Message ( @(
+                        "$($MyInvocation.MyCommand): "
+                        $($script:localizedData.NoExistingSansMessage -f $Subject, $ca, $FriendlyName, $CertificateTemplate, ($correctDns -join ',') ,$certificate.Thumbprint)
+                    ) -join '' )
+                return $false
             }
-            else
+
+            if (@(Compare-Object -ReferenceObject $currentDns -DifferenceObject $correctDns).Count -gt 0)
             {
-                # There are no SANs and there should be
+                # Current DNS SANs and the desired DNS SANs do not match
+                Write-Verbose -Message ( @(
+                    "$($MyInvocation.MyCommand): "
+                    $($script:localizedData.SansMismatchMessage -f $Subject, $ca, $FriendlyName, $CertificateTemplate,($currentDns -join ',') , ($correctDns -join ',') ,$certificate.Thumbprint)
+                ) -join '' )
                 return $false
             }
         }
 
         $currentCertificateTemplateName = Get-CertificateTemplateName -Certificate $certificate
 
-        if ($CertificateTemplate -ne $currentCertificateTemplateName)
-        {
-            Write-Verbose -Message ( @(
-                    "$($MyInvocation.MyCommand): "
-                    $($script:localizedData.CertTemplateMismatch -f $Subject, $ca, $certificate.Thumbprint, $currentCertificateTemplateName)
-                ) -join '' )
-            return $false
-        } # if
-
         # Check the friendly name of the certificate matches
         if ($FriendlyName -ne $certificate.FriendlyName)
         {
             Write-Verbose -Message ( @(
                     "$($MyInvocation.MyCommand): "
-                    $($script:localizedData.CertFriendlyNameMismatch -f $Subject, $ca, $certificate.Thumbprint, $certificate.FriendlyName)
+                    $($script:localizedData.CertFriendlyNameMismatchMessage -f $Subject, $ca, $currentCertificateTemplateName, $certificate.Thumbprint, $certificate.FriendlyName)
                 ) -join '' )
             return $false
         } # if
 
         # The certificate was found and is OK - so no change required.
         Write-Verbose -Message ( @(
                 "$($MyInvocation.MyCommand): "
-                $($script:localizedData.ValidCertificateExistsMessage -f $Subject, $ca, $certificate.Thumbprint)
+                $($script:localizedData.ValidCertificateExistsMessage -f $Subject, $ca, $FriendlyName, $CertificateTemplate, $certificate.Thumbprint)
             ) -join '' )
         return $true
     } # if
 
     # A valid certificate was not found
     Write-Verbose -Message ( @(
             "$($MyInvocation.MyCommand): "
-            $($script:localizedData.NoValidCertificateMessage -f $Subject, $ca)
+            $($script:localizedData.NoValidCertificateMessage -f $Subject, $ca, $FriendlyName, $CertificateTemplate)
         ) -join '' )
     return $false
 } # end function Test-TargetResource
@@ -1037,7 +1042,7 @@ 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')))
     {
-        New-InvalidArgumentException -Message (($script:localizedData.InvalidKeySize) -f $KeyLength, $KeyType) -ArgumentName 'KeyLength'
+        New-InvalidArgumentException -Message (($script:localizedData.InvalidKeySizeError) -f $KeyLength, $KeyType) -ArgumentName 'KeyLength'
     }
 }# end function Assert-ResourceProperty
 

diff --git a/source/DSCResources/DSC_CertReq/DSC_CertReq.schema.mof b/source/DSCResources/DSC_CertReq/DSC_CertReq.schema.mof
@@ -2,6 +2,7 @@
 class DSC_CertReq : OMI_BaseResource
 {
     [Key, Description("Provide the text string to use as the subject of the certificate.")] String Subject;
+    [Key, Description("Specifies a friendly name for the certificate.")] String FriendlyName;
     [Write, Description("The type of CA in use, Standalone/Enterprise.")] String CAType;
     [Write, Description("The FQDN of the Active Directory Certificate Authority on the local area network. Leave empty to automatically locate.")] String CAServerFQDN;
     [Write, Description("The name of the certificate authority, by default this will be in format domain-servername-ca. Leave empty to automatically locate.")] String CARootName;
@@ -17,7 +18,6 @@ class DSC_CertReq : OMI_BaseResource
     [Write, Description("The URL to the Certification Enrollment Policy Service.")] String CepURL;
     [Write, Description("The URL to the Certification Enrollment Service.")] String CesURL;
     [Write, Description("Indicates whether or not the flag -adminforcemachine will be used when requesting certificates. Necessary for certain templates like e.g. DomainControllerAuthentication")] Boolean UseMachineContext;
-    [Write, Description("Specifies a friendly name for the certificate.")] String FriendlyName;
     [Write, Description("Specifies if the key type should be RSA or ECDH, defaults to RSA."), ValueMap{"RSA","ECDH"}, Values{"RSA","ECDH"}] String KeyType;
     [Write, Description("Specifies if the request type should be CMC or PKCS10, deafults to CMC."), ValueMap{"CMC","PKCS10"},Values{"CMC","PKCS10"}] String RequestType;
 };
diff --git a/source/DSCResources/DSC_CertReq/en-US/DSC_CertReq.strings.psd1 b/source/DSCResources/DSC_CertReq/en-US/DSC_CertReq.strings.psd1
@@ -1,7 +1,7 @@
 ConvertFrom-StringData @'
-    GettingCertReqStatusMessage = Getting Certificate with Subject '{0}' issued by {1}.
-    CertificateExistsMessage = Certificate with Subject '{0}' issued by {1} found with thumbprint '{2}'.
-    StartingCertReqMessage = Starting Certificate request with Subject '{0}' issued by {1}.
+    GettingCertReqStatusMessage = Getting Certificate with Subject '{0}', Issuer '{1}', FriendlyName '{2}' and CertificateTemplate '{3}'.
+    CertificateExistsMessage = Certificate with Subject '{0}', Issuer '{1}', FriendlyName '{2}' and CertificateTemplate '{3}' found with Thumbprint '{4}'.
+    StartingCertReqMessage = Starting Certificate request with Subject '{0}', Issuer '{1}', FriendlyName '{2}' and CertificateTemplate '{3}'.
     CreateRequestCertificateMessage = Creating certificate request '{1}' from '{0}'.
     CreateRequestResultCertificateMessage = Create certificate request result: {0}
     SubmittingRequestCertificateMessage = Submitting certificate request '{0}' returning '{1}' issued by {2}.
@@ -10,16 +10,17 @@ ConvertFrom-StringData @'
     AcceptingRequestCertificateMessage = Accepting certificate '{1}' issued by {0}.
     AcceptingRequestResultCertificateMessage = Accepting certificate result: {0}
     CleaningUpRequestFilesMessage = Cleaning up certificate request files '{0}'.
-    TestingCertReqStatusMessage = Testing Certificate with Subject '{0}' issued by {1}.
-    ExpiringCertificateMessage = The certificate found with subject '{0}' issued by {1} with thumbprint '{2}' is about to expire.
-    NoValidCertificateMessage = No valid certificate found with subject '{0}' issued by {1}.
-    ExpiredCertificateMessage = The certificate found with subject '{0}' issued by {1} with thumbprint '{2}' has expired.
-    ValidCertificateExistsMessage = Valid certificate '{2}' found with subject '{0}' issued by {1}.
+    TestingCertReqStatusMessage = Testing Certificate with Subject '{0}', Issuer '{1}', FriendlyName '{2}' and CertificateTemplate '{3}'.
+    ExpiringCertificateMessage = The certificate found with Subject '{0}', Issuer '{1}', FriendlyName '{2}' and CertificateTemplate '{3}' is about to expire.
+    NoValidCertificateMessage = No valid certificate found with Subject '{0}', Issuer '{1}', FriendlyName '{2}' and CertificateTemplate '{3}'.
+    ExpiredCertificateMessage = The certificate found with Subject '{0}', Issuer '{1}', FriendlyName '{2}' and CertificateTemplate '{3}' has expired: {4}.
+    NoExistingSansMessage = The certificate found with Subject '{0}', Issuer '{1}', FriendlyName '{2}' and CertificateTemplate '{3}' has no SANs, yet the following SANs are specified: {4}. Certificate has the Thumbprint '{5}'.
+    SansMismatchMessage = The certificate found with Subject '{0}', Issuer '{1}', FriendlyName '{2}' and CertificateTemplate '{3}' has the SANs '{4}', yet the following SANs are specified: {5}. Certificate has the Thumbprint '{6}'.
+    ValidCertificateExistsMessage = Valid certificate found with Subject '{0}', Issuer '{1}', FriendlyName '{2}' and CertificateTemplate '{3}': {4}
     CertificateReqNotFoundError = Certificate Request file '{0}' not found.
     CertificateCerNotFoundError = Certificate file '{0}' not found.
     CertReqOutNotFoundError = CertReq.exe output file '{0}' not found.
-    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}
+    CertFriendlyNameMismatchMessage = The certificate with Subject '{0}', Issuer '{1}', CertificateTemplate '{2}' and Thumbprint '{3}' has the wrong friendly name: {4}.
+    InvalidKeySizeError = The key length '{0}' specified is invalid for '{1}' key types.
+    GenericError = A Generic Error was thrown when accepting a Certificate. It threw the following Error message: {0}
 '@