Skip to content

Commit

Permalink
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=57665
Browse files Browse the repository at this point in the history 
Implement X-Forwarded-Host support for RemoteIpFilter and RemoteIpValve
  • Loading branch information
@markt-asf
markt-asf committed Jul 30, 2019
1 parent 425b037 commit 67c3af9
Show file tree
Hide file tree
Showing 11 changed files with 446 additions and 29 deletions.
15 changes: 11 additions & 4 deletions java/org/apache/catalina/AccessLog.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,28 +38,35 @@ public interface AccessLog {
* the AccessLog.
*/
public static final String REMOTE_ADDR_ATTRIBUTE =
"org.apache.catalina.AccessLog.RemoteAddr";
"org.apache.catalina.AccessLog.RemoteAddr";

/**
* Name of request attribute used to override remote host name recorded by
* the AccessLog.
*/
public static final String REMOTE_HOST_ATTRIBUTE =
"org.apache.catalina.AccessLog.RemoteHost";
"org.apache.catalina.AccessLog.RemoteHost";

/**
* Name of request attribute used to override the protocol recorded by the
* AccessLog.
*/
public static final String PROTOCOL_ATTRIBUTE =
"org.apache.catalina.AccessLog.Protocol";
"org.apache.catalina.AccessLog.Protocol";

/**
* Name of request attribute used to override the server name recorded by
* the AccessLog.
*/
public static final String SERVER_NAME_ATTRIBUTE =
"org.apache.catalina.AccessLog.ServerName";

/**
* Name of request attribute used to override the server port recorded by
* the AccessLog.
*/
public static final String SERVER_PORT_ATTRIBUTE =
"org.apache.catalina.AccessLog.ServerPort";
"org.apache.catalina.AccessLog.ServerPort";


/**
Expand Down
2 changes: 2 additions & 0 deletions java/org/apache/catalina/filters/LocalStrings.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ httpHeaderSecurityFilter.committed=Unable to add HTTP headers since response is
remoteCidrFilter.invalid=Invalid configuration provided for [{0}]. See previous messages for details.
remoteCidrFilter.noRemoteIp=Client does not have an IP address. Request denied.

remoteIpFilter.invalidHostHeader=Invalid value [{0}] found for Host in HTTP header [{1}]
remoteIpFilter.invalidHostWithPort=Host value [{0}] in HTTP header [{1}] included a port number which will be ignored
remoteIpFilter.invalidNumber=Illegal number for parameter [{0}]: [{1}]

requestFilter.deny=Denied request for [{0}] based on property [{1}]
Expand Down
113 changes: 105 additions & 8 deletions java/org/apache/catalina/filters/RemoteIpFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
import org.apache.tomcat.util.http.FastHttpDateFormat;
import org.apache.tomcat.util.http.parser.Host;
import org.apache.tomcat.util.res.StringManager;

/**
Expand Down Expand Up @@ -448,6 +449,8 @@ public static class XForwardedRequest extends HttpServletRequestWrapper {

protected final Map<String, List<String>> headers;

protected String localName;

protected int localPort;

protected String remoteAddr;
Expand All @@ -458,15 +461,19 @@ public static class XForwardedRequest extends HttpServletRequestWrapper {

protected boolean secure;

protected String serverName;

protected int serverPort;

public XForwardedRequest(HttpServletRequest request) {
super(request);
this.localName = request.getLocalName();
this.localPort = request.getLocalPort();
this.remoteAddr = request.getRemoteAddr();
this.remoteHost = request.getRemoteHost();
this.scheme = request.getScheme();
this.secure = request.isSecure();
this.serverName = request.getServerName();
this.serverPort = request.getServerPort();

headers = new HashMap<>();
Expand Down Expand Up @@ -530,6 +537,11 @@ public int getIntHeader(String name) {
return Integer.parseInt(value);
}

@Override
public String getLocalName() {
return localName;
}

@Override
public int getLocalPort() {
return localPort;
Expand All @@ -550,6 +562,11 @@ public String getScheme() {
return scheme;
}

@Override
public String getServerName() {
return serverName;
}

@Override
public int getServerPort() {
return serverPort;
Expand Down Expand Up @@ -578,6 +595,10 @@ public void setHeader(String name, String value) {

}

public void setLocalName(String localName) {
this.localName = localName;
}

public void setLocalPort(int localPort) {
this.localPort = localPort;
}
Expand All @@ -598,6 +619,10 @@ public void setSecure(boolean secure) {
this.secure = secure;
}

public void setServerName(String serverName) {
this.serverName = serverName;
}

public void setServerPort(int serverPort) {
this.serverPort = serverPort;
}
Expand Down Expand Up @@ -642,8 +667,12 @@ public PushBuilder newPushBuilder() {

protected static final String PROTOCOL_HEADER_HTTPS_VALUE_PARAMETER = "protocolHeaderHttpsValue";

protected static final String HOST_HEADER_PARAMETER = "hostHeader";

protected static final String PORT_HEADER_PARAMETER = "portHeader";

protected static final String CHANGE_LOCAL_NAME_PARAMETER = "changeLocalName";

protected static final String CHANGE_LOCAL_PORT_PARAMETER = "changeLocalPort";

protected static final String PROXIES_HEADER_PARAMETER = "proxiesHeader";
Expand Down Expand Up @@ -716,6 +745,10 @@ protected static String listToCommaDelimitedString(List<String> stringList) {

private String protocolHeaderHttpsValue = "https";

private String hostHeader = null;

private boolean changeLocalName = false;

private String portHeader = null;

private boolean changeLocalPort = false;
Expand Down Expand Up @@ -822,17 +855,37 @@ public void doFilter(HttpServletRequest request, HttpServletResponse response, F
}
}

if (hostHeader != null) {
String hostHeaderValue = request.getHeader(hostHeader);
if (hostHeaderValue != null) {
try {
int portIndex = Host.parse(hostHeaderValue);
if (portIndex > -1) {
log.debug(sm.getString("remoteIpFilter.invalidHostWithPort", hostHeaderValue, hostHeader));
hostHeaderValue = hostHeaderValue.substring(0, portIndex);
}

xRequest.setServerName(hostHeaderValue);
if (isChangeLocalName()) {
xRequest.setLocalName(hostHeaderValue);
}

} catch (IllegalArgumentException iae) {
log.debug(sm.getString("remoteIpFilter.invalidHostHeader", hostHeaderValue, hostHeader));
}
}
}
request.setAttribute(Globals.REQUEST_FORWARDED_ATTRIBUTE, Boolean.TRUE);

if (log.isDebugEnabled()) {
log.debug("Incoming request " + request.getRequestURI() + " with originalRemoteAddr '" + request.getRemoteAddr()
+ "', originalRemoteHost='" + request.getRemoteHost() + "', originalSecure='" + request.isSecure()
+ "', originalScheme='" + request.getScheme() + "', original[" + remoteIpHeader + "]='"
+ concatRemoteIpHeaderValue + "', original[" + protocolHeader + "]='"
+ (protocolHeader == null ? null : request.getHeader(protocolHeader)) + "' will be seen as newRemoteAddr='"
+ xRequest.getRemoteAddr() + "', newRemoteHost='" + xRequest.getRemoteHost() + "', newScheme='"
+ xRequest.getScheme() + "', newSecure='" + xRequest.isSecure() + "', new[" + remoteIpHeader + "]='"
+ xRequest.getHeader(remoteIpHeader) + "', new[" + proxiesHeader + "]='" + xRequest.getHeader(proxiesHeader) + "'");
log.debug("Incoming request " + request.getRequestURI() + " with originalRemoteAddr [" + request.getRemoteAddr() +
"], originalRemoteHost=[" + request.getRemoteHost() + "], originalSecure=[" + request.isSecure() +
"], originalScheme=[" + request.getScheme() + "], originalServerName=[" + request.getServerName() +
"], originalServerPort=[" + request.getServerPort() +
"] will be seen as newRemoteAddr=[" + xRequest.getRemoteAddr() +
"], newRemoteHost=[" + xRequest.getRemoteHost() + "], newSecure=[" + xRequest.isSecure() +
"], newScheme=[" + xRequest.getScheme() + "], newServerName=[" + xRequest.getServerName() +
"], newServerPort=[" + xRequest.getServerPort() + "]");
}
if (requestAttributesEnabled) {
request.setAttribute(AccessLog.REMOTE_ADDR_ATTRIBUTE,
Expand All @@ -843,6 +896,8 @@ public void doFilter(HttpServletRequest request, HttpServletResponse response, F
xRequest.getRemoteHost());
request.setAttribute(AccessLog.PROTOCOL_ATTRIBUTE,
xRequest.getProtocol());
request.setAttribute(AccessLog.SERVER_NAME_ATTRIBUTE,
xRequest.getServerName());
request.setAttribute(AccessLog.SERVER_PORT_ATTRIBUTE,
Integer.valueOf(xRequest.getServerPort()));
}
Expand Down Expand Up @@ -909,6 +964,10 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
}
}

public boolean isChangeLocalName() {
return changeLocalName;
}

public boolean isChangeLocalPort() {
return changeLocalPort;
}
Expand Down Expand Up @@ -968,10 +1027,18 @@ public void init() throws ServletException {
setProtocolHeaderHttpsValue(getInitParameter(PROTOCOL_HEADER_HTTPS_VALUE_PARAMETER));
}

if (getInitParameter(HOST_HEADER_PARAMETER) != null) {
setHostHeader(getInitParameter(HOST_HEADER_PARAMETER));
}

if (getInitParameter(PORT_HEADER_PARAMETER) != null) {
setPortHeader(getInitParameter(PORT_HEADER_PARAMETER));
}

if (getInitParameter(CHANGE_LOCAL_NAME_PARAMETER) != null) {
setChangeLocalName(Boolean.parseBoolean(getInitParameter(CHANGE_LOCAL_NAME_PARAMETER)));
}

if (getInitParameter(CHANGE_LOCAL_PORT_PARAMETER) != null) {
setChangeLocalPort(Boolean.parseBoolean(getInitParameter(CHANGE_LOCAL_PORT_PARAMETER)));
}
Expand Down Expand Up @@ -1005,6 +1072,22 @@ public void init() throws ServletException {
}
}

/**
* <p>
* If <code>true</code>, the return values for both {@link
* ServletRequest#getLocalName()} and {@link ServletRequest#getServerName()}
* will be modified by this Filter rather than just
* {@link ServletRequest#getServerName()}.
* </p>
* <p>
* Default value : <code>false</code>
* </p>
* @param changeLocalName The new flag value
*/
public void setChangeLocalName(boolean changeLocalName) {
this.changeLocalName = changeLocalName;
}

/**
* <p>
* If <code>true</code>, the return values for both {@link
Expand Down Expand Up @@ -1065,6 +1148,20 @@ public void setInternalProxies(String internalProxies) {
}
}

/**
* <p>
* Header that holds the incoming host, usually named
* <code>X-Forwarded-HOst</code>.
* </p>
* <p>
* Default value : <code>null</code>
* </p>
* @param hostHeader The header name
*/
public void setHostHeader(String hostHeader) {
this.hostHeader = hostHeader;
}

/**
* <p>
* Header that holds the incoming port, usually named
Expand Down
19 changes: 16 additions & 3 deletions java/org/apache/catalina/valves/AbstractAccessLogValve.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1394,11 +1394,24 @@ protected class LocalServerNameElement implements AccessLogElement {
@Override
public void addElement(CharArrayWriter buf, Date date, Request request,
Response response, long time) {
String value = null;
if (requestAttributesEnabled) {
Object serverName = request.getAttribute(SERVER_NAME_ATTRIBUTE);
if (serverName != null) {
value = serverName.toString();
}
}
if (value == null || value.length() == 0) {
value = request.getServerName();
}
if (value == null || value.length() == 0) {
value = "-";
}

if (ipv6Canonical) {
buf.append(IPv6Utils.canonize(request.getServerName()));
} else {
buf.append(request.getServerName());
value = IPv6Utils.canonize(value);
}
buf.append(value);
}
}

Expand Down
2 changes: 2 additions & 0 deletions java/org/apache/catalina/valves/LocalStrings.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,8 @@ jdbcAccessLogValve.exception=Exception performing insert access entry
remoteCidrValve.invalid=Invalid configuration provided for [{0}]. See previous messages for details.
remoteCidrValve.noRemoteIp=Client does not have an IP address. Request denied.

remoteIpValve.invalidHostHeader=Invalid value [{0}] found for Host in HTTP header [{1}]
remoteIpValve.invalidHostWithPort=Host value [{0}] in HTTP header [{1}] included a port number which will be ignored
remoteIpValve.invalidPortHeader=Invalid value [{0}] found for port in HTTP header [{1}]

requestFilterValve.configInvalid=One or more invalid configuration settings were provided for the Remote[Addr|Host]Valve which prevented the Valve and its parent containers from starting
Expand Down
Loading

0 comments on commit 67c3af9

Please sign in to comment.