binarylogic / authlogic

A simple ruby authentication solution.
http://rdoc.info/projects/binarylogic/authlogic
MIT License
4.34k stars 638 forks source link

Fixes on hash and crypto. stuff #16

Closed tredoe closed 7 years ago

tredoe commented 15 years ago

http://github.com/binarylogic/authlogic/blob/81af95e639570822667ec817f42aaad54f8865ed/lib/authlogic/random.rb#L16

By default SecureRandom [1] uses 16 bytes (which has an entropy of 128 bits [2]), so:

SecureRandom.base64(15).tr('+/=', '-_ ').strip.delete("\n")

should be:

SecureRandom.base64().tr('+/=', '-_ ').rstrip

[1] http://api.rubyonrails.org/classes/ActiveSupport/SecureRandom.html#M001107

[2] https://bitbucket.org/ares/cryha/src/tip/doc/sym_crypto.txt#cl-113

http://github.com/binarylogic/authlogic/blob/81af95e639570822667ec817f42aaad54f8865ed/lib/authlogic/random.rb#L11

SecureRandom.hex(64)

Why do use 64 bytes?

It shoulds be

SecureRandom.hex()

which uses 16 random bytes to return them into a hexadecimal string.

Use hash instead of encrypted

Another thing. I've seen that it's used 'encrypted' or 'crypted' to refer to the hashs. An hash is not a cryptographic algorithm, they are both different. Please use 'hash' to refer a SHA and any term related to crypto. to refer to AES.

thedarkone commented 15 years ago

Hey kless,

I wrote the code in question. Good call with rstrip instead of strip. I also kinda missed that SecureRandom already does .delete("\n"), so that is also redundant. The reason I only use 15 bytes is because I wanted to be fully compatible with the previous ad-hoc implementation that always produced 20 character tokens.

Same thing with hex_token, the original implementation returned 64 bytes (128 characters).

tredoe commented 15 years ago

Hi thedarkone,

thanks for contributing to this great plugin. This is the great thing respect to open software; whatever person to can check the code and make it better. In my case I know anything about security and I'm very strict respect to it.

Today it's very common (and recommended) to use an entropy of 128 bits (16 bytes) which it's used too in the initialization vectors (IV) of criptogaphic algorithms, and I'm sure that it's by that reason because SecureRandom returns 16 bytes by default.

So, at least for me, I think that the security is more important that the compatibility, and in this case I think that the change doesn't hurts.

In the next documents is explained any basic things about criptography and hashes:

https://bitbucket.org/ares/cryha/src/tip/doc/hash.txt https://bitbucket.org/ares/cryha/src/tip/doc/sym_crypto.txt

Greetings!

jaredbeck commented 7 years ago

Closing after seven years with no activity. If this is still a problem, please let us know.