-
Notifications
You must be signed in to change notification settings - Fork 315
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.
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
update hashing functions from md5 to sha256 #5456
base: dev
Are you sure you want to change the base?
Conversation
this commit updates all instances of md5 to sha256. Signed-off-by: kranurag7 <[email protected]>
+1 |
Hi! First of all, thanks for the contribution! That said, could you clarify why this change is necessary? MD5 isn’t used to hash anything sensitive, like passwords—it’s only used to generate alert group distinction, which are solely for grouping purposes. Additionally, this change seems likely to break grouping for open alert groups. Since the new hash function would generate different group distinction, new alerts that should belong to an existing group might end up in a separate one. |
I think FIPS compliant environments with strict enforcement in OpenSSL would benefit from this change, as |
Thanks for the clarification. Is it possible to implement it under the feature flag or something, so users who do not need it will still use md5 & they will not receive unexpected pages during the update? |
Hey @Konstantinov-Innokentii I would like to understand more of why it's a breaking change, changing to sha256 should be safe here right. I am thinking more on the line that md5 gives you random keys and sha256 will give you same randomness but a little longer. Is there something more detailed that I need to understand here in case I missed something here. |
Hey @kranurag7!
That's why I'm asking why this change is needed & if it's for some security compliance – is it possible to use sha256 only under feature flag. |
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
What this PR does
updates all instances of md5 to sha256.
Which issue(s) this PR closes
N/A (there were no open issues about this)
Checklist
pr:no public docs
PR label added if not required)release:
). These labels dictate how your PR willshow up in the autogenerated release notes.