Repository navigation
Use machine-scope DPAPI for acme-dns credentials #8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,6 +29,8 @@ | |
| #> | ||
|
|
||
| [CmdletBinding(DefaultParameterSetName = 'Single')] | ||
| [Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSAvoidUsingConvertToSecureStringWithPlainText', '', | ||
| Justification = 'Wraps a password decrypted from DPAPI back into a SecureString for the non-AsPlainText return path.')] | ||
| param( | ||
| [Parameter(Mandatory = $true, ParameterSetName = 'Single')] | ||
| [ValidateNotNullOrEmpty()] | ||
|
|
@@ -51,6 +53,17 @@ else { | |
| exit 1 | ||
| } | ||
|
|
||
| # DPAPI LocalMachine decrypt path needs [ProtectedData] from System.Security.dll. | ||
| # Failure to load is fatal; surface it loudly rather than letting the type | ||
| # lookup fail with a confusing error later. | ||
| try { | ||
| Add-Type -AssemblyName System.Security -ErrorAction Stop | ||
| } | ||
| catch { | ||
| Write-Error "Failed to load required assembly 'System.Security'. DPAPI-based credential decryption is unavailable: $($_.Exception.Message)" | ||
| exit 1 | ||
| } | ||
|
|
||
| Initialize-WinCertManager | ||
|
|
||
| if ($ListAll) { | ||
|
|
@@ -126,8 +139,39 @@ catch { | |
| $password = $null | ||
|
|
||
| switch ($storedData.StorageMethod) { | ||
| 'DPAPI-LocalMachine' { | ||
| # Machine-scoped DPAPI: any account on this host (including SYSTEM) | ||
| # can decrypt. This is the default for credentials registered by | ||
| # current toolkit versions. | ||
| $plainBytes = $null | ||
| try { | ||
| $protectedBytes = [Convert]::FromBase64String($storedData.EncryptedPassword) | ||
| $plainBytes = [System.Security.Cryptography.ProtectedData]::Unprotect( | ||
| $protectedBytes, $null, | ||
| [System.Security.Cryptography.DataProtectionScope]::LocalMachine | ||
| ) | ||
| $plainText = [System.Text.Encoding]::UTF8.GetString($plainBytes) | ||
| if ($AsPlainText) { | ||
| $password = $plainText | ||
| } | ||
| else { | ||
| $password = ConvertTo-SecureString -String $plainText -AsPlainText -Force | ||
| } | ||
| } | ||
| catch { | ||
| Write-Error "Failed to decrypt password (LocalMachine DPAPI): $($_.Exception.Message)" | ||
| return $null | ||
| } | ||
| finally { | ||
| if ($null -ne $plainBytes) { [Array]::Clear($plainBytes, 0, $plainBytes.Length) } | ||
| } | ||
| } | ||
|
|
||
| 'DPAPI' { | ||
| # Decrypt password from DPAPI | ||
| # Legacy CurrentUser-scoped DPAPI. Only the user that registered the | ||
| # domain can decrypt. If the renewal task runs as SYSTEM, decryption | ||
| # will fail here; use scripts\Recovery\Repair-AcmeDnsCredential.ps1 | ||
| # to migrate to DPAPI-LocalMachine. | ||
| try { | ||
| $securePassword = ConvertTo-SecureString -String $storedData.EncryptedPassword | ||
| if ($AsPlainText) { | ||
|
|
@@ -140,7 +184,7 @@ switch ($storedData.StorageMethod) { | |
| } | ||
| } | ||
| catch { | ||
| Write-Error "Failed to decrypt password. This usually means the credential was stored by a different user or on a different machine." | ||
| Write-Error "Failed to decrypt password. This usually means the credential was stored by a different user or on a different machine. Run scripts\Recovery\Repair-AcmeDnsCredential.ps1 to migrate to machine-scope DPAPI." | ||
| return $null | ||
|
Comment on lines
186
to
188
|
||
| } | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,7 +21,8 @@ | |
|
|
||
| .PARAMETER StorageMethod | ||
| How to store the credentials: CredentialManager or JsonFile. | ||
| Default: JsonFile (DPAPI encrypted) | ||
| Default: JsonFile (DPAPI encrypted, machine scope so SYSTEM-context | ||
| renewal tasks can decrypt). | ||
|
|
||
| .PARAMETER ApiKey | ||
| API key for authenticated acme-dns servers. Required for RWTS acme-dns. | ||
|
|
@@ -89,6 +90,17 @@ else { | |
| exit 1 | ||
| } | ||
|
|
||
| # DPAPI LocalMachine encrypt path needs [ProtectedData] from System.Security.dll. | ||
| # Failure to load is fatal; surface it loudly so the operator gets a clear | ||
| # error rather than a missing-type exception further down. | ||
| try { | ||
| Add-Type -AssemblyName System.Security -ErrorAction Stop | ||
| } | ||
| catch { | ||
| Write-Error "Failed to load required assembly 'System.Security'. DPAPI-based credential protection is unavailable: $($_.Exception.Message)" | ||
| exit 1 | ||
| } | ||
|
|
||
| # Initialize | ||
| Initialize-WinCertManager | ||
| Write-Log "Registering domain '$Domain' with acme-dns server: $AcmeDnsServer" -Level Info | ||
|
|
@@ -191,8 +203,44 @@ $credentialData = [PSCustomObject]@{ | |
| # Store credentials | ||
| switch ($StorageMethod) { | ||
| 'JsonFile' { | ||
| # Use DPAPI encryption for the password | ||
| $encryptedPassword = ConvertFrom-SecureString -SecureString $securePassword | ||
| # DPAPI machine-scope encryption: any account on this host (including | ||
| # SYSTEM, which runs the win-acme renewal task) can decrypt. Without | ||
| # this, a renewal triggered by SYSTEM cannot read credentials that | ||
| # were registered by an interactive operator. | ||
| # | ||
| # The plaintext password is copied directly out of the BSTR as raw | ||
| # UTF-16 bytes and transcoded to UTF-8 via Encoding.Convert. We avoid | ||
| # creating a managed System.String (e.g., via PtrToStringBSTR) so | ||
| # there is no garbage-collected immutable plaintext copy lingering | ||
| # in memory after this block exits. | ||
| $bstr = [IntPtr]::Zero | ||
| $utf16Bytes = $null | ||
| $plainBytes = $null | ||
| try { | ||
| $bstr = [System.Runtime.InteropServices.Marshal]::SecureStringToBSTR($securePassword) | ||
| # BSTR length-in-bytes prefix lives 4 bytes before the pointer. | ||
| $byteCount = [System.Runtime.InteropServices.Marshal]::ReadInt32($bstr, -4) | ||
| $utf16Bytes = New-Object byte[] $byteCount | ||
| [System.Runtime.InteropServices.Marshal]::Copy($bstr, $utf16Bytes, 0, $byteCount) | ||
| $plainBytes = [System.Text.Encoding]::Convert( | ||
| [System.Text.Encoding]::Unicode, | ||
| [System.Text.Encoding]::UTF8, | ||
| $utf16Bytes | ||
| ) | ||
|
|
||
|
Comment on lines
+220
to
+230
|
||
| $protected = [System.Security.Cryptography.ProtectedData]::Protect( | ||
| $plainBytes, $null, | ||
| [System.Security.Cryptography.DataProtectionScope]::LocalMachine | ||
| ) | ||
| $encryptedPassword = [Convert]::ToBase64String($protected) | ||
| } | ||
| finally { | ||
| if ($null -ne $utf16Bytes) { [Array]::Clear($utf16Bytes, 0, $utf16Bytes.Length) } | ||
| if ($null -ne $plainBytes) { [Array]::Clear($plainBytes, 0, $plainBytes.Length) } | ||
| if ($bstr -ne [IntPtr]::Zero) { | ||
| [System.Runtime.InteropServices.Marshal]::ZeroFreeBSTR($bstr) | ||
| } | ||
| } | ||
|
|
||
| $storageData = [PSCustomObject]@{ | ||
| Domain = $credentialData.Domain | ||
|
|
@@ -203,7 +251,7 @@ switch ($StorageMethod) { | |
| EncryptedPassword = $encryptedPassword | ||
| AllowFrom = $credentialData.AllowFrom | ||
| RegisteredAt = $credentialData.RegisteredAt | ||
| StorageMethod = 'DPAPI' | ||
| StorageMethod = 'DPAPI-LocalMachine' | ||
| } | ||
|
|
||
| $storageData | ConvertTo-Json -Depth 5 | Set-Content -Path $credentialFile -Force | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The troubleshooting steps reference
.\scripts\Recovery\Repair-AcmeDnsCredential.ps1, butscripts/Recoverydoesn't exist in the repo unless PR #7 lands first. To avoid broken guidance if merge order changes (or users read docs from this commit), consider updating the text to explicitly note the dependency/availability or include the recovery script in this PR.