Major revamp of security. Passwords are first SHA1 hashed and then

that hash is bcrypted.  Bcrypted hashes are stored in a new column on
users.  This separate column is only to allow for rollbacks,
eventually the old SHA1 hash column will be removed.  Sensitive cookie
details are now encrypted to prevent user tampering and more stringent
checks on secret_token and session_secret_key are enforced.

Gemfile

@@ -32,7 +32,7 @@ gem 'net-sftp'
 gem 'newrelic_rpm'
 gem 'term-ansicolor', :require => "term/ansicolor"
 gem 'diff-lcs', :require => "diff/lcs/array"
-gem 'bcrypt-ruby'
+gem 'bcrypt-ruby', :require => "bcrypt"
 
 group :development do
   gem 'ruby-prof'

app/logical/session_creator.rb

@@ -15,8 +15,8 @@ class SessionCreator
       user.update_column(:last_logged_in_at, Time.now)
       
       if remember.present?
-        cookies[:user_name] = {:expires => 1.year.from_now, :value => user.name}
+        cookies.permanent.signed[:user_name] = user.name
-        cookies[:cookie_password_hash] = {:expires => 1.year.from_now, :value => user.cookie_password_hash}
+        cookies.permanent.signed[:password_hash] = user.bcrypt_password_hash
       end
       
       session[:user_id] = user.id

app/logical/session_loader.rb

@@ -32,7 +32,7 @@ private
   end
   
   def load_cookie_user
-    CurrentUser.user = User.find_by_name(cookies[:user_name])
+    CurrentUser.user = User.find_by_name(cookies.signed[:user_name])
     CurrentUser.ip_addr = request.remote_ip
   end
   
@@ -41,7 +41,7 @@ private
   end
   
   def cookie_password_hash_valid?
-    cookies[:cookie_password_hash] && User.authenticate_cookie_hash(cookies[:user_name], cookies[:cookie_password_hash])
+    cookies[:password_hash] && User.authenticate_cookie_hash(cookies.signed[:user_name], cookies.signed[:password_hash])
   end
   
   def update_last_logged_in_at

app/models/user.rb

@@ -121,19 +121,24 @@ class User < ActiveRecord::Base
   end
   
   module PasswordMethods
+    def bcrypt_password
+      BCrypt::Password.new(bcrypt_password_hash)
+    end
+
     def encrypt_password_on_create
-      self.password_hash = User.sha1(password)
+      self.password_hash = ""
+      self.bcrypt_password_hash = User.bcrypt(password)
     end
     
     def encrypt_password_on_update
       return if password.blank?
       return if old_password.blank?
       
-      if User.sha1(old_password) == password_hash
+      if bcrypt_password == User.sha1(old_password)
-        self.password_hash = User.sha1(password)
+        self.bcrypt_password_hash = User.bcrypt(password)
         return true
       else
-        errors[:old_password] << "is incorrect"
+        errors[:old_password] = "is incorrect"
         return false
       end
     end
@@ -149,7 +154,7 @@ class User < ActiveRecord::Base
       end
 
       pass << rand(100).to_s
-      update_column(:password_hash, User.sha1(pass))
+      update_column(:bcrypt_password_hash, User.bcrypt(pass))
       pass    
     end
     
@@ -168,29 +173,31 @@ class User < ActiveRecord::Base
       end
 
       def authenticate_hash(name, hash)
-        where(["lower(name) = ? AND password_hash = ?", name.downcase, hash]).first != nil
+        user = find_by_name(name)
+        if user && user.bcrypt_password == hash
+          user
+        else
+          nil
+        end
       end
-    
+
       def authenticate_cookie_hash(name, hash)
-        user = User.find_by_name(name)
+        user = find_by_name(name)
-        return nil if user.nil?
+        if user && user.bcrypt_password_hash == hash
-        return hash == user.cookie_password_hash
+          user
+        else
+          nil
+        end
+      end
+      
+      def bcrypt(pass)
+        BCrypt::Password.create(sha1(pass))
       end
 
       def sha1(pass)
         Digest::SHA1.hexdigest("#{Danbooru.config.password_salt}--#{pass}--")
       end
     end
-
-    def cookie_password_hash
-      hash = password_hash
-      
-      (name.size + 8).times do
-        hash = User.sha1(hash)
-      end
-      
-      return hash
-    end
   end
   
   module FavoriteMethods
