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 a certificate info metric #8253

Merged
merged 1 commit into from
Feb 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
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
Add a certificate info metric
When the ingress controller loads certificates  (new ones or following a
secret update), it performs a series of check to ensure its validity.

In our systems, we detected a case where, when the secret object is
compromised, for example when the certificate does not match the secret
key, different pods of the ingress controller are serving a different
version of the certificate.

This behaviour is due to the cache mechanism of the ingress controller,
keeping the last known certificate in case of corruption. When this
happens, old ingress-controller pods will keep serving the old one,
while new pods, by failing to load the corrupted certificates, would
use the default certificate, causing invalid certificates for its
clients.

This generates a random error on the client side, depending on the
actual pod instance it reaches.

In order to allow detecting occurences of those situations, add a metric
to expose, for all ingress controlller pods, detailed informations of
the currently loaded certificate.

This will, for example, allow setting an alert when there is a
certificate discrepency across all ingress controller pods using a query
similar to `sum(nginx_ingress_controller_ssl_certificate_info{host="name.tld"})by(serial_number)`

This also allows to catch other exceptions loading certificates (failing
to load the certificate from the k8s API, ...

Co-authored-by: Daniel Ricart <danielricart@users.noreply.github.com>
  • Loading branch information
2 people authored and tjamet committed Feb 17, 2022
commit 1289bf9e264946b1f2577472f2bcb5b5d03227ab
35 changes: 34 additions & 1 deletion internal/ingress/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ func (n *NGINXController) syncIngress(interface{}) error {
hosts, servers, pcfg := n.getConfiguration(ings)

n.metricCollector.SetSSLExpireTime(servers)
n.metricCollector.SetSSLInfo(servers)

if n.runningConfig.Equal(pcfg) {
klog.V(3).Infof("No configuration change detected, skipping backend reload")
Expand Down Expand Up @@ -211,7 +212,8 @@ func (n *NGINXController) syncIngress(interface{}) error {

ri := getRemovedIngresses(n.runningConfig, pcfg)
re := getRemovedHosts(n.runningConfig, pcfg)
n.metricCollector.RemoveMetrics(ri, re)
rc := getRemovedCertificateSerialNumbers(n.runningConfig, pcfg)
n.metricCollector.RemoveMetrics(ri, re, rc)

n.runningConfig = pcfg

Expand Down Expand Up @@ -1625,6 +1627,37 @@ func getRemovedHosts(rucfg, newcfg *ingress.Configuration) []string {
return old.Difference(new).List()
}

func getRemovedCertificateSerialNumbers(rucfg, newcfg *ingress.Configuration) []string {
oldCertificates := sets.NewString()
newCertificates := sets.NewString()

for _, server := range rucfg.Servers {
if server.SSLCert == nil {
continue
}
identifier := server.SSLCert.Identifier()
if identifier != "" {
if !oldCertificates.Has(identifier) {
oldCertificates.Insert(identifier)
}
}
}

for _, server := range newcfg.Servers {
if server.SSLCert == nil {
continue
}
identifier := server.SSLCert.Identifier()
if identifier != "" {
if !newCertificates.Has(identifier) {
newCertificates.Insert(identifier)
}
}
}

return oldCertificates.Difference(newCertificates).List()
}

func getRemovedIngresses(rucfg, newcfg *ingress.Configuration) []string {
oldIngresses := sets.NewString()
newIngresses := sets.NewString()
Expand Down
1 change: 1 addition & 0 deletions internal/ingress/controller/nginx.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@ func (n *NGINXController) Start() {
// manually update SSL expiration metrics
// (to not wait for a reload)
n.metricCollector.SetSSLExpireTime(n.runningConfig.Servers)
n.metricCollector.SetSSLInfo(n.runningConfig.Servers)
},
OnStoppedLeading: func() {
n.metricCollector.OnStoppedLeading(electionID)
Expand Down
89 changes: 85 additions & 4 deletions internal/ingress/metric/collectors/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ import (
var (
operation = []string{"controller_namespace", "controller_class", "controller_pod"}
ingressOperation = []string{"controller_namespace", "controller_class", "controller_pod", "namespace", "ingress"}
sslLabelHost = []string{"namespace", "class", "host"}
sslLabelHost = []string{"namespace", "class", "host", "secret_name"}
sslInfoLabels = []string{"namespace", "class", "host", "secret_name", "identifier", "issuer_organization", "issuer_common_name", "serial_number", "public_key_algorithm"}
)

// Controller defines base metrics about the ingress controller
Expand All @@ -46,6 +47,7 @@ type Controller struct {
checkIngressOperation *prometheus.CounterVec
checkIngressOperationErrors *prometheus.CounterVec
sslExpireTime *prometheus.GaugeVec
sslInfo *prometheus.GaugeVec

constLabels prometheus.Labels
labels prometheus.Labels
Expand Down Expand Up @@ -152,6 +154,14 @@ func NewController(pod, namespace, class string) *Controller {
},
sslLabelHost,
),
sslInfo: prometheus.NewGaugeVec(
prometheus.GaugeOpts{
Namespace: PrometheusNamespace,
Name: "ssl_certificate_info",
Help: `Hold all labels associated to a certificate`,
},
sslInfoLabels,
),
leaderElection: prometheus.NewGaugeVec(
prometheus.GaugeOpts{
Namespace: PrometheusNamespace,
Expand Down Expand Up @@ -229,6 +239,7 @@ func (cm Controller) Describe(ch chan<- *prometheus.Desc) {
cm.checkIngressOperation.Describe(ch)
cm.checkIngressOperationErrors.Describe(ch)
cm.sslExpireTime.Describe(ch)
cm.sslInfo.Describe(ch)
cm.leaderElection.Describe(ch)
cm.buildInfo.Describe(ch)
}
Expand All @@ -243,6 +254,7 @@ func (cm Controller) Collect(ch chan<- prometheus.Metric) {
cm.checkIngressOperation.Collect(ch)
cm.checkIngressOperationErrors.Collect(ch)
cm.sslExpireTime.Collect(ch)
cm.sslInfo.Collect(ch)
cm.leaderElection.Collect(ch)
cm.buildInfo.Collect(ch)
}
Expand All @@ -256,20 +268,89 @@ func (cm *Controller) SetSSLExpireTime(servers []*ingress.Server) {
labels[k] = v
}
labels["host"] = s.Hostname
labels["secret_name"] = s.SSLCert.Name

cm.sslExpireTime.With(labels).Set(float64(s.SSLCert.ExpireTime.Unix()))
}
}
}

// SetSSLInfo creates a metric with all certificates informations
func (cm *Controller) SetSSLInfo(servers []*ingress.Server) {
for _, s := range servers {
if s.SSLCert != nil && s.SSLCert.Certificate != nil && s.SSLCert.Certificate.SerialNumber != nil {
labels := make(prometheus.Labels, len(cm.labels)+1)
for k, v := range cm.labels {
labels[k] = v
}
labels["identifier"] = s.SSLCert.Identifier()
labels["host"] = s.Hostname
labels["secret_name"] = s.SSLCert.Name
labels["namespace"] = s.SSLCert.Namespace
labels["issuer_common_name"] = s.SSLCert.Certificate.Issuer.CommonName
labels["issuer_organization"] = ""
if len(s.SSLCert.Certificate.Issuer.Organization) > 0 {
labels["issuer_organization"] = s.SSLCert.Certificate.Issuer.Organization[0]
}
labels["serial_number"] = s.SSLCert.Certificate.SerialNumber.String()
labels["public_key_algorithm"] = s.SSLCert.Certificate.PublicKeyAlgorithm.String()

cm.sslInfo.With(labels).Set(1)
}
}
}

// RemoveMetrics removes metrics for hostnames not available anymore
func (cm *Controller) RemoveMetrics(hosts []string, registry prometheus.Gatherer) {
func (cm *Controller) RemoveMetrics(hosts, certificates []string, registry prometheus.Gatherer) {
cm.removeSSLExpireMetrics(true, hosts, registry)
cm.removeCertificatesMetrics(true, certificates, registry)
}

// RemoveAllSSLExpireMetrics removes metrics for expiration of SSL Certificates
func (cm *Controller) RemoveAllSSLExpireMetrics(registry prometheus.Gatherer) {
// RemoveAllSSLMetrics removes metrics for expiration of SSL Certificates
func (cm *Controller) RemoveAllSSLMetrics(registry prometheus.Gatherer) {
cm.removeSSLExpireMetrics(false, []string{}, registry)
cm.removeCertificatesMetrics(false, []string{}, registry)
}

func (cm *Controller) removeCertificatesMetrics(onlyDefinedHosts bool, certificates []string, registry prometheus.Gatherer) {
mfs, err := registry.Gather()
if err != nil {
klog.Errorf("Error gathering metrics: %v", err)
return
}

toRemove := sets.NewString(certificates...)

for _, mf := range mfs {
metricName := mf.GetName()
if fmt.Sprintf("%v_ssl_certificate_info", PrometheusNamespace) != metricName {
continue
}

for _, m := range mf.GetMetric() {
labels := make(map[string]string, len(m.GetLabel()))
for _, labelPair := range m.GetLabel() {
labels[*labelPair.Name] = *labelPair.Value
}

// remove labels that are constant
deleteConstants(labels)

identifier, ok := labels["identifier"]
if !ok {
continue
}
if onlyDefinedHosts && !toRemove.Has(identifier) {
continue
}

klog.V(2).Infof("Removing prometheus metric from gauge %v for identifier %v", metricName, identifier)
removed := cm.sslInfo.Delete(labels)
if !removed {
klog.V(2).Infof("metric %v for identifier %v with labels not removed: %v", metricName, identifier, labels)
}
}
}
}

func (cm *Controller) removeSSLExpireMetrics(onlyDefinedHosts bool, hosts []string, registry prometheus.Gatherer) {
Expand Down
Loading