-
Notifications
You must be signed in to change notification settings - Fork 378
Allow using custom Auth Certificate lifetime #2347
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
base: main
Are you sure you want to change the base?
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
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.
Pull Request Overview
This PR introduces support for custom Auth Certificate lifetimes in the MonitorExchangeAuthCertificate script to address customer requirements with tenant policies that restrict certificate lifetimes to 365 days or less.
- A new
New-ExchangeSelfSignedCertificatefunction that allows specifying custom certificate lifetimes - Enhanced script parameters to support custom certificate lifetimes and enforce new certificate creation
- Documentation updates reflecting the new functionality
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/Admin/MonitorExchangeAuthCertificate.md | Added documentation for new parameters EnforceNewAuthCertificateCreation and CustomCertificateLifetimeInDays |
| Shared/CertificateFunctions/New-ExchangeSelfSignedCertificate.ps1 | New function implementing custom self-signed certificate generation with configurable lifetime |
| Admin/MonitorExchangeAuthCertificate/MonitorExchangeAuthCertificate.ps1 | Main script updates to support new parameters and certificate creation modes |
| Admin/MonitorExchangeAuthCertificate/DataCollection/Get-ExchangeAuthCertificateStatus.ps1 | Enhanced status detection logic to support enforced certificate creation |
| Admin/MonitorExchangeAuthCertificate/ConfigurationAction/New-ExchangeAuthCertificate.ps1 | Updated certificate creation logic to use custom function when lifetime is specified |
| .build/cspell-words.txt | Added new technical terms to spell check dictionary |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Shared/CertificateFunctions/New-ExchangeSelfSignedCertificate.ps1
Outdated
Show resolved
Hide resolved
Admin/MonitorExchangeAuthCertificate/ConfigurationAction/New-ExchangeAuthCertificate.ps1
Outdated
Show resolved
Hide resolved
Admin/MonitorExchangeAuthCertificate/MonitorExchangeAuthCertificate.ps1
Outdated
Show resolved
Hide resolved
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
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.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 10 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (-not([System.String]::IsNullOrEmpty($utf8FriendlyName))) { | ||
| if ($PSCmdlet.ShouldProcess("Adding FriendlyName $utf8FriendlyName")) { | ||
| $certificate.FriendlyName = $utf8FriendlyName | ||
| } | ||
| } |
Copilot
AI
Nov 21, 2025
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.
When running with -WhatIf, the $certificate variable is never created (line 265-270), but line 274 attempts to set $certificate.FriendlyName without checking if $certificate exists. This will cause a null reference error in WhatIf mode.
The code should verify $certificate is not null before attempting to use it, or skip this operation in WhatIf mode.
| Write-Verbose "Exporting and re-importing certificate with Exportable flag to make it exportable..." | ||
| if ($PSCmdlet.ShouldProcess("Making certificate exportable")) { | ||
| $pfxBytes = $certificate.Export( | ||
| [System.Security.Cryptography.X509Certificates.X509ContentType]::Pfx | ||
| ) | ||
| $certificateWithExportableKey = [System.Security.Cryptography.X509Certificates.X509Certificate2]::new() | ||
| $certificateWithExportableKey.Import( | ||
| $pfxBytes, | ||
| $null, | ||
| ([System.Security.Cryptography.X509Certificates.X509KeyStorageFlags]::Exportable -bor | ||
| [System.Security.Cryptography.X509Certificates.X509KeyStorageFlags]::PersistKeySet -bor | ||
| [System.Security.Cryptography.X509Certificates.X509KeyStorageFlags]::MachineKeySet) | ||
| ) |
Copilot
AI
Nov 21, 2025
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.
When running with -WhatIf, the $certificate variable is never created (line 265-270), but line 296 attempts to export it with $certificate.Export() without checking if $certificate exists. This will cause a null reference error in WhatIf mode.
The entire certificate export/import block should be wrapped in a WhatIf check or skip execution when $certificate is null.
| Write-Verbose "Exporting and re-importing certificate with Exportable flag to make it exportable..." | |
| if ($PSCmdlet.ShouldProcess("Making certificate exportable")) { | |
| $pfxBytes = $certificate.Export( | |
| [System.Security.Cryptography.X509Certificates.X509ContentType]::Pfx | |
| ) | |
| $certificateWithExportableKey = [System.Security.Cryptography.X509Certificates.X509Certificate2]::new() | |
| $certificateWithExportableKey.Import( | |
| $pfxBytes, | |
| $null, | |
| ([System.Security.Cryptography.X509Certificates.X509KeyStorageFlags]::Exportable -bor | |
| [System.Security.Cryptography.X509Certificates.X509KeyStorageFlags]::PersistKeySet -bor | |
| [System.Security.Cryptography.X509Certificates.X509KeyStorageFlags]::MachineKeySet) | |
| ) | |
| if ($null -ne $certificate) { | |
| Write-Verbose "Exporting and re-importing certificate with Exportable flag to make it exportable..." | |
| if ($PSCmdlet.ShouldProcess("Making certificate exportable")) { | |
| $pfxBytes = $certificate.Export( | |
| [System.Security.Cryptography.X509Certificates.X509ContentType]::Pfx | |
| ) | |
| $certificateWithExportableKey = [System.Security.Cryptography.X509Certificates.X509Certificate2]::new() | |
| $certificateWithExportableKey.Import( | |
| $pfxBytes, | |
| $null, | |
| ([System.Security.Cryptography.X509Certificates.X509KeyStorageFlags]::Exportable -bor | |
| [System.Security.Cryptography.X509Certificates.X509KeyStorageFlags]::PersistKeySet -bor | |
| [System.Security.Cryptography.X509Certificates.X509KeyStorageFlags]::MachineKeySet) | |
| ) | |
| } |
| if ($PSCmdlet.ShouldProcess("Adding certificate to LocalMachine\My store")) { | ||
| $machineStore.Open([System.Security.Cryptography.X509Certificates.OpenFlags]::ReadWrite) | ||
| $machineStore.Add($certificateWithExportableKey) | ||
| $machineStore.Close() | ||
| } |
Copilot
AI
Nov 21, 2025
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.
In WhatIf mode, $certificateWithExportableKey is never created (line 295-307), but the code attempts to add it to the certificate store at line 319. This will cause a null reference error.
The store operations should check if $certificateWithExportableKey exists before attempting to use it.
| if ($TrustCertificate -and $PSCmdlet.ShouldProcess("Adding certificate to LocalMachine\Root store")) { | ||
| $trustedRootStore.Open([System.Security.Cryptography.X509Certificates.OpenFlags]::ReadWrite) | ||
| $trustedRootStore.Add($certificateWithExportableKey) | ||
| $trustedRootStore.Close() | ||
| } |
Copilot
AI
Nov 21, 2025
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.
In WhatIf mode, $certificateWithExportableKey is never created, but it's being added to the trusted root store at line 334 without null checking. This will cause a null reference error.
The store operations should check if $certificateWithExportableKey exists before attempting to use it.
| return [PSCustomObject]@{ | ||
| Subject = $subject.Name | ||
| Thumbprint = $certificateThumbprint | ||
| } |
Copilot
AI
Nov 21, 2025
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.
In WhatIf mode, $subject is defined but $certificateThumbprint is set to a mock value (line 282). However, at line 383, the function returns $certificateThumbprint which may be undefined if an early return occurred or if the flow didn't reach line 278-283.
Consider initializing $certificateThumbprint at the beginning of the process block to avoid potential undefined variable issues.
| $newAuthCertificate = New-ExchangeCertificate @newAuthCertificateParams | ||
| if ($NewAuthCertificateLifetimeInDays -gt 0) { | ||
| Write-Verbose "Creating a custom self-signed certificate with a lifetime of $NewAuthCertificateLifetimeInDays days" | ||
| $newAuthCertificate = New-ExchangeSelfSignedCertificate @newCustomAuthCertificateParams |
Copilot
AI
Nov 21, 2025
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.
When New-ExchangeSelfSignedCertificate is called with -WhatIf, it may return a result with a mock thumbprint, but it's not guaranteed to complete successfully due to the WhatIf handling issues in that function. The code should add a check after line 208 to verify that $newAuthCertificate was successfully created before proceeding, similar to the existing null check pattern at line 241.
| $newAuthCertificate = New-ExchangeSelfSignedCertificate @newCustomAuthCertificateParams | |
| $newAuthCertificate = New-ExchangeSelfSignedCertificate @newCustomAuthCertificateParams | |
| if ($null -eq $newAuthCertificate) { | |
| throw "Failed to create a new Auth Certificate. The certificate object is null." | |
| } |
| You can use this switch parameter to let the script stage a new next Auth Certificate which will become automatically active within 24 hours. | ||
| .PARAMETER CustomCertificateLifetimeInDays | ||
| You can use this parameter to specify a custom lifetime for the newly created Auth certificate. | ||
| By default, the self-signed certificate is created with a lifetime of 5 years. |
Copilot
AI
Nov 21, 2025
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 documentation states "By default, the self-signed certificate is created with a lifetime of 5 years," but it doesn't clarify what happens when CustomCertificateLifetimeInDays is set to 0. The parameter accepts 0 as valid, but it's unclear whether this means "use the default 5-year lifetime" or if it has a different meaning.
Consider updating the documentation to explicitly state: "By default (or when set to 0), the self-signed certificate is created with a lifetime of 5 years."
| By default, the self-signed certificate is created with a lifetime of 5 years. | |
| By default (or when set to 0), the self-signed certificate is created with a lifetime of 5 years. |
| [Parameter(Mandatory = $false, ParameterSetName = "NewPrimaryAuthCert")] | ||
| [Parameter(Mandatory = $false, ParameterSetName = "NewNextAuthCert")] | ||
| [ValidateScript({ $_ -ge 0 })] | ||
| [int]$NewAuthCertificateLifetimeInDays, |
Copilot
AI
Nov 21, 2025
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 $NewAuthCertificateLifetimeInDays parameter lacks a default value. When not explicitly provided by the caller, it will be $null or 0 (depending on PowerShell's type coercion). This could lead to unexpected behavior. The parameter should have an explicit default value (e.g., = 0) to make the API contract clear, especially since line 206 checks if ($NewAuthCertificateLifetimeInDays -gt 0) which suggests 0 or less means "use default".
| [int]$NewAuthCertificateLifetimeInDays, | |
| [int]$NewAuthCertificateLifetimeInDays = 0, |
| "LocalMachine" | ||
| ) | ||
|
|
||
| if ($TrustCertificate -and $PSCmdlet.ShouldProcess("Adding certificate to LocalMachine\Root store")) { |
Copilot
AI
Nov 21, 2025
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 redundant check $TrustCertificate -and is unnecessary here since this code is already inside an if ($TrustCertificate) block (line 324). The condition should just be if ($PSCmdlet.ShouldProcess(...)).
| if ($TrustCertificate -and $PSCmdlet.ShouldProcess("Adding certificate to LocalMachine\Root store")) { | |
| if ($PSCmdlet.ShouldProcess("Adding certificate to LocalMachine\Root store")) { |
| [Parameter(Mandatory = $false, ParameterSetName = "MonitorExchangeAuthCertificateManually")] | ||
| [Parameter(Mandatory = $false, ParameterSetName = "ConfigureAutomaticExecutionViaScheduledTask")] | ||
| [Parameter(Mandatory = $false, ParameterSetName = "EnforceNewNextAuthCertificateConfiguration")] | ||
| [ValidateScript({ $_ -ge 0 })] |
Copilot
AI
Nov 21, 2025
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 validation { $_ -ge 0 } allows 0 as a valid value for CustomCertificateLifetimeInDays. However, when this value is 0, the code at line 206 checks if ($NewAuthCertificateLifetimeInDays -gt 0), meaning a value of 0 will use the default certificate generation path. This creates ambiguity: should 0 mean "use default" or should it be rejected as invalid?
Consider either:
- Changing the validation to
{ $_ -gt 0 }if 0 is not a valid lifetime, or - Adding clearer documentation that
0means "use default 5-year lifetime".
| [ValidateScript({ $_ -ge 0 })] | |
| [ValidateScript({ $_ -gt 0 })] |
Description:
With the release of the dedicated Exchange hybrid application feature (https://learn.microsoft.com/en-us/Exchange/hybrid-deployment/deploy-dedicated-hybrid-app), customers must create an application in Microsoft Entra ID which is used by Exchange Server running in hybrid mode. As part of the application creation process, the public key of the current and new next Auth Certificate are uploaded to the application in Entra ID. The certificate is used for
JWT assertion-based authentication.Some customers have a policy enforced within their tenant, which prevents them from uploading a certificates with a lifetime longer than 365 days as part of the
keyCredentialConfiguration. TheNew-ExchangeCertificate, which is used to generate the self-signed certificate, used as Auth Certificate, issues self-signed certificates with a lifetime of 5 years by default. It doesn't allow to specify a different lifetime for self-signed certificates yet. This is something we plan to introduce in future. As a workaround we decided to update theMonitorExchangeAuthCertificate.ps1script so that it allows customers specify the lifetime of the self-signed certificate used as Auth Certificate.Fix:
Introduce a new function
New-ExchangeSelfSignedCertificatethat allows specifying a customer lifetime for the self-signed certificate. It generates a self-signed certificate by using the expected cryptographic service provider (CSP) with extensions which are expected for the Auth Certificate.Validation:
Lab / Validation by test team is pending