Improve the crypt() builtin #2
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Improve the checks to find crypt() and error if we can't find it. Previously, this might have failed during building if there is no libcrypt, which is becoming very slightly more a possibility since glibc may be obsoleting their libcrypt implementation. Additionally, obsolete the fallback behavior if we have no crypt() implementation. Modern systems should have crypt() available, either through the standard library, or something more modern, like libxcrypt, and we shouldn't fall back to the insecure "just copy the string" crypt() implementation without explicitly being told to.
To call crypt(), we need to supply a salt/valid settings. It's not very easy to portably know what might be supported or available in a given crypt implementation, but there's fortunately a function more modern crypt libraries have to make salts for us. Check if it's there so we can use it.
In case someone wants to avoid using crypt_gensalt if it's available, to stick with the legacy DES crypt hashes (e.g. in case they want to use the database with older versions of the engine), add a build option to disable crypt_gensalt usage. This would be more convenient as a run-time option, but that's more complicated.
If enabled, use crypt_gensalt to make salts for our crypt() calls. This means we can always get a modern, secure hash, while maintaining backwards-compatibility. This should make passwords at rest more secure.
Make sure that libxcrypt has all hashes enabled for nix builds, so that if a database has DES hashes, crypt won't refuse to process them.
Add info on the crypt changes, and add tips in case an older build system gets confused by changes to configure.ac after updating.
Add a build-aux directory to store all the extra scripts autoconf and automake add to help the build process without cluttering up the main directory. Additionally, add an m4 directory for any future m4 macros we add, or which aclocal installs, so that, e.g., users don't need to have autoconf-archive.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
crypt()builtin currently uses DES for hashing passwords, which is very insecure.If available and enabled, we now use
crypt_gensalt()to make a usable salt for whatever algorithm the library supplyingcrypt()thinks is strongest.Since this is provided by a fair few newer libcrypt libraries, such as libxcrypt and crypt_blowfish, it's fairly widespread.
Additionally, glibc recently removed its libcrypt implementation, so many distros are now using a more modern libcrypt supplied by a project focused on cryptography, like libxcrypt, making it more likely to be available, and making it necessary to ensure libcrypt is properly found and linked if needed.
In short, this should make it fairly easy to have more secure password hashes, and at least in databases I have, it should just work without any major issues or changes needed to the database code.
If someone wants to explicitly stick with legacy DES hashes only, to keep a database compatible with older versions of the engine,
--disable-crypt-gensaltcan be passed toconfigureto disable the new hash salts.