jro_apps/redmine Commit - r4816:ce84bb1a0194

Adds random salt to user passwords (#7410)....

Jean-Philippe Lang -

r4816:ce84bb1a0194

parent child

Context file:

r4816:ce84bb1a0194

Collapse all files

db/migrate/20110223180944_add_users_salt.rb created 644 +9 0

			@@ -0,0 +1,9
		1	class AddUsersSalt < ActiveRecord::Migration
		2	def self.up
		3	add_column :users, :salt, :string, :limit => 64
		4	end
		5
		6	def self.down
		7	remove_column :users, :salt
		8	end
		9	end

db/migrate/20110223180953_salt_user_passwords.rb created 644 +13 0

			@@ -0,0 +1,13
		1	class SaltUserPasswords < ActiveRecord::Migration
		2
		3	def self.up
		4	say_with_time "Salting user passwords, this may take some time..." do
		5	User.salt_unsalted_passwords!
		6	end
		7	end
		8
		9	def self.down
		10	# Unsalted passwords can not be restored
		11	raise ActiveRecord::IrreversibleMigration, "Can't decypher salted passwords. This migration can not be rollback'ed."
		12	end
		13	end

app/models/user.rb +33 -3

                def before_save
                  # update hashed_password if password was set
-                 self.hashed_password = User.hash_password(self.password) if self.password && self.auth_source_id.blank?
+                 if self.password && self.auth_source_id.blank?
+                   salt_password(password)
+                 end
                end
                def reload(*args)
                      return nil unless user.auth_source.authenticate(login, password)
                    else
                      # authentication with local password
-                     return nil unless User.hash_password(password) == user.hashed_password
+                     return nil unless user.check_password?(password)
                    end
                  else
                    # user is not yet registered, try to authenticate with available sources
                  update_attribute(:status, STATUS_LOCKED)
                end
+               # Returns true if +clear_password+ is the correct user's password, otherwise false
                def check_password?(clear_password)
                  if auth_source_id.present?
                    auth_source.authenticate(self.login, clear_password)
                  else
-                   User.hash_password(clear_password) == self.hashed_password
+                   User.hash_password("#{salt}#{User.hash_password clear_password}") == hashed_password
                  end
                end
+               # Generates a random salt and computes hashed_password for +clear_password+
+               # The hashed password is stored in the following form: SHA1(salt + SHA1(password))
+               def salt_password(clear_password)
+                 self.salt = User.generate_salt
+                 self.hashed_password = User.hash_password("#{salt}#{User.hash_password clear_password}")
+               end
                # Does the backend storage allow this user to change their password?
                def change_password_allowed?
                  end
                  anonymous_user
                end
+               # Salts all existing unsalted passwords
+               # It changes password storage scheme from SHA1(password) to SHA1(salt + SHA1(password))
+               # This method is used in the SaltPasswords migration and is to be kept as is
+               def self.salt_unsalted_passwords!
+                 transaction do
+                   User.find_each(:conditions => "salt IS NULL OR salt = ''") do |user|
+                     next if user.hashed_password.blank?
+                     salt = User.generate_salt
+                     hashed_password = User.hash_password("#{salt}#{user.hashed_password}")
+                     User.update_all("salt = '#{salt}', hashed_password = '#{hashed_password}'", ["id = ?", user.id] )
+                   end
+                 end
+               end
                protected
                def self.hash_password(clear_password)
                  Digest::SHA1.hexdigest(clear_password || "")
                end
+               # Returns a 128bits random salt as a hex string (32 chars long)
+               def self.generate_salt
+                 ActiveSupport::SecureRandom.hex(16)
+               end
              end
              class AnonymousUser < User

extra/svn/Redmine.pm +5 -4

                my ($self, $parms, $arg) = @_;
                $self->{RedmineDSN} = $arg;
                my $query = "SELECT
-                              hashed_password, auth_source_id, permissions
+                              hashed_password, salt, auth_source_id, permissions
                            FROM members, projects, users, roles, member_roles
                            WHERE
                              projects.id=members.project_id
                $sth->execute($redmine_user, $project_id);
                my $ret;
-               while (my ($hashed_password, $auth_source_id, $permissions) = $sth->fetchrow_array) {
+               while (my ($hashed_password, $salt, $auth_source_id, $permissions) = $sth->fetchrow_array) {
                    unless ($auth_source_id) {
-             	  my $method = $r->method;
-                       if ($hashed_password eq $pass_digest && ((defined $read_only_methods{$method} && $permissions =~ /:browse_repository/) || $permissions =~ /:commit_access/) ) {
+             	  			my $method = $r->method;
+                       my $salted_password = Digest::SHA1::sha1_hex($salt.$pass_digest);
+             					if ($hashed_password eq $salted_password && ((defined $read_only_methods{$method} && $permissions =~ /:browse_repository/) || $permissions =~ /:commit_access/) ) {
                            $ret = 1;
                            last;
                        }

test/fixtures/users.yml +13 -5

                status: 1
                last_login_on:
                language: en
-               hashed_password: 4e4aeb7baaf0706bd670263fef42dad15763b608
+               # password = foo
+               salt: 3126f764c3c5ac61cbfc103f25f934cf
+               hashed_password: 9e4dd7eeb172c12a0691a6d9d3a269f7e9fe671b
                updated_on: 2006-07-19 19:34:07 +02:00
                admin: false
                mail: rhill@somenet.foo
                status: 1
                last_login_on: 2006-07-19 22:57:52 +02:00
                language: en
-               hashed_password: d033e22ae348aeb5660fc2140aec35850c4da997
+               # password = admin
+               salt: 82090c953c4a0000a7db253b0691a6b4
+               hashed_password: b5b6ff9543bf1387374cdfa27a54c96d236a7150
                updated_on: 2006-07-19 22:57:52 +02:00
                admin: true
                mail: admin@somenet.foo
                status: 1
                last_login_on: 2006-07-19 22:42:15 +02:00
                language: en
-               hashed_password: a9a653d4151fa2c081ba1ffc2c2726f3b80b7d7d
+               # password = jsmith
+               salt: 67eb4732624d5a7753dcea7ce0bb7d7d
+               hashed_password: bfbe06043353a677d0215b26a5800d128d5413bc
                updated_on: 2006-07-19 22:42:15 +02:00
                admin: false
                mail: jsmith@somenet.foo
                status: 1
                last_login_on:
                language: en
-               hashed_password: 7feb7657aa7a7bf5aef3414a5084875f27192415
+               # password = foo
+               salt: 7599f9963ec07b5a3b55b354407120c0
+               hashed_password: 8f659c8d7c072f189374edacfa90d6abbc26d8ed
                updated_on: 2006-07-19 19:33:19 +02:00
                admin: false
                mail: dlopper@somenet.foo
                status: 3
                last_login_on:
                language: en
-               hashed_password: 7feb7657aa7a7bf5aef3414a5084875f27192415
+               hashed_password: 1
                updated_on: 2006-07-19 19:33:19 +02:00
                admin: false
                mail: dlopper2@somenet.foo

test/unit/user_test.rb +33 -1

                  user = User.try_to_login("admin", "hello")
                  assert_kind_of User, user
                  assert_equal "admin", user.login
-                 assert_equal User.hash_password("hello"), user.hashed_password
                end
                def test_name_format
                  assert_equal nil, user
                end
+               context ".try_to_login" do
+                 context "with good credentials" do
+                   should "return the user" do
+                     user = User.try_to_login("admin", "admin")
+                     assert_kind_of User, user
+                     assert_equal "admin", user.login
+                   end
+                 end
+                 context "with wrong credentials" do
+                   should "return nil" do
+                     assert_nil User.try_to_login("admin", "foo")
+                   end
+                 end
+               end
                if ldap_configured?
                  context "#try_to_login using LDAP" do
                    context "with failed connection to the LDAP server" do
                    should 'be added and tested'
                  end
                end
+               def test_salt_unsalted_passwords
+                 # Restore a user with an unsalted password
+                 user = User.find(1)
+                 user.salt = nil
+                 user.hashed_password = User.hash_password("unsalted")
+                 user.save!
+                 User.salt_unsalted_passwords!
+                 user.reload
+                 # Salt added
+                 assert !user.salt.blank?
+                 # Password still valid
+                 assert user.check_password?("unsalted")
+                 assert_equal user, User.try_to_login(user.login, "unsalted")
+               end
                if Object.const_defined?(:OpenID)

General Comments 0

Write
Preview

You need to be logged in to leave comments. Login now

No TODOs yet

	Site-wide shortcuts
/	Use quick search box
g h	Goto home page
g g	Goto my private gists page
g G	Goto my public gists page
g 0-9	Goto bookmarked items from 0-9
n r	New repository page
n g	New gist page

	Repositories
g s	Goto summary page
g c	Goto changelog page
g f	Goto files page
g F	Goto files page with file search activated
g p	Goto pull requests page
g o	Goto repository settings
g O	Goto repository access permissions settings
t s	Toggle sidebar on some pages