Skip to content

Commit

Permalink
Initial implementation of Root cert validation
Browse files Browse the repository at this point in the history
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
  • Loading branch information
atc0005 committed Dec 20, 2024
1 parent 5e29b8b commit ef9247d
Show file tree
Hide file tree
Showing 13 changed files with 837 additions and 197 deletions.
37 changes: 19 additions & 18 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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`

Expand Down Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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`
Expand Down
31 changes: 31 additions & 0 deletions cmd/check_cert/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
34 changes: 34 additions & 0 deletions cmd/check_cert/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

}
48 changes: 48 additions & 0 deletions cmd/lscert/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion internal/certs/advice/root-cert-found.txt
Original file line number Diff line number Diff line change
@@ -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.
31 changes: 23 additions & 8 deletions internal/certs/certs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Loading

0 comments on commit ef9247d

Please sign in to comment.