Password migration/upgrade

NOTE: All existing passwords in development no longer work after
this change! Change your users password using the rails console.

Automatically convert and ugrade old passwords to using bcrypt
Removed the seemingly pointless transformation and hashing on top
of the actual password with a static salt.
Disabled logging in using password hashes, because that's just not
secure in any way, and negates cracking passwords at all.
Disabled sending the password hash to the client as a cookie, even
if it was signed.
Disabled legacy API logins.

app/controllers/maintenance/user/deletions_controller.rb

@@ -8,8 +8,6 @@ module Maintenance
         deletion = UserDeletion.new(CurrentUser.user, params[:password])
         deletion.delete!
         session.delete(:user_id)
-        cookies.delete(:cookie_password_hash)
-        cookies.delete(:user_name)
         redirect_to(posts_path, :notice => "You are now logged out")
       end
     end

app/controllers/sessions_controller.rb

@@ -16,8 +16,6 @@ class SessionsController < ApplicationController
 
   def destroy
     session.delete(:user_id)
-    cookies.delete(:user_name)
-    cookies.delete(:password_hash)
     redirect_to(posts_path, :notice => "You are now logged out.")
   end
 

app/logical/pbkdf2.rb

@@ -0,0 +1,80 @@
+# Password Hashing With PBKDF2 (http://crackstation.net/hashing-security.htm).
+# Copyright (c) 2013, Taylor Hornby
+# All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are met:
+#
+# 1. Redistributions of source code must retain the above copyright notice,
+# this list of conditions and the following disclaimer.
+#
+# 2. Redistributions in binary form must reproduce the above copyright notice,
+# this list of conditions and the following disclaimer in the documentation
+# and/or other materials provided with the distribution.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+# ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
+# LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+# CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+# SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+# ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+# POSSIBILITY OF SUCH DAMAGE.
+
+require 'securerandom'
+require 'openssl'
+require 'base64'
+
+module PBKDF2
+
+  PBKDF2_ITERATIONS = 20000
+  SALT_BYTE_SIZE = 24
+  HASH_BYTE_SIZE = 24
+
+  HASH_SECTIONS = 4
+  SECTION_DELIMITER = ':'
+  ITERATIONS_INDEX = 1
+  SALT_INDEX = 2
+  HASH_INDEX = 3
+
+  def self.create_hash( password )
+    salt = SecureRandom.base64( SALT_BYTE_SIZE )
+    pbkdf2 = OpenSSL::PKCS5::pbkdf2_hmac_sha1(
+        password,
+        salt,
+        PBKDF2_ITERATIONS,
+        HASH_BYTE_SIZE
+    )
+    return ["sha1", PBKDF2_ITERATIONS, salt, Base64.encode64( pbkdf2 )].join( SECTION_DELIMITER )
+  end
+
+  def self.validate_password( password, correctHash )
+    params = correctHash.split( SECTION_DELIMITER )
+    return false if params.length != HASH_SECTIONS
+
+    pbkdf2 = Base64.decode64( params[HASH_INDEX] )
+    testHash = OpenSSL::PKCS5::pbkdf2_hmac_sha1(
+        password,
+        params[SALT_INDEX],
+        params[ITERATIONS_INDEX].to_i,
+        pbkdf2.length
+    )
+
+    return pbkdf2 == testHash
+  end
+
+  def self.needs_upgrade( hash )
+    params = hash.split( SECTION_DELIMITER )
+    if params.length != HASH_SECTIONS
+      return true
+    end
+    if params[ITERATIONS_INDEX] != PBKDF2_ITERATIONS.to_s
+      return true
+    end
+    return false
+  end
+
+end

app/logical/session_creator.rb

@@ -15,19 +15,6 @@ class SessionCreator
     if User.authenticate(name, password)
       user = User.find_by_name(name)
 
-      if remember.present?
-        cookies.permanent.signed[:user_name] = {
-          :value => user.name,
-          :secure => secure,
-          :httponly => true
-        }
-        cookies.permanent[:password_hash] = {
-          :value => user.bcrypt_cookie_password_hash,
-          :secure => secure,
-          :httponly => true
-        }
-      end
-
       session[:user_id] = user.id
       user.update_column(:last_ip_addr, ip_addr)
       return true

app/logical/session_loader.rb

@@ -18,8 +18,6 @@ class SessionLoader
       load_for_test(Thread.current[:test_user_id])
     elsif session[:user_id]
       load_session_user
-    elsif cookie_password_hash_valid?
-      load_cookie_user
     else
       load_session_for_api
     end
@@ -46,12 +44,8 @@ private
   def load_session_for_api
     if request.authorization
       authenticate_basic_auth
-      
     elsif params[:login].present? && params[:api_key].present?
       authenticate_api_key(params[:login], params[:api_key])
