Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add support for detecting misordered certificate chain entries #1004

Open
atc0005 opened this issue Nov 4, 2024 · 6 comments
Open

Add support for detecting misordered certificate chain entries #1004

atc0005 opened this issue Nov 4, 2024 · 6 comments
Assignees
Labels
app/lscert documentation Improvements or additions to documentation enhancement New feature or request output/extended Long Service Output (aka, "extended" or "detailed") output/perfdata Service Perf Data (aka, "performance data") plugin/check_cert
Milestone

Comments

@atc0005
Copy link
Owner

atc0005 commented Nov 4, 2024

This is implicitly covered by the goal to provide certificate chain verification (#2), but is worth covering explicitly so it can be singled out as an item in a problems report.

See also:

@atc0005 atc0005 added documentation Improvements or additions to documentation enhancement New feature or request plugin/check_cert app/lscert output/perfdata Service Perf Data (aka, "performance data") output/extended Long Service Output (aka, "extended" or "detailed") labels Nov 4, 2024
@atc0005 atc0005 added this to the Future milestone Nov 4, 2024
@atc0005 atc0005 self-assigned this Nov 4, 2024
@atc0005 atc0005 modified the milestones: Future, v0.21.0 Nov 6, 2024
@atc0005 atc0005 modified the milestones: v0.21.0, v0.22.0, v0.23.0, v0.24.0 Nov 16, 2024
atc0005 added a commit to atc0005/cert-payload that referenced this issue Nov 23, 2024
OVERVIEW

Add support for generating a certificate metadata payload in JSON
format from a specified metadata payload format version.

Add support for support for decoding a given (valid) certificate
metadata payload. The format is automatically detected from a list of
valid format versions.

The intent is to support all stable format versions indefinitely.

As of this commit / PR, format 0 is still under active development.
This format version is an "unstable" metadata format and is not
covered by this goal; format version 0 is subject to change often as
development continues. Format version 1 is implemented at this time as
a stub version for testing purposes; once stable the plan is to
promote version 0 content as the initial version 1.

CHANGES

Primary changes:

- add support for generating a JSON payload from a specified metadata
  payload format version
  - this can be generated by calling the `Encode` function from a
    specific format version or by calling the top-level `Encode`
    function and specifying a valid format version number (e.g., `0`
    or `1`)
- add support for decoding a given (valid) certificate metadata
  payload
  - the intent is to support decoding any given payload matching the
    set of supported format versions (e.g., `0`, `1`)
  - the caller provides an instance of a specific format version of
    the certificate metadata payload and the `Decode` function for
    that format version is used
  - once a format version is stable, the intent is to support creating
    and decoding it using this library indefinitely
    - this should allow the sysadmin using the `check_cert` plugin to
      specify what version of the payload format they wish to create
    - this should allow the sysadmin using a reporting tool to consume
      a certificate metadata payload generated by the `check_cert`
      plugin in the same fixed version as the one they asked the
      `check_cert` plugin to create
    - this process should continue to work as-is until the sysadmin
      decides to explicitly change the certificate metadata payload
      format version they're working with; updating this dependency
      should not break payload generation or consumption

Other changes:

- add identification of misordered certificate chains
- add example "test" to illustrate library usage
  - initial example uses format 0; the plan is to update the example
    once format 1 is released/stable
- documentation updates
- general refactoring work

REFERENCES

- #19
- #31
- #46
- atc0005/check-cert#1004
atc0005 added a commit to atc0005/cert-payload that referenced this issue Nov 23, 2024
OVERVIEW

Add support for generating a certificate metadata payload in JSON
format from a specified metadata payload format version.

Add support for support for decoding a given (valid) certificate
metadata payload. The format is automatically detected from a list of
valid format versions.

The intent is to support all stable format versions indefinitely.

As of this commit / PR, format 0 is still under active development.
This format version is an "unstable" metadata format and is not
covered by this goal; format version 0 is subject to change often as
development continues. Format version 1 is implemented at this time as
a stub version for testing purposes; once stable the plan is to
promote version 0 content as the initial version 1.

CHANGES

Primary changes:

- add support for generating a JSON payload from a specified metadata
  payload format version
  - this can be generated by calling the `Encode` function from a
    specific format version or by calling the top-level `Encode`
    function and specifying a valid format version number (e.g., `0`
    or `1`)
- add support for decoding a given (valid) certificate metadata
  payload
  - the intent is to support decoding any given payload matching the
    set of supported format versions (e.g., `0`, `1`)
  - the caller provides an instance of a specific format version of
    the certificate metadata payload and the `Decode` function for
    that format version is used
  - once a format version is stable, the intent is to support creating
    and decoding it using this library indefinitely
    - this should allow the sysadmin using the `check_cert` plugin to
      specify what version of the payload format they wish to create
    - this should allow the sysadmin using a reporting tool to consume
      a certificate metadata payload generated by the `check_cert`
      plugin in the same fixed version as the one they asked the
      `check_cert` plugin to create
    - this process should continue to work as-is until the sysadmin
      decides to explicitly change the certificate metadata payload
      format version they're working with; updating this dependency
      should not break payload generation or consumption

Other changes:

- add identification of misordered certificate chains
- add example "test" to illustrate library usage
  - initial example uses format 0; the plan is to update the example
    once format 1 is released/stable
- documentation updates
- general refactoring work

REFERENCES

- #19
- #31
- #46
- atc0005/check-cert#1004
atc0005 added a commit that referenced this issue Nov 24, 2024
CHANGES

NOTE: All of the following changes pertain to the `cert_check` plugin
and associated dependencies.

Primary changes:

- create certificate metadata payload (if requested) regardless of
  successful certificate chain validation attempt
  - e.g., even if the connection attempt times out or DNS resolution
    fails
- add `payload-format` flag to allow explicitly specifying which
  certificate metadata payload format version will be used when
  generating the payload for inclusion in plugin output
  - this is intended to provide stability when updating the
    `cert-payload` dependency as it "locks-in" the format version
    instead of just always using the latest format and creating a
    conflict with reporting tools that expect to work with specific
    formats
- enable full `go-nagios` dependency debug logging *if* debug logging
  is requested plugin
- *heavily* refactor `addCertChainPayload` function
  - to outsource bulk of certificate metadata payload generation to
    the `cert-payload` project
  - to record the resolved IP Address used to retrieve the certificate
    chain
  - add warning if (unstable) certificate metadata payload format 0 is
    chosen
    - while format version 1 is not *yet* stable (and serves mostly as
      a stub entry to test against), the plan is to stabilize it
      fairly soon
  - emit "pretty printed" JSON payload if debug or trace logging
    levels is requested

Additional changes:

- misordered certificate chain detection is now performed by the
  cert-payload project and recorded in the certificate metadata
  payload
  - the `check_cert` plugin is not (yet) flagging the service check as
    passing/failing based on this criteria; misordered chain
    validation is to be added in a future project release

REFERENCES

- atc0005/cert-payload#19
- atc0005/cert-payload#31
- atc0005/cert-payload#46
- #1004
atc0005 added a commit that referenced this issue Nov 24, 2024
CHANGES

NOTE: All of the following changes pertain to the `cert_check` plugin
and associated dependencies.

Primary changes:

- create certificate metadata payload (if requested) regardless of
  successful certificate chain validation attempt
  - e.g., even if the connection attempt times out or DNS resolution
    fails
- add `payload-format` flag to allow explicitly specifying which
  certificate metadata payload format version will be used when
  generating the payload for inclusion in plugin output
  - this is intended to provide stability when updating the
    `cert-payload` dependency as it "locks-in" the format version
    instead of just always using the latest format and creating a
    conflict with reporting tools that expect to work with specific
    formats
- enable full `go-nagios` dependency debug logging *if* debug logging
  is requested plugin
- *heavily* refactor `addCertChainPayload` function
  - to outsource bulk of certificate metadata payload generation to
    the `cert-payload` project
  - to record the resolved IP Address used to retrieve the certificate
    chain
  - add warning if (unstable) certificate metadata payload format 0 is
    chosen
    - while format version 1 is not *yet* stable (and serves mostly as
      a stub entry to test against), the plan is to stabilize it
      fairly soon
  - emit "pretty printed" JSON payload if debug or trace logging
    levels is requested

Additional changes:

- misordered certificate chain detection is now performed by the
  cert-payload project and recorded in the certificate metadata
  payload
  - the `check_cert` plugin is not (yet) flagging the service check as
    passing/failing based on this criteria; misordered chain
    validation is to be added in a future project release

REFERENCES

- atc0005/cert-payload#19
- atc0005/cert-payload#31
- atc0005/cert-payload#46
- #1004
atc0005 added a commit that referenced this issue Nov 25, 2024
CHANGES

NOTE: All of the following changes pertain to the `cert_check` plugin
and associated dependencies.

Primary changes:

- create certificate metadata payload (if requested) regardless of
  successful certificate chain validation attempt
  - e.g., even if the connection attempt times out or DNS resolution
    fails
- add `payload-format` flag to allow explicitly specifying which
  certificate metadata payload format version will be used when
  generating the payload for inclusion in plugin output
  - this is intended to provide stability when updating the
    `cert-payload` dependency as it "locks-in" the format version
    instead of just always using the latest format and creating a
    conflict with reporting tools that expect to work with specific
    formats
- enable full `go-nagios` dependency debug logging *if* debug logging
  is requested plugin
- *heavily* refactor `addCertChainPayload` function
  - to outsource bulk of certificate metadata payload generation to
    the `cert-payload` project
  - to record the resolved IP Address used to retrieve the certificate
    chain
  - add warning if (unstable) certificate metadata payload format 0 is
    chosen
    - while format version 1 is not *yet* stable (and serves mostly as
      a stub entry to test against), the plan is to stabilize it
      fairly soon
  - emit "pretty printed" JSON payload if debug or trace logging
    levels is requested

Additional changes:

- misordered certificate chain detection is now performed by the
  cert-payload project and recorded in the certificate metadata
  payload
  - the `check_cert` plugin is not (yet) flagging the service check as
    passing/failing based on this criteria; misordered chain
    validation is to be added in a future project release

REFERENCES

- atc0005/cert-payload#19
- atc0005/cert-payload#31
- atc0005/cert-payload#46
- #1004
atc0005 added a commit to atc0005/cert-payload that referenced this issue Dec 3, 2024
OVERVIEW

Add support for generating a certificate metadata payload in JSON
format from a specified metadata payload format version.

Add support for support for decoding a given (valid) certificate
metadata payload. The format is automatically detected from a list of
valid format versions.

The intent is to support all stable format versions indefinitely.

As of this commit / PR, format 0 is still under active development.
This format version is an "unstable" metadata format and is not
covered by this goal; format version 0 is subject to change often as
development continues. Format version 1 is implemented at this time as
a stub version for testing purposes; once stable the plan is to
promote version 0 content as the initial version 1.

CHANGES

Primary changes:

- add support for generating a JSON payload from a specified metadata
  payload format version
  - this can be generated by calling the `Encode` function from a
    specific format version or by calling the top-level `Encode`
    function and specifying a valid format version number (e.g., `0`
    or `1`)
- add support for decoding a given (valid) certificate metadata
  payload
  - the intent is to support decoding any given payload matching the
    set of supported format versions (e.g., `0`, `1`)
  - the caller provides an instance of a specific format version of
    the certificate metadata payload and the `Decode` function for
    that format version is used
  - once a format version is stable, the intent is to support creating
    and decoding it using this library indefinitely
    - this should allow the sysadmin using the `check_cert` plugin to
      specify what version of the payload format they wish to create
    - this should allow the sysadmin using a reporting tool to consume
      a certificate metadata payload generated by the `check_cert`
      plugin in the same fixed version as the one they asked the
      `check_cert` plugin to create
    - this process should continue to work as-is until the sysadmin
      decides to explicitly change the certificate metadata payload
      format version they're working with; updating this dependency
      should not break payload generation or consumption

Other changes:

- add identification of misordered certificate chains
- add example "test" to illustrate library usage
  - initial example uses format 0; the plan is to update the example
    once format 1 is released/stable
- documentation updates
- general refactoring work

REFERENCES

- #19
- #31
- #46
- atc0005/check-cert#1004
@atc0005
Copy link
Owner Author

atc0005 commented Dec 11, 2024

Question: what happens if there is only one cert in the chain?

@atc0005
Copy link
Owner Author

atc0005 commented Dec 11, 2024

Question: what happens if there is only one cert in the chain?

Opted to handle this as an error condition for this validation check. While it probably should be handled by a separate validation check, I think adding it here first can be helpful.

Currently I'm treating an incomplete cert chain (of length 1) as CRITICAL state with a misordered cert chain as WARNING state. The thinking is that most modern browsers will handle reordering a cert chain without much complaint while some modern clients will object to just a leaf cert present in a cert chain.

Between the two problems, a cert chain of just a leaf seems like it would have the biggest impact on users and so would warrant the strongest non-OK state rating.

@atc0005
Copy link
Owner Author

atc0005 commented Dec 11, 2024

This may need to be tracked by a GH issue of its own, but this came up in a conversation today:

communicate which intermediate/root bundle link should be used when we receive the Sectigo email

This was from a follow-up conversation about the topic of sysadmins unintentionally installing the wrong intermediates bundle from Sectigo (InCommon).

In that earlier discussion I shared these guidelines for use when selecting certificate download links from "InCommon SSL certificate" notifications from Sectigo:

  1. as Certificate only, PEM encoded
    • ➖ provides only the leaf certificate
    • 🆗 if you next append Issuing CA certificates only > as Intermediate(s)/Root only, PEM encoded to this cert
  2. as Certificate (w/ issuer after), PEM encoded
    • ✅ everything already in the correct order
  3. as Certificate (w/ chain), PEM encoded
    • ❌ everything reversed
  4. Issuing CA certificates only > as Root/Intermediate(s) only, PEM encoded
    • ➖ intermediates only
    • ❌ everything reversed
  5. Issuing CA certificates only > as Intermediate(s)/Root only, PEM encoded
    • ➖ intermediates only
    • 🆗 if you next append this to the file downloaded via the as Certificate only, PEM encoded option

Option 4 from this is (unfortunately) commonly paired with Option 1 to form a certificate chain. Instead, Option 2 will usually give the best results.

In the conversation today I responded with (snippet):

the plugin can be updated to offer some advice on this also.

I think on the next iteration I’ll update the logic to look for indicators of a InCommon/Sectigo intermediate being used to sign the leaf cert and offer a suggestion for the correct bundle link.

This should hopefully work well both for cert chains containing just the leaf and cert chains with the intermediates out of order.

The logic used to provide this advice could be extended to match intermediates used by other common certificate vendors and provide remediation advice for those as well.

atc0005 added a commit that referenced this issue Dec 18, 2024
OVERVIEW

This set of changes applies to both the `check_cert` plugin and the
`lscert` CLI tool. While the primary focus is adding a prototype
implementation of "chain order" validation logic, other ergonomic and
display changes have been applied to the `lscert` tool. Some of those
changes are likely to be refined further in near future releases as
additional validation checks are applied.

CHANGES

- update `check_cert` plugin
  - add new `Chain Order` validation type
    - 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
  - add new performance data metrics
    - `certs_ordered`
    - `certs_misordered`
  - extend tests to cover new validation type
- update `lscert`
  - incorporate new validation check
  - rework summary display to use emoji status indicators
    (pass/neutral/warn/crit)
  - rename headers to emphasize "cert chain" over just "certs"
  - incorporate the same "advice" output that the `check_cert` plugin
    now emits for `Chain Order` validation problems

CREDIT

The following LLM sources were used for inspiration or starting code
samples for some of the included changes:

- ChatGPT, OpenAI
- Google Gemini
- Claude (Anthropic AI assistant)

In particular, I used all of these sources for assistance with logic
for certificate chain ordering, signature validation and misc other
tasks. While none provided solutions I could use as-is, all were
helpful in pointing me in the right direction when I needed it.

REFERENCES

- GH-1004
- GH-364
- GH-365
- GH-219
@atc0005 atc0005 pinned this issue Dec 20, 2024
atc0005 added a commit that referenced this issue Dec 20, 2024
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
@atc0005
Copy link
Owner Author

atc0005 commented Jan 26, 2025

While working on #364 (and intentionally testing against a chain with the wrong intermediates bundle and an intentionally wrong intermediates from another CA), I realized that it may be worth combining this validation check with an intermediates validation check.

As a sysadmin, I'd first want to know if invalid intermediates are included, then missing intermediates and finally if the intermediates are present, but in the wrong order.

As things stand, the misordered check results will be displayed alongside check results for either missing or invalid intermediates. Coming at this with fresh eyes, I don't know if it's helpful to see both of those "needs to be fixed" items listed.

Instead, a sysadmin should update/replace the chain to remove invalid certificates or add missing intermediates. Then, if the chain is misordered that should be listed as the ordering may change significantly when the chain is fixed/replaced.

Still tinkering, trying to decide the best way to handle this.

@atc0005
Copy link
Owner Author

atc0005 commented Jan 26, 2025

As things stand, the misordered check results will be displayed alongside check results for either missing or invalid intermediates. Coming at this with fresh eyes, I don't know if it's helpful to see both of those "needs to be fixed" items listed.

Additionally, the "misordered" validation results are likely to be wrong if the "misordered" detection logic doesn't exclude invalid certificates and first ensure that all intermediates are present.

For example, with the cert chain that I intentionally inserted a wrong cert and used an old intermediates bundle from the CA we ended up with this in the chain:

(1) InCommon RSA Server CA [intermediate]
(2) USERTrust RSA Certification Authority [intermediate]
(3) E5 [intermediate]
(4) AAA Certificate Services [root]

Position 0 (not shown) being the leaf cert.

After we have the modified "recommendation" logic exclude invalid certs, this is what it recommends:

(1) USERTrust RSA Certification Authority [intermediate]
(2) AAA Certificate Services [root]

Those two remaining certs are technically correct & ordered properly, but we're missing the issuer for the leaf certificate. We could modify the logic to just drop remaining unlinked elements from the chain, but then we'd just be recommending the leaf cert. Maybe that really is good advice whenever invalid certs are present and there are not enough certs available to construct a full chain, but it feels disjointed.

Instead, folding the logic into a single flow (assert no invalid, assert no missing, asserting not misordered) seems like the most reliable/maintainable approach.

@atc0005
Copy link
Owner Author

atc0005 commented Jan 28, 2025

Loose thoughts as I continue trying to puzzle out next steps:

A chain can be declared as having both invalid (wrong intermediates present) and missing intermediates.

A chain can not be declared as both invalid and misordered. Likewise, a chain can not have missing intermediates and be misordered.

A misordered chain requires that all expected certs be present but out of the expected order. If an authoritative collection is not provided, we can still assert that what we're provided is not misordered by using the available certs in the given chain. This is not quite as reliable as when checking against an authoritative collection (i.e., AIA-sourced intermediate certs), but should be "good enough" for handling the most common scenarios.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
app/lscert documentation Improvements or additions to documentation enhancement New feature or request output/extended Long Service Output (aka, "extended" or "detailed") output/perfdata Service Perf Data (aka, "performance data") plugin/check_cert
Projects
None yet
Development

No branches or pull requests

1 participant