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 <robertbest89@gmail.com>

diff --git a/prober/tcp_test.go b/prober/tcp_test.go
index fd219f94649b9bf1476bfe67901d51a8f7d401ab..6b4fdb3673cdbf088770b2dfb53cd88aa130ca8d 100644 (file)
--- 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 87107974b0f079270549ab63bc05d130966e7dfc..38d5533611f7b7dc3cad35657bca27dcb16838d1 100644 (file)
--- 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
                }