From 0e5a3cb74f275ba1477bd13f6b1af27d30426aa0 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Thu, 18 Aug 2022 12:26:55 -0700 Subject: [PATCH 01/10] GUACAMOLE-990: The UserContext passed to redecorate() should NOT be the internal DecoratedUserContext wrapper. --- .../org/apache/guacamole/rest/auth/DecoratedUserContext.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/guacamole/src/main/java/org/apache/guacamole/rest/auth/DecoratedUserContext.java b/guacamole/src/main/java/org/apache/guacamole/rest/auth/DecoratedUserContext.java index d773068f3..9aa26fafc 100644 --- a/guacamole/src/main/java/org/apache/guacamole/rest/auth/DecoratedUserContext.java +++ b/guacamole/src/main/java/org/apache/guacamole/rest/auth/DecoratedUserContext.java @@ -145,7 +145,7 @@ public class DecoratedUserContext extends DelegatingUserContext { if (authProvider != userContext.getAuthenticationProvider()) { // Apply next layer of wrapping around UserContext - UserContext redecorated = authProvider.redecorate(decorated, + UserContext redecorated = authProvider.redecorate(decorated.getDelegateUserContext(), userContext, authenticatedUser, credentials); // Do not allow misbehaving extensions to wipe out the From e6a61b72237f38cb52f2def3e0911b0d9c3027f2 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Thu, 18 Aug 2022 12:20:32 -0700 Subject: [PATCH 02/10] GUACAMOLE-990: Fire auth success/failure events only after authentication has absolutely succeeded or failed, including the details of any failure. Previously, these events were fired only after the user's identity had been determined (or failed to be determined). If we don't wait until after the user contexts have also been successfully obtained (or failed to be obtained), then things like MFA will not be taken into account for auth events. --- .../net/event/AuthenticationFailureEvent.java | 104 +++++++- .../event/AuthenticationProviderEvent.java | 40 +++ .../net/event/AuthenticationSuccessEvent.java | 11 +- .../guacamole/net/event/FailureEvent.java | 39 +++ .../rest/auth/AuthenticationService.java | 240 +++++++++--------- .../rest/auth/DecoratedUserContext.java | 48 ++-- .../rest/auth/DecorationService.java | 9 +- ...acamoleAuthenticationProcessException.java | 164 ++++++++++++ 8 files changed, 509 insertions(+), 146 deletions(-) create mode 100644 guacamole-ext/src/main/java/org/apache/guacamole/net/event/AuthenticationProviderEvent.java create mode 100644 guacamole-ext/src/main/java/org/apache/guacamole/net/event/FailureEvent.java create mode 100644 guacamole/src/main/java/org/apache/guacamole/rest/auth/GuacamoleAuthenticationProcessException.java diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/net/event/AuthenticationFailureEvent.java b/guacamole-ext/src/main/java/org/apache/guacamole/net/event/AuthenticationFailureEvent.java index 9808e7047..870590076 100644 --- a/guacamole-ext/src/main/java/org/apache/guacamole/net/event/AuthenticationFailureEvent.java +++ b/guacamole-ext/src/main/java/org/apache/guacamole/net/event/AuthenticationFailureEvent.java @@ -19,28 +19,91 @@ package org.apache.guacamole.net.event; +import org.apache.guacamole.net.auth.AuthenticationProvider; import org.apache.guacamole.net.auth.Credentials; +import org.apache.guacamole.net.event.listener.Listener; /** * An event which is triggered whenever a user's credentials fail to be * authenticated. The credentials that failed to be authenticated are included * within this event, and can be retrieved using getCredentials(). */ -public class AuthenticationFailureEvent implements CredentialEvent { +public class AuthenticationFailureEvent implements AuthenticationProviderEvent, + CredentialEvent, FailureEvent { /** * The credentials which failed authentication. */ - private Credentials credentials; + private final Credentials credentials; /** - * Creates a new AuthenticationFailureEvent which represents the failure - * to authenticate the given credentials. + * The AuthenticationProvider that encountered the failure. This may be + * null if the AuthenticationProvider is not known, such as if the failure + * is caused by every AuthenticationProvider passively refusing to + * authenticate the user but without explicitly rejecting the user + * (returning null for calls to {@link AuthenticationProvider#authenticateUser(org.apache.guacamole.net.auth.Credentials)}), + * or if the failure is external to any installed AuthenticationProvider + * (such as within a {@link Listener}. + */ + private final AuthenticationProvider authProvider; + + /** + * The Throwable that was thrown resulting in the failure, if any. This + * may be null if authentication failed without a known error, such as if + * the failure is caused by every AuthenticationProvider passively refusing + * to authenticate the user but without explicitly rejecting the user + * (returning null for calls to {@link AuthenticationProvider#authenticateUser(org.apache.guacamole.net.auth.Credentials)}). + */ + private final Throwable failure; + + /** + * Creates a new AuthenticationFailureEvent which represents a failure + * to authenticate the given credentials where there is no specific + * AuthenticationProvider nor Throwable associated with the failure. * - * @param credentials The credentials which failed authentication. + * @param credentials + * The credentials which failed authentication. */ public AuthenticationFailureEvent(Credentials credentials) { + this(credentials, null); + } + + /** + * Creates a new AuthenticationFailureEvent which represents a failure + * to authenticate the given credentials where there is no specific + * AuthenticationProvider causing the failure. + * + * @param credentials + * The credentials which failed authentication. + * + * @param failure + * The Throwable that was thrown resulting in the failure, or null if + * there is no such Throwable. + */ + public AuthenticationFailureEvent(Credentials credentials, Throwable failure) { + this(credentials, null, failure); + } + + /** + * Creates a new AuthenticationFailureEvent which represents a failure + * to authenticate the given credentials. + * + * @param credentials + * The credentials which failed authentication. + * + * @param authProvider + * The AuthenticationProvider that caused the failure, or null if there + * is no such AuthenticationProvider. + * + * @param failure + * The Throwable that was thrown resulting in the failure, or null if + * there is no such Throwable. + */ + public AuthenticationFailureEvent(Credentials credentials, + AuthenticationProvider authProvider, Throwable failure) { this.credentials = credentials; + this.authProvider = authProvider; + this.failure = failure; } @Override @@ -48,4 +111,35 @@ public class AuthenticationFailureEvent implements CredentialEvent { return credentials; } + /** + * {@inheritDoc} + * + *

NOTE: In the case of an authentication failure, cases where this may + * be null include if authentication failed without a definite single + * AuthenticationProvider causing that failure, such as if the failure is + * caused by every AuthenticationProvider passively refusing to + * authenticate the user but without explicitly rejecting the user + * (returning null for calls to {@link AuthenticationProvider#authenticateUser(org.apache.guacamole.net.auth.Credentials)}), + * or if the failure is external to any installed AuthenticationProvider + * (such as within a {@link Listener}. + */ + @Override + public AuthenticationProvider getAuthenticationProvider() { + return authProvider; + } + + /** + * {@inheritDoc} + * + *

NOTE: In the case of an authentication failure, cases where this may + * be null include if authentication failed without a known error, such as + * if the failure is caused by every AuthenticationProvider passively + * refusing to authenticate the user but without explicitly rejecting the + * user (returning null for calls to {@link AuthenticationProvider#authenticateUser(org.apache.guacamole.net.auth.Credentials)}). + */ + @Override + public Throwable getFailure() { + return failure; + } + } diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/net/event/AuthenticationProviderEvent.java b/guacamole-ext/src/main/java/org/apache/guacamole/net/event/AuthenticationProviderEvent.java new file mode 100644 index 000000000..7faa87697 --- /dev/null +++ b/guacamole-ext/src/main/java/org/apache/guacamole/net/event/AuthenticationProviderEvent.java @@ -0,0 +1,40 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.guacamole.net.event; + +import org.apache.guacamole.net.auth.AuthenticationProvider; + +/** + * An event which may be dispatched due to a specific AuthenticationProvider. + */ +public interface AuthenticationProviderEvent { + + /** + * Returns the AuthenticationProvider that resulted in the event, if any. + * If the event occurred without any definite causing + * AuthenticationProvider, this may be null. + * + * @return + * The AuthenticationProvider that resulted in the event, or null if no + * such AuthenticationProvider is known. + */ + AuthenticationProvider getAuthenticationProvider(); + +} diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/net/event/AuthenticationSuccessEvent.java b/guacamole-ext/src/main/java/org/apache/guacamole/net/event/AuthenticationSuccessEvent.java index 8b63bcf0e..a9b21dce8 100644 --- a/guacamole-ext/src/main/java/org/apache/guacamole/net/event/AuthenticationSuccessEvent.java +++ b/guacamole-ext/src/main/java/org/apache/guacamole/net/event/AuthenticationSuccessEvent.java @@ -20,6 +20,7 @@ package org.apache.guacamole.net.event; import org.apache.guacamole.net.auth.AuthenticatedUser; +import org.apache.guacamole.net.auth.AuthenticationProvider; import org.apache.guacamole.net.auth.Credentials; /** @@ -32,7 +33,8 @@ import org.apache.guacamole.net.auth.Credentials; * is effectively vetoed and will be subsequently processed as though the * authentication failed. */ -public class AuthenticationSuccessEvent implements UserEvent, CredentialEvent { +public class AuthenticationSuccessEvent implements UserEvent, CredentialEvent, + AuthenticationProviderEvent { /** * The AuthenticatedUser identifying the user that successfully @@ -60,7 +62,12 @@ public class AuthenticationSuccessEvent implements UserEvent, CredentialEvent { @Override public Credentials getCredentials() { - return authenticatedUser.getCredentials(); + return getAuthenticatedUser().getCredentials(); + } + + @Override + public AuthenticationProvider getAuthenticationProvider() { + return getAuthenticatedUser().getAuthenticationProvider(); } } diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/net/event/FailureEvent.java b/guacamole-ext/src/main/java/org/apache/guacamole/net/event/FailureEvent.java new file mode 100644 index 000000000..dfc337594 --- /dev/null +++ b/guacamole-ext/src/main/java/org/apache/guacamole/net/event/FailureEvent.java @@ -0,0 +1,39 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.guacamole.net.event; + +/** + * An event which represents failure of an operation, where that failure may + * be associated with a particular Throwable. + */ +public interface FailureEvent { + + /** + * Returns the Throwable that represents the failure that occurred, if any. + * If the failure was recognized but without a definite known error, this + * may be null. + * + * @return + * The Throwable that represents the failure that occurred, or null if + * no such Throwable is known. + */ + Throwable getFailure(); + +} diff --git a/guacamole/src/main/java/org/apache/guacamole/rest/auth/AuthenticationService.java b/guacamole/src/main/java/org/apache/guacamole/rest/auth/AuthenticationService.java index ce8a9fb0c..8ed76c6a6 100644 --- a/guacamole/src/main/java/org/apache/guacamole/rest/auth/AuthenticationService.java +++ b/guacamole/src/main/java/org/apache/guacamole/rest/auth/AuthenticationService.java @@ -34,7 +34,6 @@ import org.apache.guacamole.net.auth.AuthenticatedUser; import org.apache.guacamole.net.auth.AuthenticationProvider; import org.apache.guacamole.net.auth.Credentials; import org.apache.guacamole.net.auth.UserContext; -import org.apache.guacamole.net.auth.credentials.CredentialsInfo; import org.apache.guacamole.net.auth.credentials.GuacamoleCredentialsException; import org.apache.guacamole.net.auth.credentials.GuacamoleInsufficientCredentialsException; import org.apache.guacamole.net.auth.credentials.GuacamoleInvalidCredentialsException; @@ -169,14 +168,15 @@ public class AuthenticationService { * The AuthenticatedUser given by the highest-priority * AuthenticationProvider for which the given credentials are valid. * - * @throws GuacamoleException + * @throws GuacamoleAuthenticationProcessException * If the given credentials are not valid for any * AuthenticationProvider, or if an error occurs while authenticating * the user. */ private AuthenticatedUser authenticateUser(Credentials credentials) - throws GuacamoleException { + throws GuacamoleAuthenticationProcessException { + AuthenticationProvider failedAuthProvider = null; GuacamoleCredentialsException authFailure = null; // Attempt authentication against each AuthenticationProvider @@ -191,27 +191,29 @@ public class AuthenticationService { // Insufficient credentials should take precedence catch (GuacamoleInsufficientCredentialsException e) { - if (authFailure == null || authFailure instanceof GuacamoleInvalidCredentialsException) + if (authFailure == null || authFailure instanceof GuacamoleInvalidCredentialsException) { + failedAuthProvider = authProvider; authFailure = e; + } } - + // Catch other credentials exceptions and assign the first one catch (GuacamoleCredentialsException e) { - if (authFailure == null) + if (authFailure == null) { + failedAuthProvider = authProvider; authFailure = e; + } + } + + catch (GuacamoleException | RuntimeException | Error e) { + throw new GuacamoleAuthenticationProcessException("User " + + "authentication was aborted.", authProvider, e); } } - // If a specific failure occured, rethrow that - if (authFailure != null) - throw authFailure; - - // Otherwise, request standard username/password - throw new GuacamoleInvalidCredentialsException( - "Permission Denied.", - CredentialsInfo.USERNAME_PASSWORD - ); + throw new GuacamoleAuthenticationProcessException("User authentication " + + "failed.", failedAuthProvider, authFailure); } @@ -230,51 +232,29 @@ public class AuthenticationService { * A AuthenticatedUser which may have been updated due to re- * authentication. * - * @throws GuacamoleException + * @throws GuacamoleAuthenticationProcessException * If an error prevents the user from being re-authenticated. */ private AuthenticatedUser updateAuthenticatedUser(AuthenticatedUser authenticatedUser, - Credentials credentials) throws GuacamoleException { + Credentials credentials) throws GuacamoleAuthenticationProcessException { // Get original AuthenticationProvider AuthenticationProvider authProvider = authenticatedUser.getAuthenticationProvider(); - // Re-authenticate the AuthenticatedUser against the original AuthenticationProvider only - authenticatedUser = authProvider.updateAuthenticatedUser(authenticatedUser, credentials); - if (authenticatedUser == null) - throw new GuacamoleSecurityException("User re-authentication failed."); + try { - return authenticatedUser; + // Re-authenticate the AuthenticatedUser against the original AuthenticationProvider only + authenticatedUser = authProvider.updateAuthenticatedUser(authenticatedUser, credentials); + if (authenticatedUser == null) + throw new GuacamoleSecurityException("User re-authentication failed."); - } + return authenticatedUser; - /** - * Notify all bound listeners that a successful authentication - * has occurred. - * - * @param authenticatedUser - * The user that was successfully authenticated. - * - * @throws GuacamoleException - * If thrown by a listener. - */ - private void fireAuthenticationSuccessEvent(AuthenticatedUser authenticatedUser) - throws GuacamoleException { - listenerService.handleEvent(new AuthenticationSuccessEvent(authenticatedUser)); - } + } + catch (GuacamoleException | RuntimeException | Error e) { + throw new GuacamoleAuthenticationProcessException("User re-authentication failed.", authProvider, e); + } - /** - * Notify all bound listeners that an authentication attempt has failed. - * - * @param credentials - * The credentials that failed to authenticate. - * - * @throws GuacamoleException - * If thrown by a listener. - */ - private void fireAuthenticationFailedEvent(Credentials credentials) - throws GuacamoleException { - listenerService.handleEvent(new AuthenticationFailureEvent(credentials)); } /** @@ -292,61 +272,23 @@ public class AuthenticationService { * The AuthenticatedUser associated with the given session and * credentials. * - * @throws GuacamoleException + * @throws GuacamoleAuthenticationProcessException * If an error occurs while authenticating or re-authenticating the * user. */ private AuthenticatedUser getAuthenticatedUser(GuacamoleSession existingSession, - Credentials credentials) throws GuacamoleException { - - try { - - // Re-authenticate user if session exists - if (existingSession != null) { - AuthenticatedUser updatedUser = updateAuthenticatedUser( - existingSession.getAuthenticatedUser(), credentials); - fireAuthenticationSuccessEvent(updatedUser); - return updatedUser; - } - - // Otherwise, attempt authentication as a new user - AuthenticatedUser authenticatedUser = AuthenticationService.this.authenticateUser(credentials); - fireAuthenticationSuccessEvent(authenticatedUser); - - if (logger.isInfoEnabled()) - logger.info("User \"{}\" successfully authenticated from {}.", - authenticatedUser.getIdentifier(), - getLoggableAddress(credentials.getRequest())); - - return authenticatedUser; + Credentials credentials) throws GuacamoleAuthenticationProcessException { + // Re-authenticate user if session exists + if (existingSession != null) { + AuthenticatedUser updatedUser = updateAuthenticatedUser( + existingSession.getAuthenticatedUser(), credentials); + return updatedUser; } - // Log and rethrow any authentication errors - catch (GuacamoleException e) { - - fireAuthenticationFailedEvent(credentials); - - // Get request and username for sake of logging - HttpServletRequest request = credentials.getRequest(); - String username = credentials.getUsername(); - - // Log authentication failures with associated usernames - if (username != null) { - if (logger.isWarnEnabled()) - logger.warn("Authentication attempt from {} for user \"{}\" failed.", - getLoggableAddress(request), username); - } - - // Log anonymous authentication failures - else if (logger.isDebugEnabled()) - logger.debug("Anonymous authentication attempt from {} failed.", - getLoggableAddress(request)); - - // Rethrow exception - throw e; - - } + // Otherwise, attempt authentication as a new user + AuthenticatedUser authenticatedUser = AuthenticationService.this.authenticateUser(credentials); + return authenticatedUser; } @@ -371,15 +313,14 @@ public class AuthenticationService { * A List of all UserContexts associated with the given * AuthenticatedUser. * - * @throws GuacamoleException + * @throws GuacamoleAuthenticationProcessException * If an error occurs while creating or updating any UserContext. */ private List getUserContexts(GuacamoleSession existingSession, AuthenticatedUser authenticatedUser, Credentials credentials) - throws GuacamoleException { + throws GuacamoleAuthenticationProcessException { - List userContexts = - new ArrayList(authProviders.size()); + List userContexts = new ArrayList<>(authProviders.size()); // If UserContexts already exist, update them and add to the list if (existingSession != null) { @@ -392,7 +333,15 @@ public class AuthenticationService { // Update existing UserContext AuthenticationProvider authProvider = oldUserContext.getAuthenticationProvider(); - UserContext updatedUserContext = authProvider.updateUserContext(oldUserContext, authenticatedUser, credentials); + UserContext updatedUserContext; + try { + updatedUserContext = authProvider.updateUserContext(oldUserContext, authenticatedUser, credentials); + } + catch (GuacamoleException | RuntimeException | Error e) { + throw new GuacamoleAuthenticationProcessException("User " + + "authentication aborted during UserContext update.", + authProvider, e); + } // Add to available data, if successful if (updatedUserContext != null) @@ -415,7 +364,15 @@ public class AuthenticationService { for (AuthenticationProvider authProvider : authProviders) { // Generate new UserContext - UserContext userContext = authProvider.getUserContext(authenticatedUser); + UserContext userContext; + try { + userContext = authProvider.getUserContext(authenticatedUser); + } + catch (GuacamoleException | RuntimeException | Error e) { + throw new GuacamoleAuthenticationProcessException("User " + + "authentication aborted during initial " + + "UserContext creation.", authProvider, e); + } // Add to available data, if successful if (userContext != null) @@ -453,7 +410,7 @@ public class AuthenticationService { * If the authentication or re-authentication attempt fails. */ public String authenticate(Credentials credentials, String token) - throws GuacamoleException { + throws GuacamoleException { // Pull existing session if token provided GuacamoleSession existingSession; @@ -462,25 +419,72 @@ public class AuthenticationService { else existingSession = null; - // Get up-to-date AuthenticatedUser and associated UserContexts - AuthenticatedUser authenticatedUser = getAuthenticatedUser(existingSession, credentials); - List userContexts = getUserContexts(existingSession, authenticatedUser, credentials); - - // Update existing session, if it exists + AuthenticatedUser authenticatedUser; String authToken; - if (existingSession != null) { - authToken = token; - existingSession.setAuthenticatedUser(authenticatedUser); - existingSession.setUserContexts(userContexts); + + try { + + // Get up-to-date AuthenticatedUser and associated UserContexts + authenticatedUser = getAuthenticatedUser(existingSession, credentials); + List userContexts = getUserContexts(existingSession, authenticatedUser, credentials); + + // Update existing session, if it exists + if (existingSession != null) { + authToken = token; + existingSession.setAuthenticatedUser(authenticatedUser); + existingSession.setUserContexts(userContexts); + } + + // If no existing session, generate a new token/session pair + else { + authToken = authTokenGenerator.getToken(); + tokenSessionMap.put(authToken, new GuacamoleSession(environment, authenticatedUser, userContexts)); + logger.debug("Login was successful for user \"{}\".", authenticatedUser.getIdentifier()); + } + + // Report authentication success + try { + listenerService.handleEvent(new AuthenticationSuccessEvent(authenticatedUser)); + } + catch (GuacamoleException e) { + throw new GuacamoleAuthenticationProcessException("User " + + "authentication aborted by event listener.", null, e); + } + } - // If no existing session, generate a new token/session pair - else { - authToken = authTokenGenerator.getToken(); - tokenSessionMap.put(authToken, new GuacamoleSession(environment, authenticatedUser, userContexts)); - logger.debug("Login was successful for user \"{}\".", authenticatedUser.getIdentifier()); + // Log and rethrow any authentication errors + catch (GuacamoleAuthenticationProcessException e) { + + // Get request and username for sake of logging + HttpServletRequest request = credentials.getRequest(); + String username = credentials.getUsername(); + + listenerService.handleEvent(new AuthenticationFailureEvent(credentials, + e.getAuthenticationProvider(), e.getCause())); + + // Log authentication failures with associated usernames + if (username != null) { + if (logger.isWarnEnabled()) + logger.warn("Authentication attempt from {} for user \"{}\" failed.", + getLoggableAddress(request), username); + } + + // Log anonymous authentication failures + else if (logger.isDebugEnabled()) + logger.debug("Anonymous authentication attempt from {} failed.", + getLoggableAddress(request)); + + // Rethrow exception + throw e.getCauseAsGuacamoleException(); + } + if (logger.isInfoEnabled()) + logger.info("User \"{}\" successfully authenticated from {}.", + authenticatedUser.getIdentifier(), + getLoggableAddress(credentials.getRequest())); + return authToken; } diff --git a/guacamole/src/main/java/org/apache/guacamole/rest/auth/DecoratedUserContext.java b/guacamole/src/main/java/org/apache/guacamole/rest/auth/DecoratedUserContext.java index 9aa26fafc..2be283ac2 100644 --- a/guacamole/src/main/java/org/apache/guacamole/rest/auth/DecoratedUserContext.java +++ b/guacamole/src/main/java/org/apache/guacamole/rest/auth/DecoratedUserContext.java @@ -76,21 +76,29 @@ public class DecoratedUserContext extends DelegatingUserContext { * given AuthenticationProvider, or the original UserContext if the * given AuthenticationProvider originated the UserContext. * - * @throws GuacamoleException + * @throws GuacamoleAuthenticationProcessException * If the given AuthenticationProvider fails while decorating the * UserContext. */ private static UserContext decorate(AuthenticationProvider authProvider, UserContext userContext, AuthenticatedUser authenticatedUser, - Credentials credentials) throws GuacamoleException { + Credentials credentials) throws GuacamoleAuthenticationProcessException { // Skip the AuthenticationProvider which produced the UserContext // being decorated if (authProvider != userContext.getAuthenticationProvider()) { // Apply layer of wrapping around UserContext - UserContext decorated = authProvider.decorate(userContext, - authenticatedUser, credentials); + UserContext decorated; + try { + decorated = authProvider.decorate(userContext, + authenticatedUser, credentials); + } + catch (GuacamoleException | RuntimeException | Error e) { + throw new GuacamoleAuthenticationProcessException("User " + + "authentication aborted by decorating UserContext.", + authProvider, e); + } // Do not allow misbehaving extensions to wipe out the // UserContext entirely @@ -130,13 +138,13 @@ public class DecoratedUserContext extends DelegatingUserContext { * given AuthenticationProvider, or the original UserContext if the * given AuthenticationProvider originated the UserContext. * - * @throws GuacamoleException + * @throws GuacamoleAuthenticationProcessException * If the given AuthenticationProvider fails while decorating the * UserContext. */ private static UserContext redecorate(DecoratedUserContext decorated, UserContext userContext, AuthenticatedUser authenticatedUser, - Credentials credentials) throws GuacamoleException { + Credentials credentials) throws GuacamoleAuthenticationProcessException { AuthenticationProvider authProvider = decorated.getDecoratingAuthenticationProvider(); @@ -145,8 +153,16 @@ public class DecoratedUserContext extends DelegatingUserContext { if (authProvider != userContext.getAuthenticationProvider()) { // Apply next layer of wrapping around UserContext - UserContext redecorated = authProvider.redecorate(decorated.getDelegateUserContext(), - userContext, authenticatedUser, credentials); + UserContext redecorated; + try { + redecorated = authProvider.redecorate(decorated.getDelegateUserContext(), + userContext, authenticatedUser, credentials); + } + catch (GuacamoleException | RuntimeException | Error e) { + throw new GuacamoleAuthenticationProcessException("User " + + "authentication aborted by redecorating UserContext.", + authProvider, e); + } // Do not allow misbehaving extensions to wipe out the // UserContext entirely @@ -181,13 +197,13 @@ public class DecoratedUserContext extends DelegatingUserContext { * The credentials associated with the request which produced the given * UserContext. * - * @throws GuacamoleException + * @throws GuacamoleAuthenticationProcessException * If any of the given AuthenticationProviders fails while decorating * the UserContext. */ public DecoratedUserContext(AuthenticationProvider authProvider, UserContext userContext, AuthenticatedUser authenticatedUser, - Credentials credentials) throws GuacamoleException { + Credentials credentials) throws GuacamoleAuthenticationProcessException { // Wrap the result of invoking decorate() on the given AuthenticationProvider super(decorate(authProvider, userContext, authenticatedUser, credentials)); @@ -221,13 +237,13 @@ public class DecoratedUserContext extends DelegatingUserContext { * The credentials associated with the request which produced the given * UserContext. * - * @throws GuacamoleException + * @throws GuacamoleAuthenticationProcessException * If any of the given AuthenticationProviders fails while decorating * the UserContext. */ public DecoratedUserContext(AuthenticationProvider authProvider, DecoratedUserContext userContext, AuthenticatedUser authenticatedUser, - Credentials credentials) throws GuacamoleException { + Credentials credentials) throws GuacamoleAuthenticationProcessException { // Wrap the result of invoking decorate() on the given AuthenticationProvider super(decorate(authProvider, userContext, authenticatedUser, credentials)); @@ -261,13 +277,13 @@ public class DecoratedUserContext extends DelegatingUserContext { * The credentials associated with the request which produced the given * UserContext. * - * @throws GuacamoleException + * @throws GuacamoleAuthenticationProcessException * If any of the given AuthenticationProviders fails while decorating * the UserContext. */ public DecoratedUserContext(DecoratedUserContext decorated, UserContext userContext, AuthenticatedUser authenticatedUser, - Credentials credentials) throws GuacamoleException { + Credentials credentials) throws GuacamoleAuthenticationProcessException { // Wrap the result of invoking redecorate() on the given AuthenticationProvider super(redecorate(decorated, userContext, authenticatedUser, credentials)); @@ -303,13 +319,13 @@ public class DecoratedUserContext extends DelegatingUserContext { * The credentials associated with the request which produced the given * UserContext. * - * @throws GuacamoleException + * @throws GuacamoleAuthenticationProcessException * If any of the given AuthenticationProviders fails while decorating * the UserContext. */ public DecoratedUserContext(DecoratedUserContext decorated, DecoratedUserContext userContext, AuthenticatedUser authenticatedUser, - Credentials credentials) throws GuacamoleException { + Credentials credentials) throws GuacamoleAuthenticationProcessException { // Wrap the result of invoking redecorate() on the given AuthenticationProvider super(redecorate(decorated, userContext, authenticatedUser, credentials)); diff --git a/guacamole/src/main/java/org/apache/guacamole/rest/auth/DecorationService.java b/guacamole/src/main/java/org/apache/guacamole/rest/auth/DecorationService.java index b28dc03af..0b7fc1264 100644 --- a/guacamole/src/main/java/org/apache/guacamole/rest/auth/DecorationService.java +++ b/guacamole/src/main/java/org/apache/guacamole/rest/auth/DecorationService.java @@ -23,7 +23,6 @@ import java.util.Iterator; import java.util.List; import javax.inject.Inject; -import org.apache.guacamole.GuacamoleException; import org.apache.guacamole.net.auth.AuthenticatedUser; import org.apache.guacamole.net.auth.AuthenticationProvider; import org.apache.guacamole.net.auth.Credentials; @@ -65,12 +64,12 @@ public class DecorationService { * A new DecoratedUserContext which has been decorated by all * AuthenticationProviders. * - * @throws GuacamoleException + * @throws GuacamoleAuthenticationProcessException * If any AuthenticationProvider fails while decorating the UserContext. */ public DecoratedUserContext decorate(UserContext userContext, AuthenticatedUser authenticatedUser, Credentials credentials) - throws GuacamoleException { + throws GuacamoleAuthenticationProcessException { // Get first AuthenticationProvider in list Iterator current = authProviders.iterator(); @@ -119,12 +118,12 @@ public class DecorationService { * A new DecoratedUserContext which has been decorated by all * AuthenticationProviders. * - * @throws GuacamoleException + * @throws GuacamoleAuthenticationProcessException * If any AuthenticationProvider fails while decorating the UserContext. */ public DecoratedUserContext redecorate(DecoratedUserContext decorated, UserContext userContext, AuthenticatedUser authenticatedUser, - Credentials credentials) throws GuacamoleException { + Credentials credentials) throws GuacamoleAuthenticationProcessException { // If the given DecoratedUserContext contains further decorated layers, // redecorate those first diff --git a/guacamole/src/main/java/org/apache/guacamole/rest/auth/GuacamoleAuthenticationProcessException.java b/guacamole/src/main/java/org/apache/guacamole/rest/auth/GuacamoleAuthenticationProcessException.java new file mode 100644 index 000000000..ba1d72a50 --- /dev/null +++ b/guacamole/src/main/java/org/apache/guacamole/rest/auth/GuacamoleAuthenticationProcessException.java @@ -0,0 +1,164 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.guacamole.rest.auth; + +import java.io.Serializable; +import org.apache.guacamole.GuacamoleException; +import org.apache.guacamole.GuacamoleServerException; +import org.apache.guacamole.net.auth.AuthenticationProvider; +import org.apache.guacamole.net.auth.credentials.CredentialsInfo; +import org.apache.guacamole.net.auth.credentials.GuacamoleInvalidCredentialsException; +import org.apache.guacamole.protocol.GuacamoleStatus; + +/** + * An exception that occurs during Guacamole's authentication and authorization + * process, possibly associated with a specific AuthenticationProvider. + */ +public class GuacamoleAuthenticationProcessException extends GuacamoleException { + + /** + * Internal identifier unique to this version of + * GuacamoleAuthenticationProcessException, as required by Java's + * {@link Serializable} interface. + */ + private static final long serialVersionUID = 1L; + + /** + * The AuthenticationProvider that caused the failure, or null if there is + * no such specific AuthenticationProvider involved in this failure. + */ + private final transient AuthenticationProvider authProvider; + + /** + * A GuacamoleException representation of the failure that occurred. If + * the cause provided when this GuacamoleAuthenticationProcessException + * was created was a GuacamoleException, this will just be that exception. + * Otherwise, this will be a GuacamoleServerException wrapping the cause + * or a generic GuacamoleInvalidCredentialsException requesting a + * username/password if there is no specific cause at all. + */ + private final GuacamoleException guacCause; + + /** + * Converts the given Throwable to a GuacamoleException representing the + * failure that occurred. If the Throwable already is a GuacamoleException, + * this will just be that Throwable. For all other cases, a new + * GuacamoleException will be created that best represents the provided + * failure. If no failure is provided at all, a generic + * GuacamoleInvalidCredentialsException requesting a username/password is + * created. + * + * @param message + * A human-readable message describing the failure that occurred. + * + * @param cause + * The Throwable cause of the failure that occurred, if any, or null if + * the cause is not known to be a specific Throwable. + * + * @return + * A GuacamoleException representation of the message and cause + * provided. + */ + private static GuacamoleException toGuacamoleException(String message, + Throwable cause) { + + // Create generic invalid username/password exception if we have no + // specific cause + if (cause == null) + return new GuacamoleInvalidCredentialsException( + "Permission Denied.", + CredentialsInfo.USERNAME_PASSWORD + ); + + // If the specific cause is already a GuacamoleException, there's + // nothing for us to do here + if (cause instanceof GuacamoleException) + return (GuacamoleException) cause; + + // Wrap all other Throwables as generic internal errors + return new GuacamoleServerException(message, cause); + + } + + /** + * Creates a new GuacamoleAuthenticationProcessException with the given + * message, associated AuthenticationProvider, and cause. + * + * @param message + * A human readable description of the exception that occurred. + * + * @param authProvider + * The AuthenticationProvider that caused the failure, or null if there + * is no such specific AuthenticationProvider involved in this failure. + * + * @param cause + * The cause of this exception, or null if the cause is unknown or + * there is no such cause. + */ + public GuacamoleAuthenticationProcessException(String message, + AuthenticationProvider authProvider, Throwable cause) { + super(message, cause); + this.authProvider = authProvider; + this.guacCause = toGuacamoleException(message, cause); + } + + /** + * Returns the AuthenticationProvider that caused the failure, if any. If + * there is no specific AuthenticationProvider involved in this failure, + * including if the failure is due to multiple AuthenticationProviders, + * this will be null. + * + * @return + * The AuthenticationProvider that caused the failure, or null if there + * is no such specific AuthenticationProvider involved in this failure. + */ + public AuthenticationProvider getAuthenticationProvider() { + return authProvider; + } + + /** + * Returns a GuacamoleException that represents the user-facing cause of + * this exception. A GuacamoleException will be returned by this function + * in all cases, including if no specific cause was given. + * + * @return + * A GuacamoleException that represents the user-facing cause of this + * exception. + */ + public GuacamoleException getCauseAsGuacamoleException() { + return guacCause; + } + + @Override + public GuacamoleStatus getStatus() { + return getCauseAsGuacamoleException().getStatus(); + } + + @Override + public int getHttpStatusCode() { + return getCauseAsGuacamoleException().getHttpStatusCode(); + } + + @Override + public int getWebSocketCode() { + return getCauseAsGuacamoleException().getWebSocketCode(); + } + +} From 275b5bee134f8035acf0e00b2579d05c7405eb66 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Wed, 5 Jan 2022 15:08:54 -0800 Subject: [PATCH 03/10] GUACAMOLE-990: Add extension for automatically blocking brute-force auth attempts. --- extensions/guacamole-auth-ban/.ratignore | 0 extensions/guacamole-auth-ban/pom.xml | 60 ++++ .../src/main/assembly/dist.xml | 54 ++++ .../auth/ban/AuthenticationFailureStatus.java | 123 ++++++++ .../ban/AuthenticationFailureTracker.java | 278 ++++++++++++++++++ .../ban/BanningAuthenticationListener.java | 81 +++++ .../ban/BanningAuthenticationProvider.java | 121 ++++++++ .../src/main/resources/guac-manifest.json | 16 + extensions/pom.xml | 1 + 9 files changed, 734 insertions(+) create mode 100644 extensions/guacamole-auth-ban/.ratignore create mode 100644 extensions/guacamole-auth-ban/pom.xml create mode 100644 extensions/guacamole-auth-ban/src/main/assembly/dist.xml create mode 100644 extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/AuthenticationFailureStatus.java create mode 100644 extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/AuthenticationFailureTracker.java create mode 100644 extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/BanningAuthenticationListener.java create mode 100644 extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/BanningAuthenticationProvider.java create mode 100644 extensions/guacamole-auth-ban/src/main/resources/guac-manifest.json diff --git a/extensions/guacamole-auth-ban/.ratignore b/extensions/guacamole-auth-ban/.ratignore new file mode 100644 index 000000000..e69de29bb diff --git a/extensions/guacamole-auth-ban/pom.xml b/extensions/guacamole-auth-ban/pom.xml new file mode 100644 index 000000000..2be082fc2 --- /dev/null +++ b/extensions/guacamole-auth-ban/pom.xml @@ -0,0 +1,60 @@ + + + + + 4.0.0 + org.apache.guacamole + guacamole-auth-ban + jar + 1.4.0 + guacamole-auth-ban + http://guacamole.apache.org/ + + + org.apache.guacamole + extensions + 1.4.0 + ../ + + + + + + + javax.servlet + servlet-api + 2.5 + provided + + + + + org.apache.guacamole + guacamole-ext + 1.4.0 + provided + + + + + diff --git a/extensions/guacamole-auth-ban/src/main/assembly/dist.xml b/extensions/guacamole-auth-ban/src/main/assembly/dist.xml new file mode 100644 index 000000000..d046ae699 --- /dev/null +++ b/extensions/guacamole-auth-ban/src/main/assembly/dist.xml @@ -0,0 +1,54 @@ + + + + + dist + ${project.artifactId}-${project.version} + + + + tar.gz + + + + + + + + + target/licenses + + + + + target + + + *.jar + + + + + + diff --git a/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/AuthenticationFailureStatus.java b/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/AuthenticationFailureStatus.java new file mode 100644 index 000000000..87bdc5515 --- /dev/null +++ b/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/AuthenticationFailureStatus.java @@ -0,0 +1,123 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.guacamole.auth.ban; + +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; + +/** + * The current status of an authentication failure, including the number of + * times the failure has occurred. + */ +public class AuthenticationFailureStatus { + + /** + * The timestamp of the last authentication failure, as returned by + * System.nanoTime(). + */ + private long lastFailure; + + /** + * The number of failures that have occurred. + */ + private final AtomicInteger failureCount; + + /** + * The maximum number of failures that may occur before the user/address + * causing the failures is blocked. + */ + private final int maxAttempts; + + /** + * The amount of time that a user/address must remain blocked after they + * have reached the maximum number of failures. Unlike the value provided + * at construction time, this value is maintained in nanoseconds. + */ + private final long duration; + + /** + * Creates an AuthenticationFailureStatus that represents a single failure + * and is subject to the given restrictions. Additional failures may be + * flagged after creation with {@link #notifyFailed()}. + * + * @param maxAttempts + * The maximum number of failures that may occur before the + * user/address causing the failures is blocked. + * + * @param duration + * The amount of time, in seconds, that a user/address must remain + * blocked after they have reached the maximum number of failures. + */ + public AuthenticationFailureStatus(int maxAttempts, int duration) { + this.lastFailure = System.nanoTime(); + this.failureCount = new AtomicInteger(1); + this.maxAttempts = maxAttempts; + this.duration = TimeUnit.SECONDS.toNanos(duration); + } + + /** + * Updates this authentication failure, noting that the failure it + * represents has recurred. + */ + public void notifyFailed() { + lastFailure = System.nanoTime(); + failureCount.incrementAndGet(); + } + + /** + * Returns whether this authentication failure is recent enough that it + * should still be tracked. This function will return false for + * authentication failures that have not recurred for at least the duration + * provided at construction time. + * + * @return + * true if this authentication failure is recent enough that it should + * still be tracked, false otherwise. + */ + public boolean isValid() { + return System.nanoTime() - lastFailure <= duration; + } + + /** + * Returns whether the user/address causing this authentication failure + * should be blocked based on the restrictions provided at construction + * time. + * + * @return + * true if the user/address causing this failure should be blocked, + * false otherwise. + */ + public boolean isBlocked() { + return isValid() && failureCount.get() >= maxAttempts; + } + + /** + * Returns the total number of authentication failures that have been + * recorded through creating this object and invoking + * {@link #notifyFailed()}. + * + * @return + * The total number of failures that have occurred. + */ + public int getFailures() { + return failureCount.get(); + } + +} diff --git a/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/AuthenticationFailureTracker.java b/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/AuthenticationFailureTracker.java new file mode 100644 index 000000000..4f77875fc --- /dev/null +++ b/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/AuthenticationFailureTracker.java @@ -0,0 +1,278 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.guacamole.auth.ban; + +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import javax.servlet.http.HttpServletRequest; +import org.apache.guacamole.GuacamoleClientTooManyException; +import org.apache.guacamole.GuacamoleException; +import org.apache.guacamole.GuacamoleServerException; +import org.apache.guacamole.net.auth.Credentials; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Provides automated tracking and blocking of IP addresses that repeatedly + * fail authentication. + */ +public class AuthenticationFailureTracker { + + /** + * Logger for this class. + */ + private static final Logger logger = LoggerFactory.getLogger(AuthenticationFailureTracker.class); + + /** + * All authentication failures currently being tracked, stored by the + * associated IP address. + */ + private final ConcurrentMap failures = + new ConcurrentHashMap<>(); + + /** + * The maximum number of failed authentication attempts allowed before an + * address is temporarily banned. + */ + private final int maxAttempts; + + /** + * The length of time that each address should be banned after reaching the + * maximum number of failed authentication attempts, in seconds. + */ + private final int banDuration; + + /** + * Creates a new AuthenticationFailureTracker that automatically blocks + * authentication attempts based on the provided blocking criteria. + * + * @param maxAttempts + * The maximum number of failed authentication attempts allowed before + * an address is temporarily banned. + * + * @param banDuration + * The length of time that each address should be banned after reaching + * the maximum number of failed authentication attempts, in seconds. + */ + public AuthenticationFailureTracker(int maxAttempts, int banDuration) { + this.maxAttempts = maxAttempts; + this.banDuration = banDuration; + + // Inform administrator of configured behavior + if (maxAttempts <= 0) { + logger.info("Maximum failed authentication attempts has been set " + + "to {}. Automatic banning of brute-force authentication " + + "attempts will be disabled.", maxAttempts); + } + else if (banDuration <= 0) { + logger.info("Ban duration for addresses that repeatedly fail " + + "authentication has been set to {}. Automatic banning " + + "of brute-force authentication attempts will be " + + "disabled.", banDuration); + } + else { + logger.info("Addresses will be automatically banned for {} " + + "seconds after {} failed authentication attempts.", + banDuration, maxAttempts); + } + + } + + /** + * Returns whether the given Credentials do not contain any specific + * authentication parameters, including HTTP parameters. An authentication + * request that contains no parameters whatsoever will tend to be the + * first, anonymous, credential-less authentication attempt that results in + * the initial login screen rendering. + * + * @param credentials + * The Credentials object to test. + * + * @return + * true if the given Credentials contain no authentication parameters + * whatsoever, false otherwise. + */ + private boolean isEmpty(Credentials credentials) { + + // An authentication request that contains an explicit username or + // password (even if blank) is non-empty, regardless of how the values + // were passed + if (credentials.getUsername() != null || credentials.getPassword() != null) + return false; + + // All further tests depend on HTTP request details + HttpServletRequest request = credentials.getRequest(); + if (request == null) + return true; + + // An authentication request is non-empty if it contains any HTTP + // parameters at all or contains an authentication token + return !request.getParameterNames().hasMoreElements() + && request.getHeader("Guacamole-Token") == null; + + } + + /** + * Reports that the given address has just failed to authenticate and + * returns the AuthenticationFailureStatus that represents that failure. If + * the address isn't already being tracked, it will begin being tracked as + * of this call. If the address is already tracked, the returned + * AuthenticationFailureStatus will represent past authentication failures, + * as well. + * + * @param address + * The address that has failed to authenticate. + * + * @return + * An AuthenticationFailureStatus that represents this latest + * authentication failure for the given address, as well as any past + * failures. + */ + private AuthenticationFailureStatus getAuthenticationFailure(String address) { + + AuthenticationFailureStatus newFailure = new AuthenticationFailureStatus(maxAttempts, banDuration); + AuthenticationFailureStatus status = failures.putIfAbsent(address, newFailure); + if (status == null) + return newFailure; + + status.notifyFailed(); + return status; + + } + + /** + * Reports that an authentication request has been received, as well as + * whether that request is known to have failed. If the associated address + * is currently being blocked, an exception will be thrown. + * + * @param credentials + * The credentials associated with the authentication request. + * + * @param failed + * Whether the request is known to have failed. If the status of the + * request is not yet known, this should be false. + * + * @throws GuacamoleException + * If the authentication request is being blocked due to brute force + * prevention rules. + */ + private void notifyAuthenticationStatus(Credentials credentials, + boolean failed) throws GuacamoleException { + + // Do not track/ban if tracking or banning are disabled + if (maxAttempts <= 0 || banDuration <= 0) + return; + + // Ignore requests that do not contain explicit parameters of any kind + if (isEmpty(credentials)) + return; + + // Determine originating address of the authentication request + String address = credentials.getRemoteAddress(); + if (address == null) + throw new GuacamoleServerException("Source address cannot be determined."); + + // Get current failure status for the address associated with the + // authentication request, adding/updating that status if the request + // was itself a failure + AuthenticationFailureStatus status; + if (failed) { + status = getAuthenticationFailure(address); + logger.debug("Authentication has failed for address \"{}\" (current total failures: {}/{}).", + address, status.getFailures(), maxAttempts); + } + else + status = failures.get(address); + + if (status != null) { + + // Explicitly block further processing of authentication/authorization + // if too many failures have occurred + if (status.isBlocked()) { + logger.debug("Blocking authentication attempt from address \"{}\" due to number of authentication failures.", address); + throw new GuacamoleClientTooManyException("Too many failed " + + "authentication attempts. Please try again later."); + } + + // Clean up tracking of failures if the address is no longer + // relevant (all failures are sufficiently old) + else if (!status.isValid()) { + logger.debug("Removing address \"{}\" from tracking as there are no recent authentication failures.", address); + failures.remove(address); + } + + } + + } + + /** + * Reports that an authentication request has been received, but it is + * either not yet known whether the request has succeeded or failed. If the + * associated address is currently being blocked, an exception will be + * thrown. + * + * @param credentials + * The credentials associated with the authentication request. + * + * @throws GuacamoleException + * If the authentication request is being blocked due to brute force + * prevention rules. + */ + public void notifyAuthenticationRequestReceived(Credentials credentials) + throws GuacamoleException { + notifyAuthenticationStatus(credentials, false); + } + + /** + * Reports that an authentication request has been received and has + * succeeded. If the associated address is currently being blocked, an + * exception will be thrown. + * + * @param credentials + * The credentials associated with the successful authentication + * request. + * + * @throws GuacamoleException + * If the authentication request is being blocked due to brute force + * prevention rules. + */ + public void notifyAuthenticationSuccess(Credentials credentials) + throws GuacamoleException { + notifyAuthenticationStatus(credentials, false); + } + + /** + * Reports that an authentication request has been received and has + * failed. If the associated address is currently being blocked, an + * exception will be thrown. + * + * @param credentials + * The credentials associated with the failed authentication request. + * + * @throws GuacamoleException + * If the authentication request is being blocked due to brute force + * prevention rules. + */ + public void notifyAuthenticationFailed(Credentials credentials) + throws GuacamoleException { + notifyAuthenticationStatus(credentials, true); + } + +} diff --git a/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/BanningAuthenticationListener.java b/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/BanningAuthenticationListener.java new file mode 100644 index 000000000..38fd575fd --- /dev/null +++ b/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/BanningAuthenticationListener.java @@ -0,0 +1,81 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.guacamole.auth.ban; + +import org.apache.guacamole.GuacamoleException; +import org.apache.guacamole.net.auth.credentials.GuacamoleInsufficientCredentialsException; +import org.apache.guacamole.net.event.AuthenticationFailureEvent; +import org.apache.guacamole.net.event.AuthenticationSuccessEvent; +import org.apache.guacamole.net.event.listener.Listener; + +/** + * Listener implementation which automatically tracks authentication failures + * such that further authentication attempts may be automatically blocked by + * {@link BanningAuthenticationProvider} if they match configured criteria. + */ +public class BanningAuthenticationListener implements Listener { + + /** + * Shared tracker of addresses that have repeatedly failed authentication. + */ + private static AuthenticationFailureTracker tracker; + + /** + * Assigns the shared tracker instance used by both the {@link BanningAuthenticationProvider} + * and this listener. This function MUST be invoked with the tracker + * created for BanningAuthenticationProvider as soon as possible (during + * construction of BanningAuthenticationProvider), or processing of + * received events will fail internally. + * + * @param tracker + * The tracker instance to use for received authentication events. + */ + public static void setAuthenticationFailureTracker(AuthenticationFailureTracker tracker) { + BanningAuthenticationListener.tracker = tracker; + } + + @Override + public void handleEvent(Object event) throws GuacamoleException { + + if (event instanceof AuthenticationFailureEvent) { + + AuthenticationFailureEvent failure = (AuthenticationFailureEvent) event; + + // Requests for additional credentials are not failures per se, + // but continuations of a multi-request authentication attempt that + // has not yet succeeded OR failed + if (failure.getFailure() instanceof GuacamoleInsufficientCredentialsException) { + tracker.notifyAuthenticationRequestReceived(failure.getCredentials()); + return; + } + + // Consider all other errors to be failed auth attempts + tracker.notifyAuthenticationFailed(failure.getCredentials()); + + } + + else if (event instanceof AuthenticationSuccessEvent) { + AuthenticationSuccessEvent success = (AuthenticationSuccessEvent) event; + tracker.notifyAuthenticationSuccess(success.getCredentials()); + } + + } + +} diff --git a/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/BanningAuthenticationProvider.java b/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/BanningAuthenticationProvider.java new file mode 100644 index 000000000..12195f5e4 --- /dev/null +++ b/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/BanningAuthenticationProvider.java @@ -0,0 +1,121 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.guacamole.auth.ban; + +import org.apache.guacamole.GuacamoleException; +import org.apache.guacamole.environment.Environment; +import org.apache.guacamole.environment.LocalEnvironment; +import org.apache.guacamole.net.auth.AbstractAuthenticationProvider; +import org.apache.guacamole.net.auth.AuthenticatedUser; +import org.apache.guacamole.net.auth.Credentials; +import org.apache.guacamole.net.auth.UserContext; +import org.apache.guacamole.properties.IntegerGuacamoleProperty; + +/** + * AuthenticationProvider implementation that blocks further authentication + * attempts that are related to past authentication failures flagged by + * {@link BanningAuthenticationListener}. + */ +public class BanningAuthenticationProvider extends AbstractAuthenticationProvider { + + /** + * The maximum number of failed authentication attempts allowed before an + * address is temporarily banned. + */ + private static final IntegerGuacamoleProperty MAX_ATTEMPTS = new IntegerGuacamoleProperty() { + + @Override + public String getName() { + return "ban-max-invalid-attempts"; + } + + }; + + /** + * The length of time that each address should be banned after reaching the + * maximum number of failed authentication attempts, in seconds. + */ + private static final IntegerGuacamoleProperty IP_BAN_DURATION = new IntegerGuacamoleProperty() { + + @Override + public String getName() { + return "ban-address-duration"; + } + + }; + + /** + * The default maximum number of failed authentication attempts allowed + * before an address is temporarily banned. + */ + private static final int DEFAULT_MAX_ATTEMPTS = 5; + + /** + * The default length of time that each address should be banned after + * reaching the maximum number of failed authentication attempts, in + * seconds. + */ + private static final int DEFAULT_IP_BAN_DURATION = 300; + + /** + * Shared tracker of addresses that have repeatedly failed authentication. + */ + private final AuthenticationFailureTracker tracker; + + /** + * Creates a new BanningAuthenticationProvider which automatically bans + * further authentication attempts from addresses that have repeatedly + * failed to authenticate. The ban duration and maximum number of failed + * attempts allowed before banning are configured within + * guacamole.properties. + * + * @throws GuacamoleException + * If an error occurs parsing the configuration properties used by this + * extension. + */ + public BanningAuthenticationProvider() throws GuacamoleException { + + Environment environment = LocalEnvironment.getInstance(); + int maxAttempts = environment.getProperty(MAX_ATTEMPTS, DEFAULT_MAX_ATTEMPTS); + int banDuration = environment.getProperty(IP_BAN_DURATION, DEFAULT_IP_BAN_DURATION); + + tracker = new AuthenticationFailureTracker(maxAttempts, banDuration); + BanningAuthenticationListener.setAuthenticationFailureTracker(tracker); + + } + + @Override + public String getIdentifier() { + return "ban"; + } + + @Override + public AuthenticatedUser authenticateUser(Credentials credentials) throws GuacamoleException { + tracker.notifyAuthenticationRequestReceived(credentials); + return null; + } + + @Override + public UserContext getUserContext(AuthenticatedUser authenticatedUser) throws GuacamoleException { + tracker.notifyAuthenticationRequestReceived(authenticatedUser.getCredentials()); + return null; + } + +} diff --git a/extensions/guacamole-auth-ban/src/main/resources/guac-manifest.json b/extensions/guacamole-auth-ban/src/main/resources/guac-manifest.json new file mode 100644 index 000000000..87f75ea61 --- /dev/null +++ b/extensions/guacamole-auth-ban/src/main/resources/guac-manifest.json @@ -0,0 +1,16 @@ +{ + + "guacamoleVersion" : "1.4.0", + + "name" : "Brute-force Authentication Detection/Prevention", + "namespace" : "ban", + + "authProviders" : [ + "org.apache.guacamole.auth.ban.BanningAuthenticationProvider" + ], + + "listeners" : [ + "org.apache.guacamole.auth.ban.BanningAuthenticationListener" + ] + +} diff --git a/extensions/pom.xml b/extensions/pom.xml index 3bab33257..b16b3ed53 100644 --- a/extensions/pom.xml +++ b/extensions/pom.xml @@ -40,6 +40,7 @@ + guacamole-auth-ban guacamole-auth-duo guacamole-auth-header guacamole-auth-jdbc From f9d8abcfdeb50c8e8630aafb3fc19ff95be42670 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Mon, 22 Aug 2022 09:17:29 -0700 Subject: [PATCH 04/10] GUACAMOLE-990: Clear out any previous authentication token that is known to be invalid. --- .../src/app/auth/service/authenticationService.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/guacamole/src/main/frontend/src/app/auth/service/authenticationService.js b/guacamole/src/main/frontend/src/app/auth/service/authenticationService.js index 944fd4a9a..199d6cb2e 100644 --- a/guacamole/src/main/frontend/src/app/auth/service/authenticationService.js +++ b/guacamole/src/main/frontend/src/app/auth/service/authenticationService.js @@ -198,12 +198,16 @@ angular.module('auth').factory('authenticationService', ['$injector', ['catch'](requestService.createErrorCallback(function authenticationFailed(error) { // Request credentials if provided credentials were invalid - if (error.type === Error.Type.INVALID_CREDENTIALS) + if (error.type === Error.Type.INVALID_CREDENTIALS) { $rootScope.$broadcast('guacInvalidCredentials', parameters, error); + clearAuthenticationResult(); + } // Request more credentials if provided credentials were not enough - else if (error.type === Error.Type.INSUFFICIENT_CREDENTIALS) + else if (error.type === Error.Type.INSUFFICIENT_CREDENTIALS) { $rootScope.$broadcast('guacInsufficientCredentials', parameters, error); + clearAuthenticationResult(); + } // Abort rendering of page if an internal error occurs else if (error.type === Error.Type.INTERNAL_ERROR) From 2b19bc95daeead7127ddcc9b65260c2794d18fc6 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Mon, 22 Aug 2022 09:40:10 -0700 Subject: [PATCH 05/10] GUACAMOLE-990: Use translation string for "too many failed attempts" error. --- .../guacamole/auth/ban/AuthenticationFailureTracker.java | 7 ++++--- .../src/main/resources/guac-manifest.json | 4 ++++ .../src/main/resources/translations/en.json | 5 +++++ 3 files changed, 13 insertions(+), 3 deletions(-) create mode 100644 extensions/guacamole-auth-ban/src/main/resources/translations/en.json diff --git a/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/AuthenticationFailureTracker.java b/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/AuthenticationFailureTracker.java index 4f77875fc..d30f522bd 100644 --- a/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/AuthenticationFailureTracker.java +++ b/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/AuthenticationFailureTracker.java @@ -22,9 +22,9 @@ package org.apache.guacamole.auth.ban; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import javax.servlet.http.HttpServletRequest; -import org.apache.guacamole.GuacamoleClientTooManyException; import org.apache.guacamole.GuacamoleException; import org.apache.guacamole.GuacamoleServerException; +import org.apache.guacamole.language.TranslatableGuacamoleClientTooManyException; import org.apache.guacamole.net.auth.Credentials; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -207,8 +207,9 @@ public class AuthenticationFailureTracker { // if too many failures have occurred if (status.isBlocked()) { logger.debug("Blocking authentication attempt from address \"{}\" due to number of authentication failures.", address); - throw new GuacamoleClientTooManyException("Too many failed " - + "authentication attempts. Please try again later."); + throw new TranslatableGuacamoleClientTooManyException("Too " + + "many failed authentication attempts.", + "LOGIN.ERROR_TOO_MANY_ATTEMPTS"); } // Clean up tracking of failures if the address is no longer diff --git a/extensions/guacamole-auth-ban/src/main/resources/guac-manifest.json b/extensions/guacamole-auth-ban/src/main/resources/guac-manifest.json index 87f75ea61..1e6beac22 100644 --- a/extensions/guacamole-auth-ban/src/main/resources/guac-manifest.json +++ b/extensions/guacamole-auth-ban/src/main/resources/guac-manifest.json @@ -11,6 +11,10 @@ "listeners" : [ "org.apache.guacamole.auth.ban.BanningAuthenticationListener" + ], + + "translations" : [ + "translations/en.json" ] } diff --git a/extensions/guacamole-auth-ban/src/main/resources/translations/en.json b/extensions/guacamole-auth-ban/src/main/resources/translations/en.json new file mode 100644 index 000000000..2ef8a37d7 --- /dev/null +++ b/extensions/guacamole-auth-ban/src/main/resources/translations/en.json @@ -0,0 +1,5 @@ +{ + "LOGIN": { + "ERROR_TOO_MANY_ATTEMPTS" : "Too many failed authentication attempts. Please try again later." + } +} From 43f65357c8b0de44db925c66716ff0ad668c3af8 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Mon, 22 Aug 2022 10:40:52 -0700 Subject: [PATCH 06/10] GUACAMOLE-990: Limit maximum number of tracked addresses. --- doc/licenses/caffeine-2.9.3/README | 8 +++++ .../caffeine-2.9.3/dep-coordinates.txt | 1 + doc/licenses/checker-qual-3.19.0/LICENSE.txt | 22 +++++++++++++ doc/licenses/checker-qual-3.19.0/README | 8 +++++ .../checker-qual-3.19.0/dep-coordinates.txt | 1 + doc/licenses/error-prone-2.10.0/README | 8 +++++ .../error-prone-2.10.0/dep-coordinates.txt | 1 + extensions/guacamole-auth-ban/pom.xml | 21 ++++++++++++ .../auth/ban/AuthenticationFailureStatus.java | 8 ++--- .../ban/AuthenticationFailureTracker.java | 33 ++++++++++++------- .../ban/BanningAuthenticationProvider.java | 32 +++++++++++++++++- 11 files changed, 127 insertions(+), 16 deletions(-) create mode 100644 doc/licenses/caffeine-2.9.3/README create mode 100644 doc/licenses/caffeine-2.9.3/dep-coordinates.txt create mode 100644 doc/licenses/checker-qual-3.19.0/LICENSE.txt create mode 100644 doc/licenses/checker-qual-3.19.0/README create mode 100644 doc/licenses/checker-qual-3.19.0/dep-coordinates.txt create mode 100644 doc/licenses/error-prone-2.10.0/README create mode 100644 doc/licenses/error-prone-2.10.0/dep-coordinates.txt diff --git a/doc/licenses/caffeine-2.9.3/README b/doc/licenses/caffeine-2.9.3/README new file mode 100644 index 000000000..c51ba745c --- /dev/null +++ b/doc/licenses/caffeine-2.9.3/README @@ -0,0 +1,8 @@ +Caffeine (https://github.com/ben-manes/caffeine) +------------------------------------------------ + + Version: 2.9.3 + From: 'Ben Manes' (https://github.com/ben-manes) + License(s): + Apache v2.0 + diff --git a/doc/licenses/caffeine-2.9.3/dep-coordinates.txt b/doc/licenses/caffeine-2.9.3/dep-coordinates.txt new file mode 100644 index 000000000..feda8aa63 --- /dev/null +++ b/doc/licenses/caffeine-2.9.3/dep-coordinates.txt @@ -0,0 +1 @@ +com.github.ben-manes.caffeine:caffeine:jar:2.9.3 diff --git a/doc/licenses/checker-qual-3.19.0/LICENSE.txt b/doc/licenses/checker-qual-3.19.0/LICENSE.txt new file mode 100644 index 000000000..9837c6b69 --- /dev/null +++ b/doc/licenses/checker-qual-3.19.0/LICENSE.txt @@ -0,0 +1,22 @@ +Checker Framework qualifiers +Copyright 2004-present by the Checker Framework developers + +MIT License: + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE. diff --git a/doc/licenses/checker-qual-3.19.0/README b/doc/licenses/checker-qual-3.19.0/README new file mode 100644 index 000000000..4a00db7a7 --- /dev/null +++ b/doc/licenses/checker-qual-3.19.0/README @@ -0,0 +1,8 @@ +Checker Framework qualifiers (https://checkerframework.org/) +------------------------------------------------------------ + + Version: 3.19.0 + From: 'Checker Framework developers' (https://checkerframework.org/) + License(s): + MIT (bundled/checker-qual-3.19.0/LICENSE.txt) + diff --git a/doc/licenses/checker-qual-3.19.0/dep-coordinates.txt b/doc/licenses/checker-qual-3.19.0/dep-coordinates.txt new file mode 100644 index 000000000..322f3959d --- /dev/null +++ b/doc/licenses/checker-qual-3.19.0/dep-coordinates.txt @@ -0,0 +1 @@ +org.checkerframework:checker-qual:jar:3.19.0 diff --git a/doc/licenses/error-prone-2.10.0/README b/doc/licenses/error-prone-2.10.0/README new file mode 100644 index 000000000..f286aed3b --- /dev/null +++ b/doc/licenses/error-prone-2.10.0/README @@ -0,0 +1,8 @@ +Error Prone (https://errorprone.info/) +-------------------------------------- + + Version: 2.10.0 + From: 'Google Inc.' (http://www.google.com/) + License(s): + Apache v2.0 + diff --git a/doc/licenses/error-prone-2.10.0/dep-coordinates.txt b/doc/licenses/error-prone-2.10.0/dep-coordinates.txt new file mode 100644 index 000000000..9473dfcab --- /dev/null +++ b/doc/licenses/error-prone-2.10.0/dep-coordinates.txt @@ -0,0 +1 @@ +com.google.errorprone:error_prone_annotations:jar:2.10.0 diff --git a/extensions/guacamole-auth-ban/pom.xml b/extensions/guacamole-auth-ban/pom.xml index 2be082fc2..68b8d6f90 100644 --- a/extensions/guacamole-auth-ban/pom.xml +++ b/extensions/guacamole-auth-ban/pom.xml @@ -53,6 +53,27 @@ guacamole-ext 1.4.0 provided + + + + + org.checkerframework + checker-qual + + + com.google.errorprone + error_prone_annotations + + + + + + + + com.github.ben-manes.caffeine + caffeine + 2.9.3 diff --git a/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/AuthenticationFailureStatus.java b/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/AuthenticationFailureStatus.java index 87bdc5515..ac4f1efc1 100644 --- a/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/AuthenticationFailureStatus.java +++ b/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/AuthenticationFailureStatus.java @@ -53,9 +53,9 @@ public class AuthenticationFailureStatus { private final long duration; /** - * Creates an AuthenticationFailureStatus that represents a single failure - * and is subject to the given restrictions. Additional failures may be - * flagged after creation with {@link #notifyFailed()}. + * Creates an AuthenticationFailureStatus that is initialized to zero + * failures and is subject to the given restrictions. Additional failures + * may be flagged after creation with {@link #notifyFailed()}. * * @param maxAttempts * The maximum number of failures that may occur before the @@ -67,7 +67,7 @@ public class AuthenticationFailureStatus { */ public AuthenticationFailureStatus(int maxAttempts, int duration) { this.lastFailure = System.nanoTime(); - this.failureCount = new AtomicInteger(1); + this.failureCount = new AtomicInteger(0); this.maxAttempts = maxAttempts; this.duration = TimeUnit.SECONDS.toNanos(duration); } diff --git a/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/AuthenticationFailureTracker.java b/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/AuthenticationFailureTracker.java index d30f522bd..85f5cc09c 100644 --- a/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/AuthenticationFailureTracker.java +++ b/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/AuthenticationFailureTracker.java @@ -19,8 +19,8 @@ package org.apache.guacamole.auth.ban; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; +import com.github.benmanes.caffeine.cache.Cache; +import com.github.benmanes.caffeine.cache.Caffeine; import javax.servlet.http.HttpServletRequest; import org.apache.guacamole.GuacamoleException; import org.apache.guacamole.GuacamoleServerException; @@ -44,8 +44,7 @@ public class AuthenticationFailureTracker { * All authentication failures currently being tracked, stored by the * associated IP address. */ - private final ConcurrentMap failures = - new ConcurrentHashMap<>(); + private final Cache failures; /** * The maximum number of failed authentication attempts allowed before an @@ -70,8 +69,14 @@ public class AuthenticationFailureTracker { * @param banDuration * The length of time that each address should be banned after reaching * the maximum number of failed authentication attempts, in seconds. + * + * @param maxAddresses + * The maximum number of unique IP addresses that should be tracked + * before discarding older tracked failures. */ - public AuthenticationFailureTracker(int maxAttempts, int banDuration) { + public AuthenticationFailureTracker(int maxAttempts, int banDuration, + long maxAddresses) { + this.maxAttempts = maxAttempts; this.banDuration = banDuration; @@ -93,6 +98,14 @@ public class AuthenticationFailureTracker { banDuration, maxAttempts); } + // Limit maximum number of tracked addresses to configured upper bound + this.failures = Caffeine.newBuilder() + .maximumSize(maxAddresses) + .build(); + + logger.info("Up to {} unique addresses will be tracked/banned at any " + + " given time.", maxAddresses); + } /** @@ -147,10 +160,8 @@ public class AuthenticationFailureTracker { */ private AuthenticationFailureStatus getAuthenticationFailure(String address) { - AuthenticationFailureStatus newFailure = new AuthenticationFailureStatus(maxAttempts, banDuration); - AuthenticationFailureStatus status = failures.putIfAbsent(address, newFailure); - if (status == null) - return newFailure; + AuthenticationFailureStatus status = failures.get(address, + (addr) -> new AuthenticationFailureStatus(maxAttempts, banDuration)); status.notifyFailed(); return status; @@ -199,7 +210,7 @@ public class AuthenticationFailureTracker { address, status.getFailures(), maxAttempts); } else - status = failures.get(address); + status = failures.getIfPresent(address); if (status != null) { @@ -216,7 +227,7 @@ public class AuthenticationFailureTracker { // relevant (all failures are sufficiently old) else if (!status.isValid()) { logger.debug("Removing address \"{}\" from tracking as there are no recent authentication failures.", address); - failures.remove(address); + failures.invalidate(address); } } diff --git a/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/BanningAuthenticationProvider.java b/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/BanningAuthenticationProvider.java index 12195f5e4..adf54ff0c 100644 --- a/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/BanningAuthenticationProvider.java +++ b/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/BanningAuthenticationProvider.java @@ -20,6 +20,7 @@ package org.apache.guacamole.auth.ban; import org.apache.guacamole.GuacamoleException; +import org.apache.guacamole.GuacamoleServerException; import org.apache.guacamole.environment.Environment; import org.apache.guacamole.environment.LocalEnvironment; import org.apache.guacamole.net.auth.AbstractAuthenticationProvider; @@ -27,6 +28,7 @@ import org.apache.guacamole.net.auth.AuthenticatedUser; import org.apache.guacamole.net.auth.Credentials; import org.apache.guacamole.net.auth.UserContext; import org.apache.guacamole.properties.IntegerGuacamoleProperty; +import org.apache.guacamole.properties.LongGuacamoleProperty; /** * AuthenticationProvider implementation that blocks further authentication @@ -61,6 +63,20 @@ public class BanningAuthenticationProvider extends AbstractAuthenticationProvide }; + /** + * The maximum number of failed authentication attempts tracked at any + * given time. Once this number of addresses is exceeded, the oldest + * authentication attempts are rotated off on an LRU basis. + */ + private static final LongGuacamoleProperty MAX_ADDRESSES = new LongGuacamoleProperty() { + + @Override + public String getName() { + return "ban-max-addresses"; + } + + }; + /** * The default maximum number of failed authentication attempts allowed * before an address is temporarily banned. @@ -74,6 +90,13 @@ public class BanningAuthenticationProvider extends AbstractAuthenticationProvide */ private static final int DEFAULT_IP_BAN_DURATION = 300; + /** + * The maximum number of failed authentication attempts tracked at any + * given time. Once this number of addresses is exceeded, the oldest + * authentication attempts are rotated off on an LRU basis. + */ + private static final long DEFAULT_MAX_ADDRESSES = 10485760; + /** * Shared tracker of addresses that have repeatedly failed authentication. */ @@ -95,8 +118,15 @@ public class BanningAuthenticationProvider extends AbstractAuthenticationProvide Environment environment = LocalEnvironment.getInstance(); int maxAttempts = environment.getProperty(MAX_ATTEMPTS, DEFAULT_MAX_ATTEMPTS); int banDuration = environment.getProperty(IP_BAN_DURATION, DEFAULT_IP_BAN_DURATION); + long maxAddresses = environment.getProperty(MAX_ADDRESSES, DEFAULT_MAX_ADDRESSES); - tracker = new AuthenticationFailureTracker(maxAttempts, banDuration); + if (maxAddresses <= 0) + throw new GuacamoleServerException("The maximum number of " + + "addresses tracked, as specified by the " + + "\"" + MAX_ADDRESSES.getName() + "\" property, must be " + + "greater than zero."); + + tracker = new AuthenticationFailureTracker(maxAttempts, banDuration, maxAddresses); BanningAuthenticationListener.setAuthenticationFailureTracker(tracker); } From 8b981d92135253626239dd957192747eb6532c0f Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Mon, 22 Aug 2022 12:13:19 -0700 Subject: [PATCH 07/10] GUACAMOLE-990: Add Docker image support for guacamole-auth-ban. --- guacamole-docker/bin/build-guacamole.sh | 10 ++++++++++ guacamole-docker/bin/start.sh | 12 ++++++++++++ 2 files changed, 22 insertions(+) diff --git a/guacamole-docker/bin/build-guacamole.sh b/guacamole-docker/bin/build-guacamole.sh index 9f1773d83..6ffc86611 100755 --- a/guacamole-docker/bin/build-guacamole.sh +++ b/guacamole-docker/bin/build-guacamole.sh @@ -198,3 +198,13 @@ if [ -f extensions/guacamole-auth-json/target/guacamole-auth-json*.jar ]; then mkdir -p "$DESTINATION/json" cp extensions/guacamole-auth-json/target/guacamole-auth-json*.jar "$DESTINATION/json" fi + +# +# Copy automatic brute-force banning auth extension if it was built +# + +if [ -f extensions/guacamole-auth-ban/target/guacamole-auth-ban*.jar ]; then + mkdir -p "$DESTINATION/ban" + cp extensions/guacamole-auth-ban/target/guacamole-auth-ban*.jar "$DESTINATION/ban" +fi + diff --git a/guacamole-docker/bin/start.sh b/guacamole-docker/bin/start.sh index c9d205b58..632c3d03a 100755 --- a/guacamole-docker/bin/start.sh +++ b/guacamole-docker/bin/start.sh @@ -1160,6 +1160,18 @@ if [ -n "$API_SESSION_TIMEOUT" ]; then associate_apisessiontimeout fi +# Apply any overrides for default address ban behavior +set_optional_property "ban-address-duration" "$BAN_ADDRESS_DURATION" +set_optional_property "ban-max-addresses" "$BAN_MAX_ADDRESSES" +set_optional_property "ban-max-invalid-attempts" "$BAN_MAX_INVALID_ATTEMPTS" + +# Ensure guacamole-auth-ban always loads before other extensions unless +# explicitly overridden via naming or EXTENSION_PRIORITY (allowing other +# extensions to attempt authentication before guacamole-auth-ban has a chance +# to enforce any bans could allow credentials to continue to be guessed even +# after the address has been blocked via timing attacks) +ln -s /opt/guacamole/ban/guacamole-auth-*.jar "$GUACAMOLE_EXT/_guacamole-auth-ban.jar" + # Set logback level if specified if [ -n "$LOGBACK_LEVEL" ]; then unzip -o -j /opt/guacamole/guacamole.war WEB-INF/classes/logback.xml -d $GUACAMOLE_HOME From a9ed4c298206fb14503811151fab0d03ad1d570e Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Mon, 22 Aug 2022 12:19:52 -0700 Subject: [PATCH 08/10] GUACAMOLE-990: Revise guacamole-auth-ban log levels to generally always notify of problematic addresses. --- .../guacamole/auth/ban/AuthenticationFailureTracker.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/AuthenticationFailureTracker.java b/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/AuthenticationFailureTracker.java index 85f5cc09c..72b4fc8dd 100644 --- a/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/AuthenticationFailureTracker.java +++ b/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/AuthenticationFailureTracker.java @@ -206,7 +206,7 @@ public class AuthenticationFailureTracker { AuthenticationFailureStatus status; if (failed) { status = getAuthenticationFailure(address); - logger.debug("Authentication has failed for address \"{}\" (current total failures: {}/{}).", + logger.info("Authentication has failed for address \"{}\" (current total failures: {}/{}).", address, status.getFailures(), maxAttempts); } else @@ -217,7 +217,7 @@ public class AuthenticationFailureTracker { // Explicitly block further processing of authentication/authorization // if too many failures have occurred if (status.isBlocked()) { - logger.debug("Blocking authentication attempt from address \"{}\" due to number of authentication failures.", address); + logger.warn("Blocking authentication attempt from address \"{}\" due to number of authentication failures.", address); throw new TranslatableGuacamoleClientTooManyException("Too " + "many failed authentication attempts.", "LOGIN.ERROR_TOO_MANY_ATTEMPTS"); From 584db45a4fdbb950ec4e793e5ee958f9d3fcff36 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Mon, 22 Aug 2022 15:08:42 -0700 Subject: [PATCH 09/10] GUACAMOLE-990: Enabled/disable auth failure tracking via implementations of a common interface. --- .../ban/BanningAuthenticationListener.java | 1 + .../ban/BanningAuthenticationProvider.java | 34 +++++++- .../AuthenticationFailureStatus.java | 2 +- .../status/AuthenticationFailureTracker.java | 78 ++++++++++++++++++ ...InMemoryAuthenticationFailureTracker.java} | 79 +++---------------- .../NullAuthenticationFailureTracker.java | 49 ++++++++++++ 6 files changed, 172 insertions(+), 71 deletions(-) rename extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/{ => status}/AuthenticationFailureStatus.java (98%) create mode 100644 extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/status/AuthenticationFailureTracker.java rename extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/{AuthenticationFailureTracker.java => status/InMemoryAuthenticationFailureTracker.java} (74%) create mode 100644 extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/status/NullAuthenticationFailureTracker.java diff --git a/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/BanningAuthenticationListener.java b/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/BanningAuthenticationListener.java index 38fd575fd..4d8a3bbad 100644 --- a/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/BanningAuthenticationListener.java +++ b/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/BanningAuthenticationListener.java @@ -19,6 +19,7 @@ package org.apache.guacamole.auth.ban; +import org.apache.guacamole.auth.ban.status.AuthenticationFailureTracker; import org.apache.guacamole.GuacamoleException; import org.apache.guacamole.net.auth.credentials.GuacamoleInsufficientCredentialsException; import org.apache.guacamole.net.event.AuthenticationFailureEvent; diff --git a/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/BanningAuthenticationProvider.java b/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/BanningAuthenticationProvider.java index adf54ff0c..a6df43a30 100644 --- a/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/BanningAuthenticationProvider.java +++ b/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/BanningAuthenticationProvider.java @@ -19,8 +19,11 @@ package org.apache.guacamole.auth.ban; +import org.apache.guacamole.auth.ban.status.InMemoryAuthenticationFailureTracker; +import org.apache.guacamole.auth.ban.status.AuthenticationFailureTracker; import org.apache.guacamole.GuacamoleException; import org.apache.guacamole.GuacamoleServerException; +import org.apache.guacamole.auth.ban.status.NullAuthenticationFailureTracker; import org.apache.guacamole.environment.Environment; import org.apache.guacamole.environment.LocalEnvironment; import org.apache.guacamole.net.auth.AbstractAuthenticationProvider; @@ -29,6 +32,8 @@ import org.apache.guacamole.net.auth.Credentials; import org.apache.guacamole.net.auth.UserContext; import org.apache.guacamole.properties.IntegerGuacamoleProperty; import org.apache.guacamole.properties.LongGuacamoleProperty; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * AuthenticationProvider implementation that blocks further authentication @@ -37,6 +42,11 @@ import org.apache.guacamole.properties.LongGuacamoleProperty; */ public class BanningAuthenticationProvider extends AbstractAuthenticationProvider { + /** + * Logger for this class. + */ + private static final Logger logger = LoggerFactory.getLogger(BanningAuthenticationProvider.class); + /** * The maximum number of failed authentication attempts allowed before an * address is temporarily banned. @@ -126,7 +136,29 @@ public class BanningAuthenticationProvider extends AbstractAuthenticationProvide + "\"" + MAX_ADDRESSES.getName() + "\" property, must be " + "greater than zero."); - tracker = new AuthenticationFailureTracker(maxAttempts, banDuration, maxAddresses); + // Configure auth failure tracking behavior and inform administrator of + // ultimate result + if (maxAttempts <= 0) { + this.tracker = new NullAuthenticationFailureTracker(); + logger.info("Maximum failed authentication attempts has been set " + + "to {}. Automatic banning of brute-force authentication " + + "attempts will be disabled.", maxAttempts); + } + else if (banDuration <= 0) { + this.tracker = new NullAuthenticationFailureTracker(); + logger.info("Ban duration for addresses that repeatedly fail " + + "authentication has been set to {}. Automatic banning " + + "of brute-force authentication attempts will be " + + "disabled.", banDuration); + } + else { + this.tracker = new InMemoryAuthenticationFailureTracker(maxAttempts, banDuration, maxAddresses); + logger.info("Addresses will be automatically banned for {} " + + "seconds after {} failed authentication attempts. Up " + + "to {} unique addresses will be tracked/banned at any " + + "given time.", banDuration, maxAttempts, maxAddresses); + } + BanningAuthenticationListener.setAuthenticationFailureTracker(tracker); } diff --git a/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/AuthenticationFailureStatus.java b/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/status/AuthenticationFailureStatus.java similarity index 98% rename from extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/AuthenticationFailureStatus.java rename to extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/status/AuthenticationFailureStatus.java index ac4f1efc1..3292d1179 100644 --- a/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/AuthenticationFailureStatus.java +++ b/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/status/AuthenticationFailureStatus.java @@ -17,7 +17,7 @@ * under the License. */ -package org.apache.guacamole.auth.ban; +package org.apache.guacamole.auth.ban.status; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; diff --git a/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/status/AuthenticationFailureTracker.java b/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/status/AuthenticationFailureTracker.java new file mode 100644 index 000000000..9ea7f2c9e --- /dev/null +++ b/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/status/AuthenticationFailureTracker.java @@ -0,0 +1,78 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.guacamole.auth.ban.status; + +import org.apache.guacamole.GuacamoleException; +import org.apache.guacamole.net.auth.Credentials; + +/** + * Tracks past authentication results, automatically blocking the IP addresses + * of machines that repeatedly fail to authenticate. + */ +public interface AuthenticationFailureTracker { + + /** + * Reports that an authentication request has been received, but it is + * either not yet known whether the request has succeeded or failed. If the + * associated address is currently being blocked, an exception will be + * thrown. + * + * @param credentials + * The credentials associated with the authentication request. + * + * @throws GuacamoleException + * If the authentication request is being blocked due to brute force + * prevention rules. + */ + void notifyAuthenticationRequestReceived(Credentials credentials) + throws GuacamoleException; + + /** + * Reports that an authentication request has been received and has + * succeeded. If the associated address is currently being blocked, an + * exception will be thrown. + * + * @param credentials + * The credentials associated with the successful authentication + * request. + * + * @throws GuacamoleException + * If the authentication request is being blocked due to brute force + * prevention rules. + */ + void notifyAuthenticationSuccess(Credentials credentials) + throws GuacamoleException; + + /** + * Reports that an authentication request has been received and has + * failed. If the associated address is currently being blocked, an + * exception will be thrown. + * + * @param credentials + * The credentials associated with the failed authentication request. + * + * @throws GuacamoleException + * If the authentication request is being blocked due to brute force + * prevention rules. + */ + void notifyAuthenticationFailed(Credentials credentials) + throws GuacamoleException; + +} diff --git a/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/AuthenticationFailureTracker.java b/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/status/InMemoryAuthenticationFailureTracker.java similarity index 74% rename from extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/AuthenticationFailureTracker.java rename to extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/status/InMemoryAuthenticationFailureTracker.java index 72b4fc8dd..b655168e1 100644 --- a/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/AuthenticationFailureTracker.java +++ b/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/status/InMemoryAuthenticationFailureTracker.java @@ -17,7 +17,7 @@ * under the License. */ -package org.apache.guacamole.auth.ban; +package org.apache.guacamole.auth.ban.status; import com.github.benmanes.caffeine.cache.Cache; import com.github.benmanes.caffeine.cache.Caffeine; @@ -30,15 +30,16 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** - * Provides automated tracking and blocking of IP addresses that repeatedly - * fail authentication. + * AuthenticationFailureTracker implementation that tracks the failure status + * of each IP address in memory. The maximum amount of memory consumed is + * bounded by the configured maximum number of addresses tracked. */ -public class AuthenticationFailureTracker { +public class InMemoryAuthenticationFailureTracker implements AuthenticationFailureTracker { /** * Logger for this class. */ - private static final Logger logger = LoggerFactory.getLogger(AuthenticationFailureTracker.class); + private static final Logger logger = LoggerFactory.getLogger(InMemoryAuthenticationFailureTracker.class); /** * All authentication failures currently being tracked, stored by the @@ -74,38 +75,17 @@ public class AuthenticationFailureTracker { * The maximum number of unique IP addresses that should be tracked * before discarding older tracked failures. */ - public AuthenticationFailureTracker(int maxAttempts, int banDuration, + public InMemoryAuthenticationFailureTracker(int maxAttempts, int banDuration, long maxAddresses) { this.maxAttempts = maxAttempts; this.banDuration = banDuration; - // Inform administrator of configured behavior - if (maxAttempts <= 0) { - logger.info("Maximum failed authentication attempts has been set " - + "to {}. Automatic banning of brute-force authentication " - + "attempts will be disabled.", maxAttempts); - } - else if (banDuration <= 0) { - logger.info("Ban duration for addresses that repeatedly fail " - + "authentication has been set to {}. Automatic banning " - + "of brute-force authentication attempts will be " - + "disabled.", banDuration); - } - else { - logger.info("Addresses will be automatically banned for {} " - + "seconds after {} failed authentication attempts.", - banDuration, maxAttempts); - } - // Limit maximum number of tracked addresses to configured upper bound this.failures = Caffeine.newBuilder() .maximumSize(maxAddresses) .build(); - logger.info("Up to {} unique addresses will be tracked/banned at any " - + " given time.", maxAddresses); - } /** @@ -187,10 +167,6 @@ public class AuthenticationFailureTracker { private void notifyAuthenticationStatus(Credentials credentials, boolean failed) throws GuacamoleException { - // Do not track/ban if tracking or banning are disabled - if (maxAttempts <= 0 || banDuration <= 0) - return; - // Ignore requests that do not contain explicit parameters of any kind if (isEmpty(credentials)) return; @@ -234,54 +210,19 @@ public class AuthenticationFailureTracker { } - /** - * Reports that an authentication request has been received, but it is - * either not yet known whether the request has succeeded or failed. If the - * associated address is currently being blocked, an exception will be - * thrown. - * - * @param credentials - * The credentials associated with the authentication request. - * - * @throws GuacamoleException - * If the authentication request is being blocked due to brute force - * prevention rules. - */ + @Override public void notifyAuthenticationRequestReceived(Credentials credentials) throws GuacamoleException { notifyAuthenticationStatus(credentials, false); } - /** - * Reports that an authentication request has been received and has - * succeeded. If the associated address is currently being blocked, an - * exception will be thrown. - * - * @param credentials - * The credentials associated with the successful authentication - * request. - * - * @throws GuacamoleException - * If the authentication request is being blocked due to brute force - * prevention rules. - */ + @Override public void notifyAuthenticationSuccess(Credentials credentials) throws GuacamoleException { notifyAuthenticationStatus(credentials, false); } - /** - * Reports that an authentication request has been received and has - * failed. If the associated address is currently being blocked, an - * exception will be thrown. - * - * @param credentials - * The credentials associated with the failed authentication request. - * - * @throws GuacamoleException - * If the authentication request is being blocked due to brute force - * prevention rules. - */ + @Override public void notifyAuthenticationFailed(Credentials credentials) throws GuacamoleException { notifyAuthenticationStatus(credentials, true); diff --git a/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/status/NullAuthenticationFailureTracker.java b/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/status/NullAuthenticationFailureTracker.java new file mode 100644 index 000000000..9b50a3022 --- /dev/null +++ b/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/status/NullAuthenticationFailureTracker.java @@ -0,0 +1,49 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.guacamole.auth.ban.status; + +import org.apache.guacamole.GuacamoleException; +import org.apache.guacamole.net.auth.Credentials; + +/** + * AuthenticationFailureTracker implementation that does nothing. All requests + * are ignored, regardless of status, and no tracking is performed. + */ +public class NullAuthenticationFailureTracker implements AuthenticationFailureTracker { + + @Override + public void notifyAuthenticationRequestReceived(Credentials credentials) + throws GuacamoleException { + // Do nothing + } + + @Override + public void notifyAuthenticationSuccess(Credentials credentials) + throws GuacamoleException { + // Do nothing + } + + @Override + public void notifyAuthenticationFailed(Credentials credentials) + throws GuacamoleException { + // Do nothing + } + +} From 2e5d3f4fafcfe475cd8c7bccaba4d5eae94ca560 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Mon, 22 Aug 2022 15:12:01 -0700 Subject: [PATCH 10/10] GUACAMOLE-990: Disable tracking if max addresses is not a positive integer. --- .../auth/ban/BanningAuthenticationProvider.java | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/BanningAuthenticationProvider.java b/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/BanningAuthenticationProvider.java index a6df43a30..1d115d39d 100644 --- a/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/BanningAuthenticationProvider.java +++ b/extensions/guacamole-auth-ban/src/main/java/org/apache/guacamole/auth/ban/BanningAuthenticationProvider.java @@ -22,7 +22,6 @@ package org.apache.guacamole.auth.ban; import org.apache.guacamole.auth.ban.status.InMemoryAuthenticationFailureTracker; import org.apache.guacamole.auth.ban.status.AuthenticationFailureTracker; import org.apache.guacamole.GuacamoleException; -import org.apache.guacamole.GuacamoleServerException; import org.apache.guacamole.auth.ban.status.NullAuthenticationFailureTracker; import org.apache.guacamole.environment.Environment; import org.apache.guacamole.environment.LocalEnvironment; @@ -130,12 +129,6 @@ public class BanningAuthenticationProvider extends AbstractAuthenticationProvide int banDuration = environment.getProperty(IP_BAN_DURATION, DEFAULT_IP_BAN_DURATION); long maxAddresses = environment.getProperty(MAX_ADDRESSES, DEFAULT_MAX_ADDRESSES); - if (maxAddresses <= 0) - throw new GuacamoleServerException("The maximum number of " - + "addresses tracked, as specified by the " - + "\"" + MAX_ADDRESSES.getName() + "\" property, must be " - + "greater than zero."); - // Configure auth failure tracking behavior and inform administrator of // ultimate result if (maxAttempts <= 0) { @@ -151,6 +144,12 @@ public class BanningAuthenticationProvider extends AbstractAuthenticationProvide + "of brute-force authentication attempts will be " + "disabled.", banDuration); } + else if (maxAddresses <= 0) { + this.tracker = new NullAuthenticationFailureTracker(); + logger.info("Maximum number of tracked addresses has been set to " + + "{}. Automatic banning of brute-force authentication " + + "attempts will be disabled.", maxAddresses); + } else { this.tracker = new InMemoryAuthenticationFailureTracker(maxAttempts, banDuration, maxAddresses); logger.info("Addresses will be automatically banned for {} "