diff --git a/client/src/main/java/org/apache/cloudstack/ServerDaemon.java b/client/src/main/java/org/apache/cloudstack/ServerDaemon.java index 08f856655dc..ce927601653 100644 --- a/client/src/main/java/org/apache/cloudstack/ServerDaemon.java +++ b/client/src/main/java/org/apache/cloudstack/ServerDaemon.java @@ -31,7 +31,6 @@ import com.cloud.utils.server.ServerProperties; import org.apache.commons.daemon.Daemon; import org.apache.commons.daemon.DaemonContext; import org.eclipse.jetty.jmx.MBeanContainer; -import org.eclipse.jetty.server.ForwardedRequestCustomizer; import org.eclipse.jetty.server.HttpConfiguration; import org.eclipse.jetty.server.HttpConnectionFactory; import org.eclipse.jetty.server.NCSARequestLog; @@ -167,7 +166,7 @@ public class ServerDaemon implements Daemon { // HTTP config final HttpConfiguration httpConfig = new HttpConfiguration(); - httpConfig.addCustomizer( new ForwardedRequestCustomizer() ); +// it would be nice to make this dynamic but we take care of this ourselves for now: httpConfig.addCustomizer( new ForwardedRequestCustomizer() ); httpConfig.setSecureScheme("https"); httpConfig.setSecurePort(httpsPort); httpConfig.setOutputBufferSize(32768); diff --git a/framework/config/src/main/java/org/apache/cloudstack/framework/config/ConfigKey.java b/framework/config/src/main/java/org/apache/cloudstack/framework/config/ConfigKey.java index 13c594f9367..bd38ba92fd5 100644 --- a/framework/config/src/main/java/org/apache/cloudstack/framework/config/ConfigKey.java +++ b/framework/config/src/main/java/org/apache/cloudstack/framework/config/ConfigKey.java @@ -34,6 +34,7 @@ public class ConfigKey { public static final String CATEGORY_ADVANCED = "Advanced"; public static final String CATEGORY_ALERT = "Alert"; + public static final String CATEGORY_NETWORK = "Network"; public enum Scope { Global, Zone, Cluster, StoragePool, Account, ManagementServer, ImageStore, Domain diff --git a/server/src/main/java/com/cloud/api/ApiServer.java b/server/src/main/java/com/cloud/api/ApiServer.java index e88d7cf8b53..4a7259c6d33 100644 --- a/server/src/main/java/com/cloud/api/ApiServer.java +++ b/server/src/main/java/com/cloud/api/ApiServer.java @@ -234,42 +234,42 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer @Inject private MessageBus messageBus; - private static final ConfigKey IntegrationAPIPort = new ConfigKey("Advanced" + private static final ConfigKey IntegrationAPIPort = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED , Integer.class , "integration.api.port" , "0" , "Integration (unauthenticated) API port. To disable set it to 0 or negative." , false , ConfigKey.Scope.Global); - private static final ConfigKey ConcurrentSnapshotsThresholdPerHost = new ConfigKey("Advanced" + private static final ConfigKey ConcurrentSnapshotsThresholdPerHost = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED , Long.class , "concurrent.snapshots.threshold.perhost" , null , "Limits number of snapshots that can be handled by the host concurrently; default is NULL - unlimited" , true // not sure if this is to be dynamic , ConfigKey.Scope.Global); - private static final ConfigKey EncodeApiResponse = new ConfigKey("Advanced" + private static final ConfigKey EncodeApiResponse = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED , Boolean.class , "encode.api.response" , "false" , "Do URL encoding for the api response, false by default" , false , ConfigKey.Scope.Global); - static final ConfigKey JSONcontentType = new ConfigKey( "Advanced" + static final ConfigKey JSONcontentType = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED , String.class , "json.content.type" , "application/json; charset=UTF-8" , "Http response content type for .js files (default is text/javascript)" , false , ConfigKey.Scope.Global); - static final ConfigKey EnableSecureSessionCookie = new ConfigKey("Advanced" + static final ConfigKey EnableSecureSessionCookie = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED , Boolean.class , "enable.secure.session.cookie" , "false" , "Session cookie is marked as secure if this is enabled. Secure cookies only work when HTTPS is used." , false , ConfigKey.Scope.Global); - private static final ConfigKey JSONDefaultContentType = new ConfigKey ("Advanced" + private static final ConfigKey JSONDefaultContentType = new ConfigKey<> (ConfigKey.CATEGORY_ADVANCED , String.class , "json.content.type" , "application/json; charset=UTF-8" @@ -277,13 +277,34 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer , false , ConfigKey.Scope.Global); - private static final ConfigKey UseEventAccountInfo = new ConfigKey( "advanced" + private static final ConfigKey UseEventAccountInfo = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED , Boolean.class , "event.accountinfo" , "false" , "use account info in event logging" , true , ConfigKey.Scope.Global); + static final ConfigKey useForwardHeader = new ConfigKey<>(ConfigKey.CATEGORY_NETWORK + , Boolean.class + , "proxy.header.verify" + , "false" + , "enables/disables checking of ipaddresses from a proxy set header. See \"proxy.header.names\" for the headers to allow." + , true + , ConfigKey.Scope.Global); + static final ConfigKey listOfForwardHeaders = new ConfigKey<>(ConfigKey.CATEGORY_NETWORK + , String.class + , "proxy.header.names" + , "X-Forwarded-For,HTTP_CLIENT_IP,HTTP_X_FORWARDED_FOR" + , "a list of names to check for allowed ipaddresses from a proxy set header. See \"proxy.cidr\" for the proxies allowed to set these headers." + , true + , ConfigKey.Scope.Global); + static final ConfigKey proxyForwardList = new ConfigKey<>(ConfigKey.CATEGORY_NETWORK + , String.class + , "proxy.cidr" + , "" + , "a list of cidrs for which \"proxy.header.names\" are honoured if the \"Remote_Addr\" is in this list." + , true + , ConfigKey.Scope.Global); @Override public boolean configure(final String name, final Map params) throws ConfigurationException { @@ -1499,7 +1520,10 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer ConcurrentSnapshotsThresholdPerHost, EncodeApiResponse, EnableSecureSessionCookie, - JSONDefaultContentType + JSONDefaultContentType, + proxyForwardList, + useForwardHeader, + listOfForwardHeaders }; } } diff --git a/server/src/main/java/com/cloud/api/ApiServlet.java b/server/src/main/java/com/cloud/api/ApiServlet.java index 626678649d7..f6f46419c04 100644 --- a/server/src/main/java/com/cloud/api/ApiServlet.java +++ b/server/src/main/java/com/cloud/api/ApiServlet.java @@ -21,7 +21,6 @@ import java.net.InetAddress; import java.net.URLDecoder; import java.net.UnknownHostException; import java.util.Arrays; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -69,13 +68,9 @@ import com.cloud.utils.db.EntityManager; import com.cloud.utils.net.NetUtils; @Component("apiServlet") -@SuppressWarnings("serial") public class ApiServlet extends HttpServlet { public static final Logger s_logger = Logger.getLogger(ApiServlet.class.getName()); private static final Logger s_accessLogger = Logger.getLogger("apiserver." + ApiServlet.class.getName()); - private final static List s_clientAddressHeaders = Collections - .unmodifiableList(Arrays.asList("X-Forwarded-For", - "HTTP_CLIENT_IP", "HTTP_X_FORWARDED_FOR", "Remote_Addr")); private static final String REPLACEMENT = "_"; private static final String LOG_REPLACEMENTS = "[\n\r\t]"; @@ -570,17 +565,39 @@ public class ApiServlet extends HttpServlet { } return false; } + boolean doUseForwardHeaders() { + return Boolean.TRUE.equals(ApiServer.useForwardHeader.value()); + } + String[] proxyNets() { + return ApiServer.proxyForwardList.value().split(","); + } //This method will try to get login IP of user even if servlet is behind reverseProxy or loadBalancer - public static InetAddress getClientAddress(final HttpServletRequest request) throws UnknownHostException { - for(final String header : s_clientAddressHeaders) { - final String ip = getCorrectIPAddress(request.getHeader(header)); - if (ip != null) { - return InetAddress.getByName(ip); - } + public InetAddress getClientAddress(final HttpServletRequest request) throws UnknownHostException { + String ip = null; + InetAddress pretender = InetAddress.getByName(request.getRemoteAddr()); + if(doUseForwardHeaders()) { + if (NetUtils.isIpInCidrList(pretender, proxyNets())) { + for (String header : getClientAddressHeaders()) { + header = header.trim(); + ip = getCorrectIPAddress(request.getHeader(header)); + if (StringUtils.isNotBlank(ip)) { + s_logger.debug(String.format("found ip %s in header %s ", ip, header)); + break; + } + } // no address found in header so ip is blank and use remote addr + } // else not an allowed proxy address, ip is blank and use remote addr + } + if (StringUtils.isBlank(ip)) { + s_logger.trace(String.format("no ip found in headers, returning remote address %s.", pretender.getHostAddress())); + return pretender; } - return InetAddress.getByName(request.getRemoteAddr()); + return InetAddress.getByName(ip); + } + + private String[] getClientAddressHeaders() { + return ApiServer.listOfForwardHeaders.value().split(","); } private static String getCorrectIPAddress(String ip) { diff --git a/server/src/test/java/com/cloud/api/ApiServletTest.java b/server/src/test/java/com/cloud/api/ApiServletTest.java index ce1f009d3e8..250d2b06d97 100644 --- a/server/src/test/java/com/cloud/api/ApiServletTest.java +++ b/server/src/test/java/com/cloud/api/ApiServletTest.java @@ -99,15 +99,17 @@ public class ApiServletTest { @Mock AccountService accountMgr; + @Mock ConfigKey useForwardHeader; StringWriter responseWriter; ApiServlet servlet; - + ApiServlet spyServlet; @SuppressWarnings("unchecked") @Before public void setup() throws SecurityException, NoSuchFieldException, IllegalArgumentException, IllegalAccessException, IOException, UnknownHostException { servlet = new ApiServlet(); + spyServlet = Mockito.spy(servlet); responseWriter = new StringWriter(); Mockito.when(response.getWriter()).thenReturn( new PrintWriter(responseWriter)); @@ -261,32 +263,43 @@ public class ApiServletTest { @Test public void getClientAddressWithXForwardedFor() throws UnknownHostException { + String[] proxynet = {"127.0.0.0/8"}; + Mockito.when(spyServlet.proxyNets()).thenReturn(proxynet); + Mockito.when(spyServlet.doUseForwardHeaders()).thenReturn(true); Mockito.when(request.getHeader(Mockito.eq("X-Forwarded-For"))).thenReturn("192.168.1.1"); - Assert.assertEquals(InetAddress.getByName("192.168.1.1"), ApiServlet.getClientAddress(request)); + Assert.assertEquals(InetAddress.getByName("192.168.1.1"), spyServlet.getClientAddress(request)); } @Test public void getClientAddressWithHttpXForwardedFor() throws UnknownHostException { + String[] proxynet = {"127.0.0.0/8"}; + Mockito.when(spyServlet.proxyNets()).thenReturn(proxynet); + Mockito.when(spyServlet.doUseForwardHeaders()).thenReturn(true); Mockito.when(request.getHeader(Mockito.eq("HTTP_X_FORWARDED_FOR"))).thenReturn("192.168.1.1"); - Assert.assertEquals(InetAddress.getByName("192.168.1.1"), ApiServlet.getClientAddress(request)); + Assert.assertEquals(InetAddress.getByName("192.168.1.1"), spyServlet.getClientAddress(request)); } @Test - public void getClientAddressWithXRemoteAddr() throws UnknownHostException { - Mockito.when(request.getHeader(Mockito.eq("Remote_Addr"))).thenReturn("192.168.1.1"); - Assert.assertEquals(InetAddress.getByName("192.168.1.1"), ApiServlet.getClientAddress(request)); + public void getClientAddressWithRemoteAddr() throws UnknownHostException { + String[] proxynet = {"127.0.0.0/8"}; + Mockito.when(spyServlet.proxyNets()).thenReturn(proxynet); + Mockito.when(spyServlet.doUseForwardHeaders()).thenReturn(true); + Assert.assertEquals(InetAddress.getByName("127.0.0.1"), spyServlet.getClientAddress(request)); } @Test public void getClientAddressWithHttpClientIp() throws UnknownHostException { + String[] proxynet = {"127.0.0.0/8"}; + Mockito.when(spyServlet.proxyNets()).thenReturn(proxynet); + Mockito.when(spyServlet.doUseForwardHeaders()).thenReturn(true); Mockito.when(request.getHeader(Mockito.eq("HTTP_CLIENT_IP"))).thenReturn("192.168.1.1"); - Assert.assertEquals(InetAddress.getByName("192.168.1.1"), ApiServlet.getClientAddress(request)); + Assert.assertEquals(InetAddress.getByName("192.168.1.1"), spyServlet.getClientAddress(request)); } @Test public void getClientAddressDefault() throws UnknownHostException { Mockito.when(request.getRemoteAddr()).thenReturn("127.0.0.1"); - Assert.assertEquals(InetAddress.getByName("127.0.0.1"), ApiServlet.getClientAddress(request)); + Assert.assertEquals(InetAddress.getByName("127.0.0.1"), spyServlet.getClientAddress(request)); } @Test diff --git a/utils/src/main/java/com/cloud/utils/StringUtils.java b/utils/src/main/java/com/cloud/utils/StringUtils.java index 9e197a8a94b..93e66ecfc02 100644 --- a/utils/src/main/java/com/cloud/utils/StringUtils.java +++ b/utils/src/main/java/com/cloud/utils/StringUtils.java @@ -27,7 +27,7 @@ import java.util.Map; import java.util.regex.Matcher; import java.util.regex.Pattern; -public class StringUtils { +public class StringUtils extends org.apache.commons.lang3.StringUtils { private static final char[] hexChar = {'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C', 'D', 'E', 'F'}; private static Charset preferredACSCharset;