From 15d29c646b545b65bbedeed1f49329b9292671ba Mon Sep 17 00:00:00 2001
From: Tom Hughes <tom@compton.nu>
Date: Tue, 13 Aug 2013 23:07:41 +0100
Subject: [PATCH] Strengthen password hashing algorithm

---
 app/models/user.rb              |  5 ++---
 lib/osm.rb                      |  7 ------
 lib/password_hash.rb            | 39 +++++++++++++++++++++++++++++++++
 test/unit/password_hash_test.rb | 25 +++++++++++++++++++++
 4 files changed, 66 insertions(+), 10 deletions(-)
 create mode 100644 lib/password_hash.rb
 create mode 100644 test/unit/password_hash_test.rb

diff --git a/app/models/user.rb b/app/models/user.rb
index 6677d3b98..4c51089e6 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -70,7 +70,7 @@ class User < ActiveRecord::Base
         end
       end
 
-      user = nil if user and user.pass_crypt != OSM::encrypt_password(options[:password], user.pass_salt)
+      user = nil if user and not PasswordHash.check(user.pass_crypt, user.pass_salt, options[:password])
     elsif options[:token]
       token = UserToken.find_by_token(options[:token])
       user = token.user if token
@@ -240,8 +240,7 @@ private
 
   def encrypt_password
     if pass_crypt_confirmation
-      self.pass_salt = OSM::make_token(8)
-      self.pass_crypt = OSM::encrypt_password(pass_crypt, pass_salt)
+      self.pass_crypt, self.pass_salt = PasswordHash.create(pass_crypt)
       self.pass_crypt_confirmation = nil
     end
   end
diff --git a/lib/osm.rb b/lib/osm.rb
index 4a6237b12..2ed98b92d 100644
--- a/lib/osm.rb
+++ b/lib/osm.rb
@@ -5,7 +5,6 @@ module OSM
   require 'rexml/parsers/sax2parser'
   require 'rexml/text'
   require 'xml/libxml'
-  require 'digest/md5'
 
   if defined?(SystemTimer)
     Timer = SystemTimer
@@ -569,12 +568,6 @@ module OSM
     return token
   end
 
-  # Return an encrypted version of a password
-  def self.encrypt_password(password, salt)
-    return Digest::MD5.hexdigest(password) if salt.nil?
-    return Digest::MD5.hexdigest(salt + password)
-  end
-
   # Return an SQL fragment to select a given area of the globe
   def self.sql_for_area(bbox, prefix = nil)
     tilesql = QuadTile.sql_for_area(bbox, prefix)
diff --git a/lib/password_hash.rb b/lib/password_hash.rb
new file mode 100644
index 000000000..1bd80291a
--- /dev/null
+++ b/lib/password_hash.rb
@@ -0,0 +1,39 @@
+require "securerandom"
+require "openssl"
+require "base64"
+require "digest/md5"
+
+module PasswordHash
+  SALT_BYTE_SIZE = 32
+  HASH_BYTE_SIZE = 32
+  PBKDF2_ITERATIONS = 1000
+  DIGEST_ALGORITHM = "sha512"
+
+  def self.create(password)
+    salt = SecureRandom.base64(SALT_BYTE_SIZE)
+    hash = self.hash(password, salt, PBKDF2_ITERATIONS, HASH_BYTE_SIZE, DIGEST_ALGORITHM)
+    return hash, [DIGEST_ALGORITHM, PBKDF2_ITERATIONS, salt].join("!")
+  end
+
+  def self.check(hash, salt, candidate)
+    if salt.nil?
+      candidate = Digest::MD5.hexdigest(candidate)
+    elsif salt =~ /!/
+      algorithm, iterations, salt = salt.split("!")
+      size = Base64.strict_decode64(hash).length
+      candidate = self.hash(candidate, salt, iterations.to_i, size, algorithm)
+    else
+      candidate = Digest::MD5.hexdigest(salt + candidate)
+    end
+
+    return hash == candidate
+  end
+
+private
+
+  def self.hash(password, salt, iterations, size, algorithm)
+    digest = OpenSSL::Digest.new(algorithm)
+    pbkdf2 = OpenSSL::PKCS5::pbkdf2_hmac(password, salt, iterations, size, digest)
+    Base64.strict_encode64(pbkdf2)
+  end
+end
diff --git a/test/unit/password_hash_test.rb b/test/unit/password_hash_test.rb
new file mode 100644
index 000000000..61d3d4921
--- /dev/null
+++ b/test/unit/password_hash_test.rb
@@ -0,0 +1,25 @@
+require File.dirname(__FILE__) + '/../test_helper'
+
+class PasswordHashTest < ActiveSupport::TestCase
+  def test_md5_without_salt
+    assert_equal true, PasswordHash.check("5f4dcc3b5aa765d61d8327deb882cf99", nil, "password")
+    assert_equal false, PasswordHash.check("5f4dcc3b5aa765d61d8327deb882cf99", nil, "wrong")
+  end
+
+  def test_md5_with_salt
+    assert_equal true, PasswordHash.check("67a1e09bb1f83f5007dc119c14d663aa", "salt", "password")
+    assert_equal false, PasswordHash.check("67a1e09bb1f83f5007dc119c14d663aa", "salt", "wrong")
+    assert_equal false, PasswordHash.check("67a1e09bb1f83f5007dc119c14d663aa", "wrong", "password")
+  end
+
+  def test_default
+    hash1, salt1 = PasswordHash.create("password")
+    hash2, salt2 = PasswordHash.create("password")
+    assert_not_equal hash1, hash2
+    assert_not_equal salt1, salt2
+    assert_equal true, PasswordHash.check(hash1, salt1, "password")
+    assert_equal false, PasswordHash.check(hash1, salt1, "wrong")
+    assert_equal true, PasswordHash.check(hash2, salt2, "password")
+    assert_equal false, PasswordHash.check(hash2, salt2, "wrong")
+  end
+end
-- 
2.39.5