patternrubyMinor
Simple Password Utility
Viewed 0 times
simpleutilitypassword
Problem
I am somewhat new to ruby (but not to programming) and I want to develop a small web-app with Sinatra. I started creating a small password utility script that hashes and secures entered password. I am not sure it is finished. I would like to know if there are any ruby tricks to make my code better or more verbose, and what else needs to be done / changed to make passwords more secure. Any criticism is appreciated. here is my code:
require 'digest'
def random_salt
hash = ''
for i in 1..64
rand_seed = rand(0..61)
hash << 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890'[rand_seed]
end
return hash
end
# password format: salt + password + length_of_salt_and_hash
def hash_password(password)
salt = random_salt
hashed_pass = Digest::SHA512.hexdigest password.chomp
length = salt.length + hashed_pass.length
return salt + hashed_pass + length.to_s
end
# just a test
print 'Enter a password: '
word = gets
new_word = hash_password word
puts new_wordSolution
My critique is mostly on the crypto.
- Never invent your own crypto. It's difficult to do correctly, and there are already good libraries for this sort of thing. See sinatra-authentication or bcrypt-ruby.
- rand() is not a cryptographically secure random number generator, so it's a poor choice for use in crypto. Also see point #1.
- Your password salting is incorrect. The salt must be mixed with the password before hashing, not after. Also see point #1.
- Why append the length_of_salt_and_hash? That seems pointless.
- Why append salt + hashed_pass? These are usually stored as separate fields in a database record.
- Avoid magic numbers like 61 in your code. What does Roger Maris have to do with this? It's unclear and redundant that this 61 refers to the length of the string on the next line.
- You ask for "better or more verbose." Those are often opposite goals. More verbose code means more code to read and more code to maintain. (That does not mean you should prefer cryptic one-liners.)
Context
StackExchange Code Review Q#91120, answer score: 8
Revisions (0)
No revisions yet.