Add new probe_ssl_latest_verified_chain_expiry metric #636
Conversation
Resolves prometheus#340 Based on the discussion in the issue above, this metric will help determine when the SSL/TLS certificate expiration error actually happens on clients like a browser that attempts to verify certificates by building one or more chains from peer certificates. Signed-off-by: Takuya Kosugiyama <re@itkq.jp>
itkq
force-pushed
the
probe_ssl_latest_verified_chain_expiry
branch
from
4404f90
to
a70527c
Compare
prober/tls.go
Outdated
| @@ -28,6 +28,23 @@ func getEarliestCertExpiry(state *tls.ConnectionState) time.Time { | |||
| return earliest | |||
| } | |||
|
|
|||
| func getLatestVerifiedChainExpiry(state *tls.ConnectionState) time.Time { | |||
| latestVerifiedChainExpiry := time.Time{} | |||
There was a problem hiding this comment.
What does this output if there's no chains? You'll probably need insecure_skip_verify to test this.
prober/tcp.go
Outdated
| @@ -94,6 +94,10 @@ func ProbeTCP(ctx context.Context, target string, module config.Module, registry | |||
| Name: "probe_ssl_earliest_cert_expiry", | |||
| Help: "Returns earliest SSL cert expiry date", | |||
| }) | |||
| probeSSLLatestVerifiedChainExpiry := prometheus.NewGauge(prometheus.GaugeOpts{ | |||
| Name: "probe_ssl_latest_verified_chain_expiry", | |||
There was a problem hiding this comment.
That's a little of a mouthful, and naming conventions have matured since the previous one was named.
I'd go for something like probe_ssl_last_chain_expiry_timestamp_seconds.
Signed-off-by: Takuya Kosugiyama <re@itkq.jp>
Signed-off-by: Takuya Kosugiyama <re@itkq.jp>
Signed-off-by: Takuya Kosugiyama <re@itkq.jp>
Signed-off-by: Takuya Kosugiyama <re@itkq.jp>
|
Changed the name of the metric a bit and also added a unit test. |
prober/tcp.go
Outdated
| probeTLSVersion.WithLabelValues(getTLSVersion(&state)).Set(1) | ||
|
|
||
| if lastChainExpiry, ok := getLastChainExpiry(&state); ok { | ||
| registry.MustRegister(probeSSLLastChainExpiryTimestampSeconds) |
There was a problem hiding this comment.
Let's keep this consistent with the other tls metric, both in terms of registration and what happens when there's no certs.
prober/tcp.go
Outdated
| Help: "Returns latest SSL verified chain expiry date", | ||
| probeSSLLastChainExpiryTimestampSeconds := prometheus.NewGauge(prometheus.GaugeOpts{ | ||
| Name: "probe_ssl_last_chain_expiry_timestamp_seconds", | ||
| Help: "Returns last SSL chain expiry in timestamp seconds", |
There was a problem hiding this comment.
in unixtime
Looks like the existing metric help could do with some consistency too.
prober/tcp_test.go
Outdated
| // Prepare certificates to simulate a situation where | ||
| // the root certificate in the chain sent by the server | ||
| // expired but a verified certificate chain can be found | ||
| // because a new one has installed on the local machine. |
There was a problem hiding this comment.
"one" is ambiguous here. Root?
This also feels like we're testing Go's root handling rather than testing our own code. Having two chains where one is expired would seem like a better test.
There was a problem hiding this comment.
Right. I made a change to do that in d45159b
Signed-off-by: Takuya Kosugiyama <re@itkq.jp>
Signed-off-by: Takuya Kosugiyama <re@itkq.jp>
Signed-off-by: Takuya Kosugiyama <re@itkq.jp>
itkq
force-pushed
the
probe_ssl_latest_verified_chain_expiry
branch
from
fb1fcc7
to
d45159b
Compare
|
Thanks! |
Resolves #340
Based on the discussion in the issue above, this metric will help determine
when the SSL/TLS certificate expiration error actually happens on clients
like a browser that attempts to verify certificates by building one or
more chains from peer certificates.