Manageiq: Need account lockout policy

[dmetzger57]

Related RFE BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1814507

An HCL app scan in redhat cloudforms environment and it gave error about the account lockout as follows:

Threat Classification: Brute Force

Risk: It might be possible to escalate user privileges and gain administrative permissions over the web application

Causes: Insecure web application programming or configuration

Fix: Enforce account lockout after several failed login attempts

• [x] ManageIQ/manageiq#20087
• [x] ManageIQ/manageiq-schema#467
• [x] ManageIQ/manageiq-ui-classic#6989
• [x] cross_repo - https://github.com/ManageIQ/manageiq-cross_repo-tests/pull/117

[Fryguy]

Do we want complete account lockout, or just block login for a period of time like 5 minutes or something?

[KhushbuB]

Hey, The cu looks for a complete account lockout until the admin unlocks the account.
If that is not available then lock for a period of time is fine.

[skateman]

My idea is to have a failed_login_attempts integer column on the users table which will get increased if a bad password is entered for the given username. We can have a Settings.server.max_failed_login_attempts configuration parameter that we could default to let's say 3 and at a successful login we would test against this before letting the user in. Of course if the number is below the threshold and the user enters the correct password, the failed_login_attempts resets to 0.

My only question is, how should we communicate this towards the UI/API clients if there's a bad username/password. If I will start telling the client that they have N more tries before an account lockout, they can use this information to guess account usernames. So my plan would be to show the regular bad username/password as before and only show the locked account message if the user enters the correct username/password combo for a locked account. Is this okay?

Also what should happen if the admin user gets locked out? Is it enough to implement a User#unlock! method that can be called from the rails console?

[gtanzillo]

@skateman I like the approach you described. It makes sense to me that failed login attempts are communicated in the usual way, without mentioning anything about tries before a lockout and only present an account locked out message on successful username/password after the account has been locked out.

I think the rails console method (or perhaps a script in the tools directory) to unlock would be plenty good for the initial implementation. But it would be nice to have something in the UI under settings / access control that would allow a super administrator to unlock a locked out user

[skateman]

Oh right, I didn't mention it because I thought that an UI unlock button is obvious :laughing: my console solution is for the corner-case when the admin account gets locked.

[skateman]

@gtanzillo one more thing, is this related to database authentication only? External auths should implement some kind of lockout policy of their own, right? And should this also lock out the API access using tokens? Not talking about basic auth, but token auth. IMO we should do a different kind of approach for the tokens if we do any.

[Fryguy]

@skateman I like the idea of a configurable threshold where nil can represent no lockout and could be the default (to maintain the current user experience).

The only thing I don't like about a lockout feature in general is if you know one exists, a bad actor can effectively lock other people's accounts, essentially creating a denial of service for them. I'm not sure how you avoid that. Your "what about the admin" question is a direct consequence of that.

[Fryguy]

The problem with showing the lockout after a successful login is that it effectively undoes the purpose of the lockout. That is, the point of a lockout is to prevent brute forcing. If you continully show a "bad password" notification, then when they are successful and get to the lockout screen they know they have the right password. At that point the user is compromised.

Since the purpose of the lockout is to stop brute forcing, it's probably sufficient to even lockout for a period of time (say, 1-10 minutes) kind of like your phone does, which effectively prevents a broad brute force attack, with a notice like "Too many login attempts in a short period of time. Try again in X minutes".

[abellotti]

This makes sense for DB auth only as external directory servers, IPA, AD, etc. implement their own and we don't want to step over their security implementation.

[skateman]

@Fryguy the timeout for a lockout would require a datetime of when the lockout happened and a background job for unlocking accounts over time, I guess we already have something like this, I just don't know where :thinking:

But back to the original problem, after 3 unsuccessful logins I should show for any other attempts that the user is locked out regardless of the validity of the password, right?

[Fryguy]

@Fryguy the timeout for a lockout would require a datetime of when the lockout happened and a background job for unlocking accounts over time, I guess we already have something like this, I just don't know where :thinking:

Not necessarily. At the moment of the nth lockout you put an item on the queue for some time in the future to unlock it.

But back to the original problem, after 3 unsuccessful logins I should show for any other attempts that the user is locked out regardless of the validity of the password, right?

Correct

[skateman]

Now it all makes sense, thanks! @jvlcek @abellotti are you okay with these? Can I get my hands dirty*? :wink:

* I already have it implemented, just need to dig it up from my git stashes

[jvlcek]

Now it all makes sense, thanks! @jvlcek @abellotti are you okay with these? Can I get my hands dirty*? 😉

• I already have it implemented, just need to dig it up from my git stashes

Sounds like a reasonable feature to include for DB authentication. As @abellotti mentioned above:

This makes sense for DB auth only as external directory servers, IPA, AD, etc. implement their own and we don't want to step over their security implementation.

[Fryguy]

@gtanzillo Do you still think we need the ability to unlock an account if we change it so that accounts auto-unlock after a configurable time (like 2 minutes or something?) as mentioned here

[skateman]

@Fryguy @gtanzillo IMO it can still make sense to have a UI-based unlock as the user-defined unlock timeout has no upper limit.

[gtanzillo]

@skateman @Fryguy I agree with @skateman, It would still be good to have the ability to unlock in the UI since the auto-unlock wait period is configurable. Even if it's a few minutes there could be a time-critical need to get a locked out user back in.

That said, perhaps command line force unlock would be good enough for that case.

[skateman]

Okay, so no UI-solution? I can live with that :laughing:

[Fryguy]

Ah ok I hadn't thought of a really large lockout time (perhaps we could even support infinite in the future if the admin so chose). I think a console based unlock is fine for now (no UI nor API), as I expect the likelihood of its usage is low to nothing because most admins will leave a reasonable lockout time.

[Fryguy]

Implemeted via #20087