HiveBrain v1.2.0
Get Started
← Back to all entries
patternrubyMinor

Simple Password Utility

Submitted by: @import:stackexchange-codereview··
0
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_word

Solution

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.