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

Switch to base 16 for TLS serial number in packetbeat in line with ECS changes #41542

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
+20 −20
Open

Switch to base 16 for TLS serial number in packetbeat in line with ECS changes #41542

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
3b00e70
switch to base 16 for cert serial number
fearful-symmetry Nov 6, 2024
1698a96
uppercase
fearful-symmetry Nov 6, 2024
778ff3b
linter...
fearful-symmetry Nov 6, 2024
95be01e
clean up test
fearful-symmetry Nov 6, 2024
9bf37be
try to fix integration tests
fearful-symmetry Nov 6, 2024
6efedf2
add changelog
fearful-symmetry Nov 6, 2024
a53fca0
update readme
fearful-symmetry Nov 6, 2024
75da1cf
Merge branch 'main' into tls-base16
fearful-symmetry Nov 11, 2024
b45aafd
Merge remote-tracking branch 'upstream/main' into tls-base16
fearful-symmetry Nov 13, 2024
fec9267
Merge remote-tracking branch 'origin/tls-base16' into tls-base16
fearful-symmetry Nov 13, 2024
873800e
Merge branch 'main' into tls-base16
fearful-symmetry Nov 13, 2024
47309c2
Merge branch 'main' into tls-base16
fearful-symmetry Nov 14, 2024
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
2 changes: 1 addition & 1 deletion packetbeat/protos/tls/parse.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
package tls

import (
"crypto/dsa" //lint:ignore SA1019 Deprecated, but still used. So we have to handle it.

Check failure on line 21 in packetbeat/protos/tls/parse.go

View workflow job for this annotation

GitHub Actions / lint (linux) 

SA1019: "crypto/dsa" has been deprecated since Go 1.16 because it shouldn't be used: DSA is a legacy algorithm, and modern alternatives such as Ed25519 (implemented by package crypto/ed25519) should be used instead. Keys with 1024-bit moduli (L1024N160 parameters) are cryptographically weak, while bigger keys are not widely supported. Note that FIPS 186-5 no longer approves DSA for signature generation. (staticcheck)
"crypto/ecdsa"
"crypto/rsa"
"crypto/x509"
Expand Down Expand Up @@ -270,7 +270,7 @@
debugf("handshake completed")
}
// discard remaining data for this stream (encrypted)
buf.Advance(buf.Len())

Check failure on line 273 in packetbeat/protos/tls/parse.go

View workflow job for this annotation

GitHub Actions / lint (linux) 

Error return value of `buf.Advance` is not checked (errcheck)
return resultEncrypted

case recordTypeHandshake:
Expand Down Expand Up @@ -300,7 +300,7 @@
}
}

buf.Advance(limit)

Check failure on line 303 in packetbeat/protos/tls/parse.go

View workflow job for this annotation

GitHub Actions / lint (linux) 

Error return value of `buf.Advance` is not checked (errcheck)
}

if buf.Len() == 0 {
Expand Down Expand Up @@ -350,10 +350,10 @@
}
if !parser.parseHandshake(header.handshakeType,
bufferView{&parser.handshakeBuf, handshakeHeaderSize, limit}) {
parser.handshakeBuf.Advance(limit)

Check failure on line 353 in packetbeat/protos/tls/parse.go

View workflow job for this annotation

GitHub Actions / lint (linux) 

Error return value of `parser.handshakeBuf.Advance` is not checked (errcheck)
return fmt.Errorf("bad handshake %+v", header)
}
parser.handshakeBuf.Advance(limit)

Check failure on line 356 in packetbeat/protos/tls/parse.go

View workflow job for this annotation

GitHub Actions / lint (linux) 

