Skip to content

Commit 013a50e

Browse files
authored
Fix remediation output for health tests for certs (#542)
This pull request enhances the certificate validation logic in the SDN diagnostic health module by improving how certificate details are collected and reported in several health test functions. The main improvements include standardizing the way certificate properties are gathered and ensuring that results are consistently stored for reporting. **Certificate details collection and reporting:** * Refactored `Test-SdnCertificateExpired` and `Test-SdnCertificateMultiple` to accumulate certificate details into an array and assign them to `Properties` at the end of execution, ensuring consistent and structured reporting of affected certificates. [[1]](diffhunk://#diff-15898640fc68e07afa836ad8d93af4f22a4442978d9c233f39d48d44d85cfb60R1122) [[2]](diffhunk://#diff-15898640fc68e07afa836ad8d93af4f22a4442978d9c233f39d48d44d85cfb60R1141-R1143) [[3]](diffhunk://#diff-15898640fc68e07afa836ad8d93af4f22a4442978d9c233f39d48d44d85cfb60R1153-R1154) [[4]](diffhunk://#diff-15898640fc68e07afa836ad8d93af4f22a4442978d9c233f39d48d44d85cfb60R1168) [[5]](diffhunk://#diff-15898640fc68e07afa836ad8d93af4f22a4442978d9c233f39d48d44d85cfb60R1182-R1211) * In `Test-SdnNonSelfSignedCertificateInTrustedRootStore`, introduced a variable to collect certificate details and improved the process of adding certificate information to the result array. **Certificate selection logic:** * Enhanced logic in `Test-SdnCertificateMultiple` to prioritize certificates issued by `AzureStackCertificationAuthority` when determining which certificate to keep, and improved the remediation message formatting. # Change type - [x] Bug fix (non-breaking change) - [ ] Code style update (formatting, local variables) - [ ] New Feature (non-breaking change that adds new functionality without impacting existing) - [ ] Breaking change (fix or feature that may cause functionality impact) - [ ] Other # Checklist: - [x] My code follows the style and contribution guidelines of this project. - [x] I have tested and validated my code changes.
1 parent ee8e42a commit 013a50e

1 file changed

Lines changed: 32 additions & 5 deletions

File tree

src/modules/SdnDiag.Health.psm1

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -952,8 +952,11 @@ function Test-SdnNonSelfSignedCertificateInTrustedRootStore {
952952
$rootCerts = Get-ChildItem -Path 'Cert:LocalMachine\Root' | Where-Object { $_.Issuer -ne $_.Subject }
953953
if ($rootCerts -or $rootCerts.Count -gt 0) {
954954
$sdnHealthTest.Result = 'FAIL'
955-
$rootCerts | ForEach-Object {
955+
$certDetails = $rootCerts | ForEach-Object {
956956
"`t- Thumbprint: {0} Subject: {1} Issuer: {2} NotAfter: {3}" -f $_.Thumbprint, $_.Subject, $_.Issuer, $_.NotAfter
957+
}
958+
959+
$rootCerts | ForEach-Object {
957960
$array += [PSCustomObject]@{
958961
Thumbprint = $_.Thumbprint
959962
Subject = $_.Subject
@@ -1116,6 +1119,7 @@ function Test-SdnCertificateExpired {
11161119

11171120
$role = $Global:SdnDiagnostics.Config.Role
11181121
$sdnHealthTest = New-SdnHealthTest
1122+
$array = @()
11191123

11201124
try {
11211125
foreach ($r in $role) {
@@ -1134,8 +1138,9 @@ function Test-SdnCertificateExpired {
11341138
if ($certificate -or $certificate.Count -gt 0) {
11351139
$sdnHealthTest.Result = 'FAIL'
11361140
$sdnHealthTest.Remediation = "Renew the certificate(s) used for SDN components."
1141+
11371142
$certificate | ForEach-Object {
1138-
$sdnHealthTest.Properties += [PSCustomObject]@{
1143+
$array += [PSCustomObject]@{
11391144
Thumbprint = $_.Thumbprint
11401145
Subject = $_.Subject
11411146
NotAfter = $_.NotAfter
@@ -1145,6 +1150,8 @@ function Test-SdnCertificateExpired {
11451150
}
11461151
}
11471152
}
1153+
1154+
$sdnHealthTest.Properties = $array
11481155
}
11491156
catch {
11501157
$_ | Trace-Exception
@@ -1158,6 +1165,7 @@ function Test-SdnCertificateMultiple {
11581165

11591166
$role = $Global:SdnDiagnostics.Config.Role
11601167
$sdnHealthTest = New-SdnHealthTest
1168+
$array = @()
11611169

11621170
try {
11631171
foreach ($r in $role) {
@@ -1171,17 +1179,36 @@ function Test-SdnCertificateMultiple {
11711179
}
11721180
}
11731181

1182+
# if we have multiple certificates, we need to determine if any are extraneous
1183+
# we will presume that the latest certificate is the one we want to keep and if issued by AzureStackCertificationAuthority, we will prioritize that one
11741184
if ($null -ne $certificate -and $certificate.Count -gt 1) {
1175-
# eliminate the most current certificate from the array as we do not want to flag that one
1176-
$latestCert = $certificate | Sort-Object -Property NotAfter -Descending | Select-Object -First 1
1185+
if ($certificate.Issuer -contains 'CN=AzureStackCertificationAuthority') {
1186+
$latestCert = $certificate | Where-Object { $_.Issuer -eq 'CN=AzureStackCertificationAuthority' } | Sort-Object -Property NotAfter -Descending | Select-Object -First 1
1187+
}
1188+
else {
1189+
$latestCert = $certificate | Sort-Object -Property NotAfter -Descending | Select-Object -First 1
1190+
}
1191+
11771192
$certificate = $certificate | Where-Object { $_.Thumbprint -ne $latestCert.Thumbprint }
11781193
$certDetails = $certificate | ForEach-Object {
11791194
"`t- Thumbprint: {0} Subject: {1} Issuer: {2} NotAfter: {3}" -f $_.Thumbprint, $_.Subject, $_.Issuer, $_.NotAfter
11801195
}
11811196

1197+
$certificate | ForEach-Object {
1198+
$array += [PSCustomObject]@{
1199+
Thumbprint = $_.Thumbprint
1200+
Subject = $_.Subject
1201+
Issuer = $_.Issuer
1202+
NotAfter = $_.NotAfter
1203+
NotBefore = $_.NotBefore
1204+
}
1205+
}
1206+
11821207
$sdnHealthTest.Result = 'WARNING'
1183-
$sdnHealthTest.Remediation = "Examine and cleanup the certificates if no longer needed:`r`n`t{0}" -f ($certDetails -join "`r`n`t")
1208+
$sdnHealthTest.Remediation = "Examine and cleanup the certificates if no longer needed:`r`n{0}" -f ($certDetails -join "`r`n")
11841209
}
1210+
1211+
$sdnHealthTest.Properties = $array
11851212
}
11861213
catch {
11871214
$_ | Trace-Exception

0 commit comments

Comments
 (0)