Skip to content

Commit

Permalink
Panic when using inline SSL certificate or key (elastic#23858)
Browse files Browse the repository at this point in the history
* Panic when using inline SSL certificate or key

When the key or certificate was smaller than 256bytes the system was
throwing a panic, the problem was generate by a debug message. Instead
of logging part of the keys or certificate in the log we are just
writing "inline".

Fixes: elastic#23820

* changelog
  • Loading branch information
ph authored Feb 9, 2021
1 parent d55f76b commit 359cd74
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
- API address is a required setting in `add_cloudfoundry_metadata`. {pull}21759[21759]
- Update to ECS 1.7.0. {pull}22571[22571]
- Add support for SCRAM-SHA-512 and SCRAM-SHA-256 in Kafka output. {pull}12867[12867]
- Fix panic with inline SSL when the certificate or key were small than 256 bytes. {pull}23820[23820]

*Auditbeat*

Expand Down
4 changes: 1 addition & 3 deletions libbeat/common/transport/tlscommon/tls.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,9 +214,7 @@ type PEMReader struct {
// NewPEMReader returns a new PEMReader.
func NewPEMReader(certificate string) (*PEMReader, error) {
if IsPEMString(certificate) {
// Take a substring of the certificate so we do not leak the whole certificate or private key in the log.
debugStr := certificate[0:256] + "..."
return &PEMReader{reader: ioutil.NopCloser(strings.NewReader(certificate)), debugStr: debugStr}, nil
return &PEMReader{reader: ioutil.NopCloser(strings.NewReader(certificate)), debugStr: "inline"}, nil
}

r, err := os.Open(certificate)
Expand Down
43 changes: 43 additions & 0 deletions libbeat/common/transport/tlscommon/tls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,7 @@ NHy+PwkYsHhbrPl4dgStTNXLenJLIJ+Ke0Pcld4ZPfYdSyu/Tv4rNswZBNpNsW9K
nygO9KTJuUiBrLr0AHEnqko=
-----END PRIVATE KEY-----
`

t.Run("embed", func(t *testing.T) {
// Create a dummy configuration and append the CA after.
cfg, err := load(`
Expand All @@ -568,6 +569,48 @@ supported_protocols: null
assert.NotNil(t, tlsC)
})

t.Run("embed small key", func(t *testing.T) {
// Create a dummy configuration and append the CA after.
cfg, err := load(`
enabled: true
verification_mode: null
certificate: null
key: null
key_passphrase: null
certificate_authorities:
cipher_suites: null
curve_types: null
supported_protocols: null
`)
certificate := `
-----BEGIN CERTIFICATE-----
MIIBmzCCAUCgAwIBAgIRAOQpDyaFimzmueynALHkFEcwCgYIKoZIzj0EAwIwJjEk
MCIGA1UEChMbVEVTVCAtIEVsYXN0aWMgSW50ZWdyYXRpb25zMB4XDTIxMDIwMjE1
NTkxMFoXDTQxMDEyODE1NTkxMFowJjEkMCIGA1UEChMbVEVTVCAtIEVsYXN0aWMg
SW50ZWdyYXRpb25zMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEBc7UEvBd+5SG
Z6QQfgBaPh/VAlf7ovpa/wfSmbHfBhee+dTvdAO1p90lannCkZmc7OfWAlQ1eTgJ
QW668CJwE6NPME0wDgYDVR0PAQH/BAQDAgeAMBMGA1UdJQQMMAoGCCsGAQUFBwMB
MAwGA1UdEwEB/wQCMAAwGAYDVR0RBBEwD4INZWxhc3RpYy1hZ2VudDAKBggqhkjO
PQQDAgNJADBGAiEAhpGWL4lxsdb3+hHv0y4ppw6B7IJJLCeCwHLyHt2Dkx4CIQD6
OEU+yuHzbWa18JVkHafxwnpwQmxwZA3VNitM/AyGTQ==
-----END CERTIFICATE-----
`
key := `
-----BEGIN PRIVATE KEY-----
MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgFDQJ1CPLXrUbUFqj
ED8dqsGuVQdcPK7CHpsCeTtAgQqhRANCAAQFztQS8F37lIZnpBB+AFo+H9UCV/ui
+lr/B9KZsd8GF5751O90A7Wn3SVqecKRmZzs59YCVDV5OAlBbrrwInAT
-----END PRIVATE KEY-----
`
cfg.Certificate.Certificate = certificate
cfg.Certificate.Key = key

tlsC, err := LoadTLSConfig(cfg)
assert.NoError(t, err)

assert.NotNil(t, tlsC)
})

t.Run("From disk", func(t *testing.T) {
k, err := ioutil.TempFile("", "certificate.key")
k.WriteString(key)
Expand Down

0 comments on commit 359cd74

Please sign in to comment.