From 3914b61c80720526c8bd1b95bb2937a73d41e473 Mon Sep 17 00:00:00 2001 From: Antoine Toulme Date: Sat, 18 Mar 2023 01:20:45 -0700 Subject: [PATCH 1/9] replace sha1 with sha2-256 --- .chloggen/introduce_sha2Hasher.yaml | 16 ++++++++++++++++ .../coreinternal/attraction/attraction.go | 10 +++++----- .../attraction/attraction_test.go | 19 +++++++++---------- internal/coreinternal/attraction/hasher.go | 9 ++++----- .../attributes_log_test.go | 8 ++++---- .../attributes_metric_test.go | 8 ++++---- .../attributes_trace_test.go | 8 ++++---- 7 files changed, 46 insertions(+), 32 deletions(-) create mode 100644 .chloggen/introduce_sha2Hasher.yaml diff --git a/.chloggen/introduce_sha2Hasher.yaml b/.chloggen/introduce_sha2Hasher.yaml new file mode 100644 index 000000000000..ffb1b2b00b4d --- /dev/null +++ b/.chloggen/introduce_sha2Hasher.yaml @@ -0,0 +1,16 @@ +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: enhancement + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: attributesprocessor + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Stop using SHA-1 and use SHA2-256 instead for hashing + +# One or more tracking issues related to the change +issues: [4759, 5576] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: diff --git a/internal/coreinternal/attraction/attraction.go b/internal/coreinternal/attraction/attraction.go index 74de9dd5a9c1..1ab89ed33664 100644 --- a/internal/coreinternal/attraction/attraction.go +++ b/internal/coreinternal/attraction/attraction.go @@ -84,8 +84,8 @@ type ActionKeyValue struct { // Either Value, FromAttribute or FromContext must be set. // DELETE - Deletes the attribute. If the key doesn't exist, // no action is performed. - // HASH - Calculates the SHA-1 hash of an existing value and overwrites the - // value with it's SHA-1 hash result. + // HASH - Calculates the SHA-256 hash of an existing value and overwrites the + // value with it's SHA-256 hash result. // EXTRACT - Extracts values using a regular expression rule from the input // 'key' to target keys specified in the 'rule'. If a target key // already exists, it will be overridden. @@ -132,8 +132,8 @@ const ( // Supports pattern which is matched against attribute key. DELETE Action = "delete" - // HASH calculates the SHA-1 hash of an existing value and overwrites the - // value with it's SHA-1 hash result. + // HASH calculates the SHA-256 hash of an existing value and overwrites the + // value with it's SHA-256 hash result. // Supports pattern which is matched against attribute key. HASH Action = "hash" @@ -405,7 +405,7 @@ func getSourceAttributeValue(ctx context.Context, action attributeAction, attrs func hashAttribute(key string, attrs pcommon.Map) { if value, exists := attrs.Get(key); exists { - sha1Hasher(value) + sha2Hasher(value) } } diff --git a/internal/coreinternal/attraction/attraction_test.go b/internal/coreinternal/attraction/attraction_test.go index 83c32c308be9..26c9fe0c64b9 100644 --- a/internal/coreinternal/attraction/attraction_test.go +++ b/internal/coreinternal/attraction/attraction_test.go @@ -16,7 +16,7 @@ package attraction import ( "context" - "crypto/sha1" // #nosec + "crypto/sha256" "encoding/binary" "errors" "fmt" @@ -630,7 +630,7 @@ func TestAttributes_HashValue(t *testing.T) { "updateme": "foo", }, expectedAttributes: map[string]interface{}{ - "updateme": sha1Hash([]byte("foo")), + "updateme": sha2Hash([]byte("foo")), }, }, // Ensure int data types are hashed correctly @@ -640,7 +640,7 @@ func TestAttributes_HashValue(t *testing.T) { "updateme": intVal, }, expectedAttributes: map[string]interface{}{ - "updateme": sha1Hash(intBytes), + "updateme": sha2Hash(intBytes), }, }, // Ensure double data types are hashed correctly @@ -650,7 +650,7 @@ func TestAttributes_HashValue(t *testing.T) { "updateme": doubleVal, }, expectedAttributes: map[string]interface{}{ - "updateme": sha1Hash(doubleBytes), + "updateme": sha2Hash(doubleBytes), }, }, // Ensure bool data types are hashed correctly @@ -660,7 +660,7 @@ func TestAttributes_HashValue(t *testing.T) { "updateme": true, }, expectedAttributes: map[string]interface{}{ - "updateme": sha1Hash([]byte{1}), + "updateme": sha2Hash([]byte{1}), }, }, // Ensure bool data types are hashed correctly @@ -670,7 +670,7 @@ func TestAttributes_HashValue(t *testing.T) { "updateme": false, }, expectedAttributes: map[string]interface{}{ - "updateme": sha1Hash([]byte{0}), + "updateme": sha2Hash([]byte{0}), }, }, // Ensure regex pattern is being used @@ -681,7 +681,7 @@ func TestAttributes_HashValue(t *testing.T) { "donotupdatemebyregexp": false, }, expectedAttributes: map[string]interface{}{ - "updatemebyregexp": sha1Hash([]byte{0}), + "updatemebyregexp": sha2Hash([]byte{0}), "donotupdatemebyregexp": false, }, }, @@ -939,9 +939,8 @@ func TestValidConfiguration(t *testing.T) { } -func sha1Hash(b []byte) string { - // #nosec - h := sha1.New() +func sha2Hash(b []byte) string { + h := sha256.New() h.Write(b) return fmt.Sprintf("%x", h.Sum(nil)) } diff --git a/internal/coreinternal/attraction/hasher.go b/internal/coreinternal/attraction/hasher.go index d685b5020d59..715809a7e86f 100644 --- a/internal/coreinternal/attraction/hasher.go +++ b/internal/coreinternal/attraction/hasher.go @@ -15,7 +15,7 @@ package attraction // import "github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal/attraction" import ( - "crypto/sha1" // #nosec + "crypto/sha256" "encoding/binary" "encoding/hex" "math" @@ -33,11 +33,11 @@ var ( byteFalse = [1]byte{0} ) -// sha1Hasher hashes an AttributeValue using SHA1 and returns a +// sha2Hasher hashes an AttributeValue using SHA2-256 and returns a // hashed version of the attribute. In practice, this would mostly be used // for string attributes but we support all types for completeness/correctness // and eliminate any surprises. -func sha1Hasher(attr pcommon.Value) { +func sha2Hasher(attr pcommon.Value) { var val []byte switch attr.Type() { case pcommon.ValueTypeStr: @@ -58,8 +58,7 @@ func sha1Hasher(attr pcommon.Value) { var hashed string if len(val) > 0 { - // #nosec - h := sha1.New() + h := sha256.New() _, _ = h.Write(val) val = h.Sum(nil) hashedBytes := make([]byte, hex.EncodedLen(len(val))) diff --git a/processor/attributesprocessor/attributes_log_test.go b/processor/attributesprocessor/attributes_log_test.go index 290a1cdd2b0c..b500faaaa27d 100644 --- a/processor/attributesprocessor/attributes_log_test.go +++ b/processor/attributesprocessor/attributes_log_test.go @@ -299,7 +299,7 @@ func TestLogAttributes_Hash(t *testing.T) { "user.email": "john.doe@example.com", }, expectedAttributes: map[string]interface{}{ - "user.email": "73ec53c4ba1747d485ae2a0d7bfafa6cda80a5a9", + "user.email": "836f82db99121b3481011f16b49dfa5fbc714a0d1b1b9f784a1ebbbf5b39577f", }, }, { @@ -308,7 +308,7 @@ func TestLogAttributes_Hash(t *testing.T) { "user.id": 10, }, expectedAttributes: map[string]interface{}{ - "user.id": "71aa908aff1548c8c6cdecf63545261584738a25", + "user.id": "a111f275cc2e7588000001d300a31e76336d15b9d314cd1a1d8f3d3556975eed", }, }, { @@ -317,7 +317,7 @@ func TestLogAttributes_Hash(t *testing.T) { "user.balance": 99.1, }, expectedAttributes: map[string]interface{}{ - "user.balance": "76429edab4855b03073f9429fd5d10313c28655e", + "user.balance": "05fabd78b01be9692863cb0985f600c99da82979af18db5c55173c2a30adb924", }, }, { @@ -326,7 +326,7 @@ func TestLogAttributes_Hash(t *testing.T) { "user.authenticated": true, }, expectedAttributes: map[string]interface{}{ - "user.authenticated": "bf8b4530d8d246dd74ac53a13471bba17941dff7", + "user.authenticated": "4bf5122f344554c53bde2ebb8cd2b7e3d1600ad631c385a5d7cce23c7785459a", }, }, } diff --git a/processor/attributesprocessor/attributes_metric_test.go b/processor/attributesprocessor/attributes_metric_test.go index 2d2166361bce..07685731526b 100644 --- a/processor/attributesprocessor/attributes_metric_test.go +++ b/processor/attributesprocessor/attributes_metric_test.go @@ -308,7 +308,7 @@ func TestMetricAttributes_Hash(t *testing.T) { "user.email": "john.doe@example.com", }, expectedAttributes: map[string]interface{}{ - "user.email": "73ec53c4ba1747d485ae2a0d7bfafa6cda80a5a9", + "user.email": "836f82db99121b3481011f16b49dfa5fbc714a0d1b1b9f784a1ebbbf5b39577f", }, }, { @@ -317,7 +317,7 @@ func TestMetricAttributes_Hash(t *testing.T) { "user.id": 10, }, expectedAttributes: map[string]interface{}{ - "user.id": "71aa908aff1548c8c6cdecf63545261584738a25", + "user.id": "a111f275cc2e7588000001d300a31e76336d15b9d314cd1a1d8f3d3556975eed", }, }, { @@ -326,7 +326,7 @@ func TestMetricAttributes_Hash(t *testing.T) { "user.balance": 99.1, }, expectedAttributes: map[string]interface{}{ - "user.balance": "76429edab4855b03073f9429fd5d10313c28655e", + "user.balance": "05fabd78b01be9692863cb0985f600c99da82979af18db5c55173c2a30adb924", }, }, { @@ -335,7 +335,7 @@ func TestMetricAttributes_Hash(t *testing.T) { "user.authenticated": true, }, expectedAttributes: map[string]interface{}{ - "user.authenticated": "bf8b4530d8d246dd74ac53a13471bba17941dff7", + "user.authenticated": "4bf5122f344554c53bde2ebb8cd2b7e3d1600ad631c385a5d7cce23c7785459a", }, }, } diff --git a/processor/attributesprocessor/attributes_trace_test.go b/processor/attributesprocessor/attributes_trace_test.go index e075225ce3a5..c9ae45340852 100644 --- a/processor/attributesprocessor/attributes_trace_test.go +++ b/processor/attributesprocessor/attributes_trace_test.go @@ -321,7 +321,7 @@ func TestAttributes_Hash(t *testing.T) { "user.email": "john.doe@example.com", }, expectedAttributes: map[string]interface{}{ - "user.email": "73ec53c4ba1747d485ae2a0d7bfafa6cda80a5a9", + "user.email": "836f82db99121b3481011f16b49dfa5fbc714a0d1b1b9f784a1ebbbf5b39577f", }, }, { @@ -330,7 +330,7 @@ func TestAttributes_Hash(t *testing.T) { "user.id": 10, }, expectedAttributes: map[string]interface{}{ - "user.id": "71aa908aff1548c8c6cdecf63545261584738a25", + "user.id": "a111f275cc2e7588000001d300a31e76336d15b9d314cd1a1d8f3d3556975eed", }, }, { @@ -339,7 +339,7 @@ func TestAttributes_Hash(t *testing.T) { "user.balance": 99.1, }, expectedAttributes: map[string]interface{}{ - "user.balance": "76429edab4855b03073f9429fd5d10313c28655e", + "user.balance": "05fabd78b01be9692863cb0985f600c99da82979af18db5c55173c2a30adb924", }, }, { @@ -348,7 +348,7 @@ func TestAttributes_Hash(t *testing.T) { "user.authenticated": true, }, expectedAttributes: map[string]interface{}{ - "user.authenticated": "bf8b4530d8d246dd74ac53a13471bba17941dff7", + "user.authenticated": "4bf5122f344554c53bde2ebb8cd2b7e3d1600ad631c385a5d7cce23c7785459a", }, }, } From e2cb3047c4b38208b276973d91a15a0e545ce0a0 Mon Sep 17 00:00:00 2001 From: Antoine Toulme Date: Sat, 18 Mar 2023 23:41:30 -0700 Subject: [PATCH 2/9] add resourceprocessor to components involved --- .chloggen/introduce_sha2Hasher.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.chloggen/introduce_sha2Hasher.yaml b/.chloggen/introduce_sha2Hasher.yaml index ffb1b2b00b4d..d26e6f819fa7 100644 --- a/.chloggen/introduce_sha2Hasher.yaml +++ b/.chloggen/introduce_sha2Hasher.yaml @@ -2,7 +2,7 @@ change_type: enhancement # The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) -component: attributesprocessor +component: attributesprocessor, resourceprocessor # A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). note: Stop using SHA-1 and use SHA2-256 instead for hashing From cc2dbb11bdaf35062bb433ae4f53760f6764a4cc Mon Sep 17 00:00:00 2001 From: Antoine Toulme Date: Wed, 22 Mar 2023 00:32:08 -0400 Subject: [PATCH 3/9] use feature gate --- .../coreinternal/attraction/attraction.go | 13 ++++++- .../attraction/attraction_test.go | 27 ++++++++++--- internal/coreinternal/attraction/hasher.go | 39 +++++++++++++++++++ internal/coreinternal/go.mod | 1 + internal/coreinternal/go.sum | 2 + 5 files changed, 75 insertions(+), 7 deletions(-) diff --git a/internal/coreinternal/attraction/attraction.go b/internal/coreinternal/attraction/attraction.go index 1ab89ed33664..9a1c012efd16 100644 --- a/internal/coreinternal/attraction/attraction.go +++ b/internal/coreinternal/attraction/attraction.go @@ -21,10 +21,17 @@ import ( "strings" "go.opentelemetry.io/collector/client" + "go.opentelemetry.io/collector/featuregate" "go.opentelemetry.io/collector/pdata/pcommon" "go.uber.org/zap" ) +var enableSha256Gate = featuregate.GlobalRegistry().MustRegister( + "coreinternal.attraction.hash.sha256", + featuregate.StageAlpha, + featuregate.WithRegisterDescription("When enabled, switches hashing algorithm from SHA-1 to SHA-2 256"), +) + // Settings specifies the processor settings. type Settings struct { // Actions specifies the list of attributes to act on. @@ -405,7 +412,11 @@ func getSourceAttributeValue(ctx context.Context, action attributeAction, attrs func hashAttribute(key string, attrs pcommon.Map) { if value, exists := attrs.Get(key); exists { - sha2Hasher(value) + if enableSha256Gate.IsEnabled() { + sha2Hasher(value) + } else { + sha1Hasher(value) + } } } diff --git a/internal/coreinternal/attraction/attraction_test.go b/internal/coreinternal/attraction/attraction_test.go index 26c9fe0c64b9..8a8998332a97 100644 --- a/internal/coreinternal/attraction/attraction_test.go +++ b/internal/coreinternal/attraction/attraction_test.go @@ -16,6 +16,7 @@ package attraction import ( "context" + "crypto/sha1" "crypto/sha256" "encoding/binary" "errors" @@ -630,7 +631,7 @@ func TestAttributes_HashValue(t *testing.T) { "updateme": "foo", }, expectedAttributes: map[string]interface{}{ - "updateme": sha2Hash([]byte("foo")), + "updateme": hash([]byte("foo")), }, }, // Ensure int data types are hashed correctly @@ -640,7 +641,7 @@ func TestAttributes_HashValue(t *testing.T) { "updateme": intVal, }, expectedAttributes: map[string]interface{}{ - "updateme": sha2Hash(intBytes), + "updateme": hash(intBytes), }, }, // Ensure double data types are hashed correctly @@ -650,7 +651,7 @@ func TestAttributes_HashValue(t *testing.T) { "updateme": doubleVal, }, expectedAttributes: map[string]interface{}{ - "updateme": sha2Hash(doubleBytes), + "updateme": hash(doubleBytes), }, }, // Ensure bool data types are hashed correctly @@ -660,7 +661,7 @@ func TestAttributes_HashValue(t *testing.T) { "updateme": true, }, expectedAttributes: map[string]interface{}{ - "updateme": sha2Hash([]byte{1}), + "updateme": hash([]byte{1}), }, }, // Ensure bool data types are hashed correctly @@ -670,7 +671,7 @@ func TestAttributes_HashValue(t *testing.T) { "updateme": false, }, expectedAttributes: map[string]interface{}{ - "updateme": sha2Hash([]byte{0}), + "updateme": hash([]byte{0}), }, }, // Ensure regex pattern is being used @@ -681,7 +682,7 @@ func TestAttributes_HashValue(t *testing.T) { "donotupdatemebyregexp": false, }, expectedAttributes: map[string]interface{}{ - "updatemebyregexp": sha2Hash([]byte{0}), + "updatemebyregexp": hash([]byte{0}), "donotupdatemebyregexp": false, }, }, @@ -939,6 +940,20 @@ func TestValidConfiguration(t *testing.T) { } +func hash(b []byte) string { + if enableSha256Gate.IsEnabled() { + return sha2Hash(b) + } else { + return sha1Hash(b) + } +} + +func sha1Hash(b []byte) string { + h := sha1.New() + h.Write(b) + return fmt.Sprintf("%x", h.Sum(nil)) +} + func sha2Hash(b []byte) string { h := sha256.New() h.Write(b) diff --git a/internal/coreinternal/attraction/hasher.go b/internal/coreinternal/attraction/hasher.go index 715809a7e86f..1a7fd739658f 100644 --- a/internal/coreinternal/attraction/hasher.go +++ b/internal/coreinternal/attraction/hasher.go @@ -15,6 +15,7 @@ package attraction // import "github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal/attraction" import ( + "crypto/sha1" "crypto/sha256" "encoding/binary" "encoding/hex" @@ -33,6 +34,44 @@ var ( byteFalse = [1]byte{0} ) +// Deprecated: use sha2Hasher instead. +// sha1Hasher hashes an AttributeValue using SHA1 and returns a +// hashed version of the attribute. In practice, this would mostly be used +// for string attributes but we support all types for completeness/correctness +// and eliminate any surprises. +func sha1Hasher(attr pcommon.Value) { + var val []byte + switch attr.Type() { + case pcommon.ValueTypeStr: + val = []byte(attr.Str()) + case pcommon.ValueTypeBool: + if attr.Bool() { + val = byteTrue[:] + } else { + val = byteFalse[:] + } + case pcommon.ValueTypeInt: + val = make([]byte, int64ByteSize) + binary.LittleEndian.PutUint64(val, uint64(attr.Int())) + case pcommon.ValueTypeDouble: + val = make([]byte, float64ByteSize) + binary.LittleEndian.PutUint64(val, math.Float64bits(attr.Double())) + } + + var hashed string + if len(val) > 0 { + // #nosec + h := sha1.New() + _, _ = h.Write(val) + val = h.Sum(nil) + hashedBytes := make([]byte, hex.EncodedLen(len(val))) + hex.Encode(hashedBytes, val) + hashed = string(hashedBytes) + } + + attr.SetStr(hashed) +} + // sha2Hasher hashes an AttributeValue using SHA2-256 and returns a // hashed version of the attribute. In practice, this would mostly be used // for string attributes but we support all types for completeness/correctness diff --git a/internal/coreinternal/go.mod b/internal/coreinternal/go.mod index 8fc985101bf4..712de96cbdb2 100644 --- a/internal/coreinternal/go.mod +++ b/internal/coreinternal/go.mod @@ -5,6 +5,7 @@ go 1.19 require ( github.com/stretchr/testify v1.8.2 go.opentelemetry.io/collector v0.74.0 + go.opentelemetry.io/collector/featuregate v0.74.0 go.opentelemetry.io/collector/pdata v1.0.0-rc8 go.opentelemetry.io/collector/semconv v0.74.0 go.uber.org/zap v1.24.0 diff --git a/internal/coreinternal/go.sum b/internal/coreinternal/go.sum index 31be60aac51b..2c2a889c413b 100644 --- a/internal/coreinternal/go.sum +++ b/internal/coreinternal/go.sum @@ -46,6 +46,8 @@ github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9dec go.opentelemetry.io/collector v0.74.0 h1:0s2DKWczGj/pLTsXGb1P+Je7dyuGx9Is4/Dri1+cS7g= go.opentelemetry.io/collector v0.74.0/go.mod h1:7NjZAvkhQ6E+NLN4EAH2hw3Nssi+F14t7mV7lMNXCto= go.opentelemetry.io/collector/consumer v0.74.0 h1:+kjT/ixG+4SVSHg7u9mQe0+LNDc6PuG8Wn2hoL/yGYk= +go.opentelemetry.io/collector/featuregate v0.74.0 h1:hzkzhi6pvjqEK5+CkVBJX69wpEEYqgtTFMHGlZFsQyE= +go.opentelemetry.io/collector/featuregate v0.74.0/go.mod h1:pmVMr98Ps6QKyEHiVPN7o3Qd8K//M2NapfOv5BMWvA0= go.opentelemetry.io/collector/pdata v1.0.0-rc8 h1:vBikWdZFsRiT5dVsLQhnE99w3edM7eem3Q9dSqMlStE= go.opentelemetry.io/collector/pdata v1.0.0-rc8/go.mod h1:BVCBhWgclYCh7Oi6BkMiQfRa6MXv1uRTlKXuL5oBby8= go.opentelemetry.io/collector/semconv v0.74.0 h1:tPpbz87CPu/pM2/fSEKBJWXTvWvUJvEChbQkzdhWQHE= From 7dffd1f1613c9a7cc551c78fab2ea37cfe383bbb Mon Sep 17 00:00:00 2001 From: Antoine Toulme Date: Wed, 22 Mar 2023 11:06:41 -0400 Subject: [PATCH 4/9] add deprecation version --- internal/coreinternal/attraction/hasher.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/coreinternal/attraction/hasher.go b/internal/coreinternal/attraction/hasher.go index 1a7fd739658f..5d0000b9b52a 100644 --- a/internal/coreinternal/attraction/hasher.go +++ b/internal/coreinternal/attraction/hasher.go @@ -34,7 +34,7 @@ var ( byteFalse = [1]byte{0} ) -// Deprecated: use sha2Hasher instead. +// Deprecated: [v0.75.0] use sha2Hasher instead. // sha1Hasher hashes an AttributeValue using SHA1 and returns a // hashed version of the attribute. In practice, this would mostly be used // for string attributes but we support all types for completeness/correctness From f753eaddb8cef053b93dc34aaf175ca6355b3f00 Mon Sep 17 00:00:00 2001 From: Antoine Toulme Date: Wed, 22 Mar 2023 18:30:27 -0400 Subject: [PATCH 5/9] fix tests and add subtext to changelog --- .chloggen/introduce_sha2Hasher.yaml | 2 +- processor/attributesprocessor/attributes_log_test.go | 8 ++++---- processor/attributesprocessor/attributes_metric_test.go | 8 ++++---- processor/attributesprocessor/attributes_trace_test.go | 8 ++++---- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/.chloggen/introduce_sha2Hasher.yaml b/.chloggen/introduce_sha2Hasher.yaml index d26e6f819fa7..6ac44d26d14d 100644 --- a/.chloggen/introduce_sha2Hasher.yaml +++ b/.chloggen/introduce_sha2Hasher.yaml @@ -13,4 +13,4 @@ issues: [4759, 5576] # (Optional) One or more lines of additional information to render under the primary note. # These lines will be padded with 2 spaces and then inserted directly into the document. # Use pipe (|) for multiline entries. -subtext: +subtext: enable switching to use SHA-2 256 with the feature gate `coreinternal.attraction.hash.sha256` diff --git a/processor/attributesprocessor/attributes_log_test.go b/processor/attributesprocessor/attributes_log_test.go index b500faaaa27d..290a1cdd2b0c 100644 --- a/processor/attributesprocessor/attributes_log_test.go +++ b/processor/attributesprocessor/attributes_log_test.go @@ -299,7 +299,7 @@ func TestLogAttributes_Hash(t *testing.T) { "user.email": "john.doe@example.com", }, expectedAttributes: map[string]interface{}{ - "user.email": "836f82db99121b3481011f16b49dfa5fbc714a0d1b1b9f784a1ebbbf5b39577f", + "user.email": "73ec53c4ba1747d485ae2a0d7bfafa6cda80a5a9", }, }, { @@ -308,7 +308,7 @@ func TestLogAttributes_Hash(t *testing.T) { "user.id": 10, }, expectedAttributes: map[string]interface{}{ - "user.id": "a111f275cc2e7588000001d300a31e76336d15b9d314cd1a1d8f3d3556975eed", + "user.id": "71aa908aff1548c8c6cdecf63545261584738a25", }, }, { @@ -317,7 +317,7 @@ func TestLogAttributes_Hash(t *testing.T) { "user.balance": 99.1, }, expectedAttributes: map[string]interface{}{ - "user.balance": "05fabd78b01be9692863cb0985f600c99da82979af18db5c55173c2a30adb924", + "user.balance": "76429edab4855b03073f9429fd5d10313c28655e", }, }, { @@ -326,7 +326,7 @@ func TestLogAttributes_Hash(t *testing.T) { "user.authenticated": true, }, expectedAttributes: map[string]interface{}{ - "user.authenticated": "4bf5122f344554c53bde2ebb8cd2b7e3d1600ad631c385a5d7cce23c7785459a", + "user.authenticated": "bf8b4530d8d246dd74ac53a13471bba17941dff7", }, }, } diff --git a/processor/attributesprocessor/attributes_metric_test.go b/processor/attributesprocessor/attributes_metric_test.go index 07685731526b..2d2166361bce 100644 --- a/processor/attributesprocessor/attributes_metric_test.go +++ b/processor/attributesprocessor/attributes_metric_test.go @@ -308,7 +308,7 @@ func TestMetricAttributes_Hash(t *testing.T) { "user.email": "john.doe@example.com", }, expectedAttributes: map[string]interface{}{ - "user.email": "836f82db99121b3481011f16b49dfa5fbc714a0d1b1b9f784a1ebbbf5b39577f", + "user.email": "73ec53c4ba1747d485ae2a0d7bfafa6cda80a5a9", }, }, { @@ -317,7 +317,7 @@ func TestMetricAttributes_Hash(t *testing.T) { "user.id": 10, }, expectedAttributes: map[string]interface{}{ - "user.id": "a111f275cc2e7588000001d300a31e76336d15b9d314cd1a1d8f3d3556975eed", + "user.id": "71aa908aff1548c8c6cdecf63545261584738a25", }, }, { @@ -326,7 +326,7 @@ func TestMetricAttributes_Hash(t *testing.T) { "user.balance": 99.1, }, expectedAttributes: map[string]interface{}{ - "user.balance": "05fabd78b01be9692863cb0985f600c99da82979af18db5c55173c2a30adb924", + "user.balance": "76429edab4855b03073f9429fd5d10313c28655e", }, }, { @@ -335,7 +335,7 @@ func TestMetricAttributes_Hash(t *testing.T) { "user.authenticated": true, }, expectedAttributes: map[string]interface{}{ - "user.authenticated": "4bf5122f344554c53bde2ebb8cd2b7e3d1600ad631c385a5d7cce23c7785459a", + "user.authenticated": "bf8b4530d8d246dd74ac53a13471bba17941dff7", }, }, } diff --git a/processor/attributesprocessor/attributes_trace_test.go b/processor/attributesprocessor/attributes_trace_test.go index c9ae45340852..e075225ce3a5 100644 --- a/processor/attributesprocessor/attributes_trace_test.go +++ b/processor/attributesprocessor/attributes_trace_test.go @@ -321,7 +321,7 @@ func TestAttributes_Hash(t *testing.T) { "user.email": "john.doe@example.com", }, expectedAttributes: map[string]interface{}{ - "user.email": "836f82db99121b3481011f16b49dfa5fbc714a0d1b1b9f784a1ebbbf5b39577f", + "user.email": "73ec53c4ba1747d485ae2a0d7bfafa6cda80a5a9", }, }, { @@ -330,7 +330,7 @@ func TestAttributes_Hash(t *testing.T) { "user.id": 10, }, expectedAttributes: map[string]interface{}{ - "user.id": "a111f275cc2e7588000001d300a31e76336d15b9d314cd1a1d8f3d3556975eed", + "user.id": "71aa908aff1548c8c6cdecf63545261584738a25", }, }, { @@ -339,7 +339,7 @@ func TestAttributes_Hash(t *testing.T) { "user.balance": 99.1, }, expectedAttributes: map[string]interface{}{ - "user.balance": "05fabd78b01be9692863cb0985f600c99da82979af18db5c55173c2a30adb924", + "user.balance": "76429edab4855b03073f9429fd5d10313c28655e", }, }, { @@ -348,7 +348,7 @@ func TestAttributes_Hash(t *testing.T) { "user.authenticated": true, }, expectedAttributes: map[string]interface{}{ - "user.authenticated": "4bf5122f344554c53bde2ebb8cd2b7e3d1600ad631c385a5d7cce23c7785459a", + "user.authenticated": "bf8b4530d8d246dd74ac53a13471bba17941dff7", }, }, } From 68a025265b6874380c1332bb9daa36eb185a4587 Mon Sep 17 00:00:00 2001 From: Antoine Toulme Date: Wed, 22 Mar 2023 18:30:59 -0400 Subject: [PATCH 6/9] apply changelog entry note change --- .chloggen/introduce_sha2Hasher.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.chloggen/introduce_sha2Hasher.yaml b/.chloggen/introduce_sha2Hasher.yaml index 6ac44d26d14d..98a5caea7c1a 100644 --- a/.chloggen/introduce_sha2Hasher.yaml +++ b/.chloggen/introduce_sha2Hasher.yaml @@ -5,7 +5,7 @@ change_type: enhancement component: attributesprocessor, resourceprocessor # A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). -note: Stop using SHA-1 and use SHA2-256 instead for hashing +note: Add feature gate to support using SHA2-256 instead of SHA-1 for hashing # One or more tracking issues related to the change issues: [4759, 5576] From b04d8c3185f630fb78ecb68b34ae0005bc525169 Mon Sep 17 00:00:00 2001 From: Antoine Toulme Date: Thu, 23 Mar 2023 11:54:47 -0700 Subject: [PATCH 7/9] correct comment --- internal/coreinternal/attraction/attraction.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/internal/coreinternal/attraction/attraction.go b/internal/coreinternal/attraction/attraction.go index 9a1c012efd16..2c3b17d64db7 100644 --- a/internal/coreinternal/attraction/attraction.go +++ b/internal/coreinternal/attraction/attraction.go @@ -91,8 +91,10 @@ type ActionKeyValue struct { // Either Value, FromAttribute or FromContext must be set. // DELETE - Deletes the attribute. If the key doesn't exist, // no action is performed. - // HASH - Calculates the SHA-256 hash of an existing value and overwrites the - // value with it's SHA-256 hash result. + // HASH - Calculates the SHA-1 hash of an existing value and overwrites the + // value with its SHA-1 hash result. If the feature gate + // `coreinternal.attraction.hash.sha256` is enabled, it uses SHA2-256 + // instead. // EXTRACT - Extracts values using a regular expression rule from the input // 'key' to target keys specified in the 'rule'. If a target key // already exists, it will be overridden. From 6db379e48ee2d04fb23ecec8e02ee6d19cbacd6a Mon Sep 17 00:00:00 2001 From: Antoine Toulme Date: Thu, 23 Mar 2023 13:28:17 -0700 Subject: [PATCH 8/9] fix linting --- internal/coreinternal/attraction/attraction_test.go | 5 ++--- internal/coreinternal/attraction/hasher.go | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/internal/coreinternal/attraction/attraction_test.go b/internal/coreinternal/attraction/attraction_test.go index 8a8998332a97..f9cd3ff21905 100644 --- a/internal/coreinternal/attraction/attraction_test.go +++ b/internal/coreinternal/attraction/attraction_test.go @@ -16,7 +16,7 @@ package attraction import ( "context" - "crypto/sha1" + "crypto/sha1" // #nosec "crypto/sha256" "encoding/binary" "errors" @@ -943,9 +943,8 @@ func TestValidConfiguration(t *testing.T) { func hash(b []byte) string { if enableSha256Gate.IsEnabled() { return sha2Hash(b) - } else { - return sha1Hash(b) } + return sha1Hash(b) } func sha1Hash(b []byte) string { diff --git a/internal/coreinternal/attraction/hasher.go b/internal/coreinternal/attraction/hasher.go index 5d0000b9b52a..86f35c1328dc 100644 --- a/internal/coreinternal/attraction/hasher.go +++ b/internal/coreinternal/attraction/hasher.go @@ -15,7 +15,7 @@ package attraction // import "github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal/attraction" import ( - "crypto/sha1" + "crypto/sha1" // #nosec "crypto/sha256" "encoding/binary" "encoding/hex" From 2aa43d372d41cfbc6f6c41c4dc3409fbeaf0c8bb Mon Sep 17 00:00:00 2001 From: Antoine Toulme Date: Thu, 23 Mar 2023 13:38:04 -0700 Subject: [PATCH 9/9] bring back one more #nosec --- internal/coreinternal/attraction/attraction_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/coreinternal/attraction/attraction_test.go b/internal/coreinternal/attraction/attraction_test.go index f9cd3ff21905..3c5dde9002d5 100644 --- a/internal/coreinternal/attraction/attraction_test.go +++ b/internal/coreinternal/attraction/attraction_test.go @@ -948,6 +948,7 @@ func hash(b []byte) string { } func sha1Hash(b []byte) string { + // #nosec h := sha1.New() h.Write(b) return fmt.Sprintf("%x", h.Sum(nil))