Project

General

Profile

Actions

Feature #14915

closed

Deprecate String#crypt

Added by jeremyevans0 (Jeremy Evans) over 6 years ago. Updated over 5 years ago.

Status:
Rejected
Assignee:
-
Target version:
-
[ruby-core:87959]

Description

This method is system and implementation dependent, and the
portable usage mentioned in the documentation is not truly
portable (doesn't work on OpenBSD) and insecure as it uses DES.
For systems that lack a crypt(3) implementation, Ruby will
happily substitute a version that only supports DES. It's 2018,
using DES should be avoided if at all possible.

The only internal usage of String#crypt in Ruby is in Webrick,
where it uses DES for basic authentication with an htpasswd file.
That could and should be changed to use a more secure hash by
default (bcrypt since that's the most secure htpasswd format),
or at least allow the user to customize Webrick's authentication.
I expect there are few if any users actively using Webrick's
htpasswd support.

This moves the String#crypt implementation to the string/crypt
extension, but leaves the String#crypt core method. The core
method prints a deprecation warning, then loads the string/crypt
extension. The string/crypt extension undefines the String#crypt
core method, then defines the previous implementation.

Because extensions use extconf.rb instead of configure for their
configuration, this ports the related configure.ac code to
extconf.rb. I'm not sure that is done correctly and works on
all platforms, it will need testing.

For systems that lack a crypt(3) implementation, this modifies the
fallback code to only define crypt_r, since that is the only
function that String#crypt will call in that case.

While the patch just deprecates String#crypt, I think
we should plan to remove support from ruby:

2.6: core method deprecated
2.7: core method removed, string/crypt extension ships with ruby
2.8: string/crypt extension moves to external gem, not shipped


Files

0001-Deprecate-String-crypt-move-implementation-to-string.patch (20.5 KB) 0001-Deprecate-String-crypt-move-implementation-to-string.patch jeremyevans0 (Jeremy Evans), 07/16/2018 05:47 PM
0001-Deprecate-String-crypt.patch (7.48 KB) 0001-Deprecate-String-crypt.patch jeremyevans0 (Jeremy Evans), 07/26/2018 07:42 PM
0001-Deprecate-String-crypt.patch (7.35 KB) 0001-Deprecate-String-crypt.patch jeremyevans0 (Jeremy Evans), 11/29/2018 08:27 AM
0001-Deprecate-String-crypt.patch (7.43 KB) 0001-Deprecate-String-crypt.patch jeremyevans0 (Jeremy Evans), 11/29/2018 03:20 PM
deprecate-string-crypt.patch (6.7 KB) deprecate-string-crypt.patch jeremyevans0 (Jeremy Evans), 05/18/2019 03:06 PM

Related issues 1 (0 open1 closed)

Related to Ruby master - Feature #14940: Support bcrypt password hashing in webrickClosednormalperson (Eric Wong)Actions

Updated by normalperson (Eric Wong) over 6 years ago

wrote:

Because extensions use extconf.rb instead of configure for their
configuration, this ports the related configure.ac code to
extconf.rb. I'm not sure that is done correctly and works on
all platforms, it will need testing.

More exts increase build and maintenance time. Right now, the
easiest and safest step would be to only deprecate it, but
keep it in core.

While I don't care for #crypt, I'd like to move some tiny exts
like fiber, io/wait, io/nonblock directly into core; because
DSOs increase memory usage and slow down startup:

https://udrepper.livejournal.com/8790.html

While the patch just deprecates String#crypt, I think
we should plan to remove support from ruby:

2.6: core method deprecated
2.7: core method removed, string/crypt extension ships with ruby
2.8: string/crypt extension moves to external gem, not shipped

Way too fast. I agree with deprecation, but any removal should
take long-term distro release cycles (5-10 years) into account.

Updated by jeremyevans0 (Jeremy Evans) over 6 years ago

normalperson (Eric Wong) wrote:

wrote:

Because extensions use extconf.rb instead of configure for their
configuration, this ports the related configure.ac code to
extconf.rb. I'm not sure that is done correctly and works on
all platforms, it will need testing.

More exts increase build and maintenance time. Right now, the
easiest and safest step would be to only deprecate it, but
keep it in core.

I agree that would be easiest and safest. However, at some point it should be moved out, and I think it's better to start that process early.

While I don't care for #crypt, I'd like to move some tiny exts
like fiber, io/wait, io/nonblock directly into core; because
DSOs increase memory usage and slow down startup:

https://udrepper.livejournal.com/8790.html

Note that DSOs only increase memory usage and slow down startup if they are actually used. This would increase memory usage and slow down startup for users of String#crypt. However, this would decrease memory usage and speed up startup for the 99.9%+ of ruby users that do not use String#crypt, since it would result in a smaller libruby.

FWIW, I'm fine including fiber, io/wait, and io/nonblock directly in core, because those extensions are small and the functionality they provide is useful in a large number of situations.

While the patch just deprecates String#crypt, I think
we should plan to remove support from ruby:

2.6: core method deprecated
2.7: core method removed, string/crypt extension ships with ruby
2.8: string/crypt extension moves to external gem, not shipped

Way too fast. I agree with deprecation, but any removal should
take long-term distro release cycles (5-10 years) into account.

That's fine. In the patch I don't specify it will be removed, just that it is deprecated.

Note that not removing String#crypt also has risks. Mainly it risks unsuspecting users using it without understanding that doesn't really provide security. I think we are doing a disservice to those users by basically promoting an insecure approach. Most code that would be broken by the removal of String#crypt is insecure and should probably be updated.

Updated by shyouhei (Shyouhei Urabe) over 6 years ago

I agree that crypt(3) is too difficult to use correctly. Properly avoiding traditional modes to generate adequate hash are up to users, which is not how we do security these days. OpenBSD splits the functionality into crypt_checkpass(3) and crypt_newhash(3), which are far better (meseems).

Said that, Ruby's String#crypt has been there for a while and we should expect wild applications. You have to carefully watch your step for its removal.

normalperson (Eric Wong) wrote:

While I don't care for #crypt, I'd like to move some tiny exts
like fiber, io/wait, io/nonblock directly into core; because
DSOs increase memory usage and slow down startup:

For String#crypt, because crypt(3) typically resides in libc, no extra DSO shall be necessary by theory. How about call it from pure ruby (maybe using fiddle)?

Updated by jeremyevans0 (Jeremy Evans) over 6 years ago

shyouhei (Shyouhei Urabe) wrote:

I agree that crypt(3) is too difficult to use correctly. Properly avoiding traditional modes to generate adequate hash are up to users, which is not how we do security these days. OpenBSD splits the functionality into crypt_checkpass(3) and crypt_newhash(3), which are far better (meseems).

Said that, Ruby's String#crypt has been there for a while and we should expect wild applications. You have to carefully watch your step for its removal.

Definitely. I don't think we should remove String#crypt without deprecating it. It's mostly a question of how long the deprecation period should be. Maybe 1 year is too short. However, I think 10 years is definitely too long.

normalperson (Eric Wong) wrote:

While I don't care for #crypt, I'd like to move some tiny exts
like fiber, io/wait, io/nonblock directly into core; because
DSOs increase memory usage and slow down startup:

For String#crypt, because crypt(3) typically resides in libc, no extra DSO shall be necessary by theory.

I think Eric means that if we move String#crypt to string/crypt, that's an extra string/crypt.so DSO you have to load if you use String#crypt.

How about call it from pure ruby (maybe using fiddle)?

String#crypt can be easily emulated using fiddle if crypt is in libc:

require 'fiddle'
libc = Fiddle.dlopen('/path/to/libc.so')
crypt = Fiddle::Function.new(
  libc['crypt'],
  [Fiddle::TYPE_VOIDP, Fiddle::TYPE_VOIDP],
  Fiddle::TYPE_VOIDP
)
crypt.call('password', 'sl').to_s

It's more work if crypt_r is used, though.

Updated by mame (Yusuke Endoh) over 6 years ago

I agree with deprecation and removal of String#crypt. If it were not included in the core, and if it were proposed today, the proposal would be definitely rejected as "it should be implemented as a gem".

This moves the String#crypt implementation to the string/crypt
extension, but leaves the String#crypt core method. The core
method prints a deprecation warning, then loads the string/crypt
extension.

It looks complex a bit to me, and I don't see its advantage. How about:

2.6: core method deprecated, and a compatibility-layer gem released
3.0?: core method removed

?

Unfortunately, the name "crypt" is already taken, though...

Incidentally, I found some usages of String#crypt in some gems.

https://github.com/eventmachine/eventmachine/blob/537127fa22a058e3d7b248a214df210e7c77dc95/lib/em/protocols/postgres3.rb#L187
https://github.com/activeldap/activeldap/blob/a63e89c384cf7f5bb95451a3749e6dc4aa799969/lib/active_ldap/user_password.rb#L38

Updated by normalperson (Eric Wong) over 6 years ago

wrote:

normalperson (Eric Wong) wrote:

While I don't care for #crypt, I'd like to move some tiny exts
like fiber, io/wait, io/nonblock directly into core; because
DSOs increase memory usage and slow down startup:

https://udrepper.livejournal.com/8790.html

Note that DSOs only increase memory usage and slow down
startup if they are actually used. This would increase memory
usage and slow down startup for users of String#crypt.
However, this would decrease memory usage and speed up startup
for the 99.9%+ of ruby users that do not use String#crypt,
since it would result in a smaller libruby.

Problem is right now we have no idea how many people use
String#crypt and how they will be affected by its deprecation.
It will take several years to know that, as we have seen
regressions which don't get reported for several releases
because distros (and users) are slow to upgrade.

Even with deprecation warnings, it can be too annoying to some
regular users who don't write code and just run some Ruby apps.

There's a lot of stuff I'd remove from Ruby to reduce footprint
and startup times first :)

Note that not removing String#crypt also has risks. Mainly it
risks unsuspecting users using it without understanding that
doesn't really provide security. I think we are doing a
disservice to those users by basically promoting an insecure
approach. Most code that would be broken by the removal of
String#crypt is insecure and should probably be updated.

I don't disagree, but we need to educate developers, first;
instead of just annoying users who don't write or maintain code.

Updated by shyouhei (Shyouhei Urabe) over 6 years ago

I can list up 10+ more reasons why using String#crypt is a bad idea today, but it seems we all agree on that point. The problem is how to let programmers avoid uisng it.

In addition to deprecate the method, how about removing DES from String#crypt ? DES is completely usafe today and shall not be used anyway. Using DES with String#crypt requires a programmer's ignorance and/or casual mistakes. By prohibiting the algorithm, I think we can educate programmers to abandon the method to something better, like bcrypt or argon2.

mame (Yusuke Endoh) wrote:

Incidentally, I found some usages of String#crypt in some gems.

https://github.com/eventmachine/eventmachine/blob/537127fa22a058e3d7b248a214df210e7c77dc95/lib/em/protocols/postgres3.rb#L187
https://github.com/activeldap/activeldap/blob/a63e89c384cf7f5bb95451a3749e6dc4aa799969/lib/active_ldap/user_password.rb#L38

Interesting. So for instance PostgreSQL requires DES password hash to be generated? That does not work on OpenBSD anymore, because they dropped DES support years ago.

Updated by jeremyevans0 (Jeremy Evans) over 6 years ago

mame (Yusuke Endoh) wrote:

I agree with deprecation and removal of String#crypt. If it were not included in the core, and if it were proposed today, the proposal would be definitely rejected as "it should be implemented as a gem".

This moves the String#crypt implementation to the string/crypt
extension, but leaves the String#crypt core method. The core
method prints a deprecation warning, then loads the string/crypt
extension.

It looks complex a bit to me, and I don't see its advantage. How about:

2.6: core method deprecated, and a compatibility-layer gem released
3.0?: core method removed

The advantage of having the core method require the extension (either shipped with ruby or an external gem) is that you get the same behavior. If the extension has different behavior than the core method, because configure feature detection is different than extconf.rb feature detection, it's going to be hard to debug, especially with so few users of this code.

That being said, I'm fine with just deprecating the core method, shipping an external gem, and removing the core method in 3.0.

Unfortunately, the name "crypt" is already taken, though...

"string-crypt" is not taken, which is the reason I named the extension "string/crypt".

Incidentally, I found some usages of String#crypt in some gems.

https://github.com/eventmachine/eventmachine/blob/537127fa22a058e3d7b248a214df210e7c77dc95/lib/em/protocols/postgres3.rb#L187

The last PostgreSQL version that supported crypt passwords was 8.3, which stopped being supported over 5 years ago (PostgreSQL removed support for crypt passwords in 8.4, and no longer supports <9.3). You would have to be connecting to a pre-8.4 server for it to be possible to connect with crypt passwords, and even in old PostgreSQL versions they recommended against crypt passwords, stating "crypt to be used only if you must support pre-7.2 clients".

I think it's safe to say nobody is executing that code and would notice if it broke.

https://github.com/activeldap/activeldap/blob/a63e89c384cf7f5bb95451a3749e6dc4aa799969/lib/active_ldap/user_password.rb#L38

This gem depends on ~10 other gems if you count transitive dependencies, some heavy weight such as activemodel, and is actively maintained by a ruby committer. I'm guessing kou wouldn't have a problem depending on another gem for the crypt support, but I don't want to speak for him.

Updated by mame (Yusuke Endoh) over 6 years ago

This ticket has been discussed at the developers' meeting today.

Some committers had a skeptic opinion against the removal, but agreed with it in some conditions.

We first need to remove the usage of String#crypt from WEBrick. @normalperson (Eric Wong), the maintainer of WEBrick, what do you think? Is it possible and acceptable? If it is not, we can't deprecate String#crpyt as long as WEBrick is bundled.
If normalperson agrees with the removal, we then ask @jeremyevans0 (Jeremy Evans) to release compatibility-layer gem by 2.6 release. If the release is succeeded, we can deprecate the core method, with a deprecation message like "String#crypt is deprecated; use `string-crypt' gem".
And finally, after @normalperson (Eric Wong) removed the dependency from WEBrick in any way, we will remove the core method, maybe in 3.x.

Updated by normalperson (Eric Wong) over 6 years ago

wrote:

We first need to remove the usage of String#crypt from WEBrick. @normalperson (Eric Wong), the maintainer of WEBrick, what do you think? Is it possible and acceptable? If it is not, we can't deprecate String#crpyt as long as WEBrick is bundled.
If normalperson agrees with the removal, we then ask @jeremyevans0 (Jeremy Evans) to release compatibility-layer gem by 2.6 release. If the release is succeeded, we can deprecate the core method, with a deprecation message like "String#crypt is deprecated; use `string-crypt' gem".
And finally, after @normalperson (Eric Wong) removed the dependency from WEBrick in any way, we will remove the core method, maybe in 3.x.

It's only the BasicAuth code and I have no idea how many people
use it; probably not many, but I don't want to break a single
user setup without adequate notice.

Even if BasicAuth is broken/removed/requires-extra-download,
most WEBrick can use the rest without difficulty.

WEBrick has several other authentication methods; which one do
we recommend users migrate to? I am not an expert in this area
(I don't like authentication at all).

Updated by shyouhei (Shyouhei Urabe) over 6 years ago

normalperson (Eric Wong) wrote:

It's only the BasicAuth code and I have no idea how many people
use it; probably not many, but I don't want to break a single
user setup without adequate notice.

Understand your concern. The problem is, String#crypt is a thin
wrapper of crypt(3) whose behaviour is not under our control. It
might change from time to time.

Looking at the current implementation of
webrick/httpauth/basicuath.rb, The usage of String#crypt silently
expects that crypt(3) acts in traditional (== DES) mode. This
mode has been dropped in OpenBSD for years. It now expects some
fixed-format string as its argument. See also:
https://man.openbsd.org/crypt.3

So in short, the status quo is already broken. I recommend you
to add "adequate notice" ASAP.

Even if BasicAuth is broken/removed/requires-extra-download,
most WEBrick can use the rest without difficulty.

WEBrick has several other authentication methods; which one do
we recommend users migrate to? I am not an expert in this area
(I don't like authentication at all).

There are two separate issues I think.

  • Basic authentication itself does not require DES. It is an
    implementation detail. You can safely migrate to another
    hash function at will. Ruby's openssl library has for
    instance OpenSSL::KDF.scrypt().

  • On the other hand, WEBrick now supports .htpasswd file, which
    contain DES hash strings. This does not interface with other
    hash functions.

There are three possible solutions for this situation I think:

  1. Keep supporting DES-formatted .htpasswd file. In order to do
    so you arguably need your own DES implementation, given
    OpenBSD lacks one.

  2. Give up DES, but contiue supporting BasicAuth in different
    hasing function (maybe using OpenSSL library). This requires
    user-side migration of existing .htpasswd files into some new
    format, but nothing more.

  3. Give up BasicAuth at all to migrate to DigestAuth. Requires
    both developer-side and user-side to work on it.

On Wed, Jul 18, 2018 at 5:42 PM, Eric Wong wrote:

wrote:

We first need to remove the usage of String#crypt from WEBrick. @normalperson (Eric Wong), the maintainer of WEBrick, what do you think? Is it possible and acceptable? If it is not, we can't deprecate String#crpyt as long as WEBrick is bundled.
If normalperson agrees with the removal, we then ask @jeremyevans0 (Jeremy Evans) to release compatibility-layer gem by 2.6 release. If the release is succeeded, we can deprecate the core method, with a deprecation message like "String#crypt is deprecated; use `string-crypt' gem".
And finally, after @normalperson (Eric Wong) removed the dependency from WEBrick in any way, we will remove the core method, maybe in 3.x.

It's only the BasicAuth code and I have no idea how many people
use it; probably not many, but I don't want to break a single
user setup without adequate notice.

Even if BasicAuth is broken/removed/requires-extra-download,
most WEBrick can use the rest without difficulty.

WEBrick has several other authentication methods; which one do
we recommend users migrate to? I am not an expert in this area
(I don't like authentication at all).

Updated by jeremyevans0 (Jeremy Evans) over 6 years ago

shyouhei (Shyouhei Urabe) wrote:

  • Basic authentication itself does not require DES. It is an
    implementation detail. You can safely migrate to another
    hash function at will. Ruby's openssl library has for
    instance OpenSSL::KDF.scrypt().

Note that OpenSSL::KDF.scrypt is only defined if the OpenSSL library defines EVP_PBE_scrypt, so it isn't always available. I believe this was added in OpenSSL 1.1.0, and it isn't currently available in LibreSSL. OpenSSL::KDF.pbkdf2_hmac is always defined, so that could be used as long as the openssl extension can be required.

Personally, I think it's best to punt and require the user provide implementations for creating a password hash and checking the password hash via two callable options (e.g. :create_password_hash and :check_password_hash). That way the user can determine which password hash format they want to use, and it doesn't tie Webrick to one particular hash implementation. If that is considered acceptable I can work on a patch for Webrick that keeps the current default behavior with a deprecation warning if the callable options are not provided.

Updated by normalperson (Eric Wong) over 6 years ago

"Urabe, Shyouhei" wrote:

normalperson (Eric Wong) wrote:

It's only the BasicAuth code and I have no idea how many people
use it; probably not many, but I don't want to break a single
user setup without adequate notice.

Understand your concern. The problem is, String#crypt is a thin
wrapper of crypt(3) whose behaviour is not under our control. It
might change from time to time.

OK, then we can refer users to their operating system
documentation when it breaks. In other words, we can shift
responsibility for breakage over to whoever provides crypt(3)
for their system.

What we should do now is try to educate users to avoid crypt(3)
before they start using it.

However, if users already depend on it, lets not break their use
case before their operating system breaks it for them.

Again, if system (libc) breaks something, there's nothing we can
do. However, lets not prematurely break things; but try to warn
and educate users to avoid future breakage.

Btw, glibc is considering such a shift to using 3rd-party libcrypt:

https://public-inbox.org/libc-alpha/20180625134403.2B6DC43994575@oldenburg.str.redhat.com/t/#u

Looking at the current implementation of
webrick/httpauth/basicuath.rb, The usage of String#crypt silently
expects that crypt(3) acts in traditional (== DES) mode. This
mode has been dropped in OpenBSD for years. It now expects some
fixed-format string as its argument. See also:
https://man.openbsd.org/crypt.3
https://man.openbsd.org/crypt.3

So in short, the status quo is already broken. I recommend you
to add "adequate notice" ASAP.

See proposed patch below.

Even if BasicAuth is broken/removed/requires-extra-download,
most WEBrick can use the rest without difficulty.

WEBrick has several other authentication methods; which one do
we recommend users migrate to? I am not an expert in this area
(I don't like authentication at all).

There are two separate issues I think.

  • Basic authentication itself does not require DES. It is an
    implementation detail. You can safely migrate to another
    hash function at will. Ruby's openssl library has for
    instance OpenSSL::KDF.scrypt().

  • On the other hand, WEBrick now supports .htpasswd file, which
    contain DES hash strings. This does not interface with other
    hash functions.

There are three possible solutions for this situation I think:

  1. Keep supporting DES-formatted .htpasswd file. In order to do
    so you arguably need your own DES implementation, given
    OpenBSD lacks one.

Nope, too much work to support broken behavior.

  1. Give up DES, but contiue supporting BasicAuth in different
    hasing function (maybe using OpenSSL library). This requires
    user-side migration of existing .htpasswd files into some new
    format, but nothing more.

Maybe, as Jeremy proposed in [ruby-core:88002]. But does it
make sense to add more options instead of telling people to use
existing options (perhaps DigestAuth)?

Both require work on the user-side to reconfigure, so I'd rather
we tell them to use an existing functionality rather than new
option (easier for people on older Ruby or webrick versions).

  1. Give up BasicAuth at all to migrate to DigestAuth. Requires
    both developer-side and user-side to work on it.

Why is there need for developer-side work on it? (aside from
documentation change)

Anyways, 3. seems ideal, we can slowly let users deal with it.
Anybody who complains when crypt(3) breaks, we can tell them
"we told you so" once we make documentation updates:

diff --git a/lib/webrick/httpauth/basicauth.rb b/lib/webrick/httpauth/basicauth.rb
index e23420fdfa..c774035ea1 100644
--- a/lib/webrick/httpauth/basicauth.rb
+++ b/lib/webrick/httpauth/basicauth.rb
@@ -18,6 +18,10 @@ module HTTPAuth
 ##
 # Basic Authentication for WEBrick
 #
+    # Do not use this for new deployments, use DigestAuth and
+    # Htdigest instead.  Htpasswd and BasicAuth are insecure
+    # and will be break incompatibly, soon.
+    #
 # Use this class to add basic authentication to a WEBrick servlet.
 #
 # Here is an example of how to set up a BasicAuth:
diff --git a/lib/webrick/httpauth/htpasswd.rb b/lib/webrick/httpauth/htpasswd.rb
index 976eeeb13e..783394220d 100644
--- a/lib/webrick/httpauth/htpasswd.rb
+++ b/lib/webrick/httpauth/htpasswd.rb
@@ -16,6 +16,10 @@ module WEBrick
module HTTPAuth

 ##
+    # Do not use this for new deployments, use Htdigest instead.
+    # Htpasswd and BasicAuth are insecure and will break incompatibly,
+    # soon.
+    #
 # Htpasswd accesses apache-compatible password files.  Passwords are
 # matched to a realm where they are valid.  For security, the path for a
 # password database should be stored outside of the paths available to the

Updated by normalperson (Eric Wong) over 6 years ago

wrote:

Personally, I think it's best to punt and require the user
provide implementations for creating a password hash and
checking the password hash via two callable options (e.g.
:create_password_hash and :check_password_hash). That way the
user can determine which password hash format they want to
use, and it doesn't tie Webrick to one particular hash
implementation. If that is considered acceptable I can work
on a patch for Webrick that keeps the current default behavior
with a deprecation warning if the callable options are not
provided.

Is the worth the effort for you or anybody to work on
such an option (which still requires user education) vs
telling them to use Htdigest, which already exists?

Updated by jeremyevans0 (Jeremy Evans) over 6 years ago

normalperson (Eric Wong) wrote:

wrote:

Personally, I think it's best to punt and require the user
provide implementations for creating a password hash and
checking the password hash via two callable options (e.g.
:create_password_hash and :check_password_hash). That way the
user can determine which password hash format they want to
use, and it doesn't tie Webrick to one particular hash
implementation. If that is considered acceptable I can work
on a patch for Webrick that keeps the current default behavior
with a deprecation warning if the callable options are not
provided.

Is the worth the effort for you or anybody to work on
such an option (which still requires user education) vs
telling them to use Htdigest, which already exists?

I think that a single iteration of MD5 is not a significant security improvement on DES:

    class DigestAuth
      def self.make_passwd(realm, user, pass)
        pass ||= ""
        Digest::MD5::hexdigest([user, realm, pass].join(":"))
      end
    end

When using HTTP, digest auth is better than basic auth if your threat model includes connection sniffing (passive or active MITM). When using HTTPS with trusted certificates (define trusted however you like), I'd much prefer basic auth with a strong hash function than digest auth.

Updated by shyouhei (Shyouhei Urabe) over 6 years ago

On Fri, Jul 20, 2018 at 5:51 PM, Eric Wong wrote:

"Urabe, Shyouhei" wrote:

normalperson (Eric Wong) wrote:

It's only the BasicAuth code and I have no idea how many people
use it; probably not many, but I don't want to break a single
user setup without adequate notice.

Understand your concern. The problem is, String#crypt is a thin
wrapper of crypt(3) whose behaviour is not under our control. It
might change from time to time.

OK, then we can refer users to their operating system
documentation when it breaks. In other words, we can shift
responsibility for breakage over to whoever provides crypt(3)
for their system.

What we should do now is try to educate users to avoid crypt(3)
before they start using it.

However, if users already depend on it, lets not break their use
case before their operating system breaks it for them.

Again, if system (libc) breaks something, there's nothing we can
do. However, lets not prematurely break things; but try to warn
and educate users to avoid future breakage.

Makes sense to me.

Btw, glibc is considering such a shift to using 3rd-party libcrypt:

https://public-inbox.org/libc-alpha/20180625134403.2B6DC43994575@oldenburg.str.redhat.com/t/#u

Looking at the current implementation of
webrick/httpauth/basicuath.rb, The usage of String#crypt silently
expects that crypt(3) acts in traditional (== DES) mode. This
mode has been dropped in OpenBSD for years. It now expects some
fixed-format string as its argument. See also:
https://man.openbsd.org/crypt.3
https://man.openbsd.org/crypt.3

So in short, the status quo is already broken. I recommend you
to add "adequate notice" ASAP.

See proposed patch below.

OK, the patch seems fine.

Even if BasicAuth is broken/removed/requires-extra-download,
most WEBrick can use the rest without difficulty.

WEBrick has several other authentication methods; which one do
we recommend users migrate to? I am not an expert in this area
(I don't like authentication at all).

There are two separate issues I think.

  • Basic authentication itself does not require DES. It is an
    implementation detail. You can safely migrate to another
    hash function at will. Ruby's openssl library has for
    instance OpenSSL::KDF.scrypt().

  • On the other hand, WEBrick now supports .htpasswd file, which
    contain DES hash strings. This does not interface with other
    hash functions.

There are three possible solutions for this situation I think:

  1. Keep supporting DES-formatted .htpasswd file. In order to do
    so you arguably need your own DES implementation, given
    OpenBSD lacks one.

Nope, too much work to support broken behavior.

  1. Give up DES, but contiue supporting BasicAuth in different
    hasing function (maybe using OpenSSL library). This requires
    user-side migration of existing .htpasswd files into some new
    format, but nothing more.

Maybe, as Jeremy proposed in [ruby-core:88002]. But does it
make sense to add more options instead of telling people to use
existing options (perhaps DigestAuth)?

Both require work on the user-side to reconfigure, so I'd rather
we tell them to use an existing functionality rather than new
option (easier for people on older Ruby or webrick versions).

  1. Give up BasicAuth at all to migrate to DigestAuth. Requires
    both developer-side and user-side to work on it.

Why is there need for developer-side work on it? (aside from
documentation change)

Maybe this was my misunderstanding. Are BasicAuth and DigestAuth
made API-compatible? If that is the case what developers should
do might be just replacing BasicAuth to DigestAuth, and Htpasswd
to Htdigest.

Anyways, 3. seems ideal, we can slowly let users deal with it.
Anybody who complains when crypt(3) breaks, we can tell them
"we told you so" once we make documentation updates:

Updated by jeremyevans0 (Jeremy Evans) over 6 years ago

shyouhei (Shyouhei Urabe) wrote:

Maybe this was my misunderstanding. Are BasicAuth and DigestAuth
made API-compatible? If that is the case what developers should
do might be just replacing BasicAuth to DigestAuth, and Htpasswd
to Htdigest.

No, basic auth and digest auth are very different, you can't just substitute one for the other. Digest auth requires MD5 for one and doesn't support a more secure format for password hash storage, while basic auth submits the password in plain text and can use any hash algorithm for storing passwords. Some HTTP clients support basic auth and not digest auth.

Digest auth really doesn't make sense when you are using HTTPS, it was designed to protect against password disclosure when using HTTP with a passive MITM (an attacker who can observe but not modify traffic). These days, there are very few cases where digest auth would be a good idea.

Updated by normalperson (Eric Wong) over 6 years ago

wrote:

shyouhei (Shyouhei Urabe) wrote:

  • Basic authentication itself does not require DES. It is an
    implementation detail. You can safely migrate to another
    hash function at will. Ruby's openssl library has for
    instance OpenSSL::KDF.scrypt().

Note that OpenSSL::KDF.scrypt is only defined if the OpenSSL
library defines EVP_PBE_scrypt, so it isn't always available.
I believe this was added in OpenSSL 1.1.0, and it isn't
currently available in LibreSSL. OpenSSL::KDF.pbkdf2_hmac is
always defined, so that could be used as long as the openssl
extension can be required.

Personally, I think it's best to punt and require the user
provide implementations for creating a password hash and
checking the password hash via two callable options (e.g.
:create_password_hash and :check_password_hash).

Can you provide pre-defined, accepted-as-secure implementations
of these which we can recommend for common use cases which would
be compatible with other webservers? I'd rather not
introduce/recommend hashes which are incompatible with other
webservers, but two-way compatibility with existing servers
is a good thing.

It could optionally recommend/use 3rd-party gem if available
(e.g. 'bcrypt'), or the bundled 'openssl' ext.

That way the user can determine which password hash format
they want to use, and it doesn't tie Webrick to one particular
hash implementation. If that is considered acceptable I can
work on a patch for Webrick that keeps the current default
behavior with a deprecation warning if the callable options
are not provided.

That seems reasonable, but I want to avoid situations where
users cargo-cult blocks of code into the config they don't
understand. They could get stuck with an option which is
eventually found insecure and we'd have no way of warning them.

Thanks.

Updated by jeremyevans0 (Jeremy Evans) over 6 years ago

normalperson (Eric Wong) wrote:

Can you provide pre-defined, accepted-as-secure implementations
of these which we can recommend for common use cases which would
be compatible with other webservers? I'd rather not
introduce/recommend hashes which are incompatible with other
webservers, but two-way compatibility with existing servers
is a good thing.

It could optionally recommend/use 3rd-party gem if available
(e.g. 'bcrypt'), or the bundled 'openssl' ext.

.htpasswd is an Apache-specific format, and Apache currently supports only 5 password formats: bcrypt, MD5, SHA1, crypt, plain text (https://httpd.apache.org/docs/2.4/misc/password_encryptions.html). Only bcrypt and MD5 are considered secure. The MD5 format uses a Apache-specific algorithm with 1000 iterations of MD5, and I'm not aware of a ruby implementation of it. I believe the bcrypt format is a standard bcrypt except that it uses 2y instead of 2a or 2b as the version, but I have not tested this yet. I will test it and assuming the bcrypt format is compatible, that is probably the format we should recommend.

That seems reasonable, but I want to avoid situations where
users cargo-cult blocks of code into the config they don't
understand. They could get stuck with an option which is
eventually found insecure and we'd have no way of warning them.

That's would be nice, but I'm not sure how attainable that is in terms of users understanding the options. A more attainable goal is a single example that is considered secure and compatible today, which I think leaves only bcrypt.

Updated by jeremyevans0 (Jeremy Evans) over 6 years ago

jeremyevans0 (Jeremy Evans) wrote:

.htpasswd is an Apache-specific format, and Apache currently supports only 5 password formats: bcrypt, MD5, SHA1, crypt, plain text (https://httpd.apache.org/docs/2.4/misc/password_encryptions.html). Only bcrypt and MD5 are considered secure. The MD5 format uses a Apache-specific algorithm with 1000 iterations of MD5, and I'm not aware of a ruby implementation of it. I believe the bcrypt format is a standard bcrypt except that it uses 2y instead of 2a or 2b as the version, but I have not tested this yet. I will test it and assuming the bcrypt format is compatible, that is probably the format we should recommend.

Apache htpasswd generates passwords using $2y$, but the hashes are the same as $2a$ format, and Apache handles $2a$ format just fine. The bcrypt gem does not consider $2y$ to match, but will work correctly if $2y$ is changed to $2a$. According to Wikipedia (https://en.wikipedia.org/wiki/Bcrypt#Versioning_history), $2y$ instead of $2a$ was just to note that the hash was not generated by a bad version of the crypt_blowfish PHP library, otherwise the formats are identical.

So to check the passwords in ruby, you would just need to do: BCrypt::Password.new(password_hash.sub(/\A\$2y$/, '$2a$')) == submitted_password

Tested using Apache 2.4.33 with the following .htaccess file:

AuthType Basic
AuthName "Restricted Files"
AuthUserFile "/var/www/passwd"
Require user foo bar

Where /var/www/passwd was populated:

cd /var/www
touch passwd
htpasswd -bB passwd foo foofoofoo
ruby -r bcrypt -e 'puts BCrypt::Password.create("barbarbar")' > passwd

Testing that both foo and bar can login with the passwords, and that the ruby bcrypt library can correctly check password hashes generated by htpasswd if you do the $2y$ to $2a$ conversion as shown above.

Should I prepare a patch to Webrick to accept :create_password_hash and :check_password_hash options, and add documentation with an example using bcrypt?

Updated by normalperson (Eric Wong) over 6 years ago

Thanks for the research on this matter

wrote:

Should I prepare a patch to Webrick to accept
:create_password_hash and :check_password_hash options, and
add documentation with an example using bcrypt?

Can you do it as a single option which doesn't involve having
users copy+paste code? Something like:

:password_hash => (:bcrypt|:crypt|:blahblah)

It can raise and tell users to install 'bcrypt' if 'bcrypt'
is missing.

Updated by jeremyevans0 (Jeremy Evans) over 6 years ago

normalperson (Eric Wong) wrote:

Can you do it as a single option which doesn't involve having
users copy+paste code? Something like:

:password_hash => (:bcrypt|:crypt|:blahblah)

It can raise and tell users to install 'bcrypt' if 'bcrypt'
is missing.

One issue with that is it doesn't offer a way to specify the password hash cost. For example, the bcrypt gem defaults to cost 10, and htpasswd defaults to cost 5 (cost 10 is 2**(10-5) times as many iterations as cost 5). It also doesn't offer a way to use custom authentication (e.g. using basic authentication to authenticate against LDAP). It's simple, but inflexible.

However, if that is still the API you want, I'll work on that and submit it as a separate feature.

Updated by shyouhei (Shyouhei Urabe) over 6 years ago

Off topic but,

jeremyevans0 (Jeremy Evans) wrote:

Apache htpasswd generates passwords using $2y$, but the hashes are the same as $2a$ format, and Apache handles $2a$ format just fine. The bcrypt gem does not consider $2y$ to match, but will work correctly if $2y$ is changed to $2a$. According to Wikipedia (https://en.wikipedia.org/wiki/Bcrypt#Versioning_history), $2y$ instead of $2a$ was just to note that the hash was not generated by a bad version of the crypt_blowfish PHP library, otherwise the formats are identical.

Is $2a$ okay? The wikipedia entry you linked says that OpenBSD people abandoned $2a$ to move to $2b$. Sounds very much like it has flaw(s). Also, the author of $2y$ variant now releases crypt_blowfish version 1.3 which states:

Version 1.3 adds support for the $2b$ prefix introduced in OpenBSD 5.5+, which behaves exactly the same as crypt_blowfish's $2y$.

see also: http://www.openwall.com/crypt/

I hope this is not an issue. But at least sub(/\A\$2y$/, '$2a$') seems something we should not.

Updated by normalperson (Eric Wong) over 6 years ago

wrote:

normalperson (Eric Wong) wrote:

Can you do it as a single option which doesn't involve having
users copy+paste code? Something like:

:password_hash => (:bcrypt|:crypt|:blahblah)

It can raise and tell users to install 'bcrypt' if 'bcrypt'
is missing.

One issue with that is it doesn't offer a way to specify the
password hash cost. For example, the bcrypt gem defaults to
cost 10, and htpasswd defaults to cost 5 (cost 10 is 2**(10-5)
times as many iterations as cost 5). It also doesn't offer a
way to use custom authentication (e.g. using basic
authentication to authenticate against LDAP). It's simple,
but inflexible.

Allowing options which other servers do not support is not
something I want, as it could be a way to lock people into
WEBrick. Being compatible with htpasswd to allow users
of other servers to easily migrate in any direction is more
important.

I haven't looked into LDAP authentication; but maybe that can
use a URI to the LDAP server instead of path. I don't know how
Apache or other servers do it, even; but we should try to steal
configuration/setup ideas from others servers to minimize
migration costs in either direction and not introduce things
which make it difficult to migrate away from.

Updated by jeremyevans0 (Jeremy Evans) over 6 years ago

shyouhei (Shyouhei Urabe) wrote:

Off topic but,

jeremyevans0 (Jeremy Evans) wrote:

Apache htpasswd generates passwords using $2y$, but the hashes are the same as $2a$ format, and Apache handles $2a$ format just fine. The bcrypt gem does not consider $2y$ to match, but will work correctly if $2y$ is changed to $2a$. According to Wikipedia (https://en.wikipedia.org/wiki/Bcrypt#Versioning_history), $2y$ instead of $2a$ was just to note that the hash was not generated by a bad version of the crypt_blowfish PHP library, otherwise the formats are identical.

Is $2a$ okay? The wikipedia entry you linked says that OpenBSD people abandoned $2a$ to move to $2b$. Sounds very much like it has flaw(s).

There is only a difference between $2a$ and $2b$ on OpenBSD if the password length is 256 characters or longer. Such passwords are very rare. The bcrypt gem currently only supports and generates $2a hashes, but much like $2y$, if you change $2b$ to $2a$, the hash will match.

I hope this is not an issue. But at least sub(/\A\$2y\$/, '$2a$') seems something we should not.

sub(/\A\$2y\$/, '$2b$') is only fine after the bcrypt gem is modified to recognize $2b$. I tested the bcrypt gem, and the algorithm it uses is compatible with OpenBSD's $2b$ format:

pass = 'a'*500
s = `encrypt #{pass}`.chomp
# => "$2b$08$2iobD98d6MzMpS6/ViH8euWUmKGBgp4D8txRHAmCeIcfAS7ZtKPxW"
BCrypt::Password.new(s) == pass
# => false
BCrypt::Password.new(s.sub(/\A\$2b\$/, '$2a$')) == pass
# => true

So there are no security issues in the sub(/\A\$2y\$/, '$2a$'). I would say that until the bcrypt gem is modified to recognize $2b$ and $2y$, that we use sub(/\A\$2[yb]\$/, '$2a$') for maximum compatibility.

Updated by jeremyevans0 (Jeremy Evans) over 6 years ago

normalperson (Eric Wong) wrote:

Allowing options which other servers do not support is not
something I want, as it could be a way to lock people into
WEBrick. Being compatible with htpasswd to allow users
of other servers to easily migrate in any direction is more
important.

Fair enough, I'll work on adding support for :password_hash=>:bcrypt|:crypt (currently defaulting to :crypt if not given for backwords compatibility).

I haven't looked into LDAP authentication; but maybe that can
use a URI to the LDAP server instead of path. I don't know how
Apache or other servers do it, even; but we should try to steal
configuration/setup ideas from others servers to minimize
migration costs in either direction and not introduce things
which make it difficult to migrate away from.

I don't think it makes sense to add specific support for other authentication options. If we aren't going to offer generic support, then we should limit the feature addition to the ability to use bcrypt. That allows people the ability to use an .htpasswd file with a strong password hash, and allows String#crypt to be deprecated and then moved to a gem.

Updated by shyouhei (Shyouhei Urabe) over 6 years ago

jeremyevans0 (Jeremy Evans) wrote:

So there are no security issues in the sub(/\A\$2y\$/, '$2a$').

Great. Thank you for confirmation.

Updated by jeremyevans0 (Jeremy Evans) over 6 years ago

I've released a string-crypt gem based on the previous patch: https://github.com/jeremyevans/ruby-string-crypt

The gem has some minor differences in extconf.rb, because it needs to avoid overlap with macros defined in ruby/config.h. It also supports an environment variable during compilation to force use of the internal DES-crypt implementation, useful for testing support (especially on OpenBSD where crypt(3) doesn't support DES-crypt). I would appreciate users of other operating systems testing it and letting me know if anything breaks.

Attached is an updated patch that just deprecates String#crypt without making any changes, and updates the tests to handle the deprecation warnings.

Updated by jeremyevans0 (Jeremy Evans) about 6 years ago

mame (Yusuke Endoh) wrote:

This ticket has been discussed at the developers' meeting today.

Some committers had a skeptic opinion against the removal, but agreed with it in some conditions.

We first need to remove the usage of String#crypt from WEBrick. @normalperson (Eric Wong), the maintainer of WEBrick, what do you think? Is it possible and acceptable? If it is not, we can't deprecate String#crpyt as long as WEBrick is bundled.
If normalperson agrees with the removal, we then ask @jeremyevans0 (Jeremy Evans) to release compatibility-layer gem by 2.6 release. If the release is succeeded, we can deprecate the core method, with a deprecation message like "String#crypt is deprecated; use `string-crypt' gem".

The Webrick issue has been dealt with, and the string-crypt gem was released back in July. Can we apply the most recent patch in this ticket to deprecate String#crypt?

Updated by jeremyevans0 (Jeremy Evans) about 6 years ago

jeremyevans0 (Jeremy Evans) wrote:

mame (Yusuke Endoh) wrote:

This ticket has been discussed at the developers' meeting today.

Some committers had a skeptic opinion against the removal, but agreed with it in some conditions.

We first need to remove the usage of String#crypt from WEBrick. @normalperson (Eric Wong), the maintainer of WEBrick, what do you think? Is it possible and acceptable? If it is not, we can't deprecate String#crpyt as long as WEBrick is bundled.
If normalperson agrees with the removal, we then ask @jeremyevans0 (Jeremy Evans) to release compatibility-layer gem by 2.6 release. If the release is succeeded, we can deprecate the core method, with a deprecation message like "String#crypt is deprecated; use `string-crypt' gem".

The Webrick issue has been dealt with, and the string-crypt gem was released back in July. Can we apply the most recent patch in this ticket to deprecate String#crypt?

I've updated the patch to apply against trunk. I would like this change to make 2.6 unless there are objections. Can a committer perform a final review and commit if acceptable?

Updated by normalperson (Eric Wong) about 6 years ago

wrote:

File 0001-Deprecate-String-crypt.patch added

I am against the rb_warn call. Documentation change is enough.

Typical users (non-programmers) may not know how to quiet
the warning or fix the problem. They will just be annoyed,
and turned off of using Ruby.

Updated by mame (Yusuke Endoh) about 6 years ago

IMO, documentation is not enough. A warning may annoy a user, but sudden removal (in future) is much worse.

As a first step, how about rb_warning instead of rb_warn? It prints a warning only when $VERBOSE is enabled. A well-mannered developer can be aware of the deprecation, will fix the issue, and may even write how to fix in stackoverflow. A non-programmer user won't be annoyed.
And then, we can replace it with rb_warn in the next version. Of course all programs won't be ready, but thing would be better. Anyway, a fix is needed as long as we will remove String#crypt in future.

Updated by jeremyevans0 (Jeremy Evans) about 6 years ago

mame (Yusuke Endoh) wrote:

IMO, documentation is not enough. A warning may annoy a user, but sudden removal (in future) is much worse.

As a first step, how about rb_warning instead of rb_warn? It prints a warning only when $VERBOSE is enabled. A well-mannered developer can be aware of the deprecation, will fix the issue, and may even write how to fix in stackoverflow. A non-programmer user won't be annoyed.
And then, we can replace it with rb_warn in the next version. Of course all programs won't be ready, but thing would be better. Anyway, a fix is needed as long as we will remove String#crypt in future.

Here's an updated patch that switches to using rb_warning, with appropriate updates to the tests. This also fixes the the Webrick test changes to use spaces instead of tabs, consistent with the rest of the file.

Updated by normalperson (Eric Wong) about 6 years ago

wrote:

IMO, documentation is not enough. A warning may annoy a user,
but sudden removal (in future) is much worse.

There should never be a sudden removal.

Honestly, I don't think we should make the first move on this
issue until more-widely used projects (e.g. glibc) start
warning or deprecating on it.

As a first step, how about rb_warning instead of rb_warn? It prints a warning only when $VERBOSE is enabled. A well-mannered developer can be aware of the deprecation, will fix the issue, and may even write how to fix in stackoverflow. A non-programmer user won't be annoyed.

It's better, I suppose. I don't know if we can guarantee $VERBOSE
isn't widely-used by some programs?

Updated by shyouhei (Shyouhei Urabe) about 6 years ago

Well, it seems we do not agree whether we should add a warning. But is anybody against on documenting the deprecation? I think we can:

  • Consider String#crypt deprecated as of 2.6.
  • Add NEWS about it.
  • Add rdoc about it.

Can I do the above? Or do we need to wait glibc for them?

Updated by normalperson (Eric Wong) about 6 years ago

wrote:

Well, it seems we do not agree whether we should add a
warning. But is anybody against on documenting the
deprecation? I think we can:

  • Consider String#crypt deprecated as of 2.6.
  • Add NEWS about it.
  • Add rdoc about it.

Can I do the above? Or do we need to wait glibc for them?

Sure, please do that. We can get Ruby programmers to stop using
it, at least.

I am against the warning because users of Ruby programs aren't
all programmers and some users aren't even bug reporters.

Actions #37

Updated by shyouhei (Shyouhei Urabe) about 6 years ago

  • Status changed from Open to Closed

Applied in changeset trunk|r66154.


string.c: [DOC] deprecate String#crypt [ci skip] [Feature #14915]

Updated by shyouhei (Shyouhei Urabe) about 6 years ago

  • Status changed from Closed to Open

I did not intend to close this ticket. What remains to be discussed is:

  • Should we need to add warnings?
  • If that is the case, when and how?

Updated by mame (Yusuke Endoh) about 6 years ago

I think that the warning is needed. People (at least, I) don't read documents. Even if I noticed, I cannot remember where my code uses String#crypt.

I understand that the warning will annoy normal non-programmer users. However, some of the users may report the warning to the author, which leads to the fix before the removal. If no warning is printed, no one will notice the deprecation, and the future removal will break the program suddenly. This is much worse than annoying warnings.

Updated by normalperson (Eric Wong) about 6 years ago

wrote:

I understand that the warning will annoy normal non-programmer
users. However, some of the users may report the warning to
the author, which leads to the fix before the removal.

Some users will also replace the Ruby program with something
else. I've seen it happen too many times with other warnings.

Instead of stderr which every user sees, how about we start
warning to syslog, which only admins will look at?

If no warning is printed, no one will notice the deprecation,
and the future removal will break the program suddenly. This
is much worse than annoying warnings.

Of course we never remove anything suddenly; and I trust
C library maintainers won't, either.

Updated by Eregon (Benoit Daloze) almost 6 years ago

If we want to remove a feature, we deprecate and warn first, that has always been the case in MRI.

@shyouhei (Shyouhei Urabe) Please add the warning. Otherwise, no users will consider it deprecated.

@normalperson (Eric Wong) If you disagree, please start a general discussion about warnings in another ticket.

Updated by normalperson (Eric Wong) almost 6 years ago

wrote:

If we want to remove a feature, we deprecate and warn first, that has always been the case in MRI.

I prefer no removals.

@normalperson (Eric Wong) If you disagree, please start a general discussion about warnings in another ticket.

But I give up caring for backwards compatibility (as far as Ruby goes).
Easier for me to stop using the language :P

Updated by enebo (Thomas Enebo) almost 6 years ago

I just read this issue and one thing confuses me...non-programmers will not see output from a webrick server will they (and only if basic auth is used)? Or is the argument that we cannot know which non-programmers use a ruby programs which happens to use crypt (and also runs it in verbose mode) so we cannot issue a warning? This second argument would apply to any warnings added to ruby.

Updated by jeremyevans0 (Jeremy Evans) over 5 years ago

Attached is an updated patch against trunk for deprecating String#crypt. This patch always warns when String#crypt is called, instead of just warning in verbose mode, since I would like the deprecation to be obvious in 2.7 for any remaining users, allowing us to remove String#crypt in 3.0.

Updated by shevegen (Robert A. Heiler) over 5 years ago

I have not read all comments (sorry) and it is way outside of my league to come to any meaningful
conclusion; I only skimmed through it and since it may be mentioned at the next developer meeting
I may comment (I only saw it because it was linked in recently). However had, this functionality
could become a gem, and put onto e. g. rubygems.org, could it not? Then people who need it could
install it; we had moved ncurses or curses into a gem too.

But I give up caring for backwards compatibility (as far as Ruby goes).
Easier for me to stop using the language :P

Personally I am not affected by the change either way (I don't think I ever used either the
specific webrick functionality or String#crypt myself), but if it is a gem that can be
installed, it should not be a problem? I think the amount of people affected may be small,
and they could only be affected if they actively upgrade to a ruby version that may have
the deprecation/removed variant, which I guess will take a long time - and if that is the
case, they could use that gem? I am a bit confused about the comment even though there is
the tongue-in-cheek symbol. Unless I have missed it, other gems existed that allowed for
this. Take the syck gem; it is maintained in the sense that incompatible changes may
be added to it (possibly). I used it for a very long time but finally made the switch
into unicode. I think I used the syck gem ever since ruby 1.9.x.

I am not completely sure why this should not be possible for the functionality here. And not
being able to move away from old/older code also creates other problems, so there are always
trade-offs either way.

By the way, if memory footprint, DSO size etc... is a concern, isn't that what ultimately
mruby could solve? E. g. if we would have a super-modular, super-small ruby that could
also be used as base for MRI eventually and be modelled into the "domain" of the particular
niche where it runs (computer system). (Just thinking here in principle ... I
guess there is a lot of work to be done either way.)

Actions #46

Updated by akr (Akira Tanaka) over 5 years ago

  • Related to Feature #14940: Support bcrypt password hashing in webrick added

Updated by usa (Usaku NAKAMURA) over 5 years ago

As a matter of practice, OpenBSD still has crypt.
It means that it's too difficult to remove this function at this timing, IMO.
We'll remove it in the future, I guess, but not now.

Updated by akr (Akira Tanaka) over 5 years ago

I feel warning is too strong.
crypt function is required to implement password checking for /etc/passwd.

Updated by naruse (Yui NARUSE) over 5 years ago

Just removing deprecated feature doesn't provide additional value.

In this case UNIX crypt(3) is sometimes used as a basic building block.
Removing a wrapper of such block may cause a compatibility issue.

As usa says why OpenBSD still has crypt(3) is considered because of such reason.
If OpenBSD's crypt(3) is removed, it can be a time to remove this feature.

Updated by jeremyevans0 (Jeremy Evans) over 5 years ago

  • Status changed from Open to Rejected

OpenBSD deprecated crypt(3) in November 2014 (https://github.com/openbsd/src/commit/608633c1c51b59b1f0d77d3d5b646ff8d27407aa). However, I doubt that crypt(3) will be removed in the near future, if ever. OpenBSD didn't remove gets(3) until March 2014, and that function was impossible to use securely.

I agree that there are backwards compatibility issues with removing String#crypt, even if the usage is uncommon. It is possible to use String#crypt correctly, though such usage may not be portable. The documentation for String#crypt now does a good job of explaining the issues, so hopefully that is enough. I'm going to close this issue now. We can reopen this issue or open a new issue if we decide to deprecate String#crypt in the future.

One remaining issue with String#crypt is that the specs for it are not portable (they fail on OpenBSD), as they rely on the use of DES encryption (including specs for webrick's use of it for http authentication). However, I will try to fix those specs so they pass on OpenBSD as well as continuing to work on operating systems that implement DES encryption by default. I had an initial attempt at this a few years ago (#11363), I'll work on an update to that issue.

Updated by mame (Yusuke Endoh) over 5 years ago

@jeremyevans0 (Jeremy Evans) Let me confirm the state of this ticket. The current rdoc says:

Please do
not use this method any longer.  It is legacy; provided only for
backward compatibility with ruby scripts in earlier days.

...

If for some reason you cannot migrate to other secure contemporary
password hashing algorithms, install the string-crypt gem and
<code>require 'string/crypt'</code> to continue using it.

It looks to me still deprecated, though it does not include the word "deprecated" explicitly. Is this intentional?

Ruby 2.6 NEWS file explicitly says "String#crypt is now deprecated. [Feature #14915]". So, if the deprecation itself is canceled, it might be good to mention the cancellation in Ruby 2.7 NEWS.

If it is actually deprecated, and if we have no plan to remove it, we don't have to do anything, I think.

Updated by jeremyevans0 (Jeremy Evans) over 5 years ago

I believe the current documentation and code is intentional. I think it is best to consider String#crypt deprecated even if we are not emitting warnings for it and we do not have immediate plans to remove it. I would still like to remove String#crypt, even if that removal doesn't happen until Ruby 4.

Updated by mame (Yusuke Endoh) over 5 years ago

Okay, it is considered deprecated, but the removal won't happen in the near future. Thank you, I see!

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0