Merge pull request #413 from wh33ly/HotfixSubject

Website binding certificate subject compare fails with multiple entries
dsccommunity · Mar 16, 2019 · 3ecc117 · 3ecc117
2 parents cd15c1a + 3abb213
commit 3ecc117
Show file tree

Hide file tree

Showing 3 changed files with 73 additions and 4 deletions.
diff --git a/DSCResources/Helper.psm1 b/DSCResources/Helper.psm1
@@ -165,7 +165,10 @@ function Find-Certificate
 
     if ($PSBoundParameters.ContainsKey('Subject'))
     {
-        $certFilters += @('($_.Subject -eq $Subject)')
+        $certFilters += @('(@(Compare-Object `
+                            -ReferenceObject (($_.Subject -split ", ").trim()|sort-object) `
+                            -DifferenceObject (($subject -split ",").trim()|sort-object)| `
+                            Where-Object -Property SideIndicator -eq "=>").Count -eq 0)')
     } # if
 
     if ($PSBoundParameters.ContainsKey('Issuer'))
@@ -180,17 +183,26 @@ function Find-Certificate
 
     if ($PSBoundParameters.ContainsKey('DNSName'))
     {
-        $certFilters += @('(@(Compare-Object -ReferenceObject $_.DNSNameList.Unicode -DifferenceObject $DNSName | Where-Object -Property SideIndicator -eq "=>").Count -eq 0)')
+        $certFilters += @('(@(Compare-Object `
+                            -ReferenceObject $_.DNSNameList.Unicode `
+                            -DifferenceObject $DNSName | `
+                            Where-Object -Property SideIndicator -eq "=>").Count -eq 0)')
     } # if
 
     if ($PSBoundParameters.ContainsKey('KeyUsage'))
     {
-        $certFilters += @('(@(Compare-Object -ReferenceObject ($_.Extensions.KeyUsages -split ", ") -DifferenceObject $KeyUsage | Where-Object -Property SideIndicator -eq "=>").Count -eq 0)')
+        $certFilters += @('(@(Compare-Object `
+                            -ReferenceObject ($_.Extensions.KeyUsages -split ", ") `
+                            -DifferenceObject $KeyUsage | `
+                            Where-Object -Property SideIndicator -eq "=>").Count -eq 0)')
     } # if
 
     if ($PSBoundParameters.ContainsKey('EnhancedKeyUsage'))
     {
-        $certFilters += @('(@(Compare-Object -ReferenceObject ($_.EnhancedKeyUsageList.FriendlyName) -DifferenceObject $EnhancedKeyUsage | Where-Object -Property SideIndicator -eq "=>").Count -eq 0)')
+        $certFilters += @('(@(Compare-Object `
+                            -ReferenceObject ($_.EnhancedKeyUsageList.FriendlyName) `
+                            -DifferenceObject $EnhancedKeyUsage | `
+                            Where-Object -Property SideIndicator -eq "=>").Count -eq 0)')
     } # if
 
     # Join all the filters together

diff --git a/README.md b/README.md
@@ -319,6 +319,10 @@ This resource manages the IIS configuration section locking (overrideMode) to co
 ## Versions
 
 ### Unreleased
+* Fix subject comparison multiple entries for helper function `Find-Certificate` that could not find the test
+  helper function `Install-NewSelfSignedCertificateExScript`.
+* Updated unit test for helper function `Find-Certificate` to check for multiple
+  subject names in different orders.
 
 ### 2.5.0.0
 

diff --git a/Tests/Unit/Helper.Tests.ps1 b/Tests/Unit/Helper.Tests.ps1
@@ -36,6 +36,9 @@ try
             $certEKUReverse = @('Client authentication','Server Authentication')
             $certEKUNoMatch = $certEKU + @('Encrypting File System')
             $certSubject = 'CN=contoso, DC=com'
+            $certSubjectLong = 'CN=contoso, [email protected], O=Fabrikam., OU=IT, L=Location, S=State, C=Country'
+            $certSubjectNoSpace = 'CN=contoso,[email protected],O=Fabrikam.,OU=IT,L=Location,S=State,C=Country'
+            $certSubjectLongReverse = '[email protected],O=Fabrikam.,L=Location,CN=contoso,OU=IT,S=State,C=Country'
             $certFriendlyName = 'Contoso Test Cert'
             $validCert = New-SelfSignedCertificateEx `
                 -Subject $certSubject `
@@ -51,6 +54,21 @@ try
             $validCert = Get-Item -Path "cert:\CurrentUser\My\$validThumbprint"
             Remove-Item -Path $validCert.PSPath -Force
 
+            # Generate the long subject certificate for testing but remove it from the store straight away
+            $validCertSubjectLong = New-SelfSignedCertificateEx `
+                -Subject $certSubjectLong `
+                -KeyUsage $certKeyUsage `
+                -KeySpec 'Exchange' `
+                -EKU $certEKU `
+                -SubjectAlternativeName $certDNSNames `
+                -FriendlyName $certFriendlyName `
+                -StoreLocation 'CurrentUser' `
+                -Exportable
+            # Pull the generated certificate from the store so we have the friendlyname
+            $longThumbprint = $validCertSubjectLong.Thumbprint
+            $validCertSubjectLong = Get-Item -Path "cert:\CurrentUser\My\$longThumbprint"
+            Remove-Item -Path $validCertSubjectLong.PSPath -Force
+
             # Generate the Expired certificate for testing but remove it from the store straight away
             $expiredCert = New-SelfSignedCertificateEx `
                 -Subject $certSubject `
@@ -94,6 +112,11 @@ try
                         return @( $expiredCert )
                     }
 
+                    'cert:\LocalMachine\LongSubject'
+                    {
+                        return @( $validCertSubjectLong )
+                    }
+
                     default
                     {
                         throw 'mock called with unexpected value {0}' -f $Path
@@ -201,6 +224,36 @@ try
                 }
             }
 
+            Context 'Subject only is passed and certificate with a different subject order exists' {
+                It 'should not throw exception' {
+                    { $script:result = Find-Certificate -Subject $certSubjectLongReverse -Store 'LongSubject' } | Should Not Throw
+                }
+
+                It 'should return expected certificate' {
+                    $script:result.Thumbprint | Should Be $longThumbprint
+                }
+
+                It 'should call expected mocks' {
+                    Assert-MockCalled -CommandName Test-Path -Exactly -Times 1
+                    Assert-MockCalled -CommandName Get-ChildItem -Exactly -Times 1
+                }
+            }
+
+            Context 'Subject only is passed and certificate subject without spaces exists' {
+                It 'should not throw exception' {
+                    { $script:result = Find-Certificate -Subject $certSubjectNoSpace -Store 'LongSubject' } | Should Not Throw
+                }
+
+                It 'should return expected certificate' {
+                    $script:result.Thumbprint | Should Be $longThumbprint
+                }
+
+                It 'should call expected mocks' {
+                    Assert-MockCalled -CommandName Test-Path -Exactly -Times 1
+                    Assert-MockCalled -CommandName Get-ChildItem -Exactly -Times 1
+                }
+            }
+
             Context 'Issuer only is passed and matching certificate exists' {
                 It 'should not throw exception' {
                     { $script:result = Find-Certificate -Issuer $certSubject } | Should Not Throw