rmbolger / Posh-ACME

PowerShell module and ACME client to create certificates from Let's Encrypt (or other ACME CA)
https://poshac.me/docs/latest/
MIT License
778 stars 191 forks source link

Helpful error message is blocking execution? #577

Open mpking828 opened 3 weeks ago

mpking828 commented 3 weeks ago

I was provided this script by someone else to allow a Milestone Camera System to use Let's Encrypt. It's worked great for a few years.

    param([string]$LogPath)

    function WriteLog {
        Param ([string]$message)
        Add-Content -Path $LogPath -Value "$(Get-Date) - $message"
    }

    try {
        $thumbprint = (Get-PACertificate).Thumbprint
        $cert = Submit-Renewal -WarningAction Stop -ErrorAction Stop
        $cert | Set-MobileServerCertificate  #Activate the certificate
        WriteLog "New certificate installed with thumbprint $($cert.Thumbprint)"
        WriteLog "Removing old certificate with thumbprint $thumbprint"
        Get-ChildItem Cert:\LocalMachine\My |
            Where-Object Thumbprint -eq $thumbprint |
            Remove-Item

    } catch {
        WriteLog $_.Exception.Message
        throw
    }

Usually it just puts this error in the log daily: 10/14/2024 03:04:11 - The running command stopped because the preference variable "WarningPreference" or common parameter is set to Stop: Order 'host.domain.com' is not recommended for renewal yet. Use -Force to override.

However, starting on the 19th it started throwing this error: 10/19/2024 03:37:25 - The running command stopped because the preference variable "WarningPreference" or common parameter is set to Stop: The ACME Server has indicated this order's certificate should be renewed AS SOON AS POSSIBLE.

I got an email today from Let'sEncrypt that the certificate expires on the 17th of Nov (it's what triggered me to check the error log)

That is the problem I'm trying to fix.

Looks like the helpful error message saying renew now is triggering the exception logic.

Here is my first attempt to fix it. (Essentially just adding the IF statement from https://poshac.me/docs/latest/Tutorial/#task-scheduler-cron).

Think this will get the job done? I'm open to suggestions

    param([string]$LogPath)

    function WriteLog {
        Param ([string]$message)
        Add-Content -Path $LogPath -Value "$(Get-Date) - $message"
    }

    try {
        $thumbprint = (Get-PACertificate).Thumbprint
        if ($cert = Submit-Renewal) {
            $cert | Set-MobileServerCertificate #Activate the certificate
            WriteLog "New certificate installed with thumbprint $($cert.Thumbprint)"
            WriteLog "Removing old certificate with thumbprint $thumbprint"
            Get-ChildItem Cert:\LocalMachine\My |
                Where-Object Thumbprint -eq $thumbprint |
                Remove-Item
        }
        else {
            WriteLog "Certificate was not renewed")
        }
    } catch {
        WriteLog $_.Exception.Message
        throw
    }
rmbolger commented 3 weeks ago

Hi @mpking828, thanks for reaching out. Looks like you already got it figured out. But yes, warnings aren't intended to stop script execution so the -WarningAction Stop was the main cause of your issue. You might want to put the -ErrorAction Stop back in though which will make it more likely that your catch statement will correctly catch and log any actual errors.

mpking828 commented 3 weeks ago

Reporting back.

My code was throwing a syntax error. Co-Pilot corrected it this way:

param([string]$LogPath)

function WriteLog {
    Param ([string]$message)
    Add-Content -Path $LogPath -Value "$(Get-Date) - $message"
}

try {
    $thumbprint = (Get-PACertificate).Thumbprint
    $cert = Submit-Renewal -ErrorAction Stop
    if ($cert) {
        $cert | Set-MobileServerCertificate
        WriteLog "New certificate installed with thumbprint $($cert.Thumbprint)"
        WriteLog "Removing old certificate with thumbprint $thumbprint"
        Get-ChildItem Cert:\LocalMachine\My | 
            Where-Object Thumbprint -eq $thumbprint | 
            Remove-Item
    } else {
        WriteLog "Certificate was not renewed"
    }
} catch {
    WriteLog $_.Exception.Message
    throw
}

I guess this is more a powershell question than a Posh-ACME question, but when it does the Submit-Renewal, if you run it from the command prompt, you get messages like: Order 'host.domain.com' is not recommended for renewal yet. Use -Force to override. Any way I can capture that (again) and write it to the log?

rmbolger commented 3 weeks ago

Ah yes, the assignment inside the if statement might have needed some additional parenthesis depending on the PowerShell version. Co-Pilot's suggestion to move it out is fine too.

The easiest way to capture and log warnings specifically would be to use the -WarningVariable parameter on the Submit-Renewal call. This saves any warning messages to an array variable of your choosing and you can then loop through them and call your WriteLog function. So something like this:

try {
    $thumbprint = (Get-PACertificate).Thumbprint
    $cert = Submit-Renewal -ErrorAction Stop -WarningVariable warnings
    $warnings | ForEach-Object { WriteLog $_ }
    if ($cert) {

Another option would be to get rid of the custom logging code in the script and switch to using PowerShell's native Start-Transcript. It basically logs all commands and their output into a file of your choice until you call Stop-Transcript.