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 of outdated hashing algorithm #462

Open
ghost opened this issue Apr 6, 2023 · 1 comment
Open

Use of outdated hashing algorithm #462

ghost opened this issue Apr 6, 2023 · 1 comment

Comments

@ghost
Copy link

ghost commented Apr 6, 2023

MD5 is in use in hub/src/server/drivers/diskDriver.ts . MD5 has "collisions" - two separate strings can resolve to the same hash. A good breakdown is https://www.mscs.dal.ca/~selinger/md5collision/ . Node.js also recommends avoiding MD5 - https://nodejs.org/api/crypto.html#support-for-weak-or-compromised-algorithms

Solution: in line 241, replace:

const hash = crypto.createHash('md5')

with a more collision-resistant algorithm such as:

const hash = crypto.createHash('sha512')
@jcnelson
Copy link
Member

jcnelson commented Apr 7, 2023

I'm well aware that MD5 is broken.

This code path uses the MD5 hash function to create an ETag HTTP header. Strictly speaking, it does not need to be an MD5, but the consequences of a collision are already priced into Gaia's data availability guarantees. If the Gaia client is not fetching data directly from the Gaia hub over SSL (in which case, the user already trusts the Gaia hub to faithfully generate ETags), then the only way I can see an MD5 collision bringing harm to the user is if a malicious MITM service (e.g. an evil CDN) deliberately generated the collision in order to serve stale data to clients, while also persuading them that the data had not been modified. Of course, if this is the goal of the malicious CDN, they don't need to break the ETag hash; they could simply lie.

Either way, changing this to sha512 should be inconsequential. But, I just wanted to make sure you're aware that we did consider the fact that MD5 is not a high-quality cryptographic hash. We are not using it as such.

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

No branches or pull requests

1 participant