Error return value of `parser.handshakeBuf.Advance` is not checked (errcheck)
}
if parser.handshakeBuf.Len() == 0 {
parser.handshakeBuf.Reset()
Expand Down Expand Up @@ -639,7 +639,7 @@
certMap := mapstr.M{
"signature_algorithm": cert.SignatureAlgorithm.String(),
"public_key_algorithm": toString(cert.PublicKeyAlgorithm),
"serial_number": cert.SerialNumber.Text(10),
"serial_number": strings.ToUpper(cert.SerialNumber.Text(16)),
nicholasberlin marked this conversation as resolved.
Show resolved Hide resolved
"issuer": toMap(&cert.Issuer),
"subject": toMap(&cert.Subject),
"not_before": cert.NotBefore,
Expand Down
2 changes: 1 addition & 1 deletion packetbeat/protos/tls/parse_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ func TestCertificates(t *testing.T) {
"not_before": "2015-11-03 00:00:00 +0000 UTC",
"public_key_algorithm": "RSA",
"public_key_size": "2048",
"serial_number": "19132437207909210467858529073412672688",
"serial_number": "E64C5FBC236ADE14B172AEB41C78CB0",
"signature_algorithm": "SHA256-RSA",
"issuer.common_name": "DigiCert SHA2 High Assurance Server CA",
"issuer.country": "US",
Expand Down
15 changes: 8 additions & 7 deletions packetbeat/protos/tls/tls_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ import (
"testing"
"time"

"github.com/google/go-cmp/cmp"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/elastic/beats/v7/libbeat/beat"
"github.com/elastic/beats/v7/libbeat/common"
Expand Down Expand Up @@ -312,7 +312,7 @@ func TestOCSPStatus(t *testing.T) {
"not_after": time.Date(2035, 3, 4, 9, 0, 0, 0, time.UTC),
"public_key_algorithm": "RSA",
"public_key_size": 4096,
"serial_number": "1492448539999078269498416841973088004758827",
"serial_number": "1121E97D5D37348C572C555A3A59B7B65D2B",
Copy link
Member

Choose a reason for hiding this comment

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

Note that all of the fields under tls.detailed.* are not part of ECS. I cannot find any definition of this field within elastic/beats that needs updated, but it is defined at https://github.com/elastic/integrations/blob/7d7358618583e3d34e83e645d8bb7c83eda58253/packages/network_traffic/data_stream/tls/fields/protocol.yml#L210-L212 so that side will also need an update.

I recommend updating that definition at the same time in which you update the ECS version12 for the network_traffic integration (after we have an ECS release).

Footnotes

  1. https://github.com/elastic/integrations/blob/7d7358618583e3d34e83e645d8bb7c83eda58253/packages/network_traffic/_dev/build/build.yml ↩

  2. https://github.com/search?q=repo%3Aelastic%2Fintegrations+path%3A%2F%5Epackages%5C%2Fnetwork_traffic%5C%2Fdata_stream%5C%2F.*default%5C.yml%2F+%2Fecs%5C.version%2F&type=code ↩

"signature_algorithm": "SHA256-RSA",
"subject": mapstr.M{
"common_name": "Orange Devices PKI TV LAB CA",
Expand All @@ -335,7 +335,7 @@ func TestOCSPStatus(t *testing.T) {
"not_before": time.Date(2020, 3, 2, 17, 0, 0, 0, time.UTC),
"public_key_algorithm": "RSA",
"public_key_size": 4096,
"serial_number": "1492246295378596931754418352553114016724120",
"serial_number": "112151567790FB40C755010CA9169CF4B498",
"signature_algorithm": "SHA256-RSA",
"subject": mapstr.M{
"common_name": "Orange Devices Root LAB CA",
Expand Down Expand Up @@ -402,7 +402,7 @@ func TestOCSPStatus(t *testing.T) {
"not_before": time.Date(2021, 6, 3, 13, 38, 16, 0, time.UTC),
"public_key_algorithm": "ECDSA",
"public_key_size": 256,
"serial_number": "189790697042017246339292011338547986350262673379",
"serial_number": "213E825A875EB349390D11117C6C14F894135FE3",
"signature_algorithm": "SHA256-RSA",
"subject": mapstr.M{
"common_name": "server2 test PKI TV LAB",
Expand All @@ -421,9 +421,10 @@ func TestOCSPStatus(t *testing.T) {
}

got := results.events[0].Fields
if !cmp.Equal(got, want) {
t.Errorf("unexpected result: %s", cmp.Diff(got, want))
}
require.Equal(t, want, got)
// if !cmp.Equal(got, want) {
// t.Errorf("unexpected result: %s", cmp.Diff(got, want))
// }
fearful-symmetry marked this conversation as resolved.
Show resolved Hide resolved
}

func TestFragmentedHandshake(t *testing.T) {
Expand Down
Loading