Skip to content
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 additional tags for x509 Input Plugin #6686

Merged
merged 5 commits into from
Nov 26, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion etc/telegraf.conf
Original file line number Diff line number Diff line change
Expand Up @@ -3652,7 +3652,7 @@
# # timeout = "5ms"


# # A plugin to collect stats from Opensmtpd - a validating, recursive, and caching DNS resolver
# # A plugin to collect stats from Opensmtpd - a validating, recursive, and caching DNS resolver
# [[inputs.opensmtpd]]
# ## If running as a restricted user you can prepend sudo for additional access:
# #use_sudo = false
Expand Down Expand Up @@ -4514,6 +4514,14 @@
# ## Timeout for SSL connection
# # timeout = "5s"
#
# ## Include Certificate Issuer information in tags
# # include_issuer = false
#
# ## Include Certificate SAN in tag
# # include_san = false
# ## Separator between each SAN in tag
# # san_separator = ","
#
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you undo this file, we will take care of it afterwards. We usually just update this once before release to avoid conflicts.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

# ## Optional TLS Config
# # tls_ca = "/etc/telegraf/ca.pem"
# # tls_cert = "/etc/telegraf/cert.pem"
Expand Down
15 changes: 15 additions & 0 deletions plugins/inputs/x509_cert/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,19 @@ file or network connection.
## Timeout for SSL connection
# timeout = "5s"

## Include Certificate Issuer information in tags
# include_issuer = false

## Include Certificate SAN in tag
# include_san = false
## Separator between each SAN in tag
# san_separator = ","

## Optional TLS Config
# tls_ca = "/etc/telegraf/ca.pem"
# tls_cert = "/etc/telegraf/cert.pem"
# tls_key = "/etc/telegraf/key.pem"

```


Expand All @@ -33,6 +42,12 @@ file or network connection.
- province
- locality
- verification
- serial_number
- signature_algorithm
- public_key_algorithm
- issuer_common_name
- issuer_serial_number
- san
- fields:
- verification_code (int)
- verification_error (string)
Expand Down
55 changes: 45 additions & 10 deletions plugins/inputs/x509_cert/x509_cert.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ package x509_cert
import (
"crypto/tls"
"crypto/x509"
"crypto/x509/pkix"
"encoding/pem"
"fmt"
"io/ioutil"
Expand All @@ -26,6 +25,14 @@ const sampleConfig = `
## Timeout for SSL connection
# timeout = "5s"

## Include Certificate Issuer information in tags
# include_issuer = false

## Include Certificate SAN in tag
# include_san = false
## Separator between each SAN in tag
# san_separator = ","
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove all of these options, the plugin user can use tagexclude/taginclude instead. On the SAN separator we can just always use ,, I think there is little reason to modify it but I suppose it could be done with a processor, and comma is consistent with our other uses.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. I have removed them.


## Optional TLS Config
# tls_ca = "/etc/telegraf/ca.pem"
# tls_cert = "/etc/telegraf/cert.pem"
Expand All @@ -35,9 +42,12 @@ const description = "Reads metrics from a SSL certificate"

// X509Cert holds the configuration of the plugin.
type X509Cert struct {
Sources []string `toml:"sources"`
Timeout internal.Duration `toml:"timeout"`
tlsCfg *tls.Config
Sources []string `toml:"sources"`
Timeout internal.Duration `toml:"timeout"`
IncludeIssuer bool `toml:"include_issuer"`
IncludeSAN bool `toml:"include_san"`
SANSeperator string `toml:"san_separator"`
tlsCfg *tls.Config
_tls.ClientConfig
}

Expand Down Expand Up @@ -129,10 +139,15 @@ func getFields(cert *x509.Certificate, now time.Time) map[string]interface{} {
return fields
}

func getTags(subject pkix.Name, location string) map[string]string {
func (c *X509Cert) getTags(cert *x509.Certificate, location string) map[string]string {
subject := cert.Subject
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick, just get cert.Subject.CommonName on line 147 without creating a local variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

subject is used in more than one place, but I've removed the local variable.


tags := map[string]string{
"source": location,
"common_name": subject.CommonName,
"source": location,
"common_name": subject.CommonName,
"serial_number": cert.SerialNumber.Text(16),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How large is the serial_number? Can you add unit tests with examples of all the data used by these changes?

Also, how does this compare to the cert.Subject.SerialNumber?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The certificate serial number is at most 20 bytes and this will be at most 40 Hex characters. This is supposed to be unique per CA.

On the other hand, based on what I know, cert.Subject.SerialNumber is an optional serial number that the public key owner can include. It's probably not as useful to disambiguate certificates than the certificate serial.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added a new test.

"signature_algorithm": cert.SignatureAlgorithm.String(),
"public_key_algorithm": cert.PublicKeyAlgorithm.String(),
}

if len(subject.Organization) > 0 {
Expand All @@ -151,6 +166,23 @@ func getTags(subject pkix.Name, location string) map[string]string {
tags["locality"] = subject.Locality[0]
}

if c.IncludeIssuer {
issuer := cert.Issuer
tags["issuer_common_name"] = issuer.CommonName
tags["issuer_serial_number"] = issuer.SerialNumber
}

if c.IncludeSAN {
san := append(cert.DNSNames, cert.EmailAddresses...)
for _, ip := range cert.IPAddresses {
san = append(san, ip.String())
}
for _, uri := range cert.URIs {
san = append(san, uri.String())
}
tags["san"] = strings.Join(san, c.SANSeperator)
}

return tags
}

Expand All @@ -172,7 +204,7 @@ func (c *X509Cert) Gather(acc telegraf.Accumulator) error {

for i, cert := range certs {
fields := getFields(cert, now)
tags := getTags(cert.Subject, location)
tags := c.getTags(cert, location)

// The first certificate is the leaf/end-entity certificate which needs DNS
// name validation against the URL hostname.
Expand Down Expand Up @@ -225,8 +257,11 @@ func (c *X509Cert) Init() error {
func init() {
inputs.Add("x509_cert", func() telegraf.Input {
return &X509Cert{
Sources: []string{},
Timeout: internal.Duration{Duration: 5},
Sources: []string{},
Timeout: internal.Duration{Duration: 5},
IncludeIssuer: false,
IncludeSAN: false,
SANSeperator: ",",
}
})
}