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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 2 commits into from
Nov 22, 2019
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4d074a5
[Heartbeat] Record TLS Metadata for expired/invalid certs (#14588)
andrewvc Nov 19, 2019
901da80
Fix changelog
andrewvc Nov 22, 2019
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
5 changes: 5 additions & 0 deletions CHANGELOG.next.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d

*Heartbeat*

- Fix NPEs / resource leaks when executing config checks. {pull}11165[11165]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Too much?

- Fix duplicated IPs on `mode: all` monitors. {pull}12458[12458]
- Fix integer comparison on JSON responses. {pull}13348[13348]
- Fix storage of HTTP bodies to work when JSON/Regex body checks are enabled. {pull}14223[14223]
- 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
View file
Open in desktop
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
View file
Open in desktop
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