api: client verification in servlet
This introduces new global settings to handle how client address checks
are handled by the API layer:
proxy.header.verify: enables/disables checking of ipaddresses from a
proxy set header
proxy.header.names: a list of names to check for allowed ipaddresses
from a proxy set header.
proxy.cidr: a list of cidrs for which \"proxy.header.names\" are
honoured if the \"Remote_Addr\" is in this list.diff --git a/client/src/main/java/org/apache/cloudstack/ServerDaemon.java b/client/src/main/java/org/apache/cloudstack/ServerDaemon.java
index 08f8566..ce92760 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 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 @@
// 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 13c594f..bd38ba9 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 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 e88d7cf..4a7259c 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 @@
@Inject
private MessageBus messageBus;
- private static final ConfigKey<Integer> IntegrationAPIPort = new ConfigKey<Integer>("Advanced"
+ private static final ConfigKey<Integer> 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<Long> ConcurrentSnapshotsThresholdPerHost = new ConfigKey<Long>("Advanced"
+ private static final ConfigKey<Long> 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<Boolean> EncodeApiResponse = new ConfigKey<Boolean>("Advanced"
+ private static final ConfigKey<Boolean> 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<String> JSONcontentType = new ConfigKey<String>( "Advanced"
+ static final ConfigKey<String> 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<Boolean> EnableSecureSessionCookie = new ConfigKey<Boolean>("Advanced"
+ static final ConfigKey<Boolean> 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<String> JSONDefaultContentType = new ConfigKey<String> ("Advanced"
+ private static final ConfigKey<String> JSONDefaultContentType = new ConfigKey<> (ConfigKey.CATEGORY_ADVANCED
, String.class
, "json.content.type"
, "application/json; charset=UTF-8"
@@ -277,13 +277,34 @@
, false
, ConfigKey.Scope.Global);
- private static final ConfigKey<Boolean> UseEventAccountInfo = new ConfigKey<Boolean>( "advanced"
+ private static final ConfigKey<Boolean> UseEventAccountInfo = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED
, Boolean.class
, "event.accountinfo"
, "false"
, "use account info in event logging"
, true
, ConfigKey.Scope.Global);
+ static final ConfigKey<Boolean> 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<String> 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<String> 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<String, Object> params) throws ConfigurationException {
@@ -1499,7 +1520,10 @@
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 6266786..f6f4641 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.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.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<String> 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 @@
}
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 ce1f009..250d2b0 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 @@
@Mock
AccountService accountMgr;
+ @Mock ConfigKey<Boolean> 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 @@
@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 9e197a8..93e66ec 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.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;
