This is an automated email from the ASF dual-hosted git repository.
rohit pushed a commit to branch 4.19
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit 67e2061f4b87934c863cbafc5031821ad7ef6d02
Author: dahn <d...@onecht.net>
AuthorDate: Tue Mar 26 07:13:23 2024 +0100
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.
(cherry picked from commit b65546636d84a5790e0297b1b0ca8e5a67a48dbc)
Signed-off-by: Rohit Yadav <rohit.ya...@shapeblue.com>
(cherry picked from commit b1e0bf9dbd464f8fb7c22f36505dee0148e2d6f4)
Signed-off-by: Rohit Yadav <rohit.ya...@shapeblue.com>
---
.../java/org/apache/cloudstack/ServerDaemon.java | 3 +-
.../cloudstack/framework/config/ConfigKey.java | 1 +
server/src/main/java/com/cloud/api/ApiServer.java | 40 ++++++++++++++++-----
server/src/main/java/com/cloud/api/ApiServlet.java | 41 +++++++++++++++-------
.../test/java/com/cloud/api/ApiServletTest.java | 29 ++++++++++-----
.../src/main/java/com/cloud/utils/StringUtils.java | 2 +-
6 files changed, 85 insertions(+), 31 deletions(-)
diff --git a/client/src/main/java/org/apache/cloudstack/ServerDaemon.java
b/client/src/main/java/org/apache/cloudstack/ServerDaemon.java
index 763c274c7f5..fb84e1297e6 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.DaemonContext;
import org.apache.commons.lang3.StringUtils;
import org.apache.log4j.Logger;
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;
@@ -173,7 +172,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 192c556bd2a..df93f78fa83 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<T> {
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 b602ed2edbc..76a436f74d4 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<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 @@ public class ApiServer extends ManagerBase implements
HttpRequestHandler, ApiSer
, 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 @@ 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<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 @@ 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(ip);
+ }
- return InetAddress.getByName(request.getRemoteAddr());
+ 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 cefbbe2d630..4d4f0a12098 100644
--- a/server/src/test/java/com/cloud/api/ApiServletTest.java
+++ b/server/src/test/java/com/cloud/api/ApiServletTest.java
@@ -97,15 +97,17 @@ public class ApiServletTest {
@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));
@@ -259,32 +261,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 fd4f7aba698..01b6f833271 100644
--- a/utils/src/main/java/com/cloud/utils/StringUtils.java
+++ b/utils/src/main/java/com/cloud/utils/StringUtils.java
@@ -31,7 +31,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;
