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

[Heartbeat] Record TLS Metadata for expired/invalid certs (#14588) #14621

Merged
merged 2 commits into from
Nov 22, 2019
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
1 change: 1 addition & 0 deletions CHANGELOG.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,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