internetstandards / Internet.nl

Internet standards compliance test suite
https://internet.nl
171 stars 35 forks source link

Remove option for unhashed passwords #1243

Open bwbroersma opened 7 months ago

bwbroersma commented 7 months ago

Currently there is an option to provide *_AUTH and *_AUTH_RAW env variables, both allow unhashed passwords.

Initially I proposed some bash to auto-hash all non hashed passwords
function optional_hash() {
    while read -r line; do
        if [[ -z $line ]]; then
            break;
        fi;
        USR="${line%%:*}";
        PWD="${line#*:}";
        if [[ "${PWD:0:6}" == '$apr1$' || "${PWD:0:4}" == '$2y$' ]]; then
            echo "$line";
        else;
            echo "$PWD" | htpasswd -inBC11 "$USR";
        fi;
    done;
}
echo \
    'md5:$apr1$FeI1RU72$f2c6FPiROTzmozpQBKdsR/,'\   
'bf:$2y$13$pVuxwaYgNrXUIpRnuv.KX.F1DWV0JG00h85ZvNIbk7lKNeosqcFEi,'\    
'plain:special:$apr1$2y$'
| tr ',' '\n' \
| optional_hash \
> test.htpasswd

But @mxsasha proposed something way better than adding bash code: filter out the bad practice of using non encrypted passwords, so only encrypted passwords are accepted.

The current code: https://github.com/internetstandards/Internet.nl/blob/7bb8f53b97ffe467080884250bc0a81f029a978c/docker/webserver/generate_htpasswd.sh#L1-L9

Idea to drop *_AUTH and to add a grep filter to the *_AUTH_RAW:

grep -P '^[^:]+:\$(2y|apr1)\$'

Since users cannot have a :, and bcrypt and md5 passwords use the special $2y$ and $apr1. Of course this can be fooled with a plaintext password that starts with the same magic value, but that would also fool htpasswd, so the user cannot login anyway.

Demo:

echo \
    'md5:$apr1$FeI1RU72$f2c6FPiROTzmozpQBKdsR/,'\   
'bf:$2y$13$pVuxwaYgNrXUIpRnuv.KX.F1DWV0JG00h85ZvNIbk7lKNeosqcFEi,'\    
'plain:special:$apr1$2y$' \
| tr ',' '\n' \
| grep -P '^[^:]+:\$(2y|apr1)\$'
> .htpasswd
baknu commented 7 months ago

Do or should we also have a minimum password length? And do or should we also ratelimit login attempts?

bwbroersma commented 7 months ago

If only encrypted passwords are accepted there is no option to enforce a password policy on the encrypted passwords. It looks like bcrypt ($2y$) is not supported by ngx_http_auth_basic_module.

One could maybe add some custom deny policy in nginx to deny short passwords, but that would be very custom so probably not a good idea for maintainability.

_Update: double checked the algorithm support in the code and ~it's confirmed there is no bcrypt support~ there is bcrypt support via the fallback code to crypt in libc ._