Skip to content
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

Check for certificate expiration is not working #171

Open
apraamsm opened this issue Dec 4, 2018 · 4 comments
Open

Check for certificate expiration is not working #171

apraamsm opened this issue Dec 4, 2018 · 4 comments
Assignees
Labels
bug The issue is a bug. in progress The issue is being actively worked on by someone.

Comments

@apraamsm
Copy link

apraamsm commented Dec 4, 2018

Details of the scenario you tried and the problem that is occurring

When a new certificate is enrolled it is after that not checked correctly for expiration.
The following check is done;

	if ($Cert.NotAfter -le (Get-Date).AddDays(-30))
	{
		# The certificate was found but it is expiring within 30 days or has expired
		Write-Verbose -Message ( @(
				"$($MyInvocation.MyCommand): "
				$($LocalizedData.ExpiringCertificateMessage -f $Subject, $ca, $cert.Thumbprint)
			) -join '' )
		return $false
	}

During tests the certificate did not renew

I believe that when you check on NotAfter the AddDays value must be 30 instead of -30

	#if ($Cert.NotAfter -le (Get-Date).AddDays(-30))
	if ($Cert.NotAfter -le (Get-Date).AddDays(30))
	{
		# The certificate was found but it is expiring within 30 days or has expired
		Write-Verbose -Message ( @(
				"$($MyInvocation.MyCommand): "
				$($LocalizedData.ExpiringCertificateMessage -f $Subject, $ca, $cert.Thumbprint)
			) -join '' )
		return $false
	}

Verbose logs showing the problem

I dont have verbose logging. I came accross this when i found that during testing the certificate did not get renewed. After i changed the AddDays value to 30 it did.

Suggested solution to the issue

	#if ($Cert.NotAfter -le (Get-Date).AddDays(-30))
	if ($Cert.NotAfter -le (Get-Date).AddDays(30))
	{
		# The certificate was found but it is expiring within 30 days or has expired
		Write-Verbose -Message ( @(
				"$($MyInvocation.MyCommand): "
				$($LocalizedData.ExpiringCertificateMessage -f $Subject, $ca, $cert.Thumbprint)
			) -join '' )
		return $false
	}

The DSC configuration that is used to reproduce the issue (as detailed as possible)

Configuration CertTest {

   # import required modules
    Import-DSCResource -ModuleName 'PSDesiredStateConfiguration'
    Import-DSCResource -ModuleName 'xPSDesiredStateConfiguration'
    Import-DscResource -ModuleName 'CertificateDsc'

    node localhost {

    # Start Certificate Request
            CertReq DomainControllerCertificateRequest
            {
                CARootName          = '' # This is auto-discovered when not used
                CAServerFQDN        = '' # This is auto-discovered when not used
                Subject             = '' # Autogenerated for Domain Controllers
                KeyLength           = '2048'
                Exportable          = $false
                ProviderName        = '"Microsoft RSA SChannel Cryptographic Provider"'
                CertificateTemplate = 'DomainControllerCertificate'
                SubjectAltName      = '' # Autogenerated for Domain Controllers (altered the module for this)
                AutoRenew           = $true
                FriendlyName        = 'Domain Controller Certificate'
            }
    # End Certificate Request

    }
}

The operating system the target node is running

OsName : Microsoft Windows Server 2016 Standard
OsOperatingSystemSKU : StandardServerEdition
OsArchitecture : 64-bit
WindowsBuildLabEx : 14393.2608.amd64fre.rs1_release.181024-1742
OsLanguage : en-US
OsMuiLanguages : {en-US}

Version and build of PowerShell the target node is running

PSVersion 5.1.14393.2608
PSEdition Desktop
PSCompatibleVersions {1.0, 2.0, 3.0, 4.0...}
BuildVersion 10.0.14393.2608
CLRVersion 4.0.30319.42000
WSManStackVersion 3.0
PSRemotingProtocolVersion 2.3
SerializationVersion 1.1.0.1

Version of the DSC module that was used ('dev' if using current dev branch)

DEV branch

@PlagueHO PlagueHO added bug The issue is a bug. help wanted The issue is up for grabs for anyone in the community. labels Dec 6, 2018
@PlagueHO
Copy link
Member

PlagueHO commented Dec 6, 2018

Hi @apraamsm - thank you for raising this. I believe you could be right about there being some problem in the date logic there. But looking through the tests (see https://github.com/PowerShell/CertificateDsc/blob/dev/Tests/Unit/MSFT_CertReq.Tests.ps1#L500) there appears to be some tests that are passing that I'd expect to fail on this condition.

So I think I'll want to do some digging into the unit tests and ensure they're functionally correct before changing the code. I'll look into this over the weekend.

@PlagueHO PlagueHO added in progress The issue is being actively worked on by someone. and removed help wanted The issue is up for grabs for anyone in the community. labels Dec 7, 2018
@PlagueHO PlagueHO self-assigned this Dec 7, 2018
@PlagueHO
Copy link
Member

PlagueHO commented Dec 9, 2018

I'll fix this one after the PR #174 has gone through.

@PlagueHO
Copy link
Member

I'll start looking into this one next as PR #174 has been completed.

@SteveL-MSFT SteveL-MSFT added this to In progress in powershell/dscresources May 14, 2019
@SteveL-MSFT SteveL-MSFT removed this from In progress in powershell/dscresources Nov 27, 2019
@pietervandersluis
Copy link

It seems that this xcertreq has been promoted to certreq.

The certreq code has the positive 30 already there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue is a bug. in progress The issue is being actively worked on by someone.
Projects
None yet
Development

No branches or pull requests

3 participants