inejge / pwhash

A collection of password hashing routines in pure Rust
MIT License
61 stars 11 forks source link

Remove length check from unix::crypt to match crypt(3) behavior #3

Closed wwalexander closed 7 years ago

wwalexander commented 7 years ago

The documentation for unix::crypt describes it as a "Unix crypt(3) work-alike." However, when unix::crypt falls back to the DES option based on the salt, it returns an error if the argument is not 13 characters. This does not match the behavior of crypt(3), which allows any salt length. For example, this C snippet compiles and runs without error using GCC 5.4.0 on Ubuntu 16.04:

#include <assert.h>
#define _GNU_SOURCE
#include <crypt.h>
#include <stdlib.h>

int main()
{
    assert(crypt("foo", "bar") != NULL);
    return 0;
}

This PR removes the length check from unix::crypt and removes a test with a salt that is not 13 characters that expects an error.

inejge commented 7 years ago

You're right, removing the check makes the function that more crypt(3)-compatible.

My intention was to limit the creation of hashes to algorithm::hash() functions, and keep unix::crypt() mostly for checking existing hashes, where the length check works as a first-line defense against malformed input. However, I'm not against making unix::crypt() closer to its libc counterpart, and since the checks in des_crypt::unix_crypt() will catch any salt errors, I'll accept the patch.

Thanks for the PR!