From ac1780a54f5f4c190fb010e07555ef5be97498fd Mon Sep 17 00:00:00 2001 From: Tero Saarni Date: Wed, 7 Feb 2024 16:13:33 +0200 Subject: [PATCH] Added event for temporary lockout for brute force protector (#26630) This change adds event for brute force protector when user account is temporarily disabled. It also lowers the priority of free-text log for failed login attempts. Signed-off-by: Tero Saarni Signed-off-by: Alexander Schwartz Co-authored-by: Alexander Schwartz --- .../release_notes/topics/24_0_0.adoc | 8 +++++ .../server_admin/topics/events/login.adoc | 14 ++++++++ .../topics/templates/document-attributes.adoc | 2 ++ .../topics/keycloak/changes-24_0_0.adoc | 12 ++++++- .../admin/messages/messages_en.properties | 4 +++ .../java/org/keycloak/events/Details.java | 3 ++ .../java/org/keycloak/events/EventType.java | 5 ++- .../org/keycloak/services/ServicesLogger.java | 4 --- .../managers/DefaultBruteForceProtector.java | 34 +++++++++++++++---- .../testsuite/forms/BruteForceTest.java | 10 ++++++ 10 files changed, 83 insertions(+), 13 deletions(-) diff --git a/docs/documentation/release_notes/topics/24_0_0.adoc b/docs/documentation/release_notes/topics/24_0_0.adoc index 74634ebbea..d45372b960 100644 --- a/docs/documentation/release_notes/topics/24_0_0.adoc +++ b/docs/documentation/release_notes/topics/24_0_0.adoc @@ -188,6 +188,14 @@ spec: key: config.xml ---- += Temporary lockout log replaced with event + +There is now a new event `USER_DISABLED_BY_TEMPORARY_LOCKOUT` when a user is temporarily locked out by the brute force protector. +The log with ID `KC-SERVICES0053` has been removed as the new event offers the information in a structured form. + +For more details, check the +link:{upgradingguide_link}[{upgradingguide_name}]. + = Updates to cookies Cookie handling code has been refactored and improved, including a new Cookie Provider. This provides better consistency diff --git a/docs/documentation/server_admin/topics/events/login.adoc b/docs/documentation/server_admin/topics/events/login.adoc index 899bd797e9..6471c4ff2b 100644 --- a/docs/documentation/server_admin/topics/events/login.adoc +++ b/docs/documentation/server_admin/topics/events/login.adoc @@ -64,6 +64,19 @@ image:images/search-user-event.png[Search user event] |=== +*Brute force protection:* + +[cols="2",options="header"] +|=== +|Event |Description +|User disabled by permanent lockout +|Brute force protection disabled the user account permanently due to too many login failures. + +|User disabled by temporary lockout +|Brute force protection disabled the user account temporarily due to too many login failures. + +|=== + *Account events:* [cols="2",options="header"] @@ -103,6 +116,7 @@ image:images/search-user-event.png[Search user event] Each event has a corresponding error event. +[[event-listener]] ==== Event listener Event listeners listen for events and perform actions based on that event. {project_name} includes two built-in listeners, the Logging Event Listener and Email Event Listener. diff --git a/docs/documentation/topics/templates/document-attributes.adoc b/docs/documentation/topics/templates/document-attributes.adoc index e108d1bacc..8fa0d6842a 100644 --- a/docs/documentation/topics/templates/document-attributes.adoc +++ b/docs/documentation/topics/templates/document-attributes.adoc @@ -50,6 +50,8 @@ :adminguide_link_latest: {project_doc_base_url_latest}/server_admin/ :adminguide_bruteforce_name: Password guess: brute force attacks :adminguide_bruteforce_link: {adminguide_link}#password-guess-brute-force-attacks +:adminguide_eventlistener_name: Event listener +:adminguide_eventlistener_link: {adminguide_link}#event-listener :adminguide_timeouts_name: Timeouts :adminguide_timeouts_link: {adminguide_link}#_timeouts :adminguide_clearcache_name: Clearing Server Caches diff --git a/docs/documentation/upgrading/topics/keycloak/changes-24_0_0.adoc b/docs/documentation/upgrading/topics/keycloak/changes-24_0_0.adoc index 672be3dea5..fb2002b511 100644 --- a/docs/documentation/upgrading/topics/keycloak/changes-24_0_0.adoc +++ b/docs/documentation/upgrading/topics/keycloak/changes-24_0_0.adoc @@ -329,7 +329,17 @@ After removal of the Map Store the following modules were renamed: * `org.keycloak:keycloak-model-legacy-private` to `org.keycloak:keycloak-model-storage-private` * `org.keycloak:keycloak-model-legacy-services` to `org.keycloak:keycloak-model-storage-services` -and `org.keycloak:keycloak-model-legacy` module was deprecated and will be removed in the next release in favour of `org.keycloak:keycloak-model-storage` module. +and `org.keycloak:keycloak-model-legacy` module was deprecated and will be removed in the next release in favour of `org.keycloak:keycloak-model-storage` module. + += Temporary lockout log replaced with event + +There is now a new event `USER_DISABLED_BY_TEMPORARY_LOCKOUT` when a user is temporarily locked out by the brute force protector. +The log with ID `KC-SERVICES0053` has been removed as the new event offers the information in a structured form. + +As it is a success event, the new event is logged by default at the `DEBUG` level. +Use the setting `spi-events-listener-jboss-logging-success-level` as described in the link:{adminguide_eventlistener_link}[{adminguide_eventlistener_name} chapter in the {adminguide_name}] to change the log level of all success events. + +To trigger custom actions or custom log entries, write a custom event listener as described in the Event Listener SPI in the link:{developerguide_link}[{developerguide_name}]. = Updates to cookies 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 e193beba86..786db18861 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 @@ -226,6 +226,8 @@ usermodel.attr.label=User Attribute eventTypes.REGISTER.name=Register eventTypes.USER_DISABLED_BY_PERMANENT_LOCKOUT.name=User disabled by permanent lockout eventTypes.USER_DISABLED_BY_PERMANENT_LOCKOUT_ERROR.name=User disabled by permanent lockout error +eventTypes.USER_DISABLED_BY_TEMPORARY_LOCKOUT.name=User disabled by temporary lockout +eventTypes.USER_DISABLED_BY_TEMPORARY_LOCKOUT_ERROR.name=User disabled by temporary lockout error deleteUser=Delete user addedNodeSuccess=Node successfully added eventTypes.INTROSPECT_TOKEN_ERROR.description=Introspect token error @@ -1293,6 +1295,8 @@ removeUser=Remove users ownerManagedAccess=User-Managed access enabled eventTypes.USER_DISABLED_BY_PERMANENT_LOCKOUT.description=User disabled by permanent lockout eventTypes.USER_DISABLED_BY_PERMANENT_LOCKOUT_ERROR.description=User disabled by permanent lockout error +eventTypes.USER_DISABLED_BY_TEMPORARY_LOCKOUT.description=User disabled by temporary lockout +eventTypes.USER_DISABLED_BY_TEMPORARY_LOCKOUT_ERROR.description=User disabled by temporary lockout error userModelAttributeNameHelp=Name of the model attribute to be added when importing user from LDAP templateHelp=Template to use to format the username to import. Substitutions are enclosed in ${}. For example\: '${ALIAS}.${CLAIM.sub}'. ALIAS is the provider alias. CLAIM. references an ID or Access token claim. The substitution can be converted to upper or lower case by appending |uppercase or |lowercase to the substituted value, e.g. '${CLAIM.sub | lowercase} permissions=Permissions diff --git a/server-spi-private/src/main/java/org/keycloak/events/Details.java b/server-spi-private/src/main/java/org/keycloak/events/Details.java index e2d2eae927..db2db25c4e 100755 --- a/server-spi-private/src/main/java/org/keycloak/events/Details.java +++ b/server-spi-private/src/main/java/org/keycloak/events/Details.java @@ -87,4 +87,7 @@ public interface Details { String CREDENTIAL_TYPE = "credential_type"; String SELECTED_CREDENTIAL_ID = "selected_credential_id"; String AUTHENTICATION_ERROR_DETAIL = "authentication_error_detail"; + + String NOT_BEFORE = "not_before"; + String NUM_FAILURES = "num_failures"; } diff --git a/server-spi-private/src/main/java/org/keycloak/events/EventType.java b/server-spi-private/src/main/java/org/keycloak/events/EventType.java index 7211b3c818..43158a2928 100755 --- a/server-spi-private/src/main/java/org/keycloak/events/EventType.java +++ b/server-spi-private/src/main/java/org/keycloak/events/EventType.java @@ -158,7 +158,10 @@ public enum EventType implements EnumWithStableIndex { PUSHED_AUTHORIZATION_REQUEST_ERROR(0x10000 + PUSHED_AUTHORIZATION_REQUEST.getStableIndex(), false), USER_DISABLED_BY_PERMANENT_LOCKOUT(52, true), - USER_DISABLED_BY_PERMANENT_LOCKOUT_ERROR(0x10000 + USER_DISABLED_BY_PERMANENT_LOCKOUT.getStableIndex(), false); + USER_DISABLED_BY_PERMANENT_LOCKOUT_ERROR(0x10000 + USER_DISABLED_BY_PERMANENT_LOCKOUT.getStableIndex(), false), + + USER_DISABLED_BY_TEMPORARY_LOCKOUT(53,true), + USER_DISABLED_BY_TEMPORARY_LOCKOUT_ERROR(0x10000 + USER_DISABLED_BY_TEMPORARY_LOCKOUT.getStableIndex(), false); private final int stableIndex; private final boolean saveByDefault; diff --git a/services/src/main/java/org/keycloak/services/ServicesLogger.java b/services/src/main/java/org/keycloak/services/ServicesLogger.java index 8c01b99fb8..9e26497e5e 100644 --- a/services/src/main/java/org/keycloak/services/ServicesLogger.java +++ b/services/src/main/java/org/keycloak/services/ServicesLogger.java @@ -253,10 +253,6 @@ public interface ServicesLogger extends BasicLogger { @Message(id=52, value="Failed processing type") void failedProcessingType(@Cause Exception e); - @LogMessage(level = WARN) - @Message(id=53, value="login failure for user %s from ip %s") - void loginFailure(String user, String ip); - @LogMessage(level = ERROR) @Message(id=54, value="Unknown action: %s") void unknownAction(String action); diff --git a/services/src/main/java/org/keycloak/services/managers/DefaultBruteForceProtector.java b/services/src/main/java/org/keycloak/services/managers/DefaultBruteForceProtector.java index c4f01c2d1a..a21bc70039 100644 --- a/services/src/main/java/org/keycloak/services/managers/DefaultBruteForceProtector.java +++ b/services/src/main/java/org/keycloak/services/managers/DefaultBruteForceProtector.java @@ -30,6 +30,10 @@ import org.keycloak.models.UserLoginFailureModel; import org.keycloak.models.UserModel; import org.keycloak.services.ServicesLogger; +import java.time.Instant; +import java.time.LocalDateTime; +import java.time.ZoneId; +import java.time.ZoneOffset; import java.util.ArrayList; import java.util.Collections; import java.util.concurrent.CountDownLatch; @@ -176,12 +180,7 @@ public class DefaultBruteForceProtector implements Runnable, BruteForceProtector logger.debugv("user {0} locked permanently due to too many login attempts", user.getUsername()); user.setEnabled(false); user.setSingleAttribute(DISABLED_REASON, DISABLED_BY_PERMANENT_LOCKOUT); - // Send event - new EventBuilder(realm, session, event.clientConnection) - .event(EventType.USER_DISABLED_BY_PERMANENT_LOCKOUT) - .detail(Details.REASON, "brute_force_attack detected") - .user(user) - .success(); + sendEvent(session, realm, userLoginFailure, EventType.USER_DISABLED_BY_PERMANENT_LOCKOUT); return; } @@ -191,6 +190,7 @@ public class DefaultBruteForceProtector implements Runnable, BruteForceProtector int notBefore = (int) (currentTime / 1000) + waitSeconds; logger.debugv("set notBefore: {0}", notBefore); userLoginFailure.setFailedLoginNotBefore(notBefore); + sendEvent(session, realm, userLoginFailure, EventType.USER_DISABLED_BY_TEMPORARY_LOCKOUT); } return; } @@ -219,6 +219,7 @@ public class DefaultBruteForceProtector implements Runnable, BruteForceProtector int notBefore = (int) (currentTime / 1000) + waitSeconds; logger.debugv("set notBefore: {0}", notBefore); userLoginFailure.setFailedLoginNotBefore(notBefore); + sendEvent(session, realm, userLoginFailure, EventType.USER_DISABLED_BY_TEMPORARY_LOCKOUT); } } @@ -237,6 +238,26 @@ public class DefaultBruteForceProtector implements Runnable, BruteForceProtector return realm; } + protected void sendEvent(KeycloakSession session, RealmModel realm, UserLoginFailureModel userLoginFailure, EventType type) { + EventBuilder builder = new EventBuilder(realm, session) + .ipAddress(userLoginFailure.getLastIPFailure()) + .event(type) + .detail(Details.REASON, "brute_force_attack detected") + .detail(Details.NUM_FAILURES, String.valueOf(userLoginFailure.getNumFailures())) + .user(userLoginFailure.getUserId()); + + if (type == EventType.USER_DISABLED_BY_TEMPORARY_LOCKOUT) { + long secondsSinceEpoch = userLoginFailure.getFailedLoginNotBefore(); + Instant instant = Instant.ofEpochSecond(secondsSinceEpoch); + LocalDateTime timestamp = LocalDateTime.ofInstant(instant, ZoneId.systemDefault()); + + builder.detail(Details.NOT_BEFORE, timestamp.toString()); + } + + // Send event. + builder.success(); + } + public void start() { new Thread(this, "Brute Force Protector").start(); } @@ -315,7 +336,6 @@ public class DefaultBruteForceProtector implements Runnable, BruteForceProtector } protected void logFailure(LoginEvent event) { - ServicesLogger.LOGGER.loginFailure(event.userId, event.clientConnection.getRemoteAddr()); failures++; long delta = 0; if (lastFailure > 0) { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BruteForceTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BruteForceTest.java index d7f27ae098..3dc4765076 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BruteForceTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BruteForceTest.java @@ -515,6 +515,16 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest { } } + @Test + public void testTemporaryLockout() throws Exception { + loginInvalidPassword("test-user@localhost"); + loginInvalidPassword("test-user@localhost", false); + + List actualEvents = Arrays.asList(events.poll(), events.poll()); + assertIsContained(events.expect(EventType.USER_DISABLED_BY_TEMPORARY_LOCKOUT).client((String) null).detail(Details.REASON, "brute_force_attack detected"), actualEvents); + assertIsContained(events.expect(EventType.LOGIN_ERROR).error(Errors.INVALID_USER_CREDENTIALS), actualEvents); + } + @Test public void testResetLoginFailureCount() { RealmRepresentation realm = testRealm().toRepresentation();