daniel-frak / keycloak-user-migration

A Keycloak plugin for migrating users from legacy systems
MIT License
304 stars 136 forks source link

No defense against brute force attacks [enhancement idea] #178

Open xgp opened 4 months ago

xgp commented 4 months ago

There is currently brute force attack detection and mitigation built into Keycloak for existing users. This means that Keycloak keeps a cache of recurring login attempts, and can be configured to lockout user login attempts when it reaches a certain threshold of failures.

Because this is for existing users, it does not help the case where a user is being migrated using the keycloak-user-migration extension. Because of this, it is possible to bypass this defense, and send unlimited requests to the REST endpoints used by the extension. This presents an attack surface that may be unacceptable to organizations that have policies that require brute force attack detection and mitigation.

A potential enhancement to this extension would be to provide a simplified brute force attack detection and mitigation functionality.

xgp commented 4 months ago

One thing to note in a potential implementation is that the InfinispanUserLoginFailureProvider which is the default implementation that is returned from session.loginFailures() doesn't seem to care about the format of the userId parameters that are passed in. An experiment that might make an implementation quite a bit easier would be to pass the email/username as the userId to see if it works. Then you get an implementation for "free".

daniel-frak commented 4 months ago

Doesn't this mean that Keycloak doesn't have brute force protection against user registration, and so you can still overwhelm it by registering users, even without a migration plugin?

If not, and this is truly something that the plugin should handle, then I'd be happy to accept a PR for this :)

Unfortunately, I don't think I'll have the time to implement this myself.

xgp commented 4 months ago

Doesn't this mean that Keycloak doesn't have brute force protection against user registration, and so you can still overwhelm it by registering users, even without a migration plugin?

It's not meant to be generalized protection against all kinds of attacks. They specifically define a "brute force attack" as an attempt to guess a user’s password by trying to log in multiple times. Keycloak has brute force detection capabilities for this specific type of attack, and can temporarily disable a user account if the number of login failures exceeds a specified threshold.

If not, and this is truly something that the plugin should handle, then I'd be happy to accept a PR for this :)

Because this extension allows testing of a user's password using multiple login attempts, but does not mitigate using Keycloak's mechanism, it would need it's own mechanism of counting and disabling attempts in order to match the expected functionality of Keycloak.

Unfortunately, I don't think I'll have the time to implement this myself.

No problem. I think we can implement this, so if you wouldn't mind keeping it open, I'll see when when have some time to do this.