From 8c7b89986c49cd30e6c2e5115b22e0e3399a93fc Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 20 Mar 2015 12:03:42 -0700 Subject: [PATCH 1/2] GUAC-1135: Restore logging of IP addresses during authentication. Restore logging of failed auth attempts. --- .../net/basic/rest/auth/TokenRESTService.java | 76 ++++++++++++++++++- 1 file changed, 73 insertions(+), 3 deletions(-) diff --git a/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/auth/TokenRESTService.java b/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/auth/TokenRESTService.java index 3aac87e65..9e46507bc 100644 --- a/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/auth/TokenRESTService.java +++ b/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/rest/auth/TokenRESTService.java @@ -24,6 +24,7 @@ package org.glyptodon.guacamole.net.basic.rest.auth; import com.google.inject.Inject; import java.io.UnsupportedEncodingException; +import java.util.regex.Pattern; import javax.servlet.http.HttpServletRequest; import javax.ws.rs.DELETE; import javax.ws.rs.FormParam; @@ -76,7 +77,53 @@ public class TokenRESTService { * Logger for this class. */ private static final Logger logger = LoggerFactory.getLogger(TokenRESTService.class); - + + /** + * Regular expression which matches any IPv4 address. + */ + private static final String IPV4_ADDRESS_REGEX = "([0-9]{1,3}\\.[0-9]{1,3}\\.[0-9]{1,3}\\.[0-9]{1,3})"; + + /** + * Regular expression which matches any IPv6 address. + */ + private static final String IPV6_ADDRESS_REGEX = "([0-9a-fA-F]*(:[0-9a-fA-F]*){0,7})"; + + /** + * Regular expression which matches any IP address, regardless of version. + */ + private static final String IP_ADDRESS_REGEX = "(" + IPV4_ADDRESS_REGEX + "|" + IPV6_ADDRESS_REGEX + ")"; + + /** + * Pattern which matches valid values of the de-facto standard + * "X-Forwarded-For" header. + */ + private static final Pattern X_FORWARDED_FOR = Pattern.compile("^" + IP_ADDRESS_REGEX + "(, " + IP_ADDRESS_REGEX + ")*$"); + + /** + * Returns a formatted string containing an IP address, or list of IP + * addresses, which represent the HTTP client and any involved proxies. As + * the headers used to determine proxies can easily be forged, this data is + * superficially validated to ensure that it at least looks like a list of + * IPs. + * + * @param request + * The HTTP request to format. + * + * @return + * A formatted string containing one or more IP addresses. + */ + private String getLoggableAddress(HttpServletRequest request) { + + // Log X-Forwarded-For, if present and valid + String header = request.getHeader("X-Forwarded-For"); + if (header != null && X_FORWARDED_FOR.matcher(header).matches()) + return "[" + header + ", " + request.getRemoteAddr() + "]"; + + // If header absent or invalid, just use source IP + return request.getRemoteAddr(); + + } + /** * Authenticates a user, generates an auth token, associates that auth token * with the user's UserContext for use by further requests. If an existing @@ -160,9 +207,17 @@ public class TokenRESTService { userContext = authProvider.updateUserContext(existingSession.getUserContext(), credentials); /// Otherwise, generate a new user context - else + else { + userContext = authProvider.getUserContext(credentials); + // Log successful authentication + if (userContext != null && logger.isInfoEnabled()) + logger.info("User \"{}\" successfully authenticated from {}.", + userContext.self().getIdentifier(), getLoggableAddress(request)); + + } + } catch(GuacamoleException e) { logger.error("Exception caught while authenticating user.", e); @@ -171,9 +226,24 @@ public class TokenRESTService { } // Authentication failed. - if (userContext == null) + if (userContext == null) { + + // 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), username); + throw new HTTPException(Status.UNAUTHORIZED, "Permission Denied."); + } + // Update existing session, if it exists String authToken; if (existingSession != null) { From 24a7525ab532c7094359c69aac2f5cc391dcc850 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 20 Mar 2015 13:10:15 -0700 Subject: [PATCH 2/2] GUAC-1135: Derive remote host from X-Forwarded-For, if present. --- .../auth/jdbc/user/AuthenticatedUser.java | 41 ++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/glyptodon/guacamole/auth/jdbc/user/AuthenticatedUser.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/glyptodon/guacamole/auth/jdbc/user/AuthenticatedUser.java index f778bbb44..480000e62 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/glyptodon/guacamole/auth/jdbc/user/AuthenticatedUser.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/glyptodon/guacamole/auth/jdbc/user/AuthenticatedUser.java @@ -22,6 +22,8 @@ package org.glyptodon.guacamole.auth.jdbc.user; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import javax.servlet.http.HttpServletRequest; import org.glyptodon.guacamole.net.auth.Credentials; @@ -47,9 +49,34 @@ public class AuthenticatedUser { */ private final String remoteHost; + /** + * Regular expression which matches any IPv4 address. + */ + private static final String IPV4_ADDRESS_REGEX = "([0-9]{1,3}\\.[0-9]{1,3}\\.[0-9]{1,3}\\.[0-9]{1,3})"; + + /** + * Regular expression which matches any IPv6 address. + */ + private static final String IPV6_ADDRESS_REGEX = "([0-9a-fA-F]*(:[0-9a-fA-F]*){0,7})"; + + /** + * Regular expression which matches any IP address, regardless of version. + */ + private static final String IP_ADDRESS_REGEX = "(" + IPV4_ADDRESS_REGEX + "|" + IPV6_ADDRESS_REGEX + ")"; + + /** + * Pattern which matches valid values of the de-facto standard + * "X-Forwarded-For" header. + */ + private static final Pattern X_FORWARDED_FOR = Pattern.compile("^" + IP_ADDRESS_REGEX + "(, " + IP_ADDRESS_REGEX + ")*$"); + /** * Derives the remote host of the authenticating user from the given - * credentials object. + * credentials object. The remote host is derived from X-Forwarded-For + * in addition to the actual source IP of the request, and thus is not + * trusted. The derived remote host is really only useful for logging, + * unless the server is configured such that X-Forwarded-For is guaranteed + * to be trustworthy. * * @param credentials * The credentials to derive the remote host from. @@ -59,8 +86,20 @@ public class AuthenticatedUser { * authenticating. */ private static String getRemoteHost(Credentials credentials) { + HttpServletRequest request = credentials.getRequest(); + + // Use X-Forwarded-For, if present and valid + String header = request.getHeader("X-Forwarded-For"); + if (header != null) { + Matcher matcher = X_FORWARDED_FOR.matcher(header); + if (matcher.matches()) + return matcher.group(1); + } + + // If header absent or invalid, just use source IP return request.getRemoteAddr(); + } /**