libxcrypt icon indicating copy to clipboard operation
libxcrypt copied to clipboard

Remove gensalt support for obsolete hashing methods?

Open zackw opened this issue 5 years ago • 4 comments

This came up during the review of yescrypt, but should be discussed separately.

We support generation of new "setting strings", via crypt_gensalt, for all of the supported hashing methods, including methods that are no longer secure (NTHASH, DES, MD5, SHA1; depending how you look at it, even SHA256 and -512 are getting shaky) and methods that exist only for backward bug-compatibility (alternative bcrypt). If we removed these, we could delete a bunch of code that exists only to support this capability, and we would eliminate a footgun (I'm especially concerned here with people passing "" as the prefix, instead of NULL, to crypt_gensalt). And it would then be consistent to provide gensalt support for yescrypt but not scrypt. However, it's conceivable that someone has a concrete need to use crypt_gensalt for obsolete methods; I don't know exactly why @besser82 implemented them in the first place, but I assume he had a use case in mind. I think we do not have a need to generate setting strings for obsolete methods in our own test suite (apart from testing the generation process itself); I can't think of any need that couldn't be met by generating setting strings by hand and embedding them in the test programs, which is what a lot of them already do.

It was my intention to address this via the runtime configurability feature proposed in #4 and #26; if people could activate obsolete gensalt modes if they really need them, there would be no downside to disabling them by default. However, I have no idea when I will have time to implement that, and it certainly won't be soon, so I think there's a case for going ahead and removing the code.

For concreteness, suppose we dropped gensalt support for "", "_", "$1$", "$2a$", "$2x$", "$2y$", "$3$", "$md5", and "$sha1". Can anyone think of negative consequences to making that change?

zackw avatar Aug 29 '18 14:08 zackw

To kick off the discussion, the most obvious scenario I can think of, where people would still need to hash new passphrases with obsolete methods, is a "computer lab" with many different types of Unix workstation, using centralized authentication via NIS-style distribution of hashes. For anything weaker than SHA256, I think there's a strong case that this is no longer acceptable practice and they need to either switch to Kerberos-style ticket-based authentication (so that only the authentication server ever deals with passphrases in the first place), or upgrade or retire any computers that are limited to weaker hashing methods. Absent a published cryptographic break on Ulrich's SHA256 and SHA512 methods, I can't see dropping gensalt support for them anytime soon; you can make them take significant time by bumping up the rounds parameter, and there are a lot of Linux boxen out there still using glibc's libcrypt.

zackw avatar Aug 29 '18 15:08 zackw

The $2*$ forms are all bcrypt, which certainly should not be considered deprecated. Are you just intending to deprecate the bug-compat setting forms?

richfelker avatar Aug 29 '18 22:08 richfelker

@richfelker Yes, that's right. The preferred $2b$ will remain.

zackw avatar Aug 30 '18 00:08 zackw

For some more context, this idea came up as an alternative to the suggestion that yescrypt's gensalt should be capable of generating setting strings not only for yescrypt's current $y$ prefix (which also supports classic scrypt under the $y$. variety - with a dot indicating flags=0), but also for classic scrypt under the older $7$ prefix (which is supported in libsodium). We do already have decode-only support for the $7$ prefix, so such hashes would work.

Adding support for $7$ doesn't immediately fit the yescrypt_encode_params_r interface in the yescrypt tree, because that interface doesn't accept a target prefix and would currently produce $y$. for flags=0. As an option, we could revise it to produce $7$ in that case (maybe my idea of fitting classic scrypt in yescrypt's newer encoding wasn't that good after all? even though it's more consistent with yescrypt and a more compact encoding), or we could extend the interface to accept a target prefix. Meanwhile, support for $7$ can indeed be added with extra code in libxcrypt (and @besser82 said he already has a patch doing that).

It would certainly be weird to configure a system to use for new passwords "" (truncation at 8), "$2x$" (from its introduction intended for use on existing buggy hashes only), or "$3$" (not salted and not iterated), "$md5" (Solaris systems supporting it also support "$2a$"), "$sha1" (NetBSD(?) systems supporting it also support "$2a$").

OTOH, I do see a point in being consistent with supporting the same set of hashes for both authenticating against them and for generating them with random salts. At least this simplifies the documentation, or otherwise we need to somehow distinguish two levels of support.

So I'm unsure. I'd be more in support of completely dropping the weird obsolete hashes that didn't get much adoption at all ("$md5", "$sha1") or not on Unix ("$3$"). There's also "_", but it's trivial to continue to support along with "" (mostly shared code), which was widespread. [And, well, I like "_" for its historical value: it was the first hash type with variable iteration count encoded along with the hash (circa 1993, whereas bcrypt was 1997), and the first to correctly remove the password length limit (prior attempts were bigcrypt and crypt16, which allowed for cracking of the 8-char portions separately). So I don't like having it go if we keep "".]

Speaking of bcrypt, "$2a$" and "$2y$" are still relevant, including for new hashes if they are likely to be migrated to certain other existing systems.

I also see some hack value in libxcrypt optionally supporting "everything". So that e.g. the one-or-so person maybe ever wanting to migrate SunMD5 hashes to Linux can do so by using (a non-default build of?) libxcrypt. Or so that JtR's --format=crypt supports all of those hashes at once. Or for use by historians. %-)

solardiz avatar Aug 30 '18 10:08 solardiz