Add metric for remaining lifetime of certificates authenticating requests #50387

jcbsmpsn · 2017-08-09T18:00:41Z

When incoming requests to the API server are authenticated by a certificate, the expiration of the certificate can affect the validity of the authentication. With auto rotation of certificates, which is starting with kubelet certificates, the goal is to use shorter lifetimes and let the kubelet renew the certificate as desired. Monitoring certificates which are approaching expiration and not renewing would be an early warning sign that nodes are about to stop participating in the cluster.

Release note:

Add new Prometheus metric that monitors the remaining lifetime of certificates used to authenticate requests to the API server.

jcbsmpsn · 2017-08-09T18:59:15Z

/assign @crassirostris

liggitt · 2017-08-09T19:29:40Z

staging/src/k8s.io/apiserver/pkg/authentication/request/x509/x509.go

@@ -71,6 +85,8 @@ func (a *Authenticator) AuthenticateRequest(req *http.Request) (user.Info, bool,
 		}
 	}

+	remaining := req.TLS.PeerCertificates[0].NotAfter.Sub(time.Now())
+	clientCertificateExpirationGauge.WithLabelValues(req.TLS.PeerCertificates[0].Subject.CommonName).Set(float64(remaining / time.Second))


the cardinality on this is way too high. a label per client and essentially infinite possible values. see the warning at the bottom of https://prometheus.io/docs/practices/naming/

Would you consider the number of nodes in the cluster to be too high cardinality? It would be useful to have the information to identify which nodes are about to drop out of the cluster because they are failing to update certificates.

Would you consider the number of nodes in the cluster to be too high cardinality?

Yes. I'm not sure about using metrics as a means of communicating health of specific clients. I'd expect that more on node status or maybe in events.

We want something that can be set to have alerts for monitoring in production. I'll see what can be done with node status or events.

I've changed the metric to a histogram to report counts in certain buckets, no detail about source, so the cardinality is low. I'll follow up with an addition to node status so it is possible to easily query nodes about remaining credential lifetime.

liggitt · 2017-08-10T20:26:31Z

staging/src/k8s.io/apiserver/pkg/authentication/request/x509/x509.go

