Bug #1216

Salt is meaningless if it is the same every time.

Added by dmasover - 246 days ago. Updated 245 days ago.

Status :New Start :
Priority :Normal Due date :
Assigned to :scott - % Done :

0%
Category :backend
Target version :-
Resolution :

Description

Found in user.rb:
  # echo "typo" | sha1sum -
  @@salt = '20ac4d290c2293702c64b3b287ae5ea79b26a5c1'
  cattr_accessor :salt

And later:
  # Apply SHA1 encryption to the supplied password.
  # We will additionally surround the password with a salt
  # for additional security.
  def self.sha1(pass)
    Digest::SHA1.hexdigest("#{salt}--#{pass}--")
  end

Looks like the latest trunk does the same thing.

I'm not entirely sure what the point of the salt is -- the hash of the user's password will be identical if the password is the same, not only across multiple users on the same blog, but across all users on all typo blogs. I don't see how this is an improvement over no salt at all.

I'd suggest either removing the salt altogether, or basing it on something that varies user to user -- even the userid would be an improvement. Ideally, generate a random salt every time a new password is saved, and save that salt in another column -- I think this is how /etc/shadow does it, on Unix.

No patch yet because I'm not sure yet how best to resolve this without breaking existing logins. Maybe transparently re-encrypt the user's password on first login after the change?

History

03/18/2008 08:11 PM - bribera -

Any random salt -- even a static value -- provides protection against dictionary attacks and the use of rainbow tables.

For example, if I use the classic 'password' as my password, a system using MD5 and no salt will store it as:

5f4dcc3b5aa765d61d8327deb882cf99

Just look around google for a second and you'll see how easy it is to turn this back into the original password. However, if I use the default Typo salt, it's the same as this:

8401e57d09c9dfad564a348d6f4bd9bb

There's a reason that Google has no hits for this value -- it's not the hash value of a word in any language. This is definitely an improvement over no salt at all, but you're correct in asserting that generating a new salt for each password is more secure.

If anything is changed, I'd suggest is adding a blog specific field that is setup with a random string upon first installation. Any changes to an existing blog would require rehashing of all passwords, or the addition of a method to reset the password to some random goop and email the user. It could be a neat little rake task -- rake resalt_passwords or the like. But using the same salt for all of your users is far less tragic than using none at all.

Generated with:
#!ruby
require 'md5'
puts MD5.new('password').to_s
puts MD5.new('20ac4d290c2293702c64b3b287ae5ea79b26a5c1--password--').to_s

03/19/2008 05:46 PM - dmasover -

That's a fair analysis, although using the same salt for every installation, while better than no salt at all, would still suggest that if Typo became popular enough, these would become known.

Assuming the User model is reasonably isolated -- that changes to the hashing mechanism shouldn't affect the rest of the system -- I would propose something like this:

  add_column :users, :salt, :string
  # If we're extra paranoid...
  add_index :users, :salt, :unique => true

  def authenticate(login, pass)
    user = find_by_login(login)
    if user.salt.nil?
      if sha1(pass) == user.password
        # Rehash
        user.password = pass
        user.save!
        user
      else
        # Bad password
        nil
      end
    else
      # User has a salt -- normal authentication
      sha1(pass, user.salt) == user.password ? user : nil
    end
  end

So, first successful login after the change, we rehash. It wouldn't improve things for users who never login, but we can't really do much about them other than, as you said, resetting their password and emailing them.

If you're interested, I can finish this and make it into a patch.

Also available in: Atom PDF