From: Rob Best Date: Thu, 20 Aug 2020 12:40:14 +0000 (+0100) Subject: prober/tls: fix probe_ssl_last_chain_expiry_timestamp_seconds (#681) X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=7913a15d3c1b225f7427e9240c63ab5075281845;p=blackbox_exporter.git prober/tls: fix probe_ssl_last_chain_expiry_timestamp_seconds (#681) * prober/tls: fix probe_ssl_last_chain_expiry_timestamp_seconds This metric should report the earliest expiry of the chain that expires the latest out of all the verified chains. Presently, it reports the earliest expiry of the chain that expires first. The current test for this metric was using an expired root certificate which is omitted from the verified chain, so the test was passing despite this bug. I've changed it to use a root that is still valid but expires before a root held by the client. * prober/tls: improve verified cert test Include the older root certificate in the chain presented by the server as well as in the client root CAs. This ensures that the peer certificate metric identifies the older root CA as the earliest expiry while it is ignored by the verified metric in favour of the longer-lived chain. Signed-off-by: Rob Best --- diff --git a/prober/tcp_test.go b/prober/tcp_test.go index fd219f9..6b4fdb3 100644 --- a/prober/tcp_test.go +++ b/prober/tcp_test.go @@ -217,33 +217,34 @@ func TestTCPConnectionWithTLSAndVerifiedCertificateChain(t *testing.T) { testCTX, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - // From here prepare two certificate chains where one is expired + // From here prepare two certificate chains where one expires before the + // other rootPrivatekey, err := rsa.GenerateKey(rand.Reader, 2048) if err != nil { panic(fmt.Sprintf("Error creating rsa key: %s", err)) } - rootCertExpiry := time.Now().AddDate(0, 0, 2) + rootCertExpiry := time.Now().AddDate(0, 0, 3) rootCertTmpl := generateCertificateTemplate(rootCertExpiry, false) rootCertTmpl.IsCA = true _, rootCertPem := generateSelfSignedCertificateWithPrivateKey(rootCertTmpl, rootPrivatekey) - oldRootCertExpiry := time.Now().AddDate(0, 0, -1) - expiredRootCertTmpl := generateCertificateTemplate(oldRootCertExpiry, false) - expiredRootCertTmpl.IsCA = true - expiredRootCert, expiredRootCertPem := generateSelfSignedCertificateWithPrivateKey(expiredRootCertTmpl, rootPrivatekey) + olderRootCertExpiry := time.Now().AddDate(0, 0, 1) + olderRootCertTmpl := generateCertificateTemplate(olderRootCertExpiry, false) + olderRootCertTmpl.IsCA = true + olderRootCert, olderRootCertPem := generateSelfSignedCertificateWithPrivateKey(olderRootCertTmpl, rootPrivatekey) - serverCertExpiry := time.Now().AddDate(0, 0, 1) + serverCertExpiry := time.Now().AddDate(0, 0, 2) serverCertTmpl := generateCertificateTemplate(serverCertExpiry, false) - _, serverCertPem, serverKey := generateSignedCertificate(serverCertTmpl, expiredRootCert, rootPrivatekey) + _, serverCertPem, serverKey := generateSignedCertificate(serverCertTmpl, olderRootCert, rootPrivatekey) // CAFile must be passed via filesystem, use a tempfile. tmpCaFile, err := ioutil.TempFile("", "cafile.pem") if err != nil { t.Fatalf(fmt.Sprintf("Error creating CA tempfile: %s", err)) } - if _, err := tmpCaFile.Write(bytes.Join([][]byte{rootCertPem, expiredRootCertPem}, []byte("\n"))); err != nil { + if _, err := tmpCaFile.Write(bytes.Join([][]byte{rootCertPem, olderRootCertPem}, []byte("\n"))); err != nil { t.Fatalf(fmt.Sprintf("Error writing CA tempfile: %s", err)) } if err := tmpCaFile.Close(); err != nil { @@ -263,7 +264,8 @@ func TestTCPConnectionWithTLSAndVerifiedCertificateChain(t *testing.T) { serverKeyPem := pem.EncodeToMemory(&pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(serverKey)}) - keypair, err := tls.X509KeyPair(serverCertPem, serverKeyPem) + // Include the older root cert in the chain + keypair, err := tls.X509KeyPair(append(serverCertPem, olderRootCertPem...), serverKeyPem) if err != nil { panic(fmt.Sprintf("Failed to decode TLS testing keypair: %s\n", err)) } @@ -316,7 +318,7 @@ func TestTCPConnectionWithTLSAndVerifiedCertificateChain(t *testing.T) { // Check values expectedResults := map[string]float64{ - "probe_ssl_earliest_cert_expiry": float64(serverCertExpiry.Unix()), + "probe_ssl_earliest_cert_expiry": float64(olderRootCertExpiry.Unix()), "probe_ssl_last_chain_expiry_timestamp_seconds": float64(serverCertExpiry.Unix()), "probe_ssl_last_chain_info": 1, "probe_tls_version_info": 1, diff --git a/prober/tls.go b/prober/tls.go index 8710797..38d5533 100644 --- a/prober/tls.go +++ b/prober/tls.go @@ -45,7 +45,7 @@ func getLastChainExpiry(state *tls.ConnectionState) time.Time { earliestCertExpiry = cert.NotAfter } } - if lastChainExpiry.IsZero() || lastChainExpiry.After(earliestCertExpiry) { + if lastChainExpiry.IsZero() || lastChainExpiry.Before(earliestCertExpiry) { lastChainExpiry = earliestCertExpiry }