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

Move Md5 password hashing to server side #617

Open
wants to merge 2 commits into
base: development
Choose a base branch
from

Conversation

tobil4sk
Copy link
Member

This changes the api version to 4.0, where hashing is done server side. For now, old clients are still allowed to use api version 3.0 and hash on the client. Later on, submission and registration can be disabled for api version 3.0 so that the migration can take place.

This makes it easier to deal with #314 (also see the thread regarding server side hashing).

This is required so that new clients which use api version 4.0 can rely
on server side hashing.
@skial skial mentioned this pull request Dec 14, 2023
1 task
@Simn
Copy link
Member

Simn commented Dec 17, 2023

FWIW I'm in favor of this change and think we should move forward with this. I think we all agree that this should be done, the only remaining question is if it should also still be done on the client-side. @andyli seems to be the only one in favor of that, so I'd like to sort that out before comitting to anything.

@tobil4sk
Copy link
Member Author

To reiterate, these are the reasons why I would recommend against client side hashing:

  • Client side hashing leads to overall worse security, because it delays security changes. Any changes to hashing require breaking client compatibility, which means that the hashing upgrades are very tedious and will end up getting put off, like the Md5 migration has been put off since 2008 when Md5 was declared broken. If we stick to only server side hashing, future hashing related security improvements can be done seamlessly which would make haxelib more adaptable to future security requirements.
  • Client side hashing does not actually provide any extra protection to haxelib user accounts, because the hash produced by the client effectively takes the place of the password and gives access to the account.
  • Client side hashing requires a Haxe implementation of a hashing algorithm, which wouldn't be audited and is insecure, given that well audited and well established implementations already exist. If we don't do this, the haxelib client will be stuck running on Neko.
  • Client side hashing would require the client getting a salt from the server, which adds complexity.
  • With https, the password is already encrypted when sent between the client and server. If the server code was compromised there would be a threat, but at that point there are bigger things to worry about because there are much nastier things an attacker could do with that kind of access.

@tobil4sk tobil4sk changed the title Move password hashing to server side Move Md5 password hashing to server side Dec 27, 2023
@Dimensionscape
Copy link

Dimensionscape commented Sep 1, 2024

There's no reason to hash the password on the client over TLS. More importantly - migrate to bcrypt. Md5 shouldn't even be used anymore. (Edit: Nevermind! I see you're already heading in that direction with argon2 #619)

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 this pull request may close these issues.

3 participants