Skip to content

Commit

Permalink
KEYCLOAK-15985 Add Brute Force Detection Lockout Event
Browse files Browse the repository at this point in the history
  • Loading branch information
@paulbares @pedroigor
paulbares authored and pedroigor committed Sep 15, 2023
1 parent 95ecf44 commit f684a70
Show file tree
Hide file tree
Showing 4 changed files with 119 additions and 37 deletions.
4 changes: 3 additions & 1 deletion server-spi-private/src/main/java/org/keycloak/events/EventType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,10 @@ public enum EventType implements EnumWithStableIndex {

// PAR request.
PUSHED_AUTHORIZATION_REQUEST(51, false),
PUSHED_AUTHORIZATION_REQUEST_ERROR(0x10000 + PUSHED_AUTHORIZATION_REQUEST.getStableIndex(), false);
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);

private final int stableIndex;
private final boolean saveByDefault;
Expand Down
73 changes: 62 additions & 11 deletions services/src/main/java/org/keycloak/services/managers/DefaultBruteForceProtector.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@
import org.jboss.logging.Logger;
import org.keycloak.common.ClientConnection;
import org.keycloak.common.util.Time;
import org.keycloak.events.Details;
import org.keycloak.events.EventBuilder;
import org.keycloak.events.EventType;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.KeycloakSessionFactory;
import org.keycloak.models.RealmModel;
Expand Down Expand Up @@ -59,12 +62,12 @@ public class DefaultBruteForceProtector implements Runnable, BruteForceProtector
protected abstract class LoginEvent implements Comparable<LoginEvent> {
protected final String realmId;
protected final String userId;
protected final String ip;
protected final ClientConnection clientConnection;

protected LoginEvent(String realmId, String userId, String ip) {
protected LoginEvent(String realmId, String userId, ClientConnection clientConnection) {
this.realmId = realmId;
this.userId = userId;
this.ip = ip;
this.clientConnection = new AdaptedClientConnection(clientConnection);
}

@Override
Expand All @@ -82,16 +85,58 @@ public ShutdownEvent() {
protected class FailedLogin extends LoginEvent {
protected final CountDownLatch latch = new CountDownLatch(1);

public FailedLogin(String realmId, String userId, String ip) {
super(realmId, userId, ip);
public FailedLogin(String realmId, String userId, ClientConnection clientConnection) {
super(realmId, userId, clientConnection);
}
}

protected class SuccessfulLogin extends LoginEvent {
protected final CountDownLatch latch = new CountDownLatch(1);

public SuccessfulLogin(String realmId, String userId, String ip) {
super(realmId, userId, ip);
public SuccessfulLogin(String realmId, String userId, ClientConnection clientConnection) {
super(realmId, userId, clientConnection);
}
}

protected static class AdaptedClientConnection implements ClientConnection {

private final String remoteAddr;
private final String remoteHost;
private final int remotePort;
private final String localAddr;
private final int localPort;

public AdaptedClientConnection(ClientConnection c) {
this.remoteAddr = c == null ? null : c.getRemoteAddr();
this.remoteHost = c == null ? null : c.getRemoteHost();
this.remotePort = c == null ? 0 : c.getRemotePort();
this.localAddr = c == null ? null : c.getLocalAddr();
this.localPort = c == null ? 0 : c.getLocalPort();
}

@Override
public String getRemoteAddr() {
return this.remoteAddr;
}

@Override
public String getRemoteHost() {
return this.remoteHost;
}

@Override
public int getRemotePort() {
return this.remotePort;
}

@Override
public String getLocalAddr() {
return this.localAddr;
}

@Override
public int getLocalPort() {
return this.localPort;
}
}

Expand All @@ -110,7 +155,7 @@ protected void failure(KeycloakSession session, LoginEvent event) {
if (userLoginFailure == null) {
userLoginFailure = session.loginFailures().addUserLoginFailure(realm, userId);
}
userLoginFailure.setLastIPFailure(event.ip);
userLoginFailure.setLastIPFailure(event.clientConnection.getRemoteAddr());
long currentTime = Time.currentTimeMillis();
long last = userLoginFailure.getLastFailure();
long deltaTime = 0;
Expand All @@ -131,6 +176,12 @@ protected void failure(KeycloakSession session, LoginEvent event) {
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();
return;
}

Expand Down Expand Up @@ -264,7 +315,7 @@ protected void success(KeycloakSession session, LoginEvent event) {
}

protected void logFailure(LoginEvent event) {
ServicesLogger.LOGGER.loginFailure(event.userId, event.ip);
ServicesLogger.LOGGER.loginFailure(event.userId, event.clientConnection.getRemoteAddr());
failures++;
long delta = 0;
if (lastFailure > 0) {
Expand All @@ -281,7 +332,7 @@ protected void logFailure(LoginEvent event) {
@Override
public void failedLogin(RealmModel realm, UserModel user, ClientConnection clientConnection) {
try {
FailedLogin event = new FailedLogin(realm.getId(), user.getId(), clientConnection.getRemoteAddr());
FailedLogin event = new FailedLogin(realm.getId(), user.getId(), clientConnection);
queue.offer(event);
// wait a minimum of seconds for type to process so that a hacker
// cannot flood with failed logins and overwhelm the queue and not have notBefore updated to block next requests
Expand All @@ -294,7 +345,7 @@ public void failedLogin(RealmModel realm, UserModel user, ClientConnection clien

@Override
public void successfulLogin(final RealmModel realm, final UserModel user, final ClientConnection clientConnection) {
SuccessfulLogin event = new SuccessfulLogin(realm.getId(), user.getId(), clientConnection.getRemoteAddr());
SuccessfulLogin event = new SuccessfulLogin(realm.getId(), user.getId(), clientConnection);
queue.offer(event);
logger.trace("sent success event");
}
Expand Down
2 changes: 1 addition & 1 deletion .../integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/AssertEvents.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ public EventRepresentation assertEvent() {
}

public EventRepresentation assertEvent(EventRepresentation actual) {
if (expected.getError() != null && ! expected.getType().toString().endsWith("_ERROR")) {
if (expected.getError() != null && ! expected.getType().endsWith("_ERROR")) {
expected.setType(expected.getType() + "_ERROR");
}
assertThat("type", actual.getType(), is(expected.getType()));
Expand Down
77 changes: 53 additions & 24 deletions ...tion-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BruteForceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.keycloak.models.Constants;
import org.keycloak.models.UserModel;
import org.keycloak.models.utils.TimeBasedOTP;
import org.keycloak.representations.idm.EventRepresentation;
import org.keycloak.representations.idm.RealmRepresentation;
import org.keycloak.representations.idm.UserRepresentation;
import org.keycloak.services.managers.BruteForceProtector;
Expand All @@ -51,9 +52,7 @@

import jakarta.mail.internet.MimeMessage;
import java.net.MalformedURLException;
import java.util.Calendar;
import java.util.Collections;
import java.util.Map;
import java.util.*;

import static org.hamcrest.CoreMatchers.is;
import static org.junit.Assert.assertEquals;
Expand Down Expand Up @@ -477,7 +476,7 @@ public void testBrowserMissingTotp() throws Exception {
}

@Test
public void testPermanentLockout() throws Exception {
public void testPermanentLockout() {
RealmRepresentation realm = testRealm().toRepresentation();

try {
Expand All @@ -486,8 +485,15 @@ public void testPermanentLockout() throws Exception {
testRealm().update(realm);

// act
loginInvalidPassword();
loginInvalidPassword();
loginInvalidPassword("test-user@localhost");
loginInvalidPassword("test-user@localhost", false);

// As of now, there are two events: USER_DISABLED_BY_PERMANENT_LOCKOUT and LOGIN_ERROR but Order is not
// guarantee though since the brute force detector is running separately "in its own thread" named
// "Brute Force Protector".
List<EventRepresentation> actualEvents = Arrays.asList(events.poll(), events.poll());
assertIsContained(events.expect(EventType.USER_DISABLED_BY_PERMANENT_LOCKOUT).client((String) null).detail(Details.REASON, "brute_force_attack detected"), actualEvents);
assertIsContained(events.expect(EventType.LOGIN_ERROR).error(Errors.INVALID_USER_CREDENTIALS), actualEvents);

// assert
expectPermanentlyDisabled();
Expand All @@ -509,7 +515,7 @@ public void testPermanentLockout() throws Exception {
}

@Test
public void testResetLoginFailureCount() throws Exception {
public void testResetLoginFailureCount() {
RealmRepresentation realm = testRealm().toRepresentation();

try {
Expand Down Expand Up @@ -608,20 +614,19 @@ public void testResetPassword() throws Exception {
events.clear();
}

public void expectTemporarilyDisabled() throws Exception {
public void expectTemporarilyDisabled() {
expectTemporarilyDisabled("test-user@localhost", null, "password");
}

public void expectTemporarilyDisabled(String username, String userId) throws Exception {
public void expectTemporarilyDisabled(String username, String userId) {
expectTemporarilyDisabled(username, userId, "password");
}

public void expectTemporarilyDisabled(String username, String userId, String password) throws Exception {
public void expectTemporarilyDisabled(String username, String userId, String password) {
loginPage.open();
loginPage.login(username, password);

loginPage.assertCurrent();
String src = driver.getPageSource();
Assert.assertEquals("Invalid username or password.", loginPage.getInputError());
ExpectedEvent event = events.expectLogin()
.session((String) null)
Expand All @@ -634,11 +639,11 @@ public void expectTemporarilyDisabled(String username, String userId, String pas
event.assertEvent();
}

public void expectPermanentlyDisabled() throws Exception {
expectPermanentlyDisabled("test-user@localhost", null);
public void expectPermanentlyDisabled() {
expectPermanentlyDisabled("test-user@localhost");
}

public void expectPermanentlyDisabled(String username, String userId) throws Exception {
public void expectPermanentlyDisabled(String username) {
loginPage.open();
loginPage.login(username, "password");

Expand All @@ -649,17 +654,14 @@ public void expectPermanentlyDisabled(String username, String userId) throws Exc
.error(Errors.USER_DISABLED)
.detail(Details.USERNAME, username)
.removeDetail(Details.CONSENT);
if (userId != null) {
event.user(userId);
}
event.assertEvent();
}

public void loginSuccess() throws Exception {
public void loginSuccess() {
loginSuccess("test-user@localhost");
}

public void loginSuccess(String username) throws Exception {
public void loginSuccess(String username) {
loginPage.open();
loginPage.login(username, "password");

Expand All @@ -676,8 +678,6 @@ public void loginSuccess(String username) throws Exception {
String idTokenHint = oauth.doAccessTokenRequest(code, "password").getIdToken();
appPage.logout(idTokenHint);
events.clear();


}

public void loginWithTotpFailure() {
Expand Down Expand Up @@ -752,19 +752,25 @@ public void loginWithMissingTotp() throws Exception {
events.clear();
}

public void loginInvalidPassword() throws Exception {
public void loginInvalidPassword() {
loginInvalidPassword("test-user@localhost");
}

public void loginInvalidPassword(String username) throws Exception {
public void loginInvalidPassword(String username) {
loginInvalidPassword(username, true);
}

public void loginInvalidPassword(String username, boolean clearEventsQueue) {
loginPage.open();
loginPage.login(username, "invalid");

loginPage.assertCurrent();

Assert.assertEquals("Invalid username or password.", loginPage.getInputError());

events.clear();
if (clearEventsQueue) {
events.clear();
}
}

public void loginMissingPassword() {
Expand Down Expand Up @@ -826,4 +832,27 @@ private void lockUserWithPasswordGrant() throws Exception {
sendInvalidPasswordPasswordGrant();
}
}

/**
* Verifies the given {@link ExpectedEvent} is "contained" in the collection of actual events. An
* {@link ExpectedEvent expectedEvent} object is considered equal to a
* {@link EventRepresentation eventRepresentation} object if {@code
* expectedEvent.assertEvent(eventRepresentation)} does not throw any {@link AssertionError}.
*
* @param expectedEvent the expected event
* @param actualEvents the collection of {@link EventRepresentation}
*/
public void assertIsContained(ExpectedEvent expectedEvent, List<? extends EventRepresentation> actualEvents) {
List<String> messages = new ArrayList<>();
for (EventRepresentation e : actualEvents) {
try {
expectedEvent.assertEvent(e);
return;
} catch (AssertionError error) {
// silently fail
messages.add(error.getMessage());
}
}
Assert.fail(String.format("Expected event not found. Possible reasons are: %s", messages));
}
}

0 comments on commit f684a70

Please sign in to comment.