-      
-    elsif params[:login].present? && params[:password_hash].present?
-      authenticate_legacy_api_key(params[:login], params[:password_hash])
     end
   end
   
@@ -68,29 +62,12 @@ private
       raise AuthenticationFailure.new
     end
   end
-  
-  def authenticate_legacy_api_key(name, password_hash)
-    CurrentUser.user = User.authenticate_hash(name, password_hash)
-
-    if CurrentUser.user.nil?
-      raise AuthenticationFailure.new
-    end
-  end
 
   def load_session_user
     user = User.find_by_id(session[:user_id])
     CurrentUser.user = user if user
   end
 
-  def load_cookie_user
-    CurrentUser.user = User.find_by_name(cookies.signed[:user_name])
-    session[:user_id] = CurrentUser.user.id
-  end
-
-  def cookie_password_hash_valid?
-    cookies[:password_hash] && cookies.signed[:user_name] && User.authenticate_cookie_hash(cookies.signed[:user_name], cookies[:password_hash])
-  end
-
   def update_last_logged_in_at
     return if CurrentUser.is_anonymous?
     return if CurrentUser.last_logged_in_at && CurrentUser.last_logged_in_at > 1.week.ago

app/models/user.rb

@@ -176,10 +176,6 @@ class User < ApplicationRecord
       BCrypt::Password.new(bcrypt_password_hash)
     end
 
-    def bcrypt_cookie_password_hash
-      bcrypt_password_hash.slice(20, 100)
-    end
-
     def encrypt_password_on_create
       self.password_hash = ""
       self.bcrypt_password_hash = User.bcrypt(password)
@@ -189,7 +185,7 @@ class User < ApplicationRecord
       return if password.blank?
       return if old_password.blank?
 
-      if bcrypt_password == User.sha1(old_password)
+      if bcrypt_password == old_password
         self.bcrypt_password_hash = User.bcrypt(password)
         return true
       else
@@ -198,6 +194,10 @@ class User < ApplicationRecord
       end
     end
 
+    def upgrade_password(pass)
+      self.update_columns(password_hash: '', bcrypt_password_hash: User.bcrypt(pass))
+    end
+
     def reset_password
       consonants = "bcdfghjklmnpqrstvqxyz"
       vowels = "aeiou"
@@ -224,7 +224,15 @@ class User < ApplicationRecord
 
     module ClassMethods
       def authenticate(name, pass)
-        authenticate_hash(name, sha1(pass))
+        user = find_by_name(name)
+        if user && user.password_hash && PBKDF2.validate_password(pass, user.password_hash)
+          user.upgrade_password(pass)
+          user
+        elsif user && user.bcrypt_password_hash && user.bcrypt_password == pass
+          user
+        else
+          nil
+        end
       end
 
       def authenticate_api_key(name, api_key)
@@ -236,30 +244,8 @@ class User < ApplicationRecord
         nil
       end
 
-      def authenticate_hash(name, hash)
-        user = find_by_name(name)
-        if user && user.bcrypt_password == hash
-          user
-        else
-          nil
-        end
-      end
-
-      def authenticate_cookie_hash(name, hash)
-        user = find_by_name(name)
-        if user && user.bcrypt_cookie_password_hash == 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}--")
+        BCrypt::Password.create(pass)
       end
     end
   end

test/factories/user.rb

@@ -4,7 +4,7 @@ FactoryBot.define do
       "user#{n}"
     end
     password "password"
-    password_hash {User.sha1("password")}
+    password_hash {PasswordHash.create_hash("password")}
     email {FFaker::Internet.email}
     default_image_size "large"
     base_upload_limit 10

test/functional/posts_controller_test.rb

@@ -56,18 +56,6 @@ class PostsControllerTest < ActionDispatch::IntegrationTest
           assert_response 401
         end
       end
-      
-      context "using the password_hash parameter" do
-        should "succeed for password matches" do
-          get posts_path, params: {:format => "json", :login => @user.name, :password_hash => User.sha1("password")}
-          assert_response :success
-        end
-        
-        # should "fail for password mismatches" do
-        #   get posts_path, {:format => "json", :login => @user.name, :password_hash => "bad"}
-        #   assert_response 403
-        # end
-      end
     end
 
     context "index action" do

test/unit/user_test.rb

@@ -192,15 +192,6 @@ class UserTest < ActiveSupport::TestCase
     end
 
     context "password" do
-      should "match the cookie hash" do
-        @user = FactoryBot.create(:user)
-        @user.password = "zugzug5"
-        @user.password_confirmation = "zugzug5"
-        @user.save
-        @user.reload
-        assert(User.authenticate_cookie_hash(@user.name, @user.bcrypt_cookie_password_hash))
-      end
-
       should "match the confirmation" do
         @user = FactoryBot.create(:user)
         @user.old_password = "password"