diff --git a/private/testing/Get-TestConfig.ps1 b/private/testing/Get-TestConfig.ps1 index 7a366bb458c..5dedb57db6f 100644 --- a/private/testing/Get-TestConfig.ps1 +++ b/private/testing/Get-TestConfig.ps1 @@ -34,6 +34,13 @@ function Get-TestConfig { AzureSqlDbServer = $null AzureSqlDbName = $null AzureSqlDbCred = $null + # The Active Directory CA that the tests of a pending certificate request need, and a certificate template on it + # that holds every request as pending until a CA manager approves it. The account that runs the tests must be + # allowed to enroll with that template and to approve requests on that CA. No CI environment has a CA, so these + # stay empty there and those tests skip themselves. A local configuration whose lab has such a CA sets all three. + CaServer = $null + CaName = $null + ApprovalTemplate = $null } if (Test-Path $LocalConfigPath) { diff --git a/public/New-DbaComputerCertificate.ps1 b/public/New-DbaComputerCertificate.ps1 index fe322955b0b..40aaa746f5d 100644 --- a/public/New-DbaComputerCertificate.ps1 +++ b/public/New-DbaComputerCertificate.ps1 @@ -13,6 +13,10 @@ function New-DbaComputerCertificate { It makes a lot of assumptions - namely, that your account is allowed to auto-enroll and that you have permission to do everything it needs to do ;) + When the CA holds the request as pending, for example because the template requires the approval of a CA manager, no certificate + is returned. The warning names the request ID and the certreq commands that install the certificate once it is issued; the request + and its private key stay in LocalMachine\REQUEST of the computer that runs the command until then. + References: https://www.itprotoday.com/sql-server/7-steps-ssl-encryption https://azurebi.jppp.org/2016/01/23/using-lets-encrypt-certificates-for-secure-sql-server-connections/ @@ -20,6 +24,8 @@ function New-DbaComputerCertificate { The certificate is generated using AD's webserver SSL template on the client machine and pushed to the remote machine. + The command leaves nothing behind on the computer it runs on: the copy of a self-signed certificate that certreq puts into the intermediate CA store is removed right away, a request the CA did not answer is removed from the REQUEST store together with its key, and a certificate created for another computer is removed from the local store together with its key once it has been exported. + .PARAMETER ComputerName Specifies the target computer or computers where the certificate will be created and installed. Defaults to localhost. For SQL Server clusters, specify each cluster node here and use ClusterInstanceName for the cluster's virtual name. @@ -381,20 +387,20 @@ function New-DbaComputerCertificate { } } - $certDir = "$tempDir\$fqdn" + # A folder of its own for every call. A folder named only after the computer was shared by concurrent calls + # for the same computer: one call deleted the request files of the other, and a failed call could then read + # the request of the other call and remove that one instead of its own. + # The name must not start with dbatools: the maintenance task tempcleanup deletes everything in the temp folder + # that does, one minute after the module is imported, and would take the folder away while certreq writes to it. + $certDir = "$tempDir\certreq-$fqdn-$([System.Guid]::NewGuid())" $certCfg = "$certDir\request.inf" $certCsr = "$certDir\$fqdn.csr" $certCrt = "$certDir\$fqdn.crt" $certPfx = "$certDir\$fqdn.pfx" $tempPfx = "$certDir\temp-$fqdn.pfx" - if (Test-Path($certDir)) { - Write-ProgressHelper -StepNumber ($stepCounter++) -Message "Deleting files from $certDir" - $null = Remove-Item "$certDir\*.*" - } else { - Write-ProgressHelper -StepNumber ($stepCounter++) -Message "Creating $certDir" - $null = New-Item -Path $certDir -ItemType Directory -Force - } + Write-ProgressHelper -StepNumber ($stepCounter++) -Message "Creating $certDir" + $null = New-Item -Path $certDir -ItemType Directory -Force # Make sure output is compat with clusters $shortName = $fqdn.Split(".")[0] @@ -456,34 +462,100 @@ function New-DbaComputerCertificate { Add-Content $certCfg $san Add-Content $certCfg "Critical=2.5.29.17" + $ownRequests = @() if ($PScmdlet.ShouldProcess("local", "Creating certificate for $computer")) { - Write-ProgressHelper -StepNumber ($stepCounter++) -Message "Running: certreq -new $certCfg $certCsr" - $create = certreq -new $certCfg $certCsr + Write-ProgressHelper -StepNumber ($stepCounter++) -Message "Running: certreq -q -new $certCfg $certCsr" + $create = certreq -q -new $certCfg $certCsr + + # The pending request of this call, recorded now: its public key is in the request file certreq just wrote, + # and every request has its own key. Should the CA not issue the certificate, exactly this request goes. + if (-not $SelfSigned -and (Test-Path -Path $certCsr)) { + $requestFileBase64 = (Get-Content -Path $certCsr | Where-Object { $PSItem -notmatch "^-----" }) -join "" + $requestFileHex = [System.BitConverter]::ToString([System.Convert]::FromBase64String($requestFileBase64)) + $ownRequests = @(Get-ChildItem -Path Cert:\LocalMachine\REQUEST -ErrorAction SilentlyContinue | Where-Object { $requestFileHex.Contains([System.BitConverter]::ToString($PSItem.PublicKey.EncodedKeyValue.RawData)) } | ForEach-Object { $PSItem.Thumbprint }) + } } if ($SelfSigned) { $serial = (($create -Split "Serial Number:" -Split "Subject")[2]).Trim() # D: $storedCert = Get-ChildItem Cert:\LocalMachine\My -Recurse | Where-Object SerialNumber -eq $serial + # certreq installs a self-signed certificate twice: with its key in LocalMachine\My, and without the key in + # LocalMachine\CA, the intermediate CA store. The copy serves no purpose, a self-signed certificate is its own + # root, and it stays behind when the certificate is removed later. So it goes right away. + if ($storedCert) { + $caStore = New-Object System.Security.Cryptography.X509Certificates.X509Store -ArgumentList "CA", "LocalMachine" + $caStore.Open("ReadWrite") + foreach ($caCopy in $caStore.Certificates.Find("FindByThumbprint", $storedCert.Thumbprint, $false)) { + $caStore.Remove($caCopy) + } + $caStore.Close() + } + if ($computer.IsLocalHost) { $storedCert | Select-Object * | Select-DefaultView -Property FriendlyName, DnsNameList, Thumbprint, NotBefore, NotAfter, Subject, Issuer } } else { + $submit = $null + $submitExitCode = $null if ($PScmdlet.ShouldProcess("local", "Submitting certificate request for $computer to $CaServer\$CaName")) { - Write-ProgressHelper -StepNumber ($stepCounter++) -Message "certreq -submit -config `"$CaServer\$CaName`" -attrib $certTemplate $certCsr $certCrt $certPfx" - $submit = certreq -submit -config "$CaServer\$CaName" -attrib $certTemplate $certCsr $certCrt $certPfx + Write-ProgressHelper -StepNumber ($stepCounter++) -Message "certreq -q -submit -config `"$CaServer\$CaName`" -attrib $certTemplate $certCsr $certCrt $certPfx" + $submit = certreq -q -submit -config "$CaServer\$CaName" -attrib $certTemplate $certCsr $certCrt $certPfx + $submitExitCode = $LASTEXITCODE } - if ($submit -match "ssued") { - Write-ProgressHelper -StepNumber ($stepCounter++) -Message "certreq -accept -machine $certCrt" - $null = certreq -accept -machine $certCrt + # The outcome comes from the exit code and the certificate file, not from the messages: they are localized, + # and "Certificate not issued" of a refused request contains "issued" as well. certreq ends with 0 when the + # CA took the request, and writes the certificate file only when the CA issued the certificate right away. + # A CA may also hold a request as pending, for example until a CA manager approves it. + if ($submitExitCode -eq 0 -and (Test-Path -Path $certCrt)) { + Write-ProgressHelper -StepNumber ($stepCounter++) -Message "certreq -q -accept -machine $certCrt" + $null = certreq -q -accept -machine $certCrt $cert = New-Object System.Security.Cryptography.X509Certificates.X509Certificate2 ($certCrt, $null, [System.Security.Cryptography.X509Certificates.X509KeyStorageFlags]::DefaultKeySet) $storedCert = Get-ChildItem "Cert:\$store\$folder" -Recurse | Where-Object { $_.Thumbprint -eq $cert.Thumbprint } - } elseif ($submit) { + } elseif ($submitExitCode -eq 0) { + # The request is pending at the CA. It stays in LocalMachine\REQUEST with its private key, because the + # certificate the CA issues later can only be installed with that key. The request files are not needed for that. + Write-Message -Level Verbose -Message "$submit" + $requestId = ([regex]::Match("$submit", "RequestId:\s*(\d+)")).Groups[1].Value + if (-not $requestId) { + $requestId = "" + } + $requestCrt = "$computer-$requestId.crt" + $pendingMessage = "The CA $CaServer\$CaName holds the certificate request $requestId for $computer as pending, for example until a CA manager approves it. The request and its private key stay in LocalMachine\REQUEST on $env:COMPUTERNAME. Once the CA has issued the certificate, install it on $env:COMPUTERNAME with: certreq -retrieve -config `"$CaServer\$CaName`" $requestId $requestCrt, then certreq -accept -machine $requestCrt." + if (-not $computer.IsLocalHost) { + $pendingMessage += " Then export it with its private key and import it on $computer with Add-DbaComputerCertificate." + } + Remove-Item -Path $certDir -Recurse -Force -ErrorAction SilentlyContinue + Stop-Function -Message $pendingMessage -Target $computer -Continue + } elseif ($null -ne $submitExitCode) { Write-Message -Level Warning -Message "Something went wrong" Write-Message -Level Warning -Message "$create" Write-Message -Level Warning -Message "$submit" - Stop-Function -Message "Failure when attempting to create the cert on $computer. Exception: $_" -Target $computer -Continue + # The CA did not issue the certificate, so the pending request and its key would stay in + # LocalMachine\REQUEST forever. They go with the failure. + # Other requests may have been created in the meantime by someone else, so only the request of this + # call goes: the one whose public key is in the request file certreq wrote. Every request has its own key. + # The request was recorded right after certreq -new, not read back from the file now. + $pendingRequests = @(Get-ChildItem -Path Cert:\LocalMachine\REQUEST -ErrorAction SilentlyContinue | Where-Object Thumbprint -in $ownRequests | ForEach-Object { $PSItem.Thumbprint }) + foreach ($pendingRequest in $pendingRequests) { + Write-Message -Level Verbose -Message "Removing the pending request $pendingRequest and its key from LocalMachine\REQUEST" + $splatRemoveRequest = @{ + Thumbprint = $pendingRequest + Folder = "REQUEST" + DeleteKey = $true + Confirm = $false + EnableException = $true + } + try { + $null = Remove-DbaComputerCertificate @splatRemoveRequest + } catch { + Write-Message -Level Warning -Message "The pending request $pendingRequest could not be removed from LocalMachine\REQUEST: $PSItem" + } + } + # The -Continue skips the removal of the request folder at the end, and every call has a folder of its own. + Remove-Item -Path $certDir -Recurse -Force -ErrorAction SilentlyContinue + Stop-Function -Message "Failure when attempting to create the cert on $computer. $($submit | Select-Object -Last 1)" -Target $computer -Continue } if ($Computer.IsLocalHost) { @@ -501,7 +573,23 @@ function New-DbaComputerCertificate { } if ($PScmdlet.ShouldProcess("local", "Removing cert from disk but keeping it in memory")) { - $storedCert | Remove-Item + # The certificate now lives in the PFX data and belongs to the target computer. Removing only the store entry + # would leave its private key on this computer, so the key is deleted with it. + $splatRemoveLocal = @{ + Thumbprint = $storedCert.Thumbprint + DeleteKey = $true + Confirm = $false + EnableException = $true + } + try { + $localRemoval = Remove-DbaComputerCertificate @splatRemoveLocal + if ($localRemoval.PrivateKey -ne "Deleted") { + Write-Message -Level Warning -Message "The private key of the certificate $($storedCert.Thumbprint) is still on $env:COMPUTERNAME: $($localRemoval.PrivateKey)" + } + } catch { + # The PFX data is there, so the import on the target still goes ahead. + Write-Message -Level Warning -Message "The certificate $($storedCert.Thumbprint) could not be removed from LocalMachine\My on $env:COMPUTERNAME: $PSItem" + } } if ($ClusterInstanceName) { $secondaryNode = $true } @@ -527,12 +615,14 @@ function New-DbaComputerCertificate { if ($PScmdlet.ShouldProcess($computer, "Attempting to import new cert")) { if ($flags -contains "UserProtected" -and -not $computer.IsLocalHost) { + Remove-Item -Path $certDir -Recurse -Force -ErrorAction SilentlyContinue Stop-Function -Message "UserProtected flag is only valid for localhost because it causes a prompt, skipping for $computer" -Continue } try { $thumbprint = (Invoke-Command2 -ComputerName $computer -Credential $Credential -ArgumentList $certdata, $SecurePassword, $Store, $Folder, $flags -ScriptBlock $scriptBlock -ErrorAction Stop -Verbose).Thumbprint Get-DbaComputerCertificate -ComputerName $computer -Credential $Credential -Thumbprint $thumbprint } catch { + Remove-Item -Path $certDir -Recurse -Force -ErrorAction SilentlyContinue Stop-Function -Message "Issue importing new cert on $computer" -ErrorRecord $_ -Target $computer -Continue } } @@ -541,7 +631,7 @@ function New-DbaComputerCertificate { try { Remove-Item -Force -Recurse $certDir -ErrorAction SilentlyContinue } catch { - Stop-Function "Isue removing files from $certDir" -Target $certDir -ErrorRecord $_ + Stop-Function -Message "Issue removing files from $certDir" -Target $certDir -ErrorRecord $PSItem } } Write-ProgressHelper -Completed diff --git a/tests/New-DbaComputerCertificate.Tests.ps1 b/tests/New-DbaComputerCertificate.Tests.ps1 index 1328db554df..84f6ea4716a 100644 --- a/tests/New-DbaComputerCertificate.Tests.ps1 +++ b/tests/New-DbaComputerCertificate.Tests.ps1 @@ -125,6 +125,23 @@ Describe $CommandName -Tag UnitTests { # These tests used to be skipped when APPVEYOR is set, which the Azure lanes do as well. Creating a self-signed # certificate in LocalMachine\My works on the runners, so they run there now. Describe $CommandName -Tag IntegrationTests { + BeforeAll { + # The thumbprints of the requests in LocalMachine\REQUEST whose public key is in one of the given request files. + # Every request has its own key, so this names exactly the requests of those files and nothing that another + # enrollment on this computer created in the meantime. + function Get-TestRequestThumbprint { + param ([string[]]$RequestFile) + foreach ($file in $RequestFile) { + if (-not (Test-Path -Path $file)) { + continue + } + $requestBase64 = (Get-Content -Path $file | Where-Object { $PSItem -notmatch "^-----" }) -join "" + $requestHex = [System.BitConverter]::ToString([System.Convert]::FromBase64String($requestBase64)) + Get-ChildItem -Path Cert:\LocalMachine\REQUEST | Where-Object { $requestHex.Contains([System.BitConverter]::ToString($PSItem.PublicKey.EncodedKeyValue.RawData)) } | ForEach-Object { $PSItem.Thumbprint } + } + } + } + Context "Can generate a new certificate with default settings" { BeforeAll { $defaultCert = New-DbaComputerCertificate -SelfSigned -EnableException @@ -149,6 +166,12 @@ Describe $CommandName -Tag IntegrationTests { It "Returns the right default one year expiry date" { $defaultCert.NotAfter -match ((Get-Date).Date).AddMonths(12) | Should -BeTrue } + + It "Leaves no copy of the certificate in the intermediate CA store" { + # certreq installs a self-signed certificate a second time, without its key, in LocalMachine\CA. The command removes that copy. + Test-Path -Path "Cert:\LocalMachine\My\$($defaultCert.Thumbprint)" | Should -BeTrue + Test-Path -Path "Cert:\LocalMachine\CA\$($defaultCert.Thumbprint)" | Should -BeFalse + } } Context "Can generate a new certificate with custom settings" { @@ -207,7 +230,13 @@ Describe $CommandName -Tag IntegrationTests { # For a remote computer the command exports a PFX and imports it on the target. The provider has to survive that transfer. # On CI the instance is local, so the transfer only happens against a lab whose instance runs on another computer. $computerName = ([DbaInstanceParameter]$TestConfig.InstanceSingle).ComputerName + $remoteIsLocal = ([DbaInstanceParameter]$TestConfig.InstanceSingle).IsLocalHost + # The key files of this computer before the certificate for the other computer is created here. After the export + # the certificate and its key have to be gone from this computer again. + $keyFolders = "$env:ProgramData\Microsoft\Crypto\RSA\MachineKeys", "$env:ProgramData\Microsoft\Crypto\Keys" + $keyFilesBefore = @((Get-ChildItem -Path $keyFolders -File).Name) $remoteKspCert = New-DbaComputerCertificate -ComputerName $computerName -SelfSigned -Provider "Microsoft Software Key Storage Provider" + $keyFilesAfter = @((Get-ChildItem -Path $keyFolders -File).Name) $readProvider = { param ($Thumbprint) $cert = Get-ChildItem -Path "Cert:\LocalMachine\My\$Thumbprint" @@ -247,5 +276,367 @@ Describe $CommandName -Tag IntegrationTests { It "Keeps the Key Storage Provider key when the certificate is imported on a remote computer" { $remoteProvider | Should -Be "Microsoft Software Key Storage Provider" } + + It "Leaves neither the certificate nor its key on this computer after the transfer to a remote computer" { + if ($remoteIsLocal) { + Set-ItResult -Skipped -Because "the instance runs on this computer, so the certificate stays here" + } + Test-Path -Path "Cert:\LocalMachine\My\$($remoteKspCert.Thumbprint)" | Should -BeFalse + Test-Path -Path "Cert:\LocalMachine\CA\$($remoteKspCert.Thumbprint)" | Should -BeFalse + $keyFilesAfter | Where-Object { $PSItem -notin $keyFilesBefore } | Should -BeNullOrEmpty + } + } + + Context "Leaves nothing behind when the CA does not answer" { + BeforeAll { + $PSDefaultParameterValues["*-Dba*:EnableException"] = $true + + # Every request file certreq writes is copied, so that cleanup and assertions know the requests of this test by + # their keys and never touch a request another enrollment on this computer created meanwhile. certreq is real. + # No temp name in this file starts with dbatools: the maintenance task tempcleanup deletes everything in the temp + # folder that does, one minute after the module is imported, which is right in the middle of this file. + $global:dbatoolsciCsrCopyFolder = "$([System.IO.Path]::GetTempPath())csrcopy_dbatoolsci_$(Get-Random)" + $null = New-Item -Path $global:dbatoolsciCsrCopyFolder -ItemType Directory + $mockCertreq = { + certreq.exe @args + if ($args -contains "-new") { + Copy-Item -Path $args[-1] -Destination (Join-Path -Path $global:dbatoolsciCsrCopyFolder -ChildPath "$(Get-Random).csr") + } + } + Mock -ModuleName dbatools -CommandName certreq -MockWith $mockCertreq + + # A request for a CA waits in LocalMachine\REQUEST with its key. When the CA cannot be reached the command removes + # the request again, so the store and the key folders have to look as before. + $requestFolders = "$env:ProgramData\Microsoft\Crypto\RSA\MachineKeys", "$env:ProgramData\Microsoft\Crypto\Keys" + $requestKeyFilesBefore = @((Get-ChildItem -Path $requestFolders -File).Name) + $splatUnreachableCa = @{ + CaServer = "nosuchca.dbatools.invalid" + CaName = "NoSuchCA" + WarningVariable = "unreachableCaWarning" + WarningAction = "SilentlyContinue" + EnableException = $false + } + $unreachableCaResult = New-DbaComputerCertificate @splatUnreachableCa + $requestFiles = @((Get-ChildItem -Path $global:dbatoolsciCsrCopyFolder -Filter "*.csr").FullName) + $ownRequestsAfter = @(Get-TestRequestThumbprint -RequestFile $requestFiles) + $requestKeyFilesAfter = @((Get-ChildItem -Path $requestFolders -File).Name) + + $PSDefaultParameterValues.Remove("*-Dba*:EnableException") + } + + AfterAll { + $PSDefaultParameterValues["*-Dba*:EnableException"] = $true + # In case the command left its request behind after all. Only the requests of this test are removed. + foreach ($leftover in (Get-TestRequestThumbprint -RequestFile $requestFiles)) { + Remove-DbaComputerCertificate -Thumbprint $leftover -Folder REQUEST -DeleteKey + } + Remove-Item -Path $global:dbatoolsciCsrCopyFolder -Recurse -ErrorAction SilentlyContinue + Remove-Variable -Name dbatoolsciCsrCopyFolder -Scope Global -ErrorAction SilentlyContinue + $PSDefaultParameterValues.Remove("*-Dba*:EnableException") + } + + It "Warns instead of returning a certificate" { + $unreachableCaResult | Should -BeNullOrEmpty + # The command writes several warnings on this path, the last one is the message of Stop-Function. + ($unreachableCaWarning -join " ") | Should -Match "Failure when attempting to create the cert" + } + + It "Removes the pending request and its key again" { + $requestFiles | Should -HaveCount 1 + $ownRequestsAfter | Should -BeNullOrEmpty + $requestKeyFilesAfter | Where-Object { $PSItem -notin $requestKeyFilesBefore } | Should -BeNullOrEmpty + } } -} \ No newline at end of file + + Context "Keeps a request of someone else when the CA does not answer" { + BeforeAll { + $PSDefaultParameterValues["*-Dba*:EnableException"] = $true + + # Another enrollment creates its own machine request while the command waits for the CA. Only the submission is + # intercepted to create that request at exactly this moment; certreq, the store and the keys are real. + $otherSubject = "CN=dbatoolsci_otherrequest_$(Get-Random)" + $global:dbatoolsciOtherInf = "$([System.IO.Path]::GetTempPath())otherrequest_dbatoolsci_$(Get-Random).inf" + $otherCsr = "$global:dbatoolsciOtherInf.csr" + $otherInfContent = @" +[Version] +Signature="`$Windows NT`$" +[NewRequest] +Subject = "$otherSubject" +KeyLength = 2048 +Exportable = TRUE +MachineKeySet = TRUE +ProviderName = "Microsoft Software Key Storage Provider" +KeyAlgorithm = RSA +RequestType = PKCS10 +"@ + Set-Content -Path $global:dbatoolsciOtherInf -Value $otherInfContent + $global:dbatoolsciCsrCopyFolder = "$([System.IO.Path]::GetTempPath())csrcopy_dbatoolsci_$(Get-Random)" + $null = New-Item -Path $global:dbatoolsciCsrCopyFolder -ItemType Directory + $mockCertreq = { + if ($args -contains "-submit") { + $null = certreq.exe -q -new $global:dbatoolsciOtherInf "$global:dbatoolsciOtherInf.csr" + } + certreq.exe @args + if ($args -contains "-new") { + Copy-Item -Path $args[-1] -Destination (Join-Path -Path $global:dbatoolsciCsrCopyFolder -ChildPath "$(Get-Random).csr") + } + } + Mock -ModuleName dbatools -CommandName certreq -MockWith $mockCertreq + + $splatUnreachableCa = @{ + CaServer = "nosuchca.dbatools.invalid" + CaName = "NoSuchCA" + WarningVariable = "otherRequestWarning" + WarningAction = "SilentlyContinue" + EnableException = $false + } + $otherRequestResult = New-DbaComputerCertificate @splatUnreachableCa + $requestFiles = @((Get-ChildItem -Path $global:dbatoolsciCsrCopyFolder -Filter "*.csr").FullName) + $ownRequestsAfter = @(Get-TestRequestThumbprint -RequestFile $requestFiles) + $otherRequest = Get-ChildItem -Path Cert:\LocalMachine\REQUEST | Where-Object Thumbprint -in @(Get-TestRequestThumbprint -RequestFile $otherCsr) + + $PSDefaultParameterValues.Remove("*-Dba*:EnableException") + } + + AfterAll { + $PSDefaultParameterValues["*-Dba*:EnableException"] = $true + # The requests of this test: the one of the command, should it still be there, and the other one. + foreach ($leftover in (Get-TestRequestThumbprint -RequestFile ($requestFiles + $otherCsr))) { + Remove-DbaComputerCertificate -Thumbprint $leftover -Folder REQUEST -DeleteKey + } + Remove-Item -Path $global:dbatoolsciOtherInf, $otherCsr -ErrorAction SilentlyContinue + Remove-Item -Path $global:dbatoolsciCsrCopyFolder -Recurse -ErrorAction SilentlyContinue + Remove-Variable -Name dbatoolsciCsrCopyFolder, dbatoolsciOtherInf -Scope Global -ErrorAction SilentlyContinue + $PSDefaultParameterValues.Remove("*-Dba*:EnableException") + } + + It "Warns instead of returning a certificate" { + $otherRequestResult | Should -BeNullOrEmpty + ($otherRequestWarning -join " ") | Should -Match "Failure when attempting to create the cert" + } + + It "Removes only its own request" { + $requestFiles | Should -HaveCount 1 + $ownRequestsAfter | Should -BeNullOrEmpty + $otherRequest | Should -Not -BeNullOrEmpty + } + + It "Keeps the key of the other request" { + $otherKey = [System.Security.Cryptography.X509Certificates.RSACertificateExtensions]::GetRSAPrivateKey($otherRequest) + $otherKey | Should -Not -BeNullOrEmpty + # Signing needs the key file itself, the certificate only points to it. + $otherKey.SignData([byte[]](1, 2, 3), [System.Security.Cryptography.HashAlgorithmName]::SHA256, [System.Security.Cryptography.RSASignaturePadding]::Pkcs1) | Should -Not -BeNullOrEmpty + } + } + + Context "Removes only its own request when a second call for the same computer overlaps" { + BeforeAll { + $PSDefaultParameterValues["*-Dba*:EnableException"] = $true + + # While the first call waits for the CA, a second call for the same computer runs from start to end. Only the + # submission of the first call is intercepted to start the second one at exactly this moment; both calls, certreq, + # the store and the keys are real. The request files of both calls are copied to know their requests afterwards. + # With one request folder per computer the second call deleted the request file of the first one, which then + # could not find its own request and left it behind. + $global:dbatoolsciCsrCopyFolder = "$([System.IO.Path]::GetTempPath())csrcopy_dbatoolsci_$(Get-Random)" + $null = New-Item -Path $global:dbatoolsciCsrCopyFolder -ItemType Directory + $global:dbatoolsciSecondCallStarted = $false + $mockCertreq = { + if ($args -contains "-submit" -and -not $global:dbatoolsciSecondCallStarted) { + $global:dbatoolsciSecondCallStarted = $true + $splatSecondCall = @{ + CaServer = "nosuchca.dbatools.invalid" + CaName = "NoSuchCA" + WarningVariable = "secondCallWarning" + WarningAction = "SilentlyContinue" + EnableException = $false + } + $global:dbatoolsciSecondCallResult = New-DbaComputerCertificate @splatSecondCall + $global:dbatoolsciSecondCallWarning = $secondCallWarning + } + certreq.exe @args + if ($args -contains "-new") { + Copy-Item -Path $args[-1] -Destination (Join-Path -Path $global:dbatoolsciCsrCopyFolder -ChildPath "$(Get-Random).csr") + } + } + Mock -ModuleName dbatools -CommandName certreq -MockWith $mockCertreq + + $splatFirstCall = @{ + CaServer = "nosuchca.dbatools.invalid" + CaName = "NoSuchCA" + WarningVariable = "firstCallWarning" + WarningAction = "SilentlyContinue" + EnableException = $false + } + $firstCallResult = New-DbaComputerCertificate @splatFirstCall + $requestFiles = @((Get-ChildItem -Path $global:dbatoolsciCsrCopyFolder -Filter "*.csr").FullName) + $ownRequestsAfter = @(Get-TestRequestThumbprint -RequestFile $requestFiles) + + $PSDefaultParameterValues.Remove("*-Dba*:EnableException") + } + + AfterAll { + $PSDefaultParameterValues["*-Dba*:EnableException"] = $true + foreach ($leftover in (Get-TestRequestThumbprint -RequestFile $requestFiles)) { + Remove-DbaComputerCertificate -Thumbprint $leftover -Folder REQUEST -DeleteKey + } + Remove-Item -Path $global:dbatoolsciCsrCopyFolder -Recurse -ErrorAction SilentlyContinue + Remove-Variable -Name dbatoolsciCsrCopyFolder, dbatoolsciSecondCallStarted, dbatoolsciSecondCallResult, dbatoolsciSecondCallWarning -Scope Global -ErrorAction SilentlyContinue + $PSDefaultParameterValues.Remove("*-Dba*:EnableException") + } + + It "Runs the second call while the first one waits" { + $global:dbatoolsciSecondCallStarted | Should -BeTrue + $requestFiles | Should -HaveCount 2 + } + + It "Warns in both calls instead of returning a certificate" { + $firstCallResult | Should -BeNullOrEmpty + $global:dbatoolsciSecondCallResult | Should -BeNullOrEmpty + ($firstCallWarning -join " ") | Should -Match "Failure when attempting to create the cert" + ($global:dbatoolsciSecondCallWarning -join " ") | Should -Match "Failure when attempting to create the cert" + } + + It "Leaves no request of either call behind" { + $ownRequestsAfter | Should -BeNullOrEmpty + } + } + + # A real CA refuses a template it does not offer right away. Its message "Certificate not issued" contains "issued", which + # the command once took for success: it then tried to install a certificate that was never written and left the request behind. + # No CI environment has a CA, so this runs only against a lab that sets CaServer and CaName in its configuration. + Context "Removes the request when the CA refuses it" -Skip:(-not $TestConfig.CaServer) { + BeforeAll { + $PSDefaultParameterValues["*-Dba*:EnableException"] = $true + + # The request files are copied to know the request of this test by its key, see the contexts above. certreq is real. + $global:dbatoolsciCsrCopyFolder = "$([System.IO.Path]::GetTempPath())csrcopy_dbatoolsci_$(Get-Random)" + $null = New-Item -Path $global:dbatoolsciCsrCopyFolder -ItemType Directory + $mockCertreq = { + certreq.exe @args + if ($args -contains "-new") { + Copy-Item -Path $args[-1] -Destination (Join-Path -Path $global:dbatoolsciCsrCopyFolder -ChildPath "$(Get-Random).csr") + } + } + Mock -ModuleName dbatools -CommandName certreq -MockWith $mockCertreq + + $splatRefusingCa = @{ + CaServer = $TestConfig.CaServer + CaName = $TestConfig.CaName + CertificateTemplate = "dbatoolsci_NoSuchTemplate" + WarningVariable = "refusedWarning" + WarningAction = "SilentlyContinue" + EnableException = $false + } + $refusedResult = New-DbaComputerCertificate @splatRefusingCa + $requestFiles = @((Get-ChildItem -Path $global:dbatoolsciCsrCopyFolder -Filter "*.csr").FullName) + $ownRequestsAfter = @(Get-TestRequestThumbprint -RequestFile $requestFiles) + + $PSDefaultParameterValues.Remove("*-Dba*:EnableException") + } + + AfterAll { + $PSDefaultParameterValues["*-Dba*:EnableException"] = $true + foreach ($leftover in (Get-TestRequestThumbprint -RequestFile $requestFiles)) { + Remove-DbaComputerCertificate -Thumbprint $leftover -Folder REQUEST -DeleteKey + } + Remove-Item -Path $global:dbatoolsciCsrCopyFolder -Recurse -ErrorAction SilentlyContinue + Remove-Variable -Name dbatoolsciCsrCopyFolder -Scope Global -ErrorAction SilentlyContinue + $PSDefaultParameterValues.Remove("*-Dba*:EnableException") + } + + It "Warns with the reason of the CA instead of returning a certificate" { + $refusedResult | Should -BeNullOrEmpty + ($refusedWarning -join " ") | Should -Match "Failure when attempting to create the cert" + } + + It "Removes its request and the key again" { + $requestFiles | Should -HaveCount 1 + $ownRequestsAfter | Should -BeNullOrEmpty + } + } + + # A real CA whose template holds every request as pending until a CA manager approves it. No CI environment has one, + # so this runs only against a lab that sets CaServer, CaName and ApprovalTemplate in its configuration. + Context "Keeps a pending request so that the certificate can be installed once it is issued" -Skip:(-not $TestConfig.ApprovalTemplate) { + BeforeAll { + $PSDefaultParameterValues["*-Dba*:EnableException"] = $true + + # The request files are copied to know the request of this test by its key, see the contexts above. certreq is real. + $global:dbatoolsciCsrCopyFolder = "$([System.IO.Path]::GetTempPath())csrcopy_dbatoolsci_$(Get-Random)" + $null = New-Item -Path $global:dbatoolsciCsrCopyFolder -ItemType Directory + $mockCertreq = { + certreq.exe @args + if ($args -contains "-new") { + Copy-Item -Path $args[-1] -Destination (Join-Path -Path $global:dbatoolsciCsrCopyFolder -ChildPath "$(Get-Random).csr") + } + } + Mock -ModuleName dbatools -CommandName certreq -MockWith $mockCertreq + + $caConfig = "$($TestConfig.CaServer)\$($TestConfig.CaName)" + $splatPendingCa = @{ + CaServer = $TestConfig.CaServer + CaName = $TestConfig.CaName + CertificateTemplate = $TestConfig.ApprovalTemplate + WarningVariable = "pendingWarning" + WarningAction = "SilentlyContinue" + EnableException = $false + } + $pendingResult = New-DbaComputerCertificate @splatPendingCa + $requestFiles = @((Get-ChildItem -Path $global:dbatoolsciCsrCopyFolder -Filter "*.csr").FullName) + $pendingRequest = Get-ChildItem -Path Cert:\LocalMachine\REQUEST | Where-Object Thumbprint -in @(Get-TestRequestThumbprint -RequestFile $requestFiles) + $pendingRequestId = ([regex]::Match("$pendingWarning", "certificate request (\d+)")).Groups[1].Value + + # A CA manager approves the request, then the certreq commands the warning names install the certificate. They + # run with -q here, so that a failure reports instead of showing a dialog. + $approval = certutil -config $caConfig -resubmit $pendingRequestId + $retrieveCommand = [regex]::Match("$pendingWarning", "certreq -retrieve -config `"([^`"]+)`" (\d+) (\S+\.crt)") + $acceptCommand = [regex]::Match("$pendingWarning", "certreq -accept -machine (\S+\.crt)") + $issuedCrt = Join-Path -Path $global:dbatoolsciCsrCopyFolder -ChildPath $retrieveCommand.Groups[3].Value + $null = certreq.exe -q -retrieve -config $retrieveCommand.Groups[1].Value $retrieveCommand.Groups[2].Value $issuedCrt + $null = certreq.exe -q -accept -machine (Join-Path -Path $global:dbatoolsciCsrCopyFolder -ChildPath $acceptCommand.Groups[1].Value) + $issuedThumbprint = (New-Object System.Security.Cryptography.X509Certificates.X509Certificate2 -ArgumentList $issuedCrt).Thumbprint + $installedCert = Get-ChildItem -Path Cert:\LocalMachine\My | Where-Object Thumbprint -eq $issuedThumbprint + $requestAfterAccept = @(Get-TestRequestThumbprint -RequestFile $requestFiles) + + $PSDefaultParameterValues.Remove("*-Dba*:EnableException") + } + + AfterAll { + $PSDefaultParameterValues["*-Dba*:EnableException"] = $true + if ($issuedThumbprint -and (Get-ChildItem -Path Cert:\LocalMachine\My | Where-Object Thumbprint -eq $issuedThumbprint)) { + Remove-DbaComputerCertificate -Thumbprint $issuedThumbprint -DeleteKey + } + foreach ($leftover in (Get-TestRequestThumbprint -RequestFile $requestFiles)) { + Remove-DbaComputerCertificate -Thumbprint $leftover -Folder REQUEST -DeleteKey + } + Remove-Item -Path $global:dbatoolsciCsrCopyFolder -Recurse -ErrorAction SilentlyContinue + Remove-Variable -Name dbatoolsciCsrCopyFolder -Scope Global -ErrorAction SilentlyContinue + $PSDefaultParameterValues.Remove("*-Dba*:EnableException") + } + + It "Warns with the request ID instead of returning a certificate" { + $pendingResult | Should -BeNullOrEmpty + ($pendingWarning -join " ") | Should -Match "as pending" + $pendingRequestId | Should -Match "^\d+$" + } + + It "Keeps the request and its key while the CA holds it" { + $requestFiles | Should -HaveCount 1 + $pendingRequest | Should -HaveCount 1 + $pendingKey = [System.Security.Cryptography.X509Certificates.RSACertificateExtensions]::GetRSAPrivateKey($pendingRequest) + # Signing needs the key file itself, the certificate only points to it. + $pendingKey.SignData([byte[]](1, 2, 3), [System.Security.Cryptography.HashAlgorithmName]::SHA256, [System.Security.Cryptography.RSASignaturePadding]::Pkcs1) | Should -Not -BeNullOrEmpty + } + + It "Installs the certificate with its private key once a CA manager has approved it" { + "$approval" | Should -Match "-resubmit command completed successfully" + $retrieveCommand.Groups[1].Value | Should -Be $caConfig + $retrieveCommand.Groups[2].Value | Should -Be $pendingRequestId + $acceptCommand.Groups[1].Value | Should -Be $retrieveCommand.Groups[3].Value + $installedCert | Should -Not -BeNullOrEmpty + $installedCert.HasPrivateKey | Should -BeTrue + $requestAfterAccept | Should -BeNullOrEmpty + } + } +}