cirrusidentity/simplesamlphp-module-ratelimit

Key generation fails for PasswordStuffingLimit

Closed this issue · 5 comments

From the logs: loading key idp.int.ratelimit-password- from memcache

This line is here is reducing the key to an empty string;

$key = substr($hash, strlen($cryptSalt), 25);

@pradtke Have you ever tested this with Memcached as the store, because I can't get any of the limiters to work :(
Never mind, it is working.. I guess I expected some kind of feedback in the logs when a user is locked out.

Update: I see you're logging some stuff to the stats-level.. Is that on purpose?

For 'stats' level logging. That is the level UserPassBase uses for logging about unsuccessful or successful authentications, so I went with that.
https://github.com/simplesamlphp/simplesamlphp/blob/2f9ba26ae4beb79d71a8dba7f4290686de3fa16f/modules/core/lib/Auth/UserPassBase.php#L308-L312

I put stats in a different log-file, so I was a bit confused as I couldn't see any feedback... Forget about this :)
Please take a look at the PR

Hmm, I wonder if the root cause of this was your salt being incompatible with bcrypt (and code not realizing such a thing was possible). From crypt " 22 characters from the alphabet "./0-9A-Za-z". Using characters outside of this range in the salt will cause crypt() to return a zero-length string."

My salts are being generated by the command from the SSP docs which generates strings of length 32...