-
-
Notifications
You must be signed in to change notification settings - Fork 580
Feature/cert chain #1121
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Feature/cert chain #1121
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the root certificate in the chain expires in, for instance, 5 days, but the child certificate expires in 30 days, but the root certificate authority isn't owned by the user using Gatus to monitor his environment, does it make sense to alert based on the expiration of the root certificate? Maybe they automatically renew a day before it expires.
(This isn't a rhetorical question, feel free to let me know what you think. I'm just trying to figure out if this change in behavior may upset some users)
client/client.go
Outdated
hostAndPort := strings.Split(address, ":") | ||
if len(hostAndPort) != 2 { | ||
return false, nil, errors.New("invalid address for starttls, format must be host:port") | ||
return CertificateChainInfo{Error: errors.New("invalid address for starttls, format must be host:port")} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know how I feel about not passing the error as a separate return value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree. Looks weird.
That's a really good question. It would confuse customers indeed. Especially getting alerts on (root) certificate expiration when you don't control the CA.
|
FWIW, I've seen various older Linux distros handle expiring root certificates explicitly included in a certificate chain rather poorly (e.g., At that time I supported several systems that were dependent on a load-balanced LDAP service managed by another team. From what I remember the cert chain for that service included the leaf certificate (still valid), intermediate certificates (one valid, one expired) and a root certificate (also expired). Existing monitoring checks (applied by another product) passed since the check evaluated only the leaf certificate. Real world clients failed to connect. If the root certificate is not explicitly included in a cert chain, then alerting on that expiration (pending or passed) is probably not that useful (particularly where cross-signed intermediates are used). Reference link (covers the incident well): |
In light of that, I think that what I'd want is a root-level configuration such as By default, Gatus should check the full certificate chain, which means this variable would default to P.S. If you can think of a better name for the configuration, please feel free to suggest it. |
Finalised the work and tested it. Looks good. |
Here is a config I used to test. |
t.Error("expected true") | ||
if rtt == 0 { | ||
t.Error("Round-trip time returned on failure should've been 0") | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test fails on Windows system. So might be a good idea to skip?
My two cents about this: would it be possible to keep the current behavior as default and only enable full-chain certificate check on demand? |
I've got another idea instead. We could implement a new placeholder, |
Summary
#1117
Enhanced certificate monitoring to check the entire certificate chain (leaf, intermediate, and root certificates) instead of just the leaf certificate.
Added new metrics
gatus_results_certificate_chain_expiration_seconds
with subject and issuer labels to track expiration times for each certificate in the chain.Updated the endpoint evaluation logic in
evaluateSTARTTLS
andevaluateTLS
functions to store and expose the full certificate chain information, maintaining backward compatibility with existing [CERTIFICATE_EXPIRATION] placeholder.Checklist
README.md
, if applicable.