-
Notifications
You must be signed in to change notification settings - Fork 65
Description
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?