ruby-changes:51846
From: normal <ko1@a...>
Date: Thu, 26 Jul 2018 12:22:09 +0900 (JST)
Subject: [ruby-changes:51846] normal:r64060 (trunk): webrick: Support bcrypt password hashing
normal 2018-07-26 12:21:52 +0900 (Thu, 26 Jul 2018) New Revision: 64060 https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=64060 Log: webrick: Support bcrypt password hashing 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. [ruby-core:88111] [Feature #14940] From: Jeremy Evans <code@j...> Modified files: trunk/lib/webrick/httpauth/basicauth.rb trunk/lib/webrick/httpauth/htpasswd.rb trunk/test/webrick/test_httpauth.rb Index: test/webrick/test_httpauth.rb =================================================================== --- test/webrick/test_httpauth.rb (revision 64059) +++ test/webrick/test_httpauth.rb (revision 64060) @@ -37,56 +37,7 @@ class TestWEBrickHTTPAuth < Test::Unit:: https://github.com/ruby/ruby/blob/trunk/test/webrick/test_httpauth.rb#L37 } 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 @@ class TestWEBrickHTTPAuth < Test::Unit:: https://github.com/ruby/ruby/blob/trunk/test/webrick/test_httpauth.rb#L45 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 @@ class TestWEBrickHTTPAuth < Test::Unit:: https://github.com/ruby/ruby/blob/trunk/test/webrick/test_httpauth.rb#L57 } 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" - - 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 } - 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" + [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 + + 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 } } - 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) } + 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, 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)} + } + } + 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_ = / Index: lib/webrick/httpauth/htpasswd.rb =================================================================== --- lib/webrick/httpauth/htpasswd.rb (revision 64059) +++ lib/webrick/httpauth/htpasswd.rb (revision 64060) @@ -35,11 +35,29 @@ module WEBrick https://github.com/ruby/ruby/blob/trunk/lib/webrick/httpauth/htpasswd.rb#L35 ## # 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 @@ -56,6 +74,14 @@ module WEBrick https://github.com/ruby/ruby/blob/trunk/lib/webrick/httpauth/htpasswd.rb#L74 line.chomp! case line 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 @@ module WEBrick https://github.com/ruby/ruby/blob/trunk/lib/webrick/httpauth/htpasswd.rb#L128 # 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 ## Index: lib/webrick/httpauth/basicauth.rb =================================================================== --- lib/webrick/httpauth/basicauth.rb (revision 64059) +++ lib/webrick/httpauth/basicauth.rb (revision 64060) @@ -24,7 +24,7 @@ module WEBrick https://github.com/ruby/ruby/blob/trunk/lib/webrick/httpauth/basicauth.rb#L24 # # 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 @@ module WEBrick https://github.com/ruby/ruby/blob/trunk/lib/webrick/httpauth/basicauth.rb#L81 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 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/