Skip to content

Commit

Permalink
[Heartbeat] Record TLS Metadata for expired/invalid certs (#14588) (#…
Browse files Browse the repository at this point in the history
…14621)

This patch fixes #13687 .

Previously heartbeat would only traverse valid x509 cert chains, with
this PR it now traverses all certs provided by the server.

(cherry picked from commit eff54c3)
  • Loading branch information
andrewvc authored Nov 22, 2019
1 parent eb0f988 commit a4e631c
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 15 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d

*Heartbeat*

- Fix recording of SSL cert metadata for Expired/Unvalidated x509 certs. {pull}13687[13687]

*Journalbeat*

Expand Down
18 changes: 8 additions & 10 deletions heartbeat/monitors/active/dialchain/tls.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,14 @@ func TLSLayer(cfg *transport.TLSConfig, to time.Duration) Layer {
timer.stop()
event.PutValue("tls.rtt.handshake", look.RTT(timer.duration()))

addCertMetdata(event.Fields, tlsConn.ConnectionState().VerifiedChains)
addCertMetdata(event.Fields, tlsConn.ConnectionState().PeerCertificates)

return conn, nil
}), nil
}
}

func addCertMetdata(fields common.MapStr, chains [][]*x509.Certificate) {
func addCertMetdata(fields common.MapStr, certs []*x509.Certificate) {
// The behavior here might seem strange. We *always* set a notBefore, but only optionally set a notAfter.
// Why might we do this?
// The root cause is that the x509.Certificate type uses time.Time for these fields instead of *time.Time
Expand All @@ -94,15 +94,13 @@ func addCertMetdata(fields common.MapStr, chains [][]*x509.Certificate) {
// To do this correctly, we take the maximum NotBefore and the minimum NotAfter.
// This *should* always wind up being the terminal cert in the chain, but we should
// compute this correctly.
for _, chain := range chains {
for _, cert := range chain {
if chainNotValidBefore.Before(cert.NotBefore) {
chainNotValidBefore = cert.NotBefore
}
for _, cert := range certs {
if chainNotValidBefore.Before(cert.NotBefore) {
chainNotValidBefore = cert.NotBefore
}

if cert.NotAfter != zeroTime && (chainNotValidAfter == nil || chainNotValidAfter.After(cert.NotAfter)) {
chainNotValidAfter = &cert.NotAfter
}
if cert.NotAfter != zeroTime && (chainNotValidAfter == nil || chainNotValidAfter.After(cert.NotAfter)) {
chainNotValidAfter = &cert.NotAfter
}
}

Expand Down
31 changes: 26 additions & 5 deletions heartbeat/monitors/active/dialchain/tls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,19 @@ func Test_addCertMetdata(t *testing.T) {
BasicConstraintsValid: true,
}

expiredNotAfter := time.Now().Add(-time.Hour)
expiredCert := x509.Certificate{
SerialNumber: big.NewInt(1),
Subject: pkix.Name{
Organization: []string{"Acme Co"},
},
NotBefore: goodNotBefore,
NotAfter: expiredNotAfter,
KeyUsage: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature,
ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
BasicConstraintsValid: true,
}

missingNotBeforeCert := x509.Certificate{
SerialNumber: big.NewInt(1),
Subject: pkix.Name{
Expand Down Expand Up @@ -76,27 +89,35 @@ func Test_addCertMetdata(t *testing.T) {
}
tests := []struct {
name string
chains [][]*x509.Certificate
certs []*x509.Certificate
expected expected
}{
{
"Valid cert",
[][]*x509.Certificate{{&goodCert}},
[]*x509.Certificate{&goodCert},
expected{
notBefore: goodNotBefore,
notAfter: &goodNotAfter,
},
},
{
"Expired cert",
[]*x509.Certificate{&expiredCert},
expected{
notBefore: goodNotBefore,
notAfter: &expiredNotAfter,
},
},
{
"Missing not before",
[][]*x509.Certificate{{&missingNotBeforeCert}},
[]*x509.Certificate{&missingNotBeforeCert},
expected{
notAfter: &goodNotAfter,
},
},
{
"Missing not after",
[][]*x509.Certificate{{&missingNotAfterCert}},
[]*x509.Certificate{&missingNotAfterCert},
expected{
notBefore: goodNotBefore,
},
Expand All @@ -105,7 +126,7 @@ func Test_addCertMetdata(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
event := common.MapStr{}
addCertMetdata(event, tt.chains)
addCertMetdata(event, tt.certs)
v, err := event.GetValue("tls.certificate_not_valid_before")
assert.NoError(t, err)
assert.Equal(t, tt.expected.notBefore, v)
Expand Down

0 comments on commit a4e631c

Please sign in to comment.