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;
+            	  			my $method = $r->method;
-                      if ($hashed_password eq $pass_digest && ((defined $read_only_methods{$method} && $permissions =~ /:browse_repository/) || $permissions =~ /:commit_access/) ) {
+                      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

@@ -361,7 +361,6 class UserTest < ActiveSupport::TestCase
361	user = User.try_to_login("admin", "hello")	361	user = User.try_to_login("admin", "hello")
362	assert_kind_of User, user	362	assert_kind_of User, user
363	assert_equal "admin", user.login	363	assert_equal "admin", user.login
364	assert_equal User.hash_password("hello"), user.hashed_password
365	end	364	end
366		365
367	def test_name_format	366	def test_name_format
@@ -383,6 +382,22 class UserTest < ActiveSupport::TestCase
383	assert_equal nil, user	382	assert_equal nil, user
384	end	383	end
385		384
		385	context ".try_to_login" do
		386	context "with good credentials" do
		387	should "return the user" do
		388	user = User.try_to_login("admin", "admin")
		389	assert_kind_of User, user
		390	assert_equal "admin", user.login
		391	end
		392	end
		393
		394	context "with wrong credentials" do
		395	should "return nil" do
		396	assert_nil User.try_to_login("admin", "foo")
		397	end
		398	end
		399	end
		400
386	if ldap_configured?	401	if ldap_configured?
387	context "#try_to_login using LDAP" do	402	context "#try_to_login using LDAP" do
388	context "with failed connection to the LDAP server" do	403	context "with failed connection to the LDAP server" do
@@ -727,6 +742,23 class UserTest < ActiveSupport::TestCase
727	should 'be added and tested'	742	should 'be added and tested'
728	end	743	end
729	end	744	end
		745
		746	def test_salt_unsalted_passwords
		747	# Restore a user with an unsalted password
		748	user = User.find(1)
		749	user.salt = nil
		750	user.hashed_password = User.hash_password("unsalted")
		751	user.save!
		752
		753	User.salt_unsalted_passwords!
		754
		755	user.reload
		756	# Salt added
		757	assert !user.salt.blank?
		758	# Password still valid
		759	assert user.check_password?("unsalted")
		760	assert_equal user, User.try_to_login(user.login, "unsalted")
		761	end
730		762
731	if Object.const_defined?(:OpenID)	763	if Object.const_defined?(:OpenID)
732		764

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