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

Implement password hashing #17

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

wode490390
Copy link

@DaMatrix
Copy link
Member

  • md5 is no longer considered cryptographically secure
  • hashing is not encryption
  • the hash is now equivalent to being the password, meaning there's no advantage

@wode490390
Copy link
Author

@DaMatrix Clear text passwords are a serious security risk. Digest authentication has significant advantages over clear text passwords, though other security issues arise. We can use sha256 instead of md5.

@wode490390 wode490390 changed the title Implement password encryption Implement password hashing Sep 12, 2019
@DaMatrix
Copy link
Member

the point is, since the password is a) stored as plain text anyway and b) transmitted insecurely, the hash is effectively the plaintext password

@wode490390
Copy link
Author

Ah, we can use salt:
md5(md5(password) + salt)

@DaMatrix
Copy link
Member

a salt doesn't make any difference! if you're using the hash for authentication, the hash is the password. this is literally exactly as secure as it was before.

@DaMatrix
Copy link
Member

if you want a secure authentication system, you'd need something like HMAC with SHA-256 or SHA-3

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.

2 participants