@@ -456,7 +463,7 @@ class User < ActiveRecord::Base
   
   module ApiMethods
     def hidden_attributes
-      super + [:password_hash, :email, :email_verification_key]
+      super + [:password_hash, :bcrypt_password_hash, :email, :email_verification_key]
     end
     
     def serializable_hash(options = {})

config/application.rb

@@ -1,5 +1,4 @@
 require File.expand_path('../boot', __FILE__)
-
 require 'rails/all'
 
 if defined?(Bundler)
@@ -11,6 +10,7 @@ end
 
 module Danbooru
   class Application < Rails::Application
+    
     config.active_record.schema_format = :sql
     config.encoding = "utf-8"
     config.filter_parameters += [:password]
@@ -25,4 +25,3 @@ module Danbooru
     config.log_tags = [lambda {|req| "PID:#{Process.pid}"}]
   end
 end
-

config/danbooru_default_config.rb

@@ -112,11 +112,6 @@ module Danbooru
       !user.is_privileged?
     end
     
-    # This is required for Rails 2.0.
-    def session_secret_key
-      "This should be at least 30 characters long"
-    end
-    
     # Users cannot search for more than X regular tags at a time.
     def base_tag_query_limit
       6

config/initializers/secret_token.rb

@@ -1,13 +1,9 @@
-# Be sure to restart your server when you modify this file.
+require File.expand_path('../../state_checker', __FILE__)
 
-# Your secret key for verifying the integrity of signed cookies.
+StateChecker.new.check!
-# If you change this key, all old signed cookies will become invalid!
-# Make sure the secret is at least 30 characters and all random,
-# no regular words or you'll be exposed to dictionary attacks.
-
-if File.exists?(File.expand_path("~/.danbooru/secret_token"))
-  Danbooru::Application.config.secret_token = File.read(File.expand_path("~/.danbooru/secret_token"))
-else
-  Danbooru::Application.config.secret_token = SecureRandom.hex(64)
-end
 
+Danbooru::Application.config.action_dispatch.session = {
+  :key    => '_danbooru2_session',
+  :secret => File.read(File.expand_path("~/.danbooru/session_secret_key"))
+}
+Danbooru::Application.config.secret_token = File.read(File.expand_path("~/.danbooru/secret_token"))

config/state_checker.rb

@@ -0,0 +1,36 @@
+class StateChecker
+  def check!
+    check_secret_token
+    check_session_secret_key
+  end
+
+private
+  
+  def secret_token_path
+    File.expand_path("~/.danbooru/secret_token")
+  end
+  
+  def check_secret_token
+    unless File.exists?(secret_token_path)
+      raise "You must create a file in #{secret_token_path} containing a secret key. It should be a string of at least 32 random characters."
+    end
+
+    if File.stat(secret_token_path).world_readable? || File.stat(secret_token_path).world_writable?
+      raise "#{secret_token_path} must not be world readable or writable"
+    end
+  end
+
+  def session_secret_key_path
+    File.expand_path("~/.danbooru/session_secret_key")
+  end
+  
+  def check_session_secret_key
+    unless File.exists?(session_secret_key_path)
+      raise "You must create a file in #{session_secret_key_path} containing a secret key. It should be a string of at least 32 random characters."
+    end
+
+    if File.stat(session_secret_key_path).world_readable? || File.stat(session_secret_key_path).world_writable?
+      raise "#{session_secret_key_path} must not be world readable or writable"
+    end
+  end
+end

db/migrate/20130305005138_add_bcrypt_fields_to_users.rb

@@ -0,0 +1,7 @@
+class AddBcryptFieldsToUsers < ActiveRecord::Migration
+  def change
+    execute "set statement_timeout = 0"
+    
+    add_column :users, :bcrypt_password_hash, :text
+  end
+end

db/structure.sql

@@ -2596,7 +2596,8 @@ CREATE TABLE users (
     default_image_size character varying(255) DEFAULT 'large'::character varying NOT NULL,
     favorite_tags text,
     blacklisted_tags text,
-    time_zone character varying(255) DEFAULT 'Eastern Time (US & Canada)'::character varying NOT NULL
+    time_zone character varying(255) DEFAULT 'Eastern Time (US & Canada)'::character varying NOT NULL,
+    bcrypt_password_hash text
 );
 
 
@@ -6207,4 +6208,6 @@ INSERT INTO schema_migrations (version) VALUES ('20130221032344');
 
 INSERT INTO schema_migrations (version) VALUES ('20130221035518');
 
