home-assistant / core

:house_with_garden: Open source home automation that puts local control and privacy first.
https://www.home-assistant.io
Apache License 2.0
72.71k stars 30.44k forks source link

Certificate Expiry integration does not seem to check the whole cert chain. #122426

Open FergusMcMenemie-cnic opened 3 months ago

FergusMcMenemie-cnic commented 3 months ago

The problem

I have a case where the cert beinng checked went bad becuase an intermediate cert had expried. The Certificate Expiry Integration only indicated this by totally failing once the cert had expired. The cert expiry went "unknown"

Note sure how to indicate this but I guess the displayed days remaining should be based on the chains shortest lifetime.

What version of Home Assistant Core has the issue?

core-2024.7.3

What was the last working version of Home Assistant Core?

No response

What type of installation are you running?

Home Assistant OS

Integration causing the issue

Certificate Expiry

Link to integration documentation on our website

https://www.home-assistant.io/integrations/cert_expiry

Diagnostics information

No response

Example YAML snippet

No response

Anything in the logs that might be useful for us?

No response

Additional information

No response

home-assistant[bot] commented 3 months ago

Hey there @jjlawren, mind taking a look at this issue as it has been labeled with an integration (cert_expiry) you are listed as a code owner for? Thanks!

Code owner commands Code owners of `cert_expiry` can trigger bot actions by commenting: - `@home-assistant close` Closes the issue. - `@home-assistant rename Awesome new title` Renames the issue. - `@home-assistant reopen` Reopen the issue. - `@home-assistant unassign cert_expiry` Removes the current integration label and assignees on the issue, add the integration domain after the command. - `@home-assistant add-label needs-more-information` Add a label (needs-more-information, problem in dependency, problem in custom component) to the issue. - `@home-assistant remove-label needs-more-information` Remove a label (needs-more-information, problem in dependency, problem in custom component) on the issue.

(message by CodeOwnersMention)


cert_expiry documentation cert_expiry source (message by IssueLinks)

kylehakala commented 3 months ago

This is an interesting one. I’m not a code owner here (nor really able to dedicate any time to putting together any possible additions), but I think there is something worth adding here.

While the integration at face value is just for monitoring leaf/end-node certificates, I’m all in agreement with the main idea that the chain is equally as important when it comes to measuring validity.

I can imagine an implementation here that preserves existing functionality of only measuring the leaf certificate, but going forward, the default would be enabled to measure the whole chain.

So it could absolutely just be a really quick-and-dirty change to suddenly start evaluating the full chain validity, but I think that leaves us with a bit of an inaccurate test on whether the leaf certificate is valid or not.

When an issuer/intermediate expires before the leaf itself, the leaf is almost certainly still a valid certificate in every other sense: no expired, not revoked, subject matches the host name serving up the cert, etc. But there could be multiple valid chain paths that one could use to form the chain (see “Cross Signing” examples for CAs) but that’s not always certain. It could also just be a misconfigured chain, which wouldn’t make the certificate at the very end invalid, but rather the chain as a whole is invalid.

So many it’s just a matter of thoroughly documenting any potential change if made, and making it clear what is being checked in the process.

It’s pretty uncommon (intermediates/issuers expiring prior to a valid certificate which it issued) but it’s a case that is a real bummer to have to work through before realizing the hiccup when you’re not expecting it.

It’s also somewhat late and I ought not fixate too much in a comment thread this late at night about TLS or I’ll be in for some weird dreams.

issue-triage-workflows[bot] commented 1 day ago

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍 This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.