From ef9247d785875232ee90fb075c4baaacf8d049fa Mon Sep 17 00:00:00 2001 From: Adam Chalkley Date: Fri, 20 Dec 2024 07:28:45 -0600 Subject: [PATCH] Initial implementation of `Root` cert validation OVERVIEW This set of changes applies to both the `check_cert` plugin and the `lscert` CLI tool. These changes borrow heavily from recent work for the initial implementation of the `Chain Order` validation check. As with that implementation, this set of changes is subject to change as work to implement further validity checks (and refactor existing ones) continues. CHANGES - update `check_cert` plugin - add new `Root` validation type - assert that no root certificates are in the chain - remove 'note' from the `Chain Order` validation check when a root certificate is found (now handled by this check) - extend tests to cover new validation type - update `lscert` - incorporate new validation check - incorporate the same "advice" output that the `check_cert` plugin now emits for `Root` validation problems - shared - move "advice" helper funcs to separate validation-helpers file as refactoring step towards further upcoming validation checks work - the initial idea was to provide further advice for sysadmins when a root certificate was detected (this may still occur later) - update README to add coverage for new `Root` validation check - emphasize that this check is not applied by default, but that this could change in the future - sysadmins are encouraged to explicitly opt-out of validation checks that they are not interested in - the checks still run, but the results are marked as ignored and not used to trigger plugin state changes REFERENCES - GH-1004 - GH-364 - GH-365 --- README.md | 37 +- cmd/check_cert/main_test.go | 31 ++ cmd/check_cert/validate.go | 34 ++ cmd/lscert/main.go | 48 +++ internal/certs/advice/root-cert-found.txt | 2 +- internal/certs/certs.go | 31 +- internal/certs/validation-chain-order.go | 172 +-------- internal/certs/validation-helpers.go | 171 +++++++++ internal/certs/validation-root.go | 435 ++++++++++++++++++++++ internal/config/config_test.go | 34 +- internal/config/constants.go | 14 + internal/config/getters.go | 24 ++ internal/config/logging.go | 1 + 13 files changed, 837 insertions(+), 197 deletions(-) create mode 100644 internal/certs/validation-helpers.go create mode 100644 internal/certs/validation-root.go diff --git a/README.md b/README.md index 4df3256..4e62e89 100644 --- a/README.md +++ b/README.md @@ -295,35 +295,33 @@ accessible to this tool. Use FQDNs in order to retrieve certificates using - issuer - Multiple certificate validation checks - - Expiration status for all certificates in a chain + - `Expiration` status for all certificates in a chain - not expired - expiring "soon" - warning threshold - critical threshold - - Hostname value for the leaf certificate in a chain + - `Hostname` value for the leaf certificate in a chain - see subsection for skipping hostname verification when the leaf certificate is missing SANs entries in the [configuration options](#configuration-options) section for details - - Subject Alternate Names (SANs) for the leaf certificate in a chain + - `Subject Alternate Names (SANs)` for the leaf certificate in a chain - if `SKIPSANSCHECKS` keyword is supplied as the value no SANs entry checks will be performed; this keyword is useful for defining a shared Nagios check command and service check where SANs list validation may not be desired for some certificate chains (e.g., those with a very long list of entries) - - Chain Order for the order of certificates in a chain + - `Chain Order` for the order of certificates in a chain - assert that leaf certificate is first in chain, followed by the intermediate which signed it, a potential second intermediate which signed the former and so on - current implementation objects to a single leaf cert in a chain, though this behavior may be moved to a separate validation check specific to intermediates - - current implementation notes the presence of a root certificate and - cautions that some platforms will object to this, though this behavior - may be moved to a separate validation check in the future - offers advice for replacing a certificate chain when specific CA vendors are matched - currently only Sectigo/InCommon is supported, though the plan is to support multiple CAs once further feedback is gathered + - `Root` for the presence of root certificates in a chain - Optional support for skipping hostname verification for a certificate when the SANs list is empty @@ -369,10 +367,11 @@ accessible to this tool. Use FQDNs in order to retrieve certificates using - issuer - Multiple certificate validation checks - - Expiration status for all certificates in a chain - - Hostname value for the leaf certificate in a chain - - Subject Alternate Names (SANs) for the leaf certificate in a chain - - Chain Order for the order of certificates in a chain + - `Expiration` status for all certificates in a chain + - `Hostname` value for the leaf certificate in a chain + - `Subject Alternate Names (SANs)` for the leaf certificate in a chain + - `Chain Order` for the order of certificates in a chain + - `Root` for the presence of a root certificate in a chain ### `cpcert` @@ -674,6 +673,7 @@ configuration settings are applied. Some are ignored by default. | `Hostname` | Yes | Server or DNS Name values | | `SANs list` | Yes`*` | SANs entries | | `Chain Order` | No | | +| `Root` | No | | The certificate expiration validation check is applied using default thresholds if not specified by the sysadmin. The hostname verification check @@ -686,14 +686,15 @@ state); without SANs entries to validate the SANs list validation check result is of limited value. If explicitly requested and SANs entries are not provided a configuration error is emitted and the plugin terminates. -The Chain Order validation is is not applied by default; this validation check -was not present in early releases of the plugin and enabling it by default -would potentially be an unwelcome change. +The `Chain Order` and `Root` validations are is not applied by default; these +validation checks were not present in early releases of the plugin and +enabling them by default would potentially be an unwelcome change. > [!NOTE] > -> A future version *may* enable the `Chain Order` validation check by default. -> You may opt out at any time by explicitly ignoring this validation type. +> A future version *may* enable the `Chain Order` and `Root` validation checks +> by default. You may opt out at any time by explicitly ignoring these +> validation types. #### `lscert` CLI tool @@ -746,8 +747,8 @@ validation checks and any behavior changes at that time noted. | `ignore-expired-root-certs` | No | `false` | No | `true`, `false` | Whether expired root certificates should be ignored. | | `ignore-expiring-intermediate-certs` | No | `false` | No | `true`, `false` | Whether expiring intermediate certificates should be ignored. | | `ignore-expiring-root-certs` | No | `false` | No | `true`, `false` | Whether expiring root certificates should be ignored. | -| `ignore-validation-result` | No | | No | `sans`, `expiration`, `hostname`, `order` | List of keywords for certificate chain validation check result that should be explicitly ignored and not used to determine final validation state. | -| `apply-validation-result` | No | | No | `sans`, `expiration`, `hostname`, `order` | List of keywords for certificate chain validation check results that should be explicitly applied and used to determine final validation state. | +| `ignore-validation-result` | No | | No | `sans`, `expiration`, `hostname`, `order`, `root` | List of keywords for certificate chain validation check result that should be explicitly ignored and not used to determine final validation state. | +| `apply-validation-result` | No | | No | `sans`, `expiration`, `hostname`, `order`, `root` | List of keywords for certificate chain validation check results that should be explicitly applied and used to determine final validation state. | | `list-ignored-errors` | No | `false` | No | `true`, `false` | Toggles emission of ignored validation check result errors. Disabled by default to reduce confusion. | #### `lscert` diff --git a/cmd/check_cert/main_test.go b/cmd/check_cert/main_test.go index 1f74b79..8a3df63 100644 --- a/cmd/check_cert/main_test.go +++ b/cmd/check_cert/main_test.go @@ -357,6 +357,37 @@ func TestApplyValidationResults(t *testing.T) { validateFunc: config.Config.ApplyCertChainOrderValidationResults, applyResults: true, }, + + { + name: "DefaultRootResults", + server: "www.example.com", + validateFlagsAndValues: []string{}, + validateFunc: config.Config.ApplyCertRootValidationResults, + + // This validation is not part of the original set and has to be + // opted into. + applyResults: false, + }, + { + name: "IgnoreValidateRootResults", + server: "www.example.com", + validateFlagsAndValues: []string{ + "--" + config.IgnoreValidationResultFlag, + config.ValidationKeywordRoot, + }, + validateFunc: config.Config.ApplyCertRootValidationResults, + applyResults: false, + }, + { + name: "ApplyValidateRootResults", + server: "www.example.com", + validateFlagsAndValues: []string{ + "--" + config.ApplyValidationResultFlag, + config.ValidationKeywordRoot, + }, + validateFunc: config.Config.ApplyCertRootValidationResults, + applyResults: true, + }, } for _, tt := range tests { diff --git a/cmd/check_cert/validate.go b/cmd/check_cert/validate.go index 28c8511..640614b 100644 --- a/cmd/check_cert/validate.go +++ b/cmd/check_cert/validate.go @@ -174,6 +174,40 @@ func runValidationChecks(cfg *config.Config, certChain []*x509.Certificate, log Msgf("%s validation successful", chainOrderValidationResult.CheckName()) } + rootValidationOptions := certs.CertChainValidationOptions{ + IgnoreValidationResultRoot: !cfg.ApplyCertRootValidationResults(), + } + + log.Debug(). + Interface("validation_options", rootValidationOptions). + Msg("Omit Root Validation Options") + + rootValidationResult := certs.ValidateRoot( + certChain, + cfg.VerboseOutput, + rootValidationOptions, + ) + validationResults.Add(rootValidationResult) + + switch { + case rootValidationResult.IsFailed(): + log.Debug(). + Err(rootValidationResult.Err()). + Int("root_certs", rootValidationResult.NumRootCerts()). + Int("total_certs", rootValidationResult.TotalCerts()). + Msgf("%s validation failure", rootValidationResult.CheckName()) + + case rootValidationResult.IsIgnored(): + log.Debug(). + Msgf("%s validation ignored", rootValidationResult.CheckName()) + + default: + log.Debug(). + Int("root_certs", rootValidationResult.NumRootCerts()). + Int("total_certs", rootValidationResult.TotalCerts()). + Msgf("%s validation successful", rootValidationResult.CheckName()) + } + return validationResults } diff --git a/cmd/lscert/main.go b/cmd/lscert/main.go index 1ea7988..cfa774b 100644 --- a/cmd/lscert/main.go +++ b/cmd/lscert/main.go @@ -539,6 +539,54 @@ func main() { ) } + rootValidationResult := certs.ValidateRoot( + certChain, + cfg.VerboseOutput, + certs.CertChainValidationOptions{ + // IgnoreValidationResultRoot: !cfg.ApplyCertRootValidationResults(), + IgnoreValidationResultRoot: false, + }, + ) + validationResults.Add(rootValidationResult) + + switch { + case rootValidationResult.IsFailed(): + log.Debug(). + Err(rootValidationResult.Err()). + Int("root_certs", rootValidationResult.NumRootCerts()). + Int("total_certs", rootValidationResult.TotalCerts()). + Int("chain_entries_total", rootValidationResult.TotalCerts()). + Msg("Chain misordered") + + fmt.Printf( + "\n%s %s\n", + stateToPrefix(rootValidationResult), + rootValidationResult.String(), + ) + + case rootValidationResult.IsIgnored(): + log.Debug(). + Msgf("%s validation ignored", rootValidationResult.CheckName()) + + fmt.Printf( + "\n%s %s\n", + stateToPrefix(rootValidationResult), + rootValidationResult.String(), + ) + + default: + log.Debug(). + Int("root_certs", rootValidationResult.NumRootCerts()). + Int("total_certs", rootValidationResult.TotalCerts()). + Msgf("%s validation successful", rootValidationResult.CheckName()) + + fmt.Printf( + "\n%s %s\n", + stateToPrefix(rootValidationResult), + rootValidationResult.String(), + ) + } + textutils.PrintHeader("CERTIFICATE CHAIN | DETAILS") // We request these details even if the user opted to disable expiration diff --git a/internal/certs/advice/root-cert-found.txt b/internal/certs/advice/root-cert-found.txt index 95b2f0b..67aae97 100644 --- a/internal/certs/advice/root-cert-found.txt +++ b/internal/certs/advice/root-cert-found.txt @@ -1 +1 @@ -NOTE: A root cert was provided; including a root cert is *usually* not a problem, but be aware that some platforms object to this. +⚠️ WARNING: A root cert was provided; while including a root cert is *usually* not a problem, be aware that some platforms object to this. diff --git a/internal/certs/certs.go b/internal/certs/certs.go index 7be4c67..857ab01 100644 --- a/internal/certs/certs.go +++ b/internal/certs/certs.go @@ -141,6 +141,10 @@ var ( // ErrMisorderedCertificateChain indicates that a certificate chain is not // in the correct order. ErrMisorderedCertificateChain = errors.New("certificate chain misordered") + + // ErrRootCertsFound indicates that one or more root certificates were + // found when evaluating a certificate chain. + ErrRootCertsFound = errors.New("root certificates found") ) // ServiceStater represents a type that is capable of evaluating its overall @@ -185,6 +189,11 @@ type CertChainValidationOptions struct { // validation against certificates in a chain. IgnoreValidationResultChainOrder bool + // IgnoreValidationResultRoot tracks whether a request was made to + // ignore validation check results from asserting that a root certificate + // is not present in a certificate chain. + IgnoreValidationResultRoot bool + // IgnoreExpiringIntermediateCertificates tracks whether a request was // made to ignore validation check results for certificate expiration // against intermediate certificates in a certificate chain which are @@ -316,25 +325,31 @@ const ExpirationValidationOneLineSummaryExpiredTmpl string = "%s validation %s: const X509CertReliesOnCommonName string = "x509: certificate relies on legacy Common Name field, use SANs instead" // Names of certificate chain validation checks. +// +// Goals: +// +// - Clearly indicate what is being checked +// - Avoid implying a preferred state +// - Be concise +// - Consistency const ( - // checkNameExpirationValidationResult string = "Certificate Chain Expiration" - // checkNameHostnameValidationResult string = "Leaf Certificate Hostname" - // checkNameSANsListValidationResult string = "Leaf Certificate SANs List" - // checkNameExpirationValidationResult string = "Expiration Validation" - // checkNameHostnameValidationResult string = "Hostname Validation" - // checkNameSANsListValidationResult string = "SANs List Validation" checkNameExpirationValidationResult string = "Expiration" checkNameHostnameValidationResult string = "Hostname" checkNameSANsListValidationResult string = "SANs List" checkNameChainOrderValidationResult string = "Chain Order" + checkNameRootValidationResult string = "Root" ) // Baseline priority values for validation results. Higher values indicate // higher priority. const ( - // baselinePrioritySANsListValidationResult is considered the lowest + // baselinePriorityRootValidationResult is considered the lowest + // priority when compared to the severity of other validation issues. + baselinePriorityRootValidationResult int = iota + 1 + + // baselinePrioritySANsListValidationResult is considered the next lowest // priority when compared to the severity of other validation issues. - baselinePrioritySANsListValidationResult int = iota + 1 + baselinePrioritySANsListValidationResult // baselinePriorityChainOrderValidationResult is a problem, but // historically has been a common issue that has been ignored with only diff --git a/internal/certs/validation-chain-order.go b/internal/certs/validation-chain-order.go index f1d755a..7c11f59 100644 --- a/internal/certs/validation-chain-order.go +++ b/internal/certs/validation-chain-order.go @@ -21,14 +21,11 @@ import ( // implementation isn't correct. var _ CertChainValidationResult = (*ChainOrderValidationResult)(nil) -// Advice for sysadmins resolving chain order issues saved in external files +// Advice for sysadmins resolving cert chain issues saved in external files // for easier maintenance. var ( //go:embed advice/sectigo-email-download-links.txt sectigoEmailAdvice string - - //go:embed advice/root-cert-found.txt - rootCertFoundAdvice string ) // ChainOrderValidationResult is the validation result from performing @@ -137,7 +134,8 @@ func ValidateChainOrder( return ChainOrderValidationResult{ certChain: certChain, err: fmt.Errorf( - "chain order validation failed: %w", + "%s validation failed: %w", + strings.ToLower(checkNameChainOrderValidationResult), ErrMisorderedCertificateChain, ), ignored: validationOptions.IgnoreValidationResultChainOrder, @@ -230,6 +228,9 @@ func (covr ChainOrderValidationResult) IsCriticalState() bool { return true case errors.Is(covr.err, ErrIncompleteCertificateChain): + // + // FIXME: Move this to an "Intermediates" specific check (GH-364). + // // An incomplete certificate chain is considered a CRITICAL state // because required certificates are not present; because some // modern/current clients will not automatically fetch missing @@ -405,7 +406,8 @@ func (covr ChainOrderValidationResult) StatusDetail() string { case covr.err != nil: detail.WriteString( fmt.Sprintf( - "An unexpected error occurred while performing chain order validation!%s", + "An unexpected error occurred while performing %s validation!%s", + strings.ToLower(covr.CheckName()), nagios.CheckOutputEOL, ), ) @@ -597,163 +599,5 @@ func reorderChainAdvice(certChain []*x509.Certificate) string { advice.WriteString(certDownloadLinksAdvice(certChain)) - // FIXME: Move this somewhere it can be used regardless of any actual - // chain ordering issue; ideally this would be noted if all chain elements - // are in the correct order but a root certificate is provided. - // - // This may fit best as a default check/note within a new validation check - // dedicated to looking for inclusion of the root cert, or later as an - // error condition once it becomes more unusual for cert chains to contain - // a root. - if HasRootCert(certChain) { - advice.WriteString( - fmt.Sprintf( - "%s%s%s", - nagios.CheckOutputEOL, - strings.TrimSpace(rootCertFoundAdvice), - nagios.CheckOutputEOL, - ), - ) - } - - return advice.String() -} - -// incompleteChainAdvice provides advice for the sysadmin when a cert chain is -// found to be incomplete. -func incompleteChainAdvice(certChain []*x509.Certificate) string { - if len(certChain) == 0 { - return "" - } - - var advice strings.Builder - - advice.WriteString( - fmt.Sprintf( - "This issue often occurs with Windows Servers when (newer) intermediates are missing from the certificate stores.%s", - nagios.CheckOutputEOL, - ), - ) - - hostValRef := func(chain []*x509.Certificate) string { - switch { - case chain[0].Subject.CommonName != "": - return fmt.Sprintf( - " for %s ", - chain[0].Subject.CommonName, - ) - - case len(chain[0].DNSNames) > 0: - return fmt.Sprintf( - " for %s ", - chain[0].DNSNames[0], - ) - - default: - return "" - } - } - - firstCertType := ChainPosition(certChain[0], certChain) - - isMissingIntermediates := !HasIntermediateCert(certChain) - - switch { - case firstCertType == certChainPositionLeafSelfSigned: - advice.WriteString( - fmt.Sprintf( - "It is recommended that you replace the %s certificate with a valid certificate chain.%s", - certChainPositionLeafSelfSigned, - nagios.CheckOutputEOL, - ), - ) - - case isMissingIntermediates: - advice.WriteString( - fmt.Sprintf( - "It is recommended that you configure the service%sto include the missing intermediates.%s", - hostValRef(certChain), - nagios.CheckOutputEOL, - ), - ) - - advice.WriteString(certDownloadLinksAdvice(certChain)) - - default: - - // Any advice for this scenario? - } - - return advice.String() -} - -// certDownloadLinksAdvice attempts to provide sysadmins advice for what -// download links to use when repairing reported certificate chain issues. -func certDownloadLinksAdvice(certChain []*x509.Certificate) string { - if len(certChain) == 0 { - return "" - } - - var advice strings.Builder - - if !HasLeafCert(certChain) { - advice.WriteString( - "NOTE: No leaf certs detected in given certificate chain;" + - " is this an intermediates bundle that is being monitored?", - ) - } - - type adviceMapEntry struct { - CA string - CASubstrings []string - Description string - Advice string - } - - adviceMappings := []adviceMapEntry{ - { - CASubstrings: []string{ - "InCommon", - "USERTrust", - "COMODO", - "Sectigo", - }, - Description: "Known CA name prefixes used by Sectigo", - Advice: strings.TrimSpace(sectigoEmailAdvice), - }, - } - -outerLoop: - for _, cert := range certChain { - for _, adviceEntry := range adviceMappings { - for _, pattern := range adviceEntry.CASubstrings { - lowerCasePattern := strings.ToLower(pattern) - - issuerContainsCAPrefix := strings.Contains( - strings.ToLower(cert.Issuer.CommonName), - lowerCasePattern, - ) - - issuedContainsCAPrefix := strings.Contains( - strings.ToLower(cert.Subject.CommonName), - lowerCasePattern, - ) - - if issuerContainsCAPrefix || issuedContainsCAPrefix { - advice.WriteString( - fmt.Sprintf( - "%s%s%s", - nagios.CheckOutputEOL, - adviceEntry.Advice, - nagios.CheckOutputEOL, - ), - ) - - break outerLoop - } - } - } - } - return advice.String() } diff --git a/internal/certs/validation-helpers.go b/internal/certs/validation-helpers.go new file mode 100644 index 0000000..58787d7 --- /dev/null +++ b/internal/certs/validation-helpers.go @@ -0,0 +1,171 @@ +// Copyright 2024 Adam Chalkley +// +// https://github.com/atc0005/check-cert +// +// Licensed under the MIT License. See LICENSE file in the project root for +// full license information. + +package certs + +import ( + "crypto/x509" + "fmt" + "strings" + + "github.com/atc0005/go-nagios" +) + +// incompleteChainAdvice provides advice for the sysadmin when a cert chain is +// found to be incomplete. +func incompleteChainAdvice(certChain []*x509.Certificate) string { + if len(certChain) == 0 { + return "" + } + + var advice strings.Builder + + advice.WriteString( + fmt.Sprintf( + "This issue often occurs with Windows Servers when (newer) intermediates are missing from the certificate stores.%s", + nagios.CheckOutputEOL, + ), + ) + + hostValRef := func(chain []*x509.Certificate) string { + switch { + case chain[0].Subject.CommonName != "": + return fmt.Sprintf( + " for %s ", + chain[0].Subject.CommonName, + ) + + case len(chain[0].DNSNames) > 0: + return fmt.Sprintf( + " for %s ", + chain[0].DNSNames[0], + ) + + default: + return "" + } + } + + firstCertType := ChainPosition(certChain[0], certChain) + + isMissingIntermediates := !HasIntermediateCert(certChain) + // isMissingLeaf := !HasLeafCert(certChain) + + switch { + case firstCertType == certChainPositionLeafSelfSigned: + advice.WriteString( + fmt.Sprintf( + "It is recommended that you replace the %s certificate with a valid certificate chain.%s", + certChainPositionLeafSelfSigned, + nagios.CheckOutputEOL, + ), + ) + + // TODO: We'd need to consider how this advice would come across for a + // cert check which monitors an intermediates bundle; intermediate + // bundles should not contain a leaf certificate. + // + // case isMissingLeaf: + // advice.WriteString( + // fmt.Sprintf( + // "It is recommended that you configure the service%sto include the missing leaf cert.%s", + // hostValRef(certChain), + // nagios.CheckOutputEOL, + // ), + // ) + // + // advice.WriteString(certDownloadLinksAdvice(certChain)) + + case isMissingIntermediates: + advice.WriteString( + fmt.Sprintf( + "It is recommended that you configure the service%sto include the missing intermediates.%s", + hostValRef(certChain), + nagios.CheckOutputEOL, + ), + ) + + advice.WriteString(certDownloadLinksAdvice(certChain)) + + default: + + // Any advice for this scenario? + } + + return advice.String() +} + +// certDownloadLinksAdvice attempts to provide sysadmins advice for what +// download links to use when repairing reported certificate chain issues. +func certDownloadLinksAdvice(certChain []*x509.Certificate) string { + if len(certChain) == 0 { + return "" + } + + var advice strings.Builder + + if !HasLeafCert(certChain) { + advice.WriteString( + "NOTE: No leaf certs detected in given certificate chain;" + + " is this an intermediates bundle that is being monitored?", + ) + } + + type adviceMapEntry struct { + CA string + CASubstrings []string + Description string + Advice string + } + + adviceMappings := []adviceMapEntry{ + { + CASubstrings: []string{ + "InCommon", + "USERTrust", + "COMODO", + "Sectigo", + }, + Description: "Known CA name prefixes used by Sectigo", + Advice: strings.TrimSpace(sectigoEmailAdvice), + }, + } + +outerLoop: + for _, cert := range certChain { + for _, adviceEntry := range adviceMappings { + for _, pattern := range adviceEntry.CASubstrings { + lowerCasePattern := strings.ToLower(pattern) + + issuerContainsCAPrefix := strings.Contains( + strings.ToLower(cert.Issuer.CommonName), + lowerCasePattern, + ) + + issuedContainsCAPrefix := strings.Contains( + strings.ToLower(cert.Subject.CommonName), + lowerCasePattern, + ) + + if issuerContainsCAPrefix || issuedContainsCAPrefix { + advice.WriteString( + fmt.Sprintf( + "%s%s%s", + nagios.CheckOutputEOL, + adviceEntry.Advice, + nagios.CheckOutputEOL, + ), + ) + + break outerLoop + } + } + } + } + + return advice.String() +} diff --git a/internal/certs/validation-root.go b/internal/certs/validation-root.go new file mode 100644 index 0000000..4feacf8 --- /dev/null +++ b/internal/certs/validation-root.go @@ -0,0 +1,435 @@ +// Copyright 2024 Adam Chalkley +// +// https://github.com/atc0005/check-cert +// +// Licensed under the MIT License. See LICENSE file in the project root for +// full license information. + +package certs + +import ( + "crypto/x509" + _ "embed" + "errors" + "fmt" + "strings" + + "github.com/atc0005/go-nagios" +) + +// Add an "implements assertion" to fail the build if the interface +// implementation isn't correct. +var _ CertChainValidationResult = (*RootValidationResult)(nil) + +// Advice for sysadmins resolving cert chain issues saved in external files +// for easier maintenance. +var ( + //go:embed advice/root-cert-found.txt + rootCertFoundAdvice string +) + +// RootValidationResult is the validation result from performing +// expiration validation against each certificate in a chain. +type RootValidationResult struct { + // certChain is the collection of certificates that we evaluated to + // produce this validation check result. + certChain []*x509.Certificate + + // err is the "final" error describing the validation attempt. + err error + + // ignored indicates whether validation check results are ignored for the + // certificate chain. + ignored bool + + // validationOptions tracks what validation options were chosen by the + // sysadmin. + validationOptions CertChainValidationOptions + + // verboseOutput indicates whether user has requested verbose validation + // results output. + verboseOutput bool + + // numRootCerts is the number of certificates in the evaluated certificate + // chain which were found to be root certificates. + numRootCerts int + + // priorityModifier is applied when calculating the priority for a + // validation check result. If a validation check result has an associated + // error but is flagged as ignored then the base priority value is used + // and this modifier is ignored. + // + // If the validation check is not flagged as ignored than this modifier is + // used to calculate the final priority level. + priorityModifier int +} + +// ValidateRoot evaluates a given certificate chain for certificates +// determined to be a root certificate (best practice indicates it should not +// be included). If specified, a flag is set to generate verbose validation +// output. +func ValidateRoot( + certChain []*x509.Certificate, + verboseOutput bool, + validationOptions CertChainValidationOptions, +) RootValidationResult { + + // Perform basic validation of given values. + // + // What other "basics" do we check for before we assert that a root + // certificate is not present? + + if len(certChain) == 0 { + return RootValidationResult{ + certChain: certChain, + validationOptions: validationOptions, + err: fmt.Errorf( + "required certificate chain is empty: %w", + ErrNoCertsFound, + ), + ignored: validationOptions.IgnoreValidationResultRoot, + priorityModifier: priorityModifierMaximum, + } + } + + numRootCerts := NumRootCerts(certChain) + hasRootCert := numRootCerts > 0 + + switch { + case hasRootCert: + return RootValidationResult{ + certChain: certChain, + err: fmt.Errorf( + "%s validation failed: %w", + strings.ToLower(checkNameRootValidationResult), + ErrRootCertsFound, + ), + ignored: validationOptions.IgnoreValidationResultRoot, + validationOptions: validationOptions, + verboseOutput: verboseOutput, + numRootCerts: numRootCerts, + priorityModifier: priorityModifierMedium, + } + + default: + // No issues found. + return RootValidationResult{ + certChain: certChain, + err: nil, + ignored: validationOptions.IgnoreValidationResultRoot, + validationOptions: validationOptions, + verboseOutput: verboseOutput, + numRootCerts: numRootCerts, + priorityModifier: priorityModifierBaseline, + } + } +} + +// CheckName emits the human-readable name of this validation check result. +func (rvr RootValidationResult) CheckName() string { + return checkNameRootValidationResult +} + +// CertChain returns the evaluated certificate chain. +func (rvr RootValidationResult) CertChain() []*x509.Certificate { + return rvr.certChain +} + +// TotalCerts returns the number of certificates in the evaluated certificate +// chain. +func (rvr RootValidationResult) TotalCerts() int { + return len(rvr.certChain) +} + +// IsWarningState indicates whether this validation check result is in a +// WARNING state. This returns false if the validation check resulted in an OK +// or CRITICAL state, or is flagged as ignored. +func (rvr RootValidationResult) IsWarningState() bool { + switch { + case rvr.IsIgnored(): + return false + + case rvr.IsOKState(): + return false + + case rvr.IsCriticalState(): + return false + + case errors.Is(rvr.err, ErrRootCertsFound): + // A certificate chain with a root certificate is considered a WARNING + // state and not CRITICAL because the majority of modern clients + // (e.g., browsers) will ignore it. A separate validation check + // asserts that all included certificates are not expired (or + // expiring). + // + // We explicitly handle this specific error type vs letting a more + // general match handle it.This is as much to document the intent as + // to provide a hook for future use. + return true + + default: + return false + } +} + +// IsCriticalState indicates whether this validation check result is in a +// CRITICAL state. This returns false if the validation check resulted in an +// OK or WARNING state, or is flagged as ignored. +func (rvr RootValidationResult) IsCriticalState() bool { + switch { + case rvr.IsIgnored(): + return false + + case errors.Is(rvr.err, ErrNoCertsFound): + // A certificate chain missing all certificates is considered a + // CRITICAL state because required certificates are not present. There + // isn't anything we can reasonably check in this situation. + return true + + default: + return false + } +} + +// IsUnknownState indicates whether this validation check result is in an +// UNKNOWN state. +func (rvr RootValidationResult) IsUnknownState() bool { + // This state is not used for this certificate validation check. + return false +} + +// IsOKState indicates whether this validation check result is in an OK or +// passing state. For the purposes of validation check evaluation, ignored +// validation checks are considered to be a subset of OK status. +func (rvr RootValidationResult) IsOKState() bool { + return rvr.err == nil || rvr.IsIgnored() +} + +// IsIgnored indicates whether this validation check result was flagged as +// ignored for the purposes of determining final validation state. +func (rvr RootValidationResult) IsIgnored() bool { + return rvr.ignored +} + +// IsSucceeded indicates whether this validation check result is not flagged +// as ignored and no problems with the certificate chain were identified. +func (rvr RootValidationResult) IsSucceeded() bool { + return rvr.IsOKState() && !rvr.IsIgnored() +} + +// IsFailed indicates whether this validation check result is not flagged as +// ignored and problems were identified. +func (rvr RootValidationResult) IsFailed() bool { + return rvr.err != nil && !rvr.IsIgnored() +} + +// Err returns the underlying error (if any) regardless of whether this +// validation check result is flagged as ignored. +func (rvr RootValidationResult) Err() error { + return rvr.err +} + +// ServiceState returns the appropriate Service Check Status label and exit +// code for this validation check result. +func (rvr RootValidationResult) ServiceState() nagios.ServiceState { + return ServiceState(rvr) +} + +// Priority indicates the level of importance for this validation check +// result. +// +// This value is calculated by applying a priority modifier for specific +// failure conditions (recorded when the validation check result is +// initially obtained) to a baseline value specific to the validation +// check performed. +// +// If the validation check result is flagged as ignored the priority +// modifier is also ignored. +func (rvr RootValidationResult) Priority() int { + switch { + case rvr.ignored: + // Though the result is ignored, we indicate the baseline value for + // this check result to allow this result to sort properly against + // other check results which may also be ignored. This why we don't + // use a value of 0 (or equivalent) here. + return baselinePriorityRootValidationResult + default: + return baselinePriorityRootValidationResult + rvr.priorityModifier + } +} + +// Overview provides a high-level summary of this validation check result. +func (rvr RootValidationResult) Overview() string { + return fmt.Sprintf( + "[ROOT CERTS: %d, TOTAL: %d]", + rvr.NumRootCerts(), + rvr.TotalCerts(), + ) +} + +// Status is intended as a brief status of the validation check result. This +// can be used as initial lead-in text. +func (rvr RootValidationResult) Status() string { + var summary string + + switch { + case errors.Is(rvr.err, ErrRootCertsFound): + summary = fmt.Sprintf( + "%s validation %s: %d root certs present", + rvr.CheckName(), + rvr.ValidationStatus(), + rvr.NumRootCerts(), + ) + + // Catchall error handling + case rvr.err != nil: + summary = fmt.Sprintf( + "%s validation %s: unexpected error encountered while validating %d certs: %s", + rvr.CheckName(), + rvr.ValidationStatus(), + rvr.TotalCerts(), + rvr.err.Error(), + ) + + // Success / OK scenario + default: + summary = fmt.Sprintf( + "%s validation %s: %d certs present, %d root certs", + rvr.CheckName(), + rvr.ValidationStatus(), + rvr.TotalCerts(), + rvr.NumRootCerts(), + ) + } + + return summary +} + +// StatusDetail provides additional details intended to extend the shorter +// status text with information suitable as explanation for the overall state +// of the validation check result. This text may span multiple lines. +func (rvr RootValidationResult) StatusDetail() string { + // NOTE: This is called from the Report() method and is used to compose + // that larger output block. + + var detail strings.Builder + + switch { + case errors.Is(rvr.err, ErrRootCertsFound): + detail.WriteString( + fmt.Sprintf( + "A root certificate in the chain was found!%s", + nagios.CheckOutputEOL, + ), + ) + + detail.WriteString( + fmt.Sprintf( + "%s%s%s", + nagios.CheckOutputEOL, + strings.TrimSpace(rootCertFoundAdvice), + nagios.CheckOutputEOL, + ), + ) + + // detail.WriteString(reorderChainAdvice(rvr.certChain)) + + // Catchall error handling + case rvr.err != nil: + detail.WriteString( + fmt.Sprintf( + "An unexpected error occurred while performing %s validation!%s", + strings.ToLower(rvr.CheckName()), + nagios.CheckOutputEOL, + ), + ) + + detail.WriteString( + fmt.Sprintf( + "Please report the following error and provide a copy of your certificate chain for evaluation (e.g., see cpcert tool in this project).%s%s", + nagios.CheckOutputEOL, + nagios.CheckOutputEOL, + ), + ) + + detail.WriteString( + fmt.Sprintf( + "Error: %q%s", + rvr.err.Error(), + nagios.CheckOutputEOL, + ), + ) + + // Success / OK scenario + default: + // TODO: Anything extra to add for no root certs detected? + // The Status() output is likely sufficient to cover this. + } + + return detail.String() +} + +// String provides the validation check result in human-readable format. +// Because the certificates chain report is so detailed we skip emitting those +// details. +func (rvr RootValidationResult) String() string { + return fmt.Sprintf( + "%s %s", + rvr.Status(), + rvr.Overview(), + ) +} + +// Report provides the validation check result in verbose human-readable +// format. Trailing whitespace is intentionally omitted per +// CertChainValidationResult recommendation. +func (rvr RootValidationResult) Report() string { + switch { + // Show advice regardless of whether check results were ignored (for the + // purposes of determining final plugin check state). + case rvr.err != nil: + return fmt.Sprintf( + "%s %s%s%s", + rvr.Status(), + nagios.CheckOutputEOL, + nagios.CheckOutputEOL, + rvr.StatusDetail(), + ) + + default: + statusSummary := fmt.Sprintf( + "%d total certificates, %d root certificates", + rvr.TotalCerts(), + rvr.NumRootCerts(), + ) + + // Provide overview only. + return fmt.Sprintf( + "%s validation %s: %s", + rvr.CheckName(), + rvr.ValidationStatus(), + statusSummary, + ) + } +} + +// NumRootCerts indicates the number of certificates in the chain that are +// root certificates. +func (rvr RootValidationResult) NumRootCerts() int { + return rvr.numRootCerts +} + +// ValidationStatus provides a one word status value for expiration validation +// check results. If the original certificate chain was filtered then the +// validation status value is based on the filtered chain, otherwise the +// original certificate chain is used. +func (rvr RootValidationResult) ValidationStatus() string { + switch { + case rvr.IsFailed(): + return ValidationStatusFailed + case rvr.IsIgnored(): + return ValidationStatusIgnored + default: + return ValidationStatusSuccessful + } +} diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 12332ff..dc9f02e 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -346,12 +346,6 @@ func TestApplyIgnoreDecision(t *testing.T) { validateFunc: Config.ApplyCertSANsListValidationResults, applyResults: false, }, - { - name: "DefaultValidateChainOrderResults", - cfg: Config{}, - validateFunc: Config.ApplyCertChainOrderValidationResults, - applyResults: defaultApplyCertChainOrderValidationResults, - }, { name: "DefaultValidateSANsListResultsWithSANsEntries", cfg: Config{ @@ -376,6 +370,12 @@ func TestApplyIgnoreDecision(t *testing.T) { validateFunc: Config.ApplyCertSANsListValidationResults, applyResults: true, }, + { + name: "DefaultValidateChainOrderResults", + cfg: Config{}, + validateFunc: Config.ApplyCertChainOrderValidationResults, + applyResults: defaultApplyCertChainOrderValidationResults, + }, { name: "IgnoreValidateChainOrderResults", cfg: Config{ @@ -392,6 +392,28 @@ func TestApplyIgnoreDecision(t *testing.T) { validateFunc: Config.ApplyCertChainOrderValidationResults, applyResults: true, }, + { + name: "DefaultValidateRootResults", + cfg: Config{}, + validateFunc: Config.ApplyCertRootValidationResults, + applyResults: defaultApplyCertRootValidationResults, + }, + { + name: "IgnoreValidateRootResults", + cfg: Config{ + ignoreValidationResults: []string{ValidationKeywordRoot}, + }, + validateFunc: Config.ApplyCertRootValidationResults, + applyResults: false, + }, + { + name: "ApplyValidateRootResults", + cfg: Config{ + applyValidationResults: []string{ValidationKeywordRoot}, + }, + validateFunc: Config.ApplyCertRootValidationResults, + applyResults: true, + }, } for _, tt := range tests { diff --git a/internal/config/constants.go b/internal/config/constants.go index cf75a53..7d0979b 100644 --- a/internal/config/constants.go +++ b/internal/config/constants.go @@ -160,6 +160,7 @@ const ( ValidationKeywordHostname string = "hostname" ValidationKeywordSANsList string = "sans" ValidationKeywordChainOrder string = "order" + ValidationKeywordRoot string = "root" ) // Certificate type keywords used when filtering specific certificate types @@ -252,6 +253,19 @@ const ( // has been made and sufficient time has passed to allow sysadmins to // explicitly opt out. defaultApplyCertChainOrderValidationResults bool = false + + // Whether negative root presence assertion check results should be + // applied when determining overall validation state of a certificate + // chain by default. + // + // This is set based on existing behavior in prior stable releases; since + // this validation type did not exist in early stable releases we + // introduce this validation type with validation ignored by default. + // + // NOTE: A future version may enable this by default after an announcement + // has been made and sufficient time has passed to allow sysadmins to + // explicitly opt out. + defaultApplyCertRootValidationResults bool = false ) // Constants specific to the copier app. diff --git a/internal/config/getters.go b/internal/config/getters.go index 45979e8..b215e30 100644 --- a/internal/config/getters.go +++ b/internal/config/getters.go @@ -201,6 +201,29 @@ func (c Config) ApplyCertChainOrderValidationResults() bool { } } +// ApplyCertRootValidationResults indicates whether negative root presence +// assertion check results should be applied when performing final plugin +// state evaluation. Precedence is given for explicit request to ignore this +// validation result. +func (c Config) ApplyCertRootValidationResults() bool { + ignoreRequested := textutils.InList( + ValidationKeywordRoot, c.ignoreValidationResults, true, + ) + + applyRequested := textutils.InList( + ValidationKeywordRoot, c.applyValidationResults, true, + ) + + switch { + case ignoreRequested: + return false + case applyRequested: + return true + default: + return defaultApplyCertRootValidationResults + } +} + // supportedValidationCheckResultKeywords returns a list of valid validation // check keywords used by plugin type applications in this project. func supportedValidationCheckResultKeywords() []string { @@ -209,6 +232,7 @@ func supportedValidationCheckResultKeywords() []string { ValidationKeywordExpiration, ValidationKeywordSANsList, ValidationKeywordChainOrder, + ValidationKeywordRoot, } } diff --git a/internal/config/logging.go b/internal/config/logging.go index db1c212..5b60835 100644 --- a/internal/config/logging.go +++ b/internal/config/logging.go @@ -138,6 +138,7 @@ func (c *Config) setupLogging(appType AppType) error { Bool("apply_expiration_validation_results", c.ApplyCertExpirationValidationResults()). Bool("apply_sans_list_validation_results", c.ApplyCertSANsListValidationResults()). Bool("apply_chain_order_validation_results", c.ApplyCertChainOrderValidationResults()). + Bool("apply_got_root_validation_results", c.ApplyCertRootValidationResults()). // TODO: Extend with further validation check names. Logger()