GUACAMOLE-540: Merge changes ensuring remote addresses are handled/logged consistently across the various parts of the webapp.

This commit is contained in:
Michael Jumper
2018-07-01 23:09:08 -07:00
5 changed files with 45 additions and 74 deletions

View File

@@ -137,7 +137,7 @@ public class ModeledUserContext extends RestrictedObject
userRecord = new ActivityRecordModel(); userRecord = new ActivityRecordModel();
userRecord.setUsername(currentUser.getIdentifier()); userRecord.setUsername(currentUser.getIdentifier());
userRecord.setStartDate(new Date()); userRecord.setStartDate(new Date());
userRecord.setRemoteHost(currentUser.getCredentials().getRemoteHostname()); userRecord.setRemoteHost(currentUser.getCredentials().getRemoteAddress());
// Insert record representing login // Insert record representing login
userRecordMapper.insert(userRecord); userRecordMapper.insert(userRecord);

View File

@@ -19,9 +19,6 @@
package org.apache.guacamole.auth.jdbc.user; package org.apache.guacamole.auth.jdbc.user;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import javax.servlet.http.HttpServletRequest;
import org.apache.guacamole.net.auth.AuthenticatedUser; import org.apache.guacamole.net.auth.AuthenticatedUser;
import org.apache.guacamole.net.auth.AuthenticationProvider; import org.apache.guacamole.net.auth.AuthenticationProvider;
import org.apache.guacamole.net.auth.Credentials; import org.apache.guacamole.net.auth.Credentials;
@@ -46,59 +43,6 @@ public abstract class RemoteAuthenticatedUser implements AuthenticatedUser {
*/ */
private final String remoteHost; 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. 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.
*
* @return
* The remote host from which the user with the given credentials is
* 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();
}
/** /**
* Creates a new RemoteAuthenticatedUser, deriving the associated remote * Creates a new RemoteAuthenticatedUser, deriving the associated remote
* host from the given credentials. * host from the given credentials.
@@ -113,7 +57,7 @@ public abstract class RemoteAuthenticatedUser implements AuthenticatedUser {
Credentials credentials) { Credentials credentials) {
this.authenticationProvider = authenticationProvider; this.authenticationProvider = authenticationProvider;
this.credentials = credentials; this.credentials = credentials;
this.remoteHost = getRemoteHost(credentials); this.remoteHost = credentials.getRemoteAddress();
} }
@Override @Override

View File

@@ -72,8 +72,41 @@ public class Credentials implements Serializable {
*/ */
private transient HttpSession session; private transient HttpSession session;
/**
* Construct a Credentials object with the given username, password,
* and HTTP request. The information is assigned to the various
* storage objects, and the remote hostname and address is parsed out
* of the request object.
*
* @param username
* The username that was provided for authentication.
*
* @param password
* The password that was provided for authentication.
*
* @param request
* The HTTP request associated with the authentication
* request.
*/
public Credentials(String username, String password, HttpServletRequest request) {
this.username = username;
this.password = password;
this.request = request;
// Set the remote address
this.remoteAddress = request.getRemoteAddr();
// Get the remote hostname
this.remoteHostname = request.getRemoteHost();
// If session exists get it, but don't create a new one.
this.session = request.getSession(false);
}
/** /**
* Returns the password associated with this set of credentials. * Returns the password associated with this set of credentials.
*
* @return The password associated with this username/password pair, or * @return The password associated with this username/password pair, or
* null if no password has been set. * null if no password has been set.
*/ */
@@ -83,6 +116,7 @@ public class Credentials implements Serializable {
/** /**
* Sets the password associated with this set of credentials. * Sets the password associated with this set of credentials.
*
* @param password The password to associate with this username/password * @param password The password to associate with this username/password
* pair. * pair.
*/ */
@@ -92,6 +126,7 @@ public class Credentials implements Serializable {
/** /**
* Returns the username associated with this set of credentials. * Returns the username associated with this set of credentials.
*
* @return The username associated with this username/password pair, or * @return The username associated with this username/password pair, or
* null if no username has been set. * null if no username has been set.
*/ */
@@ -101,6 +136,7 @@ public class Credentials implements Serializable {
/** /**
* Sets the username associated with this set of credentials. * Sets the username associated with this set of credentials.
*
* @param username The username to associate with this username/password * @param username The username to associate with this username/password
* pair. * pair.
*/ */
@@ -110,6 +146,7 @@ public class Credentials implements Serializable {
/** /**
* Returns the HttpServletRequest associated with this set of credentials. * Returns the HttpServletRequest associated with this set of credentials.
*
* @return The HttpServletRequest associated with this set of credentials, * @return The HttpServletRequest associated with this set of credentials,
* or null if no such request exists. * or null if no such request exists.
*/ */
@@ -119,6 +156,7 @@ public class Credentials implements Serializable {
/** /**
* Sets the HttpServletRequest associated with this set of credentials. * Sets the HttpServletRequest associated with this set of credentials.
*
* @param request The HttpServletRequest to associated with this set of * @param request The HttpServletRequest to associated with this set of
* credentials. * credentials.
*/ */
@@ -128,6 +166,7 @@ public class Credentials implements Serializable {
/** /**
* Returns the HttpSession associated with this set of credentials. * Returns the HttpSession associated with this set of credentials.
*
* @return The HttpSession associated with this set of credentials, or null * @return The HttpSession associated with this set of credentials, or null
* if no such request exists. * if no such request exists.
*/ */
@@ -137,6 +176,7 @@ public class Credentials implements Serializable {
/** /**
* Sets the HttpSession associated with this set of credentials. * Sets the HttpSession associated with this set of credentials.
*
* @param session The HttpSession to associated with this set of * @param session The HttpSession to associated with this set of
* credentials. * credentials.
*/ */

View File

@@ -117,15 +117,7 @@ public class TokenRESTService {
} // end Authorization header fallback } // end Authorization header fallback
// Build credentials // Build credentials
Credentials credentials = new Credentials(); return new Credentials(username, password, request);
credentials.setUsername(username);
credentials.setPassword(password);
credentials.setRequest(request);
credentials.setSession(request.getSession(false));
credentials.setRemoteAddress(request.getRemoteAddr());
credentials.setRemoteHostname(request.getRemoteHost());
return credentials;
} }

View File

@@ -155,13 +155,8 @@ public class UserResource
@Context HttpServletRequest request) throws GuacamoleException { @Context HttpServletRequest request) throws GuacamoleException {
// Build credentials // Build credentials
Credentials credentials = new Credentials(); Credentials credentials = new Credentials(user.getIdentifier(),
credentials.setUsername(user.getIdentifier()); userPasswordUpdate.getOldPassword(), request);
credentials.setPassword(userPasswordUpdate.getOldPassword());
credentials.setRequest(request);
credentials.setSession(request.getSession(false));
credentials.setRemoteAddress(request.getRemoteAddr());
credentials.setRemoteHostname(request.getRemoteHost());
// Verify that the old password was correct // Verify that the old password was correct
try { try {