If trusted proxies are not configured, default to trusting UI's IP address

This commit is contained in:
Tim Donohue
2021-03-09 16:03:30 -06:00
parent f2f7d127aa
commit 4016280c9e
4 changed files with 153 additions and 29 deletions

View File

@@ -13,10 +13,12 @@ import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.math.BigInteger;
import java.net.InetAddress;
import java.net.MalformedURLException;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
import java.net.UnknownHostException;
import java.rmi.dgc.VMID;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
@@ -453,6 +455,33 @@ public final class Utils {
}
}
/**
* Retrieve the IP address(es) of a given URI string
* @param uriString URI string
* @return IP address(es) in a String array (or null if not found)
*/
public static String[] getIPAddresses(String uriString) {
String[] ipAddresses = null;
// First, get the hostname
String hostname = getHostName(uriString);
if (StringUtils.isNotEmpty(hostname)) {
try {
// Then, get the list of all IPs for that hostname
InetAddress[] inetAddresses = InetAddress.getAllByName(hostname);
// Convert array of InetAddress objects to array of Strings by calling getHostAddress() for each
ipAddresses = Arrays.stream(inetAddresses).map(InetAddress::getHostAddress).toArray(String[]::new);
} catch (UnknownHostException ex) {
return null;
}
}
return ipAddresses;
}
/**
* Replaces configuration placeholders within a String with the corresponding value
* from DSpace's Configuration Service.

View File

@@ -11,6 +11,7 @@ import javax.servlet.http.HttpServletRequest;
import org.apache.commons.lang3.ArrayUtils;
import org.apache.commons.lang3.StringUtils;
import org.dspace.core.Utils;
import org.dspace.service.ClientInfoService;
import org.dspace.services.ConfigurationService;
import org.dspace.statistics.util.IPTable;
@@ -41,8 +42,7 @@ public class ClientInfoServiceImpl implements ClientInfoService {
@Autowired(required = true)
public ClientInfoServiceImpl(ConfigurationService configurationService) {
this.configurationService = configurationService;
this.trustedProxies = parseTrustedProxyRanges(
configurationService.getArrayProperty("proxies.trusted.ipranges"));
this.trustedProxies = parseTrustedProxyRanges();
}
@Override
@@ -74,40 +74,86 @@ public class ClientInfoServiceImpl implements ClientInfoService {
public boolean isUseProxiesEnabled() {
if (useProxiesEnabled == null) {
useProxiesEnabled = configurationService.getBooleanProperty("useProxies", true);
log.info("useProxies=" + useProxiesEnabled);
log.info("Proxies (useProxies) enabled? " + useProxiesEnabled);
}
return useProxiesEnabled;
}
private IPTable parseTrustedProxyRanges(String[] proxyProperty) {
if (ArrayUtils.isEmpty(proxyProperty)) {
return null;
} else {
//Load all supplied proxy IP ranges into the IP table
IPTable ipTable = new IPTable();
/**
* Parse / Determine trusted proxies based on configuration. "Trusted" proxies are the IP addresses from which we'll
* allow the X-FORWARDED-FOR header. We don't accept that header from any IP address, as the header could be used
* to spoof/fake your IP address.
* <P>
* If "proxies.trusted.ipranges" configuration is specified, we trust ONLY those IP addresses.
* <P>
* If "proxies.trusted.ipranges" is UNSPECIFIED, we only trust the IP address(es) associated with ${dspace.ui.url}.
* This is necessary to allow the Angular UI server-side rendering (SSR) to send us the X-FORWARDED-FOR header,
* which it usually uses to specify the original client IP address.
* @return IPTable of trusted IP addresses/ranges, or null if none could be found.
*/
private IPTable parseTrustedProxyRanges() {
IPTable ipTable = null;
// Whether we had to look up the UI's IP address (based on its URL), or not
boolean uiUrlLookup = false;
String[] ipAddresses = configurationService.getArrayProperty("proxies.trusted.ipranges");
String uiUrl = configurationService.getProperty("dspace.ui.url");
// If configuration is empty, determine IPs of ${dspace.ui.url} as the default trusted proxy
if (ArrayUtils.isEmpty(ipAddresses)) {
// Get any IP address(es) associated with our UI
ipAddresses = Utils.getIPAddresses(uiUrl);
uiUrlLookup = true;
}
// Only continue if we have IPs to trust.
if (ArrayUtils.isNotEmpty(ipAddresses)) {
ipTable = new IPTable();
try {
for (String proxyRange : proxyProperty) {
ipTable.add(proxyRange);
// Load all IPs into our IP Table
for (String ipAddress : ipAddresses) {
ipTable.add(ipAddress);
}
} catch (IPTable.IPFormatException e) {
log.error("Property proxies.trusted.ipranges contains an invalid IP range", e);
if (uiUrlLookup) {
log.error("IP address found for dspace.ui.url={} was invalid", uiUrl, e);
} else {
log.error("Property 'proxies.trusted.ipranges' contains an invalid IP range", e);
}
ipTable = null;
}
return ipTable;
}
if (ipTable != null) {
log.info("Trusted proxy IP ranges/addresses: {}" + ipTable.toSet().toString());
}
return ipTable;
}
/**
* Whether a request is from a trusted proxy or not. Only returns true if trusted proxies are specified
* and the ipAddress is contained in those proxies. False in all other cases
* @param ipAddress IP address to check for
* @return true if trusted, false otherwise
*/
private boolean isRequestFromTrustedProxy(String ipAddress) {
try {
return trustedProxies == null || trustedProxies.contains(ipAddress);
return trustedProxies != null && trustedProxies.contains(ipAddress);
} catch (IPTable.IPFormatException e) {
log.error("Request contains invalid remote address", e);
return false;
}
}
/**
* Get the first X-FORWARDED-FOR header value which does not match the IP or another proxy IP. This is the most
* likely client IP address when proxies are in use.
* @param remoteIp remote IP address
* @param xForwardedForValue X-FORWARDED-FOR header value passed by that address
* @return likely client IP address from X-FORWARDED-FOR header
*/
private String getXForwardedForIpValue(String remoteIp, String xForwardedForValue) {
String ip = null;
@@ -119,9 +165,8 @@ public class ClientInfoServiceImpl implements ClientInfoService {
not equal to the proxy
*/
if (!StringUtils.equals(remoteIp, xfip) && StringUtils.isNotBlank(xfip)
//if we have trusted proxies, we'll assume that they are not the client IP
&& (trustedProxies == null || !isRequestFromTrustedProxy(xfip))) {
// if we have trusted proxies, we'll assume that they are not the client IP
&& !isRequestFromTrustedProxy(xfip)) {
ip = xfip.trim();
}
}

View File

@@ -7,13 +7,19 @@
*/
package org.dspace.core;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.mockito.Mockito.mockStatic;
import java.net.InetAddress;
import java.net.UnknownHostException;
import org.dspace.AbstractUnitTest;
import org.dspace.services.ConfigurationService;
import org.dspace.services.factory.DSpaceServicesFactory;
import org.junit.Test;
import org.mockito.MockedStatic;
/**
* Perform some basic unit tests for Utils Class
@@ -73,6 +79,31 @@ public class UtilsTest extends AbstractUnitTest {
assertNull("Test invalid URI returns null", Utils.getHostName("&+,?/@="));
}
/**
* Test of getIPAddresses method, of class Utils
*/
@Test
public void testGetIPAddresses() throws UnknownHostException {
// Fake a URL & two fake corresponding IP addresses as an InetAddress
String fakeUrl = "https://dspace.org";
String fakeHostname = "dspace.org";
InetAddress[] fakeInetAddresses =
new InetAddress[] { InetAddress.getByName("1.2.3.4"), InetAddress.getByName("5.6.7.8") };
// Mock responses from InetAddress
try (MockedStatic<InetAddress> mockedInetAddress = mockStatic(InetAddress.class)) {
// When fakeHostname is passed to InetAddress, return fakeInetAddresses
mockedInetAddress.when(() -> InetAddress.getAllByName(fakeHostname)).thenReturn(fakeInetAddresses);
assertNull("Test invalid URL returns null",
Utils.getIPAddresses("not/a-real;url"));
assertArrayEquals("Test fake URL returns fake IPs converted to String Array",
new String[] {"1.2.3.4", "5.6.7.8"},
Utils.getIPAddresses(fakeUrl));
}
}
/**
* Test of interpolateConfigsInString method, of class Utils
*/

View File

@@ -10,14 +10,17 @@ package org.dspace.service.impl;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.mockStatic;
import org.dspace.AbstractDSpaceTest;
import org.dspace.core.Utils;
import org.dspace.service.ClientInfoService;
import org.dspace.services.ConfigurationService;
import org.dspace.services.factory.DSpaceServicesFactory;
import org.dspace.statistics.util.DummyHttpServletRequest;
import org.junit.Before;
import org.junit.Test;
import org.mockito.MockedStatic;
/**
* Unit test class for the {@link ClientInfoServiceImpl} class which implements
@@ -100,26 +103,42 @@ public class ClientInfoServiceImplTest extends AbstractDSpaceTest {
@Test
public void getClientIpWithoutTrustedProxies() {
// Ensure proxies are on, but no trusted proxies defined
configurationService.setProperty("useProxies", true);
configurationService.setProperty("proxies.trusted.ipranges", "");
clientInfoService = new ClientInfoServiceImpl(configurationService);
// Set a URL for our UI, and a fake IP address associated with that url
String fakeUI_URL = "https://mydspace.edu/";
String fakeUI_IP = "1.2.3.4";
configurationService.setProperty("dspace.ui.url", fakeUI_URL);
String remoteIp = "127.0.0.1";
String xForwardedFor = "10.24.64.14";
try (MockedStatic<Utils> mockedUtils = mockStatic(Utils.class)) {
// Mock an IP address for mydspace.edu (have it return 1.2.3.4 as the IP address)
mockedUtils.when(() -> Utils.getIPAddresses(fakeUI_URL))
.thenReturn(new String[]{fakeUI_IP});
assertEquals("10.24.64.14",
clientInfoService.getClientIp(remoteIp, xForwardedFor));
ClientInfoService clientInfoServiceMock = new ClientInfoServiceImpl(configurationService);
xForwardedFor = "127.0.0.1,10.24.64.14";
// Define a fake X-FORWARDED-FOR value returned by our UI
String xForwardedFor = "10.24.64.14";
assertEquals("10.24.64.14",
clientInfoService.getClientIp(remoteIp, xForwardedFor));
// Verify our UI is still a trusted proxy as its X-FORWARDED-FOR is accepted
assertEquals("10.24.64.14",
clientInfoServiceMock.getClientIp(fakeUI_IP, xForwardedFor));
xForwardedFor = "10.24.64.14,127.0.0.1";
// Verify if multiple X-FORWARDED-FOR values, the one NOT matching UI's IP is returned
xForwardedFor = "1.2.3.4,10.24.64.14";
assertEquals("10.24.64.14",
clientInfoServiceMock.getClientIp(fakeUI_IP, xForwardedFor));
xForwardedFor = "10.24.64.14,1.2.3.4";
assertEquals("10.24.64.14",
clientInfoServiceMock.getClientIp(fakeUI_IP, xForwardedFor));
}
assertEquals("10.24.64.14",
clientInfoService.getClientIp(remoteIp, xForwardedFor));
}
@Test