xCertificateExport: Fixed bug causing pfx export with matchsource enabled to fail

[hackjammer]

Pull Request (PR) description
Fixed pfx import check when matchsource is enabled to use a securestring instead of attempting to use a credential object.
Also updated integration test to make this less likely to happen again (now just creating a plaintext password and a credential variable as used in the DSC resource itself).

This Pull Request (PR) fixes the following issues:
#116

---

This change is 

[codecov-io]

Codecov Report

Merging #117 into dev will not change coverage.
The diff coverage is 100%.

@@        Coverage Diff         @@
##           dev   #117   +/-   ##
==================================
  Coverage   94%    94%           
==================================
  Files        5      5           
  Lines      517    517           
  Branches     2      2           
==================================
  Hits       488    488           
  Misses      27     27           
  Partials     2      2

[PlagueHO]

Great job @hackjammer

I'm a bit worried about this change though. All the documentation I can find as well as examining the System.Security.Cryptography.X509Certificates.X509Certificate2Collection objects on Windows Server 2016/Windows 10 and Windows Server 2012 R2 indicate that the Import method does not support SecureString - only string. The type you linked to in your second link is actually for a different object type.

So, with that in mind I'd suggest the following (and see all my other comments):

Could you replace the Context 'Export PFX' { ... } integration tests in the file MSFT_xCertificateExport.Integration.Test.ps1 with:

        Context 'Export PFX and then Export PFX again to ensure no errors' {
            #region DEFAULT TESTS
            It 'Should compile and apply the MOF without throwing' {
                {
                    $ConfigData = @{
                        AllNodes = @(
                            @{
                                NodeName                    = 'localhost'
                                Path                        = $script:pfxPath
                                FriendlyName                = $certFriendlyName
                                Subject                     = $certificateSubject
                                DNSName                     = $certificateDNSNames
                                Issuer                      = $certificateSubject
                                KeyUsage                    = $certificateKeyUsage
                                EnhancedKeyUsage            = $certificateEKU
                                MatchSource                 = $true
                                Type                        = 'PFX'
                                ChainOption                 = 'BuildChain'
                                Password                    = $pfxCredential
                                PsDscAllowPlainTextPassword = $true
                            }
                        )
                    }

                    & "$($script:DSCResourceName)_Config" `
                        -OutputPath $TestDrive `
                        -ConfigurationData $ConfigData

                    Start-DscConfiguration `
                        -Path $TestDrive `
                        -ComputerName localhost `
                        -Wait `
                        -Verbose `
                        -Force `
                        -ErrorAction Stop
                } | Should -Not -Throw
            }

            It 'Should be able to call Get-DscConfiguration without throwing' {
                { $script:currentPFX = Get-DscConfiguration -Verbose -ErrorAction Stop } | Should -Not -Throw
            }
            #endregion

            It 'Should have exported a PFX certificate' {
                $script:currentPFX.IsExported | Should -Be $true
            }

            It 'Should have set the resource and the thumbprint of the exported certificate should match' {
                $exportedCertificate = New-Object -TypeName 'System.Security.Cryptography.X509Certificates.X509Certificate2Collection'
                $exportedCertificate.Import($script:certificatePath,$pfxPassword,[System.Security.Cryptography.X509Certificates.X509KeyStorageFlags]::PersistKeySet)
                $exportedCertificate[0].Thumbprint | Should -Be $script:validCertificateThumbprint
            }

            # Apply the MOF a second time to ensure no errors occur
            It 'Should apply the MOF a second time without throwing' {
                {
                    Start-DscConfiguration `
                        -Path $TestDrive `
                        -ComputerName localhost `
                        -Wait `
                        -Verbose `
                        -Force `
                        -ErrorAction Stop
                } | Should -Not -Throw
            }

            It 'Should be able to call Get-DscConfiguration without throwing' {
                { $script:currentPFX = Get-DscConfiguration -Verbose -ErrorAction Stop } | Should -Not -Throw
            }
            #endregion

            It 'Should have exported a PFX certificate' {
                $script:currentPFX.IsExported | Should -Be $true
            }

            It 'Should have set the resource and the thumbprint of the exported certificate should match' {
                $exportedCertificate = New-Object -TypeName 'System.Security.Cryptography.X509Certificates.X509Certificate2Collection'
                $exportedCertificate.Import($script:certificatePath,$pfxPassword,[System.Security.Cryptography.X509Certificates.X509KeyStorageFlags]::PersistKeySet)
                $exportedCertificate[0].Thumbprint | Should -Be $script:validCertificateThumbprint
            }
        }

I actually started fixing this one before I saw you already had - so I'd updated the integration tests to catch this one (I like to practice test driven development wherever I can). So these are the tests I created that validate this.

---

Reviewed 3 of 3 files at r1.
Review status: all files reviewed at latest revision, 3 unresolved discussions.

---

CHANGELOG.md, line 6 at r1 (raw file):

- xCertificateExport:
  - Fixed bug causing pfx export with matchsource enabled to fail

Can you add link to issue as per other changes. E.g.
- see [issue #117](https://github.com/PowerShell/xCertificate/issues/117).

---

Modules/xCertificate/DSCResources/MSFT_xCertificateExport/MSFT_xCertificateExport.psm1, line 446 at r1 (raw file):

                elseif ($Type -eq 'PFX')
                {
                    $exportedCertificate.Import($Path, $Password.Password, [System.Security.Cryptography.X509Certificates.X509KeyStorageFlags]::PersistKeySet)

You'll need to use $Password.GetNetworkCredential().Password here. To validate this I actually updated the integration tests.

---

Tests/Integration/MSFT_xCertificateExport.Integration.Tests.ps1, line 151 at r1 (raw file):

            It 'Should have set the resource and the thumbprint of the exported certificate should match' {
                $exportedCertificate = New-Object -TypeName 'System.Security.Cryptography.X509Certificates.X509Certificate2Collection'
                $exportedCertificate.Import($script:certificatePath,$pfxCredential.Password,[System.Security.Cryptography.X509Certificates.X509KeyStorageFlags]::PersistKeySet)

This won't work because the Import method of System.Security.Cryptography.X509Certificates.X509Certificate2Collection does not support a password using secure string. So you'll need to use $pfxCredential.GetNetworkCredential().Password or revert these changes back to passing the PFX as a string.

---

Comments from Reviewable

[PlagueHO]

Hi @hackjammer - this one now has some merge conflicts in it due to the previously merged PR. Can you resolve?

[PlagueHO]

@hackjammer - are you still planning on working on this one?

[johlju]

Labeling this abandoned so that someone else can continue this work.

[PlagueHO]

I'm picking this one up now, so will close this and reopen a new PR.