-INSERT INTO schema_migrations (version) VALUES ('20130221214811');
+INSERT INTO schema_migrations (version) VALUES ('20130221214811');
+
+INSERT INTO schema_migrations (version) VALUES ('20130305005138');

script/fixes/004.rb

@@ -2,4 +2,6 @@
 
 require File.expand_path(File.join(File.dirname(__FILE__), '..', '..', 'config', 'environment'))
 
-puts Post.count
+User.find_each do |user|
+  user.update_column(:bcrypt_password_hash, BCrypt::Password.create(user.password_hash))
+end

test/functional/maintenance/user/password_resets_controller_test.rb

@@ -101,7 +101,7 @@ module Maintenance
               @user = FactoryGirl.create(:user)
               @nonce = FactoryGirl.create(:user_password_reset_nonce, :email => @user.email)
               ActionMailer::Base.deliveries.clear
-              @old_password = @user.password_hash
+              @old_password = @user.bcrypt_password_hash
               post :update, :email => @nonce.email, :key => @nonce.key
             end
             
@@ -115,7 +115,7 @@ module Maintenance
             
             should "change the password" do
               @user.reload
-              assert_not_equal(@old_password, @user.password_hash)
+              assert_not_equal(@old_password, @user.bcrypt_password_hash)
             end
             
             should "delete the nonce" do

test/unit/user_test.rb

@@ -35,7 +35,7 @@ class UserTest < ActiveSupport::TestCase
         assert_difference("ModAction.count") do
           @user.invite!(User::Levels::CONTRIBUTOR)
         end
-        assert_equal("#{@user.id} level changed Member -> Contributor by #{CurrentUser.name}", ModAction.first.description)
+        assert_equal("#{@user.name} level changed Member -> Contributor by #{CurrentUser.name}", ModAction.last.description)
       end
     end
     
@@ -199,21 +199,16 @@ class UserTest < ActiveSupport::TestCase
       end
     end
     
-    context "cookie password hash" do
-      setup do
-        @user = FactoryGirl.create(:user, :name => "albert", :password_hash => "1234")
-      end
-      
-      should "be correct" do
-        assert_equal("8ac3b1d04bdb95ba92f9e355897c880e0d88ac5a", @user.cookie_password_hash)
-      end
-      
-      should "validate" do
-        assert(User.authenticate_cookie_hash(@user.name, "8ac3b1d04bdb95ba92f9e355897c880e0d88ac5a"))
-      end
-    end
-    
     context "password" do
+      should "match the cookie hash" do
+        @user = FactoryGirl.create(:user)
+        @user.password = "zugzug5"
+        @user.password_confirmation = "zugzug5"
+        @user.save
+        @user.reload
+        assert(User.authenticate_cookie_hash(@user.name, @user.bcrypt_password_hash))
+      end
+      
       should "match the confirmation" do
         @user = FactoryGirl.create(:user)
         @user.password = "zugzug5"
@@ -248,25 +243,25 @@ class UserTest < ActiveSupport::TestCase
       should "not change the password if the password and old password are blank" do
         @user = FactoryGirl.create(:user, :password => "67890")
         @user.update_attributes(:password => "", :old_password => "")
-        assert_equal(User.sha1("67890"), @user.password_hash)
+        assert(@user.bcrypt_password == User.sha1("67890"))
       end
       
       should "not change the password if the old password is incorrect" do
         @user = FactoryGirl.create(:user, :password => "67890")
         @user.update_attributes(:password => "12345", :old_password => "abcdefg")
-        assert_equal(User.sha1("67890"), @user.password_hash)
+        assert(@user.bcrypt_password == User.sha1("67890"))
       end
       
       should "not change the password if the old password is blank" do
         @user = FactoryGirl.create(:user, :password => "67890")
         @user.update_attributes(:password => "12345", :old_password => "")
-        assert_equal(User.sha1("67890"), @user.password_hash)
+        assert(@user.bcrypt_password == User.sha1("67890"))
       end
       
       should "change the password if the old password is correct" do
         @user = FactoryGirl.create(:user, :password => "67890")
         @user.update_attributes(:password => "12345", :old_password => "67890")
-        assert_equal(User.sha1("12345"), @user.password_hash)
+        assert(@user.bcrypt_password == User.sha1("12345"))
       end
       
       context "in the json representation" do