+	prometheus.HistogramOpts{
+		Name: "apiserver_client_certificate_expiration_gauge",
+		Help: "Gauge of the remaining lifetime on the certificate used to authenticate a request.",
+		Buckets: []float64{


a bucket for negative (expired) would probably be good

crassirostris

Sorry for the delay!

crassirostris · 2017-08-10T20:34:34Z

staging/src/k8s.io/apiserver/pkg/authentication/request/x509/x509.go

+		Name: "apiserver_client_certificate_expiration_gauge",
+		Help: "Gauge of the remaining lifetime on the certificate used to authenticate a request.",
+		Buckets: []float64{
+			float64(6 * time.Hour / time.Second),


(6 * time.Hour).Seconds() I think looks better

crassirostris · 2017-08-10T20:36:35Z

staging/src/k8s.io/apiserver/pkg/authentication/request/x509/x509.go


 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apiserver/pkg/authentication/authenticator"
 	"k8s.io/apiserver/pkg/authentication/user"
 )

+var clientCertificateExpirationGauge = prometheus.NewHistogram(
+	prometheus.HistogramOpts{
+		Name: "apiserver_client_certificate_expiration_gauge",


Namespace: "apiserver", Subsystem: "client", Name: "certificate_expiration_seconds"

crassirostris · 2017-08-10T20:36:51Z

staging/src/k8s.io/apiserver/pkg/authentication/request/x509/x509.go


 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apiserver/pkg/authentication/authenticator"
 	"k8s.io/apiserver/pkg/authentication/user"
 )

+var clientCertificateExpirationGauge = prometheus.NewHistogram(


I assume you mean in the name. Updated.

crassirostris · 2017-08-10T20:37:14Z

staging/src/k8s.io/apiserver/pkg/authentication/request/x509/x509.go

@@ -71,6 +92,8 @@ func (a *Authenticator) AuthenticateRequest(req *http.Request) (user.Info, bool,
 		}
 	}

+	remaining := req.TLS.PeerCertificates[0].NotAfter.Sub(time.Now())
+	clientCertificateExpirationGauge.Observe(float64(remaining / time.Second))


remaining.Seconds()

liggitt · 2017-08-10T23:27:30Z

staging/src/k8s.io/apiserver/pkg/authentication/request/x509/x509.go

+		Name:      "certificate_expiration_seconds",
+		Help:      "Gauge of the remaining lifetime on the certificate used to authenticate a request.",
+		Buckets: []float64{
+			math.Inf(-1),


I thought the buckets held the max, so a bucket for negative numbers would be set to 0?

Yes, silly me. Done.

liggitt · 2017-08-11T01:18:46Z

/lgtm

crassirostris

Last nits

crassirostris · 2017-08-11T06:38:22Z

staging/src/k8s.io/apiserver/pkg/authentication/request/x509/x509.go

+		Namespace: "apiserver",
+		Subsystem: "client",
+		Name:      "certificate_expiration_seconds",
+		Help:      "Gauge of the remaining lifetime on the certificate used to authenticate a request.",


Gauge of the remaining

I'd say "Distribution of the remaining"

crassirostris · 2017-08-11T06:38:59Z

staging/src/k8s.io/apiserver/pkg/authentication/request/x509/x509.go

+		Help:      "Gauge of the remaining lifetime on the certificate used to authenticate a request.",
+		Buckets: []float64{
+			0,
+			float64((6 * time.Hour).Seconds()),


Cast is unnecessary, .Seconds() returns float64

crassirostris · 2017-08-11T06:39:12Z

staging/src/k8s.io/apiserver/pkg/authentication/request/x509/x509.go

@@ -71,6 +95,8 @@ func (a *Authenticator) AuthenticateRequest(req *http.Request) (user.Info, bool,
 		}
 	}

+	remaining := req.TLS.PeerCertificates[0].NotAfter.Sub(time.Now())
+	clientCertificateExpirationHistogram.Observe(float64(remaining.Seconds()))


Same as above

crassirostris · 2017-08-11T06:40:38Z

@jcbsmpsn There should be an associated issue or @liggitt should comment /approve no-issue

When incoming requests to the API server are authenticated by a certificate, the expiration of the certificate can affect the validity of the authentication. With auto rotation of certificates, which is starting with kubelet certificates, the goal is to use shorter lifetimes and let the kubelet renew the certificate as desired. Monitoring certificates which are approaching expiration and not renewing would be an early warning sign that nodes are about to stop participating in the cluster.

crassirostris · 2017-08-12T09:50:42Z

/lgtm

k8s-github-robot · 2017-08-12T09:51:15Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: crassirostris, jcbsmpsn, liggitt

No associated issue. Update pull-request body to add a reference to an issue, or get approval with /approve no-issue

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

~~staging/src/k8s.io/apiserver/OWNERS~~ [liggitt]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

jcbsmpsn · 2017-08-14T19:01:47Z

/test pull-kubernetes-federation-e2e-gce

crassirostris · 2017-08-16T09:07:17Z

/retest

k8s-github-robot · 2017-08-16T16:25:18Z

/test all [submit-queue is verifying that this PR is safe to merge]

k8s-github-robot · 2017-08-16T17:19:22Z

Automatic merge from submit-queue

brancz · 2017-09-19T10:36:26Z

staging/src/k8s.io/apiserver/pkg/authentication/request/x509/x509.go

@@ -71,6 +95,8 @@ func (a *Authenticator) AuthenticateRequest(req *http.Request) (user.Info, bool,
 		}
 	}

+	remaining := req.TLS.PeerCertificates[0].NotAfter.Sub(time.Now())
+	clientCertificateExpirationHistogram.Observe(remaining.Seconds())


Wouldn't it make sense to measure this after verifying? If I understand this correctly a client could now influence these metrics by generating certificates with random expiry dates, not that I can see a particularly useful attack here, just a potential for distortion. It would be simple to just move this measurement after the certificate verification 4 lines below 🙂. Please let me know if I'm missing something.

…0387-#56444-release-1.8 Automated cherry pick of #56444 to release-1.8

…0387-#56444-release-1.7 Automatic merge from submit-queue. Automated cherry pick of #50387 #56444 release-1.7 Automated cherry pick of #50387 #56444 release 1.7

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 9, 2017

k8s-github-robot assigned timothysc and liggitt Aug 9, 2017

k8s-github-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Aug 9, 2017

k8s-ci-robot assigned crassirostris Aug 9, 2017

liggitt reviewed Aug 9, 2017

View reviewed changes

jcbsmpsn force-pushed the metric-certificate-expiration branch from 1e68ba4 to 6f9fe1e Compare August 10, 2017 18:58

liggitt reviewed Aug 10, 2017

View reviewed changes

crassirostris reviewed Aug 10, 2017

View reviewed changes

jcbsmpsn force-pushed the metric-certificate-expiration branch 2 times, most recently from 4003597 to ac4aa6d Compare August 10, 2017 23:13

liggitt reviewed Aug 10, 2017

View reviewed changes

jcbsmpsn force-pushed the metric-certificate-expiration branch from ac4aa6d to 93baf71 Compare August 11, 2017 01:15

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 11, 2017

crassirostris reviewed Aug 11, 2017

View reviewed changes

jcbsmpsn force-pushed the metric-certificate-expiration branch from 93baf71 to 49a19c6 Compare August 11, 2017 18:19

k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 11, 2017

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 12, 2017

k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 16, 2017

k8s-github-robot merged commit 6bc0b29 into kubernetes:master Aug 16, 2017

brancz reviewed Sep 19, 2017

View reviewed changes

mikedanese mentioned this pull request Dec 4, 2017

Automated cherry pick of #50387 #56444 release-1.7 #56808

Merged

jpbetz referenced this pull request Dec 6, 2017

Merge pull request #56807 from mikedanese/automated-cherry-pick-of-#5…

e0551a0

…0387-#56444-release-1.8 Automated cherry pick of #56444 to release-1.8

madstacks mentioned this pull request Jan 23, 2019

Update certificate expiration alert message kubernetes-monitoring/kubernetes-mixin#141

Merged

Add metric for remaining lifetime of certificates authenticating requests #50387

Add metric for remaining lifetime of certificates authenticating requests #50387

Conversation

jcbsmpsn commented Aug 9, 2017 • edited

jcbsmpsn commented Aug 9, 2017

liggitt Aug 9, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crassirostris left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jcbsmpsn Aug 11, 2017 • edited

Choose a reason for hiding this comment

liggitt commented Aug 11, 2017

crassirostris left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crassirostris commented Aug 11, 2017

crassirostris commented Aug 12, 2017

k8s-github-robot commented Aug 12, 2017

jcbsmpsn commented Aug 14, 2017

crassirostris commented Aug 16, 2017

k8s-github-robot commented Aug 16, 2017

k8s-github-robot commented Aug 16, 2017

brancz Sep 19, 2017 • edited

Choose a reason for hiding this comment

jcbsmpsn commented Aug 9, 2017 •

edited

liggitt Aug 9, 2017 •

edited

jcbsmpsn Aug 11, 2017 •

edited

brancz Sep 19, 2017 •

edited