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.

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

Use Blake2 instead of MD5 #263

Open
igoose1 opened this issue Aug 10, 2023 · 1 comment · May be fixed by #264
Open

Use Blake2 instead of MD5 #263

igoose1 opened this issue Aug 10, 2023 · 1 comment · May be fixed by #264

Comments

@igoose1
Copy link

igoose1 commented Aug 10, 2023

What's the point of md5 usage?

cache_key = hashlib.md5( # noqa: S324

md5 was proofed to be insecure. It's possible to find collisions fast. A potential attacker theoretically can send unique query parameters which weren't present in the past but which will be cache-hit. Although, it's difficult for me to make up an example where this is a security breach, it's still possible in a complex system.

Probably, md5 was chosen because of its speed. At least, code owners look aware of an issue. Linter warnings are ignored with "noqa" comments.

Though key builder can be changed manually, I'm suggesting to change an algorithm in the library. Blake2 is a faster and secure hashing algorithm included in Python standard library.

Benchmarks:

image

from http://www.blake2.net/

image

from https://medium.com/logos-network/benchmarking-hash-and-signature-algorithms-6079735ce05

@igoose1
Copy link
Author

igoose1 commented Aug 10, 2023

Update: I'm wrong with "A potential attacker theoretically can send unique query parameters which weren't present in the past but which will be cache-hit". Hashing is used only for code written by programmers. Then it's more difficult to make up a security issue here.

@igoose1 igoose1 linked a pull request Aug 10, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant