From 8b5cdfad7dc61cc5973c171ec4e12e7cd39351e9 Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Wed, 28 Nov 2018 20:44:58 -0800 Subject: [PATCH] Deprecate String#crypt This deprecates String#crypt, with a verbose warning message pointing users to the string-crypt gem. --- string.c | 12 ++++++++++-- test/ruby/test_m17n_comb.rb | 12 +++++++++--- test/ruby/test_string.rb | 25 ++++++++++++++++--------- test/webrick/test_httpauth.rb | 32 +++++++++++++++++++++++--------- 4 files changed, 58 insertions(+), 23 deletions(-) diff --git a/string.c b/string.c index 184d7df450..aaccf7754c 100644 --- a/string.c +++ b/string.c @@ -9228,17 +9228,25 @@ rb_str_oct(VALUE str) * salt string. While the format and the result are system and * implementation dependent, using a salt matching the regular * expression \A[a-zA-Z0-9./]{2} should be valid and - * safe on any platform, in which only the first two characters are - * significant. + * safe on most platforms, in which only the first two characters are + * significant. However, this uses DES-crypt, an insecure form of + * password hashing. * * This method is for use in system specific scripts, so if you want * a cross-platform hash function consider using Digest or OpenSSL * instead. + * + * This method is deprecated, install the string-crypt gem and + * require 'string/crypt' to continue using it. */ static VALUE rb_str_crypt(VALUE str, VALUE salt) { + rb_warning("The String#crypt method is deprecated. " \ + "Install the string-crypt gem and require \"string/crypt\" " \ + "to continue using String#crypt."); + #ifdef HAVE_CRYPT_R VALUE databuf; struct crypt_data *data; diff --git a/test/ruby/test_m17n_comb.rb b/test/ruby/test_m17n_comb.rb index 99c162a92f..f6a3a5c044 100644 --- a/test/ruby/test_m17n_comb.rb +++ b/test/ruby/test_m17n_comb.rb @@ -771,13 +771,19 @@ def test_str_crypt_nonstrict end end + private def crypt_result(str, salt) + assert_warning(/The String#crypt core method is deprecated/) do + str.crypt(salt) + end + end + private def confirm_crypt_result(str, salt) if b(salt).length < 2 - assert_raise(ArgumentError) { str.crypt(salt) } + assert_raise(ArgumentError) { crypt_result(str, salt) } return end - t = str.crypt(salt) - assert_equal(b(str).crypt(b(salt)), t, "#{encdump(str)}.crypt(#{encdump(salt)})") + t = crypt_result(str, salt) + assert_equal(crypt_result(b(str), b(salt)), t, "#{encdump(str)}.crypt(#{encdump(salt)})") assert_encoding('ASCII-8BIT', t.encoding) end diff --git a/test/ruby/test_string.rb b/test/ruby/test_string.rb index ffae04bc21..9dacff6dec 100644 --- a/test/ruby/test_string.rb +++ b/test/ruby/test_string.rb @@ -646,23 +646,30 @@ def test_count assert_raise(ArgumentError) { "foo".count } end + def crypt(str, salt) + assert_warning(/The String#crypt core method is deprecated/) do + str.crypt(salt) + end + end + def test_crypt - assert_equal(S('aaGUC/JkO9/Sc'), S("mypassword").crypt(S("aa"))) - assert_not_equal(S('aaGUC/JkO9/Sc'), S("mypassword").crypt(S("ab"))) - assert_raise(ArgumentError) {S("mypassword").crypt(S(""))} - assert_raise(ArgumentError) {S("mypassword").crypt(S("\0a"))} - assert_raise(ArgumentError) {S("mypassword").crypt(S("a\0"))} - assert_raise(ArgumentError) {S("poison\u0000null").crypt(S("aa"))} + assert_equal(S('aaGUC/JkO9/Sc'), crypt(S("mypassword"), S("aa"))) + assert_not_equal(S('aaGUC/JkO9/Sc'), crypt(S("mypassword"), S("ab"))) + assert_raise(ArgumentError) {crypt(S("mypassword"), S(""))} + assert_raise(ArgumentError) {crypt(S("mypassword"), S("\0a"))} + assert_raise(ArgumentError) {crypt(S("mypassword"), S("a\0"))} + assert_raise(ArgumentError) {crypt(S("poison\u0000null"), S("aa"))} + [Encoding::UTF_16BE, Encoding::UTF_16LE, Encoding::UTF_32BE, Encoding::UTF_32LE].each do |enc| - assert_raise(ArgumentError) {S("mypassword").crypt(S("aa".encode(enc)))} - assert_raise(ArgumentError) {S("mypassword".encode(enc)).crypt(S("aa"))} + assert_raise(ArgumentError) {crypt(S("mypassword"), S("aa".encode(enc)))} + assert_raise(ArgumentError) {crypt(S("mypassword".encode(enc)), S("aa"))} end @cls == String and assert_no_memory_leak([], 's = ""', "#{<<~"begin;"}\n#{<<~'end;'}") begin; - 1000.times { s.crypt(-"..").clear } + 1000.times { crypt(s, -"..").clear } end; end diff --git a/test/webrick/test_httpauth.rb b/test/webrick/test_httpauth.rb index 0b2ba4b88f..ceafcac0a2 100644 --- a/test/webrick/test_httpauth.rb +++ b/test/webrick/test_httpauth.rb @@ -69,6 +69,12 @@ def test_basic_auth_md5 next end + if hash_algo == :bcrypt + warning = /\A\z/ + else + warning = /The String#crypt core method is deprecated/ + end + define_method(:"test_basic_auth_htpasswd_#{hash_algo}") do log_tester = lambda {|log, access_log| log.reject! {|line| /\A\s*\z/ =~ line } @@ -89,8 +95,10 @@ def test_basic_auth_md5 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") + assert_warning(warning) do + tmp_pass.set_passwd(realm, "webrick", "supersecretpassword") + tmp_pass.set_passwd(realm, "foo", "supersecretpassword") + end tmp_pass.flush htpasswd = WEBrick::HTTPAuth::Htpasswd.new(tmpfile.path, password_hash: hash_algo) @@ -110,10 +118,12 @@ def test_basic_auth_md5 } 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)} + assert_warning(warning) do + 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 } } end @@ -131,8 +141,10 @@ def test_basic_auth_md5 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") + assert_warning(warning) do + tmp_pass.set_passwd(realm, "webrick", "supersecretpassword") + tmp_pass.set_passwd(realm, "foo", "supersecretpassword") + end tmp_pass.flush htpasswd = WEBrick::HTTPAuth::Htpasswd.new(tmpfile.path, password_hash: hash_algo) @@ -148,7 +160,9 @@ def test_basic_auth_md5 } http = Net::HTTP.new(addr, port) g = Net::HTTP::Get.new(path) - g.basic_auth("foo\ebar", "passwd") + assert_warning(warning) do + g.basic_auth("foo\ebar", "passwd") + end http.request(g){|res| assert_not_equal("hoge", res.body, log.call) } } } -- 2.19.1