-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Add certificate verification status to x509_cert input #6143
Conversation
} | ||
|
||
func (c *X509Cert) getCert(u *url.URL, timeout time.Duration) ([]*x509.Certificate, *tls.Config, error) { | ||
tlsCfg, err := c.ClientConfig.TLSConfig() |
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.
It would make sense to set this up once in an Init
function. Since it is used in the parent function, I would have this function take the tls.Config instead of returning it.
fields := getFields(cert, now) | ||
tags := getTags(cert.Subject, location) | ||
|
||
opts := x509.VerifyOptions{} | ||
if i == 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.
Here would be a great place for a comment:
// The first certificate is the leaf/end-entity certificate which needs DNS
// name validation against the URL hostname.
_, err = cert.Verify(opts) | ||
if err == nil { | ||
tags["validation"] = "success" | ||
fields["validation"] = 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.
Don't name the tag and field with the same key since it is a bit tricky to query, instead use validation
and validation_code
.
I'm not totally on board with the validation key, the validation is always a success, its just that sometimes the cert is invalid. What do you think about verification=valid
, verification=invalid
. Sending the error is a bit of a new pattern, but I think it will work.
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 do like verification
and verification_code
} else { | ||
tags["verification"] = "invalid" | ||
fields["verification"] = 1 | ||
fields["validation_error"] = err.Error() |
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.
verification_error
plugins/inputs/x509_cert/README.md
Outdated
- fields: | ||
- validation (int) | ||
- validation_error (string) |
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.
Needs updated
Resolves #4877
This always sets
insecure_skip_verify
as it verifies the cert once it's been collected/parsed.To verify a self-signed cert (remote endpoint or file), set the
tls_ca
to the self-signing ca, or the self-signed cert itself for validation to be successful.Alternate to #6139