Skip to content

Commit c8e9d86

Browse files
committed
the channel handlers deal with FullHttpMessage, switch the type to make it clear
FullHttpMessage objects need to be retained, because they contain the body as well (the reason we switched the type) guard against invalid ip literals and log a warning in case we cannot parse the ip address debug log if we don't know any relaying proxy
1 parent d819aeb commit c8e9d86

File tree

1 file changed

+16
-5
lines changed

1 file changed

+16
-5
lines changed

graylog2-server/src/main/java/org/graylog2/inputs/transports/netty/HttpForwardedForHandler.java

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,10 @@
2222
import com.google.common.net.InetAddresses;
2323
import io.netty.channel.ChannelHandlerContext;
2424
import io.netty.channel.SimpleChannelInboundHandler;
25+
import io.netty.handler.codec.http.FullHttpRequest;
2526
import io.netty.handler.codec.http.HttpHeaders;
2627
import io.netty.handler.codec.http.HttpRequest;
28+
import io.netty.util.ReferenceCountUtil;
2729
import org.graylog2.utilities.IpSubnet;
2830
import org.slf4j.Logger;
2931
import org.slf4j.LoggerFactory;
@@ -39,7 +41,7 @@
3941
import java.util.Set;
4042
import java.util.stream.Collectors;
4143

42-
public class HttpForwardedForHandler extends SimpleChannelInboundHandler<HttpRequest> {
44+
public class HttpForwardedForHandler extends SimpleChannelInboundHandler<FullHttpRequest> {
4345
private static final Logger LOG = LoggerFactory.getLogger(HttpForwardedForHandler.class);
4446

4547
// Splits comma-separated forwarded-elements
@@ -81,7 +83,7 @@ public HttpForwardedForHandler(boolean enableForwardedFor, boolean enableRealIpH
8183
}
8284

8385
@Override
84-
protected void channelRead0(ChannelHandlerContext ctx, HttpRequest msg) throws Exception {
86+
protected void channelRead0(ChannelHandlerContext ctx, FullHttpRequest msg) throws Exception {
8587
String originalIp = null;
8688

8789
// if we choose to trust X-Forwarded-For or Forwarded headers
@@ -149,10 +151,16 @@ protected void channelRead0(ChannelHandlerContext ctx, HttpRequest msg) throws E
149151
if (!InetAddresses.isInetAddress(originalIp)) {
150152
LOG.warn("Ignoring non-literal IP value for original IP address: {}", originalIp);
151153
}
152-
final InetAddress inetAddress = InetAddresses.forString(originalIp);
153-
ctx.channel().attr(RawMessageHandler.ORIGINAL_IP_KEY).setIfAbsent(new InetSocketAddress(inetAddress, 0));
154+
try {
155+
final InetAddress inetAddress = InetAddresses.forString(originalIp);
156+
ctx.channel().attr(RawMessageHandler.ORIGINAL_IP_KEY).setIfAbsent(new InetSocketAddress(inetAddress, 0));
157+
} catch (IllegalArgumentException e) {
158+
LOG.warn("The address extracted for forwarded IP address is not a valid IP literal: {}", originalIp);
159+
}
154160
}
155-
ctx.fireChannelRead(msg);
161+
// our http channel handlers are using FullHttpMessage instances, so we need to retain them, even though we are
162+
// only using header values here (otherwise we'll get ref count exceptions upstream)
163+
ctx.fireChannelRead(msg.retain());
156164
}
157165

158166
private Optional<String> findOriginalIpAndCheckProxies(List<String> ipChain) throws UnknownHostException {
@@ -164,6 +172,9 @@ private Optional<String> findOriginalIpAndCheckProxies(List<String> ipChain) thr
164172
LOG.debug("Ignoring invalid X-Forwarded-For header, list of proxies is empty");
165173
return Optional.empty();
166174
} else {
175+
if (!iterator.hasNext()) {
176+
LOG.debug("Forwarded-For list only contains a single entry, cannot check relaying proxy addresses.");
177+
}
167178
// go through the remainder of the list, which should all be proxy addresses we trust
168179
// if we hit one that we don't trust, bail out and don't use the candidate IP
169180
while (iterator.hasNext()) {

0 commit comments

Comments
 (0)