Merge pull request #1198 from atc0005/i365-add-support-for-root-cert-…

…validation

Initial implementation of `Root` cert validation

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`

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 {

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
 
 }

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

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.

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