From b0ef746f3945fa12668ea18decad30f9d1edcbcc Mon Sep 17 00:00:00 2001 From: Douglas Palmer Date: Sun, 14 Jan 2024 22:40:20 -0800 Subject: [PATCH] Permanently lock users out after X temporary lockouts during a brute force attack Closes #26172 Signed-off-by: Douglas Palmer --- .../idm/RealmRepresentation.java | 9 + .../release_notes/topics/24_0_0.adoc | 8 + .../topics/threat/brute-force.adoc | 34 ++-- .../e2e/realm_settings_events_test.spec.ts | 5 +- .../e2e/realm_settings_tabs_test.spec.ts | 5 +- .../admin/messages/messages_en.properties | 9 +- .../security-defences/BruteForceDetection.tsx | 179 ++++++++++++------ .../src/defs/realmRepresentation.ts | 1 + .../models/cache/infinispan/RealmAdapter.java | 12 ++ .../infinispan/entities/CachedRealm.java | 6 + .../infinispan/UserLoginFailureAdapter.java | 19 ++ .../entities/LoginFailureEntity.java | 16 +- .../org/keycloak/models/jpa/RealmAdapter.java | 10 + .../datastore/DefaultExportImportManager.java | 2 + .../models/utils/ModelToRepresentation.java | 2 + .../util/IdentityBrokerStateTestHelpers.java | 10 + .../java/org/keycloak/models/RealmModel.java | 2 + .../models/UserLoginFailureModel.java | 2 + .../managers/DefaultBruteForceProtector.java | 62 +++--- .../services/managers/RealmManager.java | 1 + .../admin/AttackDetectionResource.java | 2 + .../admin/AttackDetectionResourceTest.java | 21 +- .../testsuite/forms/BruteForceTest.java | 59 +++++- 23 files changed, 362 insertions(+), 114 deletions(-) diff --git a/core/src/main/java/org/keycloak/representations/idm/RealmRepresentation.java b/core/src/main/java/org/keycloak/representations/idm/RealmRepresentation.java index 47d62c7e1b..408f878ef3 100755 --- a/core/src/main/java/org/keycloak/representations/idm/RealmRepresentation.java +++ b/core/src/main/java/org/keycloak/representations/idm/RealmRepresentation.java @@ -91,6 +91,7 @@ public class RealmRepresentation { //--- brute force settings protected Boolean bruteForceProtected; protected Boolean permanentLockout; + protected Integer maxTemporaryLockouts; protected Integer maxFailureWaitSeconds; protected Integer minimumQuickLoginWaitSeconds; protected Integer waitIncrementSeconds; @@ -764,6 +765,14 @@ public class RealmRepresentation { this.permanentLockout = permanentLockout; } + public Integer getMaxTemporaryLockouts() { + return maxTemporaryLockouts; + } + + public void setMaxTemporaryLockouts(Integer maxTemporaryLockouts) { + this.maxTemporaryLockouts = maxTemporaryLockouts; + } + public Integer getMaxFailureWaitSeconds() { return maxFailureWaitSeconds; } diff --git a/docs/documentation/release_notes/topics/24_0_0.adoc b/docs/documentation/release_notes/topics/24_0_0.adoc index ea88486d58..3d1f48061c 100644 --- a/docs/documentation/release_notes/topics/24_0_0.adoc +++ b/docs/documentation/release_notes/topics/24_0_0.adoc @@ -274,6 +274,14 @@ As of this release, {project_name} supports storing and searching by user attrib For more details, check the link:{upgradingguide_link}[{upgradingguide_name}]. += Brute Force Protection changes + +There have been a couple of enhancements to the Brute Protection: + +1. When an attempt to authenticate with an OTP or Recovery Code fails due to Brute Force Protection the active Authentication Session is invalidated. Any further attempts to authenticate with that session will fail. + +2. In previous versions of Keycloak the Administrator had to choose between whether to disable users temporarily or permanently due to a Brute Force attack on their account. The administrator now has the option to permanently disable a user after a given number of temporary lockouts. + = Authorization Policy In previous versions of Keycloak when the last member of a User, Group or Client policy was deleted then that policy would also be deleted. Unfortunately this could lead to an escalation of privileges if the policy was used in an aggregate policy. To avoid privilege escalation the effect policies are no longer deleted and an administrator will need to update those policies. diff --git a/docs/documentation/server_admin/topics/threat/brute-force.adoc b/docs/documentation/server_admin/topics/threat/brute-force.adoc index c49a3d3ccc..44d641001f 100644 --- a/docs/documentation/server_admin/topics/threat/brute-force.adoc +++ b/docs/documentation/server_admin/topics/threat/brute-force.adoc @@ -47,20 +47,6 @@ When a user is temporarily locked and attempts to log in, {project_name} display |=== -*Permanent Lockout Flow* -==== -. On successful login -.. Reset `count` -. On failed login -.. Increment `count` -.. If `count` greater than _Max Login Failures_ -... Permanently disable user -.. Else if the time between this failure and the last failure is less than _Quick Login Check Milliseconds_ -... Temporarily disable user for _Minimum Quick Login Wait_ - -When {project_name} disables a user, the user cannot log in until an administrator enables the user. Enabling an account resets the `count`. -==== - *Temporary Lockout Parameters* |=== @@ -92,6 +78,7 @@ wait time will never reach the value you have set to `Max wait`. .. Calculate `wait` using _Wait Increment_ * (`count` / _Max Login Failures_). The division is an integer division rounded down to a whole number .. If `wait` equals 0 and the time between this failure and the last failure is less than _Quick Login Check Milliseconds_, set `wait` to _Minimum Quick Login Wait_. ... Temporarily disable the user for the smallest of `wait` and _Max Wait_ seconds +... Increment the temporary lockout counter `count` does not increment when a temporarily disabled account commits a login failure. ==== @@ -117,6 +104,25 @@ Note that the `Effective Wait Time` at the 5th failed attempt will disable the a the next multiple of `Max Login Failures`, in this case `10`, will the time increase from `30` to `60`. The time the account will be disabled is only increased when reaching multiples of `Max Login Failures`. +*Permanent Lockout Parameters* + +|=== +|Name |Description |Default + +|Max temporary Lockouts +|The maximum number of temporary lockouts permitted before permanent lockout occurs. +|0 +|=== + +*Permanent Lockout Flow* +==== +. Follow temporary lockout flow +. If temporary lockout counter exceeds Max temporary lockouts +.. Permanently disable user + +When {project_name} disables a user, the user cannot log in until an administrator enables the user. Enabling an account resets the `count`. +==== + The downside of {project_name} brute force detection is that the server becomes vulnerable to denial of service attacks. When implementing a denial of service attack, an attacker can attempt to log in by guessing passwords for any accounts it knows and eventually causing {project_name} to disable the accounts. Consider using intrusion prevention software (IPS). {project_name} logs every login failure and client IP address failure. You can point the IPS to the {project_name} server's log file, and the IPS can modify firewalls to block connections from these IP addresses. diff --git a/js/apps/admin-ui/cypress/e2e/realm_settings_events_test.spec.ts b/js/apps/admin-ui/cypress/e2e/realm_settings_events_test.spec.ts index 42becc1500..d81bea5ec9 100644 --- a/js/apps/admin-ui/cypress/e2e/realm_settings_events_test.spec.ts +++ b/js/apps/admin-ui/cypress/e2e/realm_settings_events_test.spec.ts @@ -251,7 +251,10 @@ describe("Realm settings events tab tests", () => { cy.findByTestId("brute-force-tab-save").should("be.disabled"); - cy.get("#bruteForceProtected").click({ force: true }); + cy.get("#kc-brute-force-mode").click(); + cy.findByTestId("select-brute-force-mode") + .contains("Lockout temporarily") + .click(); cy.findByTestId("waitIncrementSeconds").type("1"); cy.findByTestId("maxFailureWaitSeconds").type("1"); cy.findByTestId("maxDeltaTimeSeconds").type("1"); diff --git a/js/apps/admin-ui/cypress/e2e/realm_settings_tabs_test.spec.ts b/js/apps/admin-ui/cypress/e2e/realm_settings_tabs_test.spec.ts index 3e0d8617a5..084bd61dc1 100644 --- a/js/apps/admin-ui/cypress/e2e/realm_settings_tabs_test.spec.ts +++ b/js/apps/admin-ui/cypress/e2e/realm_settings_tabs_test.spec.ts @@ -158,7 +158,10 @@ describe("Realm settings tabs tests", () => { sidebarPage.goToRealmSettings(); realmSettingsPage.goToSecurityDefensesTab(); realmSettingsPage.goToSecurityDefensesBruteForceTab(); - cy.get("#bruteForceProtected").click({ force: true }); + cy.get("#kc-brute-force-mode").click(); + cy.findByTestId("select-brute-force-mode") + .contains("Lockout temporarily") + .click(); cy.findByTestId("waitIncrementSeconds").type("1"); cy.findByTestId("maxFailureWaitSeconds").type("1"); cy.findByTestId("maxDeltaTimeSeconds").type("1"); diff --git a/js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties b/js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties index 034d1a791b..ad25cb7810 100644 --- a/js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties +++ b/js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties @@ -715,6 +715,8 @@ permissionType=Specifies that this permission must be applied to all resources i policyEnforcementModes.ENFORCING=Enforcing rowSaveBtnAriaLabel=Save edits for {{messageBundle}} permanentLockout=Permanent lockout +maxTemporaryLockouts=Maximum temporary lockouts +maxTemporaryLockoutsHelp=The number of temporary lockouts permitted before the user is permanently locked out. debug=Debug webAuthnPolicyRequireResidentKey=Require discoverable credential unlockUsersConfirm=All the users that are temporarily locked will be unlocked. @@ -2903,7 +2905,7 @@ eventTypes.IDENTITY_PROVIDER_FIRST_LOGIN_ERROR.description=Identity provider fir groups=Groups emptyStateText=There aren't any realm roles in this realm. Create a realm role to get started. includeSubGroups=Include sub-group users -permanentLockoutHelp=Lock the user permanently when the user exceeds the maximum login failures. +permanentLockoutHelp=Configures whether a user is temporarily or permanently disabled after too many login failures. Permanent lockout can be configured to occur after a number of login failures or after a number of temporary lockouts. logicType.positive=Positive associatedPolicy=Associated policy accountTheme=Account theme @@ -3053,3 +3055,8 @@ userNotSaved=The user has not been saved\: {{error}} kcNumberFormat=Number Format kcNumberUnFormat=Number UnFormat tokenExpirationHelp=Sets the expiration for tokens. Expired tokens are periodically deleted from the database. +bruteForceMode.Disabled=Disabled +bruteForceMode.PermanentLockout=Lockout permanently +bruteForceMode.TemporaryLockout=Lockout temporarily +bruteForceMode.PermanentAfterTemporaryLockout=Lockout permanently after temporary lockout +bruteForceMode=Brute Force Mode diff --git a/js/apps/admin-ui/src/realm-settings/security-defences/BruteForceDetection.tsx b/js/apps/admin-ui/src/realm-settings/security-defences/BruteForceDetection.tsx index e01b1bbf8e..da76a1d0ac 100644 --- a/js/apps/admin-ui/src/realm-settings/security-defences/BruteForceDetection.tsx +++ b/js/apps/admin-ui/src/realm-settings/security-defences/BruteForceDetection.tsx @@ -4,10 +4,12 @@ import { Button, FormGroup, NumberInput, - Switch, + Select, + SelectOption, + SelectVariant, } from "@patternfly/react-core"; -import { useEffect } from "react"; -import { Controller, FormProvider, useForm, useWatch } from "react-hook-form"; +import { useEffect, useState } from "react"; +import { Controller, FormProvider, useForm } from "react-hook-form"; import { useTranslation } from "react-i18next"; import { FormAccess } from "../../components/form/FormAccess"; @@ -33,19 +35,41 @@ export const BruteForceDetection = ({ formState: { isDirty }, } = form; - const enable = useWatch({ - control, - name: "bruteForceProtected", - }); + const [isBruteForceModeOpen, setIsBruteForceModeOpen] = useState(false); + const [isBruteForceModeUpdated, setIsBruteForceModeUpdated] = useState(false); - const permanentLockout = useWatch({ - control, - name: "permanentLockout", - }); + enum BruteForceMode { + Disabled = "Disabled", + PermanentLockout = "PermanentLockout", + TemporaryLockout = "TemporaryLockout", + PermanentAfterTemporaryLockout = "PermanentAfterTemporaryLockout", + } - const setupForm = () => convertToFormValues(realm, setValue); + const bruteForceModes = [ + BruteForceMode.Disabled, + BruteForceMode.PermanentLockout, + BruteForceMode.TemporaryLockout, + BruteForceMode.PermanentAfterTemporaryLockout, + ]; + + const setupForm = () => { + convertToFormValues(realm, setValue); + setIsBruteForceModeUpdated(false); + }; useEffect(setupForm, []); + const bruteForceMode = (() => { + if (!form.getValues("bruteForceProtected")) { + return BruteForceMode.Disabled; + } + if (!form.getValues("permanentLockout")) { + return BruteForceMode.TemporaryLockout; + } + return form.getValues("maxTemporaryLockouts") == 0 + ? BruteForceMode.PermanentLockout + : BruteForceMode.PermanentAfterTemporaryLockout; + })(); + return ( + } > - ( - - )} - /> + - {enable && ( + {bruteForceMode !== BruteForceMode.Disabled && ( <> - - ( - - )} - /> - - {!permanentLockout && ( + {bruteForceMode === + BruteForceMode.PermanentAfterTemporaryLockout && ( + + } + fieldId="maxTemporaryLockouts" + hasNoPaddingTop + > + ( + field.onChange(field.value + 1)} + onMinus={() => field.onChange(field.value - 1)} + onChange={(event) => + field.onChange( + Number((event.target as HTMLInputElement).value), + ) + } + aria-label={t("maxTemporaryLockouts")} + /> + )} + /> + + )} + + {(bruteForceMode === BruteForceMode.TemporaryLockout || + bruteForceMode === + BruteForceMode.PermanentAfterTemporaryLockout) && ( <>