From 2ebc263278761b164de927a2db45e71bddd8f603 Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Wed, 25 Jul 2018 15:51:47 -0700 Subject: [PATCH] Support bcrypt password hashing in webrick This adds a password_hash keyword argument to WEBrick::HTTPAuth::Htpasswd#initialize. If set to :bcrypt, it will create bcrypt hashes instead of crypt hashes, and will raise an exception if the .htpasswd file uses crypt hashes. If :bcrypt is used, then instead of calling BasicAuth.make_passwd (which uses crypt), WEBrick::HTTPAuth::Htpasswd#set_passwd will set the bcrypt password directly. It isn't possible to change the make_passwd API to accept the password hash format, as that would break configurations who use Htpasswd#auth_type= to set a custom auth_type. This modifies WEBrick::HTTPAuth::BasicAuth to handle checking both crypt and bcrypt hashes. There are commented out requires for 'string/crypt', to handle when String#crypt is deprecated and the undeprecated version is moved to a gem. There is also a commented out warning for the case when the password_hash keyword is not specified and 'string/crypt' cannot be required. I think the warning makes sense to nudge users to using bcrypt. I've updated the tests to test nil, :crypt, and :bcrypt values for the password_hash keyword, skipping the bcrypt tests if the bcrypt library cannot be required. --- lib/webrick/httpauth/basicauth.rb | 12 +- lib/webrick/httpauth/htpasswd.rb | 39 ++++++- test/webrick/test_httpauth.rb | 175 ++++++++++++++++-------------- 3 files changed, 141 insertions(+), 85 deletions(-) diff --git a/lib/webrick/httpauth/basicauth.rb b/lib/webrick/httpauth/basicauth.rb index e23420fdfa..751885bc3e 100644 --- a/lib/webrick/httpauth/basicauth.rb +++ b/lib/webrick/httpauth/basicauth.rb @@ -24,7 +24,7 @@ module HTTPAuth # # config = { :Realm => 'BasicAuth example realm' } # - # htpasswd = WEBrick::HTTPAuth::Htpasswd.new 'my_password_file' + # htpasswd = WEBrick::HTTPAuth::Htpasswd.new 'my_password_file', password_hash: :bcrypt # htpasswd.set_passwd config[:Realm], 'username', 'password' # htpasswd.flush # @@ -81,7 +81,15 @@ def authenticate(req, res) error("%s: the user is not allowed.", userid) challenge(req, res) end - if password.crypt(encpass) != encpass + + case encpass + when /\A\$2[aby]\$/ + password_matches = BCrypt::Password.new(encpass.sub(/\A\$2[aby]\$/, '$2a$')) == password + else + password_matches = password.crypt(encpass) == encpass + end + + unless password_matches error("%s: password unmatch.", userid) challenge(req, res) end diff --git a/lib/webrick/httpauth/htpasswd.rb b/lib/webrick/httpauth/htpasswd.rb index 976eeeb13e..30e9432267 100644 --- a/lib/webrick/httpauth/htpasswd.rb +++ b/lib/webrick/httpauth/htpasswd.rb @@ -35,11 +35,29 @@ class Htpasswd ## # Open a password database at +path+ - def initialize(path) + def initialize(path, password_hash: nil) @path = path @mtime = Time.at(0) @passwd = Hash.new @auth_type = BasicAuth + @password_hash = password_hash + + case @password_hash + when nil + # begin + # require "string/crypt" + # rescue LoadError + # warn("Unable to load string/crypt, proceeding with deprecated use of String#crypt, consider using password_hash: :bcrypt") + # end + @password_hash = :crypt + when :crypt + # require "string/crypt" + when :bcrypt + require "bcrypt" + else + raise ArgumentError, "only :crypt and :bcrypt are supported for password_hash keyword argument" + end + File.open(@path,"a").close unless File.exist?(@path) reload end @@ -55,7 +73,15 @@ def reload while line = io.gets line.chomp! case line - when %r!\A[^:]+:[a-zA-Z0-9./]{13}\z! + when %r!\A[^:]+:[a-zA-Z0-9./]{13}\z! + if @password_hash == :bcrypt + raise StandardError, ".htpasswd file contains crypt password, only bcrypt passwords supported" + end + user, pass = line.split(":") + when %r!\A[^:]+:\$2[aby]\$\d{2}\$.{53}\z! + if @password_hash == :crypt + raise StandardError, ".htpasswd file contains bcrypt password, only crypt passwords supported" + end user, pass = line.split(":") when /:\$/, /:{SHA}/ raise NotImplementedError, @@ -102,7 +128,14 @@ def get_passwd(realm, user, reload_db) # Sets a password in the database for +user+ in +realm+ to +pass+. def set_passwd(realm, user, pass) - @passwd[user] = make_passwd(realm, user, pass) + if @password_hash == :bcrypt + # Cost of 5 to match Apache default, and because the + # bcrypt default of 10 will introduce significant delays + # for every request. + @passwd[user] = BCrypt::Password.create(pass, :cost=>5) + else + @passwd[user] = make_passwd(realm, user, pass) + end end ## diff --git a/test/webrick/test_httpauth.rb b/test/webrick/test_httpauth.rb index 8439be2025..dcf74689e1 100644 --- a/test/webrick/test_httpauth.rb +++ b/test/webrick/test_httpauth.rb @@ -37,56 +37,7 @@ def test_basic_auth } end - def test_basic_auth2 - log_tester = lambda {|log, access_log| - log.reject! {|line| /\A\s*\z/ =~ line } - pats = [ - /ERROR Basic WEBrick's realm: webrick: password unmatch\./, - /ERROR WEBrick::HTTPStatus::Unauthorized/ - ] - pats.each {|pat| - assert(!log.grep(pat).empty?, "webrick log doesn't have expected error: #{pat.inspect}") - log.reject! {|line| pat =~ line } - } - assert_equal([], log) - } - TestWEBrick.start_httpserver({}, log_tester) {|server, addr, port, log| - realm = "WEBrick's realm" - path = "/basic_auth2" - - Tempfile.create("test_webrick_auth") {|tmpfile| - tmpfile.close - tmp_pass = WEBrick::HTTPAuth::Htpasswd.new(tmpfile.path) - tmp_pass.set_passwd(realm, "webrick", "supersecretpassword") - tmp_pass.set_passwd(realm, "foo", "supersecretpassword") - tmp_pass.flush - - htpasswd = WEBrick::HTTPAuth::Htpasswd.new(tmpfile.path) - users = [] - htpasswd.each{|user, pass| users << user } - assert_equal(2, users.size, log.call) - assert(users.member?("webrick"), log.call) - assert(users.member?("foo"), log.call) - - server.mount_proc(path){|req, res| - auth = WEBrick::HTTPAuth::BasicAuth.new( - :Realm => realm, :UserDB => htpasswd, - :Logger => server.logger - ) - auth.authenticate(req, res) - res.body = "hoge" - } - http = Net::HTTP.new(addr, port) - g = Net::HTTP::Get.new(path) - g.basic_auth("webrick", "supersecretpassword") - http.request(g){|res| assert_equal("hoge", res.body, log.call)} - g.basic_auth("webrick", "not super") - http.request(g){|res| assert_not_equal("hoge", res.body, log.call)} - } - } - end - - def test_basic_auth3 + def test_basic_auth_sha Tempfile.create("test_webrick_auth") {|tmpfile| tmpfile.puts("webrick:{SHA}GJYFRpBbdchp595jlh3Bhfmgp8k=") tmpfile.flush @@ -94,7 +45,9 @@ def test_basic_auth3 WEBrick::HTTPAuth::Htpasswd.new(tmpfile.path) } } + end + def test_basic_auth_md5 Tempfile.create("test_webrick_auth") {|tmpfile| tmpfile.puts("webrick:$apr1$IOVMD/..$rmnOSPXr0.wwrLPZHBQZy0") tmpfile.flush @@ -104,40 +57,102 @@ def test_basic_auth3 } end - def test_bad_username_with_control_characters - log_tester = lambda {|log, access_log| - assert_equal(2, log.length) - assert_match(/ERROR Basic WEBrick's realm: foo\\ebar: the user is not allowed./, log[0]) - assert_match(/ERROR WEBrick::HTTPStatus::Unauthorized/, log[1]) - } - TestWEBrick.start_httpserver({}, log_tester) {|server, addr, port, log| - realm = "WEBrick's realm" - path = "/basic_auth" + [nil, :crypt, :bcrypt].each do |hash_algo| + begin + case hash_algo + when :crypt + # require 'string/crypt' + when :bcrypt + require 'bcrypt' + end + rescue LoadError + next + end - Tempfile.create("test_webrick_auth") {|tmpfile| - tmpfile.close - tmp_pass = WEBrick::HTTPAuth::Htpasswd.new(tmpfile.path) - tmp_pass.set_passwd(realm, "webrick", "supersecretpassword") - tmp_pass.set_passwd(realm, "foo", "supersecretpassword") - tmp_pass.flush + define_method(:"test_basic_auth_htpasswd_#{hash_algo}") do + log_tester = lambda {|log, access_log| + log.reject! {|line| /\A\s*\z/ =~ line } + pats = [ + /ERROR Basic WEBrick's realm: webrick: password unmatch\./, + /ERROR WEBrick::HTTPStatus::Unauthorized/ + ] + pats.each {|pat| + assert(!log.grep(pat).empty?, "webrick log doesn't have expected error: #{pat.inspect}") + log.reject! {|line| pat =~ line } + } + assert_equal([], log) + } + TestWEBrick.start_httpserver({}, log_tester) {|server, addr, port, log| + realm = "WEBrick's realm" + path = "/basic_auth2" - htpasswd = WEBrick::HTTPAuth::Htpasswd.new(tmpfile.path) - users = [] - htpasswd.each{|user, pass| users << user } - server.mount_proc(path){|req, res| - auth = WEBrick::HTTPAuth::BasicAuth.new( - :Realm => realm, :UserDB => htpasswd, - :Logger => server.logger - ) - auth.authenticate(req, res) - res.body = "hoge" + Tempfile.create("test_webrick_auth") {|tmpfile| + tmpfile.close + tmp_pass = WEBrick::HTTPAuth::Htpasswd.new(tmpfile.path, password_hash: hash_algo) + tmp_pass.set_passwd(realm, "webrick", "supersecretpassword") + tmp_pass.set_passwd(realm, "foo", "supersecretpassword") + tmp_pass.flush + + htpasswd = WEBrick::HTTPAuth::Htpasswd.new(tmpfile.path, password_hash: hash_algo) + users = [] + htpasswd.each{|user, pass| users << user } + assert_equal(2, users.size, log.call) + assert(users.member?("webrick"), log.call) + assert(users.member?("foo"), log.call) + + server.mount_proc(path){|req, res| + auth = WEBrick::HTTPAuth::BasicAuth.new( + :Realm => realm, :UserDB => htpasswd, + :Logger => server.logger + ) + auth.authenticate(req, res) + res.body = "hoge" + } + http = Net::HTTP.new(addr, port) + g = Net::HTTP::Get.new(path) + g.basic_auth("webrick", "supersecretpassword") + http.request(g){|res| assert_equal("hoge", res.body, log.call)} + g.basic_auth("webrick", "not super") + http.request(g){|res| assert_not_equal("hoge", res.body, log.call)} } - http = Net::HTTP.new(addr, port) - g = Net::HTTP::Get.new(path) - g.basic_auth("foo\ebar", "passwd") - http.request(g){|res| assert_not_equal("hoge", res.body, log.call) } } - } + end + + define_method(:"test_basic_auth_bad_username_htpasswd_#{hash_algo}") do + log_tester = lambda {|log, access_log| + assert_equal(2, log.length) + assert_match(/ERROR Basic WEBrick's realm: foo\\ebar: the user is not allowed./, log[0]) + assert_match(/ERROR WEBrick::HTTPStatus::Unauthorized/, log[1]) + } + TestWEBrick.start_httpserver({}, log_tester) {|server, addr, port, log| + realm = "WEBrick's realm" + path = "/basic_auth" + + Tempfile.create("test_webrick_auth") {|tmpfile| + tmpfile.close + tmp_pass = WEBrick::HTTPAuth::Htpasswd.new(tmpfile.path, password_hash: hash_algo) + tmp_pass.set_passwd(realm, "webrick", "supersecretpassword") + tmp_pass.set_passwd(realm, "foo", "supersecretpassword") + tmp_pass.flush + + htpasswd = WEBrick::HTTPAuth::Htpasswd.new(tmpfile.path, password_hash: hash_algo) + users = [] + htpasswd.each{|user, pass| users << user } + server.mount_proc(path){|req, res| + auth = WEBrick::HTTPAuth::BasicAuth.new( + :Realm => realm, :UserDB => htpasswd, + :Logger => server.logger + ) + auth.authenticate(req, res) + res.body = "hoge" + } + http = Net::HTTP.new(addr, port) + g = Net::HTTP::Get.new(path) + g.basic_auth("foo\ebar", "passwd") + http.request(g){|res| assert_not_equal("hoge", res.body, log.call) } + } + } + end end DIGESTRES_ = / -- 2.17.1