Fix logging of forwarded IPs in logs (#11854)

This commit is contained in:
Abhisar Sinha 2026-01-27 18:01:02 +05:30 committed by GitHub
parent 5fd5b62570
commit 2056c582f1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 69 additions and 62 deletions

View File

@ -18,6 +18,7 @@
//
package org.apache.cloudstack;
import com.cloud.api.ApiServlet;
import com.cloud.utils.StringUtils;
import org.eclipse.jetty.server.NCSARequestLog;
import org.eclipse.jetty.server.Request;
@ -25,6 +26,7 @@ import org.eclipse.jetty.server.Response;
import org.eclipse.jetty.util.DateCache;
import org.eclipse.jetty.util.component.LifeCycle;
import java.net.InetAddress;
import java.util.Locale;
import java.util.TimeZone;
@ -51,9 +53,8 @@ public class ACSRequestLog extends NCSARequestLog {
StringBuilder sb = buffers.get();
sb.setLength(0);
sb.append(request.getHttpChannel().getEndPoint()
.getRemoteAddress().getAddress()
.getHostAddress())
InetAddress remoteAddress = ApiServlet.getClientAddress(request);
sb.append(remoteAddress.getHostAddress())
.append(" - - [")
.append(dateCache.format(request.getTimeStamp()))
.append("] \"")

View File

@ -24,15 +24,12 @@ import java.io.IOException;
import java.io.InputStream;
import java.lang.management.ManagementFactory;
import java.net.URL;
import java.util.Arrays;
import java.util.Properties;
import com.cloud.api.ApiServer;
import org.apache.commons.daemon.Daemon;
import org.apache.commons.daemon.DaemonContext;
import org.apache.commons.lang3.StringUtils;
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.RequestLog;
@ -193,7 +190,6 @@ public class ServerDaemon implements Daemon {
httpConfig.setResponseHeaderSize(8192);
httpConfig.setSendServerVersion(false);
httpConfig.setSendDateHeader(false);
addForwardingCustomiser(httpConfig);
// HTTP Connector
createHttpConnector(httpConfig);
@ -216,21 +212,6 @@ public class ServerDaemon implements Daemon {
server.join();
}
/**
* Adds a ForwardedRequestCustomizer to the HTTP configuration to handle forwarded headers.
* The header used for forwarding is determined by the ApiServer.listOfForwardHeaders property.
* Only non empty headers are considered and only the first of the comma-separated list is used.
* @param httpConfig the HTTP configuration to which the customizer will be added
*/
private static void addForwardingCustomiser(HttpConfiguration httpConfig) {
ForwardedRequestCustomizer customiser = new ForwardedRequestCustomizer();
String header = Arrays.stream(ApiServer.listOfForwardHeaders.value().split(",")).findFirst().orElse(null);
if (com.cloud.utils.StringUtils.isNotEmpty(header)) {
customiser.setForwardedForHeader(header);
}
httpConfig.addCustomizer(customiser);
}
@Override
public void stop() throws Exception {
server.stop();

View File

@ -679,38 +679,34 @@ public class ApiServlet extends HttpServlet {
}
return false;
}
boolean doUseForwardHeaders() {
static boolean doUseForwardHeaders() {
return Boolean.TRUE.equals(ApiServer.useForwardHeader.value());
}
String[] proxyNets() {
static 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 InetAddress getClientAddress(final HttpServletRequest request) throws UnknownHostException {
String ip = null;
InetAddress pretender = InetAddress.getByName(request.getRemoteAddr());
if(doUseForwardHeaders()) {
if (NetUtils.isIpInCidrList(pretender, proxyNets())) {
public static InetAddress getClientAddress(final HttpServletRequest request) throws UnknownHostException {
final String remote = request.getRemoteAddr();
if (doUseForwardHeaders()) {
final InetAddress remoteAddr = InetAddress.getByName(remote);
if (NetUtils.isIpInCidrList(remoteAddr, proxyNets())) {
for (String header : getClientAddressHeaders()) {
header = header.trim();
ip = getCorrectIPAddress(request.getHeader(header));
final String ip = getCorrectIPAddress(request.getHeader(header));
if (StringUtils.isNotBlank(ip)) {
LOGGER.debug(String.format("found ip %s in header %s ", ip, header));
break;
LOGGER.debug("found ip {} in header {}", ip, header);
return InetAddress.getByName(ip);
}
} // 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)) {
LOGGER.trace(String.format("no ip found in headers, returning remote address %s.", pretender.getHostAddress()));
return pretender;
}
return InetAddress.getByName(ip);
LOGGER.trace("no ip found in headers, returning remote address {}.", remote);
return InetAddress.getByName(remote);
}
private String[] getClientAddressHeaders() {
private static String[] getClientAddressHeaders() {
return ApiServer.listOfForwardHeaders.value().split(",");
}

View File

@ -33,6 +33,7 @@ import org.apache.cloudstack.api.auth.APIAuthenticationType;
import org.apache.cloudstack.api.auth.APIAuthenticator;
import org.apache.cloudstack.api.command.admin.config.ListCfgsByCmd;
import org.apache.cloudstack.framework.config.ConfigKey;
import org.apache.cloudstack.framework.config.impl.ConfigDepotImpl;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
@ -97,17 +98,20 @@ public class ApiServletTest {
@Mock
AccountService accountMgr;
@Mock ConfigKey<Boolean> useForwardHeader;
@Mock
ConfigDepotImpl mockConfigDepot;
StringWriter responseWriter;
ApiServlet servlet;
ApiServlet spyServlet;
private ConfigDepotImpl originalConfigDepot;
@SuppressWarnings("unchecked")
@Before
public void setup() throws SecurityException, NoSuchFieldException,
IllegalArgumentException, IllegalAccessException, IOException, UnknownHostException {
IllegalArgumentException, IllegalAccessException, IOException {
servlet = new ApiServlet();
spyServlet = Mockito.spy(servlet);
responseWriter = new StringWriter();
Mockito.when(response.getWriter()).thenReturn(
new PrintWriter(responseWriter));
@ -131,6 +135,7 @@ public class ApiServletTest {
apiServerField.setAccessible(true);
apiServerField.set(servlet, apiServer);
setupConfigDepotMock();
}
/**
@ -151,6 +156,33 @@ public class ApiServletTest {
Field smsField = ApiDBUtils.class.getDeclaredField("s_ms");
smsField.setAccessible(true);
smsField.set(null, null);
restoreConfigDepot();
}
private void setupConfigDepotMock() throws NoSuchFieldException, IllegalAccessException {
Field depotField = ConfigKey.class.getDeclaredField("s_depot");
depotField.setAccessible(true);
originalConfigDepot = (ConfigDepotImpl) depotField.get(null);
depotField.set(null, mockConfigDepot);
Mockito.when(mockConfigDepot.getConfigStringValue(
Mockito.anyString(),
Mockito.any(ConfigKey.Scope.class),
Mockito.any()
)).thenReturn(null);
}
private void restoreConfigDepot() throws Exception {
Field depotField = ConfigKey.class.getDeclaredField("s_depot");
depotField.setAccessible(true);
depotField.set(null, originalConfigDepot);
}
private void setConfigValue(String configName, String value) {
Mockito.when(mockConfigDepot.getConfigStringValue(
Mockito.eq(configName),
Mockito.eq(ConfigKey.Scope.Global),
Mockito.isNull()
)).thenReturn(value);
}
@Test
@ -261,43 +293,40 @@ 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);
setConfigValue("proxy.header.verify", "true");
setConfigValue("proxy.cidr", "127.0.0.0/8");
Mockito.when(request.getRemoteAddr()).thenReturn("127.0.0.1");
Mockito.when(request.getHeader(Mockito.eq("X-Forwarded-For"))).thenReturn("192.168.1.1");
Assert.assertEquals(InetAddress.getByName("192.168.1.1"), spyServlet.getClientAddress(request));
Assert.assertEquals(InetAddress.getByName("192.168.1.1"), ApiServlet.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);
setConfigValue("proxy.header.verify", "true");
setConfigValue("proxy.cidr", "127.0.0.0/8");
Mockito.when(request.getHeader(Mockito.eq("HTTP_X_FORWARDED_FOR"))).thenReturn("192.168.1.1");
Assert.assertEquals(InetAddress.getByName("192.168.1.1"), spyServlet.getClientAddress(request));
Assert.assertEquals(InetAddress.getByName("192.168.1.1"), ApiServlet.getClientAddress(request));
}
@Test
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));
setConfigValue("proxy.header.verify", "true");
setConfigValue("proxy.cidr", "127.0.0.0/8");
Assert.assertEquals(InetAddress.getByName("127.0.0.1"), ApiServlet.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);
setConfigValue("proxy.header.verify", "true");
setConfigValue("proxy.cidr", "127.0.0.0/8");
Mockito.when(request.getHeader(Mockito.eq("HTTP_CLIENT_IP"))).thenReturn("192.168.1.1");
Assert.assertEquals(InetAddress.getByName("192.168.1.1"), spyServlet.getClientAddress(request));
Assert.assertEquals(InetAddress.getByName("192.168.1.1"), ApiServlet.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"), spyServlet.getClientAddress(request));
Assert.assertEquals(InetAddress.getByName("127.0.0.1"), ApiServlet.getClientAddress(request));
}
@Test