search

owncloud / brute_force_protection

Brute-force protection app for ownCloud
GNU General Public License v2.0
6 stars 5 forks source link

Login is not blocked after multiple retries #191

Closed saw-jan closed 1 year ago

saw-jan commented 1 year ago

Steps to reproduce

  1. Pull latest core master (https://github.com/owncloud/core)

  2. Checkout to commit eaf5a9fe952b2780a053d2b17026e420c6542381

  3. Run: make clean and make

  4. Enable brute_force_protection app

    sudo -u www-data ./occ a:e brute_force_protection

  5. Set the these from Settings -> [Admin] Security Screenshot from 2023-01-06 14-47-17

  6. Try login with invalid password many times

Expected

to see login blocked screen Screenshot from 2023-01-06 14-49-52

Actual

No login blocked screen Users can try to login infinitely

Details

The behavior occurred after PR https://github.com/owncloud/core/pull/40520

phil-davis commented 1 year ago

@jvillafanez @DeepDiver1975 I guess that this is a regression related to https://github.com/owncloud/core/pull/40520

jvillafanez commented 1 year ago

Yes, this is a change in the behavior caused by that PR.

Right now, with the changes in core, we first check whether the password is correct or not. If the password is wrong, we don't check the brute force protection and fail the login right away. Only when the password is right we check for the brute force protection.

Technically, the login is blocked because the login attempt might still fail due to the brute force protection (assuming it's still in the cool-down period) even if the password is right. However, we might still need to change the behavior because we leak the password.

Summing up, right now, the message appears only if the password is the correct one, otherwise, the login will fail without any particular message

jvillafanez commented 1 year ago

Not fully related, but it seems "banned" period isn't respected. Assuming the setup in this ticket, if you use the wrong password more than 2 times (maybe 2) in less than a minute, you must be banned for 5 minutes, but this doesn't happen. You can enter before those 5 minutes. As far as I understand the feature, this shouldn't happen.

saw-jan commented 1 year ago

I tried manually today, now it's working just as expected. Didn't know what happened previously. I will have a look at the test code.

saw-jan commented 1 year ago

Summing up, right now, the message appears only if the password is the correct one, otherwise, the login will fail without any particular message

So now, the blocking behavior has changed. Previously, trying to attempt login with an invalid password after a certain threshold also blocked the login. But now, after reaching the threshold, trying with only valid login will block the login.

If that's the correct intended behavior then we need to fix the test code.

CC @jvillafanez @phil-davis

jvillafanez commented 1 year ago

If that's the correct intended behavior then we need to fix the test code.

I'm not sure if this is what we want. An attacker could figure out the password, so even if he can't access to the account right now, he could after the banning period is over. This didn't happen before because the attacker couldn't figure out the password

jvillafanez commented 1 year ago

Can someone check if https://github.com/owncloud/brute_force_protection/pull/193 really fix this issue? As said in the PR, the behavior should be the same as before, and the code should be backwards-compatible.

saw-jan commented 1 year ago

as per the pipeline: https://drone.owncloud.com/owncloud/brute_force_protection/2204/12/13 the expected test scenario failed but was not in the expected step. I will have a look on my machine.