diff --git a/source/java/org/alfresco/repo/security/authentication/AuthenticationServiceImpl.java b/source/java/org/alfresco/repo/security/authentication/AuthenticationServiceImpl.java index d943ccc833..b91e8160f5 100644 --- a/source/java/org/alfresco/repo/security/authentication/AuthenticationServiceImpl.java +++ b/source/java/org/alfresco/repo/security/authentication/AuthenticationServiceImpl.java @@ -49,6 +49,7 @@ public class AuthenticationServiceImpl extends AbstractAuthenticationService imp private boolean allowsUserPasswordChange = true; private static final String AUTHENTICATION_UNSUCCESSFUL = "Authentication was not successful."; + private static final String BRUTE_FORCE_ATTACK_DETECTED = "Brute force attack was detected for user: %s"; private int protectionPeriodSeconds; private boolean protectionEnabled; private int protectionLimit; @@ -110,13 +111,9 @@ public class AuthenticationServiceImpl extends AbstractAuthenticationService imp // clear context - to avoid MT concurrency issue (causing domain mismatch) - see also 'validate' below clearCurrentSecurityContext(); preAuthenticationCheck(userName); - if (protectionEnabled) + if (isUserProtected(userName)) { - ProtectedUser protectedUser = protectedUsersCache.get(userName); - if (protectedUser != null && protectedUser.isProtected()) - { - throw new AuthenticationException(AUTHENTICATION_UNSUCCESSFUL); - } + throw new AuthenticationException(AUTHENTICATION_UNSUCCESSFUL); } authenticationComponent.authenticate(userName, password); if (tenant == null) @@ -136,25 +133,60 @@ public class AuthenticationServiceImpl extends AbstractAuthenticationService imp catch(AuthenticationException ae) { clearCurrentSecurityContext(); - if (protectionEnabled) - { - ProtectedUser protectedUser = protectedUsersCache.get(userName); - if (protectedUser == null) - { - protectedUser = new ProtectedUser(userName); - } - else - { - protectedUser.recordUnsuccessfulLogin(); - } - protectedUsersCache.put(userName, protectedUser); - } + recordFailedAuthentication(userName); throw ae; } ticketComponent.clearCurrentTicket(); getCurrentTicket(); } - + + /** + * @return true if user is 'protected' from brute force attack + */ + public boolean isUserProtected(String userName) + { + boolean isProtected = false; + if (protectionEnabled) + { + ProtectedUser protectedUser = protectedUsersCache.get(userName); + if (protectedUser != null) + { + long currentTimeStamp = System.currentTimeMillis(); + isProtected = protectedUser.getNumLogins() >= protectionLimit && + currentTimeStamp - protectedUser.getTimeStamp() < protectionPeriodSeconds * 1000; + } + } + return isProtected; + } + + /** + * Method records a failed login attempt. + * If the number of recorded failures exceeds {@link AuthenticationServiceImpl#protectionLimit} + * the user will be considered 'protected'. + */ + public void recordFailedAuthentication(String userName) + { + if (protectionEnabled) + { + ProtectedUser protectedUser = protectedUsersCache.get(userName); + if (protectedUser == null) + { + protectedUser = new ProtectedUser(userName); + } + else + { + protectedUser = new ProtectedUser(userName, protectedUser.getNumLogins() + 1); + if (protectedUser.getNumLogins() == protectionLimit && logger.isWarnEnabled()) + { + // Shows only first 2 symbols of the username and masks all other character with '*' + logger.warn(String.format(BRUTE_FORCE_ATTACK_DETECTED, + userName.substring(0,2) + new String(new char[(userName.length() - 2)]).replace("\0", "*"))); + } + } + protectedUsersCache.put(userName, protectedUser); + } + } + public String getCurrentUserName() throws AuthenticationException { return authenticationComponent.getCurrentUserName(); @@ -409,74 +441,4 @@ public class AuthenticationServiceImpl extends AbstractAuthenticationService imp return true; } - - /** - * An object to hold information about unsuccessful logins - */ - /*package*/ class ProtectedUser - { - private String userId; - /** number of consecutive unsuccessful login attempts */ - private long numLogins; - /** time stamp of last unsuccessful login attempt */ - private long timeStamp; - - public ProtectedUser(String userId) - { - this.userId = userId; - this.numLogins = 1; - this.timeStamp = System.currentTimeMillis(); - } - - public void recordUnsuccessfulLogin() - { - this.numLogins+=1; - this.timeStamp = System.currentTimeMillis(); - if (numLogins == protectionLimit + 1 && logger.isWarnEnabled()) - { - // Shows only first 2 symbols of the username and masks all other character with '*' - logger.warn("Brute force attack was detected for user " + - userId.substring(0,2) + new String(new char[(userId.length() - 2)]).replace("\0", "*")); - } - } - - public boolean isProtected() - { - long currentTimeStamp = System.currentTimeMillis(); - return numLogins >= protectionLimit && - currentTimeStamp - timeStamp < protectionPeriodSeconds * 1000; - } - - @Override - public String toString() - { - return "ProtectedUser{" + - "userId='" + userId + '\'' + - ", numLogins=" + numLogins + - ", timeStamp=" + timeStamp + - '}'; - } - - @Override - public boolean equals(Object o) - { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - - ProtectedUser that = (ProtectedUser) o; - - if (numLogins != that.numLogins) return false; - if (timeStamp != that.timeStamp) return false; - return userId.equals(that.userId); - } - - @Override - public int hashCode() - { - int result = userId.hashCode(); - result = 31 * result + (int) (numLogins ^ (numLogins >>> 32)); - result = 31 * result + (int) (timeStamp ^ (timeStamp >>> 32)); - return result; - } - } } diff --git a/source/java/org/alfresco/repo/security/authentication/ProtectedUser.java b/source/java/org/alfresco/repo/security/authentication/ProtectedUser.java new file mode 100644 index 0000000000..4c9e1aded2 --- /dev/null +++ b/source/java/org/alfresco/repo/security/authentication/ProtectedUser.java @@ -0,0 +1,102 @@ +/* + * #%L + * Alfresco Repository + * %% + * Copyright (C) 2005 - 2016 Alfresco Software Limited + * %% + * This file is part of the Alfresco software. + * If the software was purchased under a paid Alfresco license, the terms of + * the paid license agreement will prevail. Otherwise, the software is + * provided under the following open source license terms: + * + * Alfresco is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Alfresco is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Alfresco. If not, see . + * #L% + */ +package org.alfresco.repo.security.authentication; + +import java.io.Serializable; + +/** + * An object to hold information about unsuccessful logins. + * It is used for brute force attack mitigation in {@link AuthenticationServiceImpl} + * + * @since 5.2.0 + * @author amukha + */ +/*package*/ class ProtectedUser implements Serializable +{ + private static final long serialVersionUID = 1L; + + private String userId; + /** number of consecutive unsuccessful login attempts */ + private long numLogins; + /** time stamp of last unsuccessful login attempt */ + private long timeStamp; + + /*package*/ ProtectedUser(String userId) + { + this.userId = userId; + this.numLogins = 1; + this.timeStamp = System.currentTimeMillis(); + } + + /*package*/ ProtectedUser(String userId, long numLogins) + { + this.userId = userId; + this.numLogins = numLogins; + this.timeStamp = System.currentTimeMillis(); + } + + /*package*/ long getNumLogins() + { + return numLogins; + } + + /*package*/ long getTimeStamp() + { + return timeStamp; + } + + @Override + public String toString() + { + return "ProtectedUser{" + + "userId='" + userId + '\'' + + ", numLogins=" + numLogins + + ", timeStamp=" + timeStamp + + '}'; + } + + @Override + public boolean equals(Object o) + { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + + ProtectedUser that = (ProtectedUser) o; + + if (numLogins != that.numLogins) return false; + if (timeStamp != that.timeStamp) return false; + return userId.equals(that.userId); + } + + @Override + public int hashCode() + { + int result = userId.hashCode(); + result = 31 * result + (int) (numLogins ^ (numLogins >>> 32)); + result = 31 * result + (int) (timeStamp ^ (timeStamp >>> 32)); + return result; + } +} diff --git a/source/test-java/org/alfresco/repo/security/authentication/AuthenticationServiceImplTest.java b/source/test-java/org/alfresco/repo/security/authentication/AuthenticationServiceImplTest.java index 158a8ddeee..a7a322287d 100644 --- a/source/test-java/org/alfresco/repo/security/authentication/AuthenticationServiceImplTest.java +++ b/source/test-java/org/alfresco/repo/security/authentication/AuthenticationServiceImplTest.java @@ -34,6 +34,7 @@ import java.util.Collection; import java.util.HashMap; import java.util.Map; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -53,7 +54,7 @@ import static org.mockito.Mockito.verify; public class AuthenticationServiceImplTest { private AuthenticationComponent authenticationComponent = mock(AuthenticationComponent.class); - private SimpleCache cache; + private SimpleCache cache; private TicketComponent ticketComponent = mock(TicketComponent.class); private AuthenticationServiceImpl authService; @@ -73,14 +74,15 @@ public class AuthenticationServiceImplTest @Test public void testProtectedUserBadPassword() { - int attempts = 3; + int limit = 3; + int attempts = limit + 3; authService.setProtectionPeriodSeconds(99999); - authService.setProtectionLimit(attempts); + authService.setProtectionLimit(limit); authService.setProtectionEnabled(true); Exception spoofedAE = new AuthenticationException("Bad password"); doThrow(spoofedAE).when(authenticationComponent).authenticate(USERNAME, PASSWORD); - for (int i = 0; i < attempts + 3; i++) + for (int i = 0; i < attempts; i++) { try { @@ -90,7 +92,7 @@ public class AuthenticationServiceImplTest catch (AuthenticationException ae) { // normal - if (i < attempts) + if (i < limit) { assertTrue("Expected failure from AuthenticationComponent", ae == spoofedAE); } @@ -100,8 +102,9 @@ public class AuthenticationServiceImplTest } } } - verify(authenticationComponent, times(attempts)).authenticate(USERNAME, PASSWORD); - assertTrue("The user should be protected.", cache.get(USERNAME).isProtected()); + verify(authenticationComponent, times(limit)).authenticate(USERNAME, PASSWORD); + assertTrue("The user should be protected.", authService.isUserProtected(USERNAME)); + assertEquals("The number of recorded logins did not match.", attempts, cache.get(USERNAME).getNumLogins()); // test that the protection is still in place even if the password is correct doNothing().when(authenticationComponent).authenticate(USERNAME, PASSWORD); @@ -114,20 +117,22 @@ public class AuthenticationServiceImplTest { // normal } - verify(authenticationComponent, times(attempts)).authenticate(USERNAME, PASSWORD); + verify(authenticationComponent, times(limit)).authenticate(USERNAME, PASSWORD); + assertEquals("The number of recorded logins did not match.", attempts + 1, cache.get(USERNAME).getNumLogins()); } @Test public void testProtectedUserCanLoginAfterProtection() throws Exception { int timeLimit = 1; + int attempts = 2; authService.setProtectionPeriodSeconds(timeLimit); - authService.setProtectionLimit(2); + authService.setProtectionLimit(attempts); authService.setProtectionEnabled(true); doThrow(new AuthenticationException("Bad password")) .when(authenticationComponent).authenticate(USERNAME, PASSWORD); - for (int i = 0; i < 2; i++) + for (int i = 0; i < attempts; i++) { try { @@ -139,9 +144,12 @@ public class AuthenticationServiceImplTest // normal } } - assertTrue("The user should be protected.", cache.get(USERNAME).isProtected()); + assertTrue("The user should be protected.", authService.isUserProtected(USERNAME)); + assertEquals("The number of recorded logins did not match.", attempts, cache.get(USERNAME).getNumLogins()); Thread.sleep(timeLimit*1000 + 1); - assertFalse("The user should not be protected any more.", cache.get(USERNAME).isProtected()); + assertFalse("The user should not be protected any more.", authService.isUserProtected(USERNAME)); + assertEquals("The number of recorded logins should stay the same after protection period ends.", + attempts, cache.get(USERNAME).getNumLogins()); doNothing().when(authenticationComponent).authenticate(USERNAME, PASSWORD); try @@ -159,9 +167,10 @@ public class AuthenticationServiceImplTest @Test public void testProtectionDisabledBadPassword() { - int attempts = 5; + int limit = 3; + int attempts = limit + 2; authService.setProtectionPeriodSeconds(99999); - authService.setProtectionLimit(attempts - 2); + authService.setProtectionLimit(limit); authService.setProtectionEnabled(false); Exception spoofedAE = new AuthenticationException("Bad password");