loki.secretfilter: Change the way the secret is hashed #2529
Conversation
|
💻 Deploy preview available: https://deploy-preview-alloy-2529-zb444pucvq-vp.a.run.app/docs/alloy/latest/ |
|
Note that the failing tests aren't linked to this component. |
| hashBytes := hasher.Sum(nil) | ||
|
|
||
| // Only use the first half of the hash | ||
| firstHalf := hashBytes[:len(hashBytes)/2] |
There was a problem hiding this comment.
This sort of feels odd, could we use a native 128bit hash? This feels harder to use.
|
I think this does make some sense - but I would much prefer it if the hash could be configurable, rather than our choice. I.e., if a user wants a particular hash function, they can pick it and we truncate it to the specific length. Honestly, good secret encryption should use hash functions that we do not want to use in our component (e.g., compute/memory intensive ones), so the likelihood of us selecting a hash function that can be directly compared to a user's secret hash should be low - it's more relevant for identifying secrets which are being kept in plain text. |
PR Description
This PR changes the way the secret is hashed in the experimental component
loki.secretfilter.In this context, the hash of the secret is used to be able to identify the redacted secret more easily, without revealing too much information about it in the logs. It's the reason why an algorithm with a shorter output was used (SHA1): it doesn't need to be collision resistant as it is used as an identifier for the redacted secret, not as a signature.
But I thought of a different approach to keep the hash relatively short: hash it with SHA256 and output only the first half of the hash. This approach should help keep the information about the secret minimal, while relying on a more recent hashing algorithm that is more likely to be available to someone investigating a leaked secret in their logs. I believe the first half of a SHA256 hash is unique enough to help identify the secret that leaked without ambiguity, and short enough not to add too much information about the underlying secret in the logs.
@kelnage happy to have your opinion on this!
Which issue(s) this PR fixes
Notes to the Reviewer
PR Checklist