Skip to content

Commit ffc3ca2

Browse files
kroepkekroepke
committed
add unit tests for forwarded/real ip headers
add hashcode/equals to resolvable inet socket address wrapper, comparing them in tests add RawMessageHandler test fix subnet trusted proxy code (yay tests)
1 parent fdc3c87 commit ffc3ca2

File tree

4 files changed

+336
-3
lines changed

4 files changed

+336
-3
lines changed

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -167,12 +167,18 @@ private Optional<String> findOriginalIpAndCheckProxies(List<String> ipChain) thr
167167
// if we hit one that we don't trust, bail out and don't use the candidate IP
168168
while (iterator.hasNext()) {
169169
final String proxyIp = iterator.next();
170+
boolean trusted = false;
170171
for (final IpSubnet subnet : trustedProxies) {
171-
if (!subnet.contains(proxyIp)) {
172-
LOG.debug("Untrusted proxy found in X-Forwarded-For header: {}, ignoring the supplied original IP address", proxyIp);
173-
return Optional.empty();
172+
if (subnet.contains(proxyIp)) {
173+
trusted = true;
174+
// we can skip the remainder of the subnet checks for this proxy ip
175+
break;
174176
}
175177
}
178+
if (!trusted) {
179+
LOG.debug("No trusted proxy entry found in {}, ignoring the supplied original IP address {}", trustedProxies, candidate);
180+
return Optional.empty();
181+
}
176182
}
177183
}
178184
}

‎graylog2-server/src/main/java/org/graylog2/plugin/ResolvableInetSocketAddress.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
import java.net.InetAddress;
2323
import java.net.InetSocketAddress;
24+
import java.util.Objects;
2425

2526
import static java.util.Objects.requireNonNull;
2627

@@ -92,4 +93,16 @@ public String toString() {
9293
return getAddress().getHostAddress() + ":" + getPort();
9394
}
9495
}
96+
97+
@Override
98+
public boolean equals(Object o) {
99+
if (o == null || getClass() != o.getClass()) return false;
100+
final ResolvableInetSocketAddress that = (ResolvableInetSocketAddress) o;
101+
return reverseLookedUp == that.reverseLookedUp && Objects.equals(inetSocketAddress, that.inetSocketAddress);
102+
}
103+
104+
@Override
105+
public int hashCode() {
106+
return Objects.hash(inetSocketAddress, reverseLookedUp);
107+
}
95108
}

‎graylog2-server/src/test/java/org/graylog2/inputs/transports/netty/HttpForwardedForHandlerTest.java‎

Lines changed: 248 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,248 @@
1+
package org.graylog2.inputs.transports.netty;
2+
3+
import io.netty.channel.embedded.EmbeddedChannel;
4+
import io.netty.handler.codec.http.DefaultFullHttpRequest;
5+
import io.netty.handler.codec.http.FullHttpRequest;
6+
import io.netty.handler.codec.http.HttpMethod;
7+
import io.netty.handler.codec.http.HttpRequest;
8+
import io.netty.handler.codec.http.HttpVersion;
9+
import org.graylog2.utilities.IpSubnet;
10+
import org.junit.jupiter.api.Test;
11+
12+
import java.net.InetSocketAddress;
13+
import java.net.UnknownHostException;
14+
import java.nio.charset.StandardCharsets;
15+
import java.util.Set;
16+
17+
import static org.assertj.core.api.Assertions.assertThat;
18+
19+
class HttpForwardedForHandlerTest {
20+
21+
public static final byte[] EMPTY_PAYLOAD = "".getBytes(StandardCharsets.UTF_8);
22+
23+
@Test
24+
void disabledHandlerHasNoSideEffects() {
25+
final EmbeddedChannel channel = new EmbeddedChannel(
26+
new HttpForwardedForHandler(false, false, "", false, Set.of())
27+
);
28+
29+
final FullHttpRequest httpRequest = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/raw");
30+
// payload is irrelevant for this test
31+
httpRequest.content().writeBytes(EMPTY_PAYLOAD);
32+
33+
final FullHttpRequest originalRequest = httpRequest.copy();
34+
35+
channel.writeInbound(httpRequest);
36+
channel.finish();
37+
38+
// the handler shouldn't have any side effects
39+
final HttpRequest o = channel.readInbound();
40+
assertThat(o).isEqualTo(originalRequest);
41+
assertThat(channel.hasAttr(RawMessageHandler.ORIGINAL_IP_KEY)).isFalse();
42+
}
43+
44+
@Test
45+
void xForwardedForDecoded() {
46+
final EmbeddedChannel channel = new EmbeddedChannel(
47+
new HttpForwardedForHandler(true, false, "", false, Set.of())
48+
);
49+
final FullHttpRequest httpRequest = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/raw");
50+
httpRequest.headers().add("X-Forwarded-For", "6.6.6.6, 192.168.1.1");
51+
// payload is irrelevant for this test
52+
httpRequest.content().writeBytes(EMPTY_PAYLOAD);
53+
54+
channel.writeInbound(httpRequest);
55+
channel.finish();
56+
assertThat(channel.hasAttr(RawMessageHandler.ORIGINAL_IP_KEY)).isTrue();
57+
assertThat(channel.attr(RawMessageHandler.ORIGINAL_IP_KEY).get()).isEqualTo(new InetSocketAddress("6.6.6.6", 0));
58+
}
59+
60+
@Test
61+
void forwardedDecoded() {
62+
final EmbeddedChannel channel = new EmbeddedChannel(
63+
new HttpForwardedForHandler(true, false, "", false, Set.of())
64+
);
65+
final FullHttpRequest httpRequest = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/raw");
66+
httpRequest.headers().add("Forwarded", "for=6.6.6.6, by=192.168.1.1");
67+
httpRequest.headers().add("Forwarded", "by=5.4.2.2");
68+
httpRequest.headers().add("Forwarded", "for=192.168.1.1, by=10.0.1.20");
69+
// payload is irrelevant for this test
70+
httpRequest.content().writeBytes(EMPTY_PAYLOAD);
71+
72+
channel.writeInbound(httpRequest);
73+
channel.finish();
74+
assertThat(channel.hasAttr(RawMessageHandler.ORIGINAL_IP_KEY)).isTrue();
75+
assertThat(channel.attr(RawMessageHandler.ORIGINAL_IP_KEY).get()).isEqualTo(new InetSocketAddress("6.6.6.6", 0));
76+
}
77+
78+
@Test
79+
void realIp() {
80+
final EmbeddedChannel channel = new EmbeddedChannel(
81+
new HttpForwardedForHandler(true,
82+
true,
83+
"X-Real-Ip, X-Client-IP, CF-Connecting-IP, True-Client-IP, Fastly-Client-IP",
84+
false, Set.of())
85+
);
86+
final FullHttpRequest httpRequest = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/raw");
87+
httpRequest.headers().add("X-Real-Ip", "6.6.6.6");
88+
// payload is irrelevant for this test
89+
httpRequest.content().writeBytes(EMPTY_PAYLOAD);
90+
91+
channel.writeInbound(httpRequest);
92+
channel.finish();
93+
assertThat(channel.hasAttr(RawMessageHandler.ORIGINAL_IP_KEY)).isTrue();
94+
assertThat(channel.attr(RawMessageHandler.ORIGINAL_IP_KEY).get()).isEqualTo(new InetSocketAddress("6.6.6.6", 0));
95+
}
96+
97+
@Test
98+
void oneOfRealIpHeaders() {
99+
final EmbeddedChannel channel = new EmbeddedChannel(
100+
new HttpForwardedForHandler(true,
101+
true,
102+
"X-Real-Ip, X-Client-IP, CF-Connecting-IP, True-Client-IP, Fastly-Client-IP",
103+
false, Set.of())
104+
);
105+
final FullHttpRequest httpRequest = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/raw");
106+
httpRequest.headers().add("true-client-ip", "6.6.6.6");
107+
// payload is irrelevant for this test
108+
httpRequest.content().writeBytes(EMPTY_PAYLOAD);
109+
110+
channel.writeInbound(httpRequest);
111+
channel.finish();
112+
assertThat(channel.hasAttr(RawMessageHandler.ORIGINAL_IP_KEY)).isTrue();
113+
assertThat(channel.attr(RawMessageHandler.ORIGINAL_IP_KEY).get()).isEqualTo(new InetSocketAddress("6.6.6.6", 0));
114+
}
115+
116+
@Test
117+
void xForwardedForHonorsTrustedProxies() throws UnknownHostException {
118+
final IpSubnet host192 = new IpSubnet("192.168.1.1/32");
119+
final IpSubnet net10_0_0 = new IpSubnet("10.0.0.1/24");
120+
final EmbeddedChannel channel = new EmbeddedChannel(
121+
new HttpForwardedForHandler(true,
122+
false,
123+
"",
124+
true, Set.of(host192, net10_0_0))
125+
);
126+
final FullHttpRequest httpRequest = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/raw");
127+
httpRequest.headers().add("X-Forwarded-For", "6.6.6.6, 10.0.0.24, 192.168.1.1");
128+
// payload is irrelevant for this test
129+
httpRequest.content().writeBytes(EMPTY_PAYLOAD);
130+
131+
channel.writeInbound(httpRequest);
132+
channel.finish();
133+
assertThat(channel.hasAttr(RawMessageHandler.ORIGINAL_IP_KEY)).isTrue();
134+
assertThat(channel.attr(RawMessageHandler.ORIGINAL_IP_KEY).get()).isEqualTo(new InetSocketAddress("6.6.6.6", 0));
135+
}
136+
137+
@Test
138+
void xForwardedForHonorsTrustedProxiesOneUntrusted() throws UnknownHostException {
139+
final IpSubnet host192 = new IpSubnet("192.168.1.1/32");
140+
final IpSubnet net10_0_0 = new IpSubnet("10.0.0.1/24");
141+
final EmbeddedChannel channel = new EmbeddedChannel(
142+
new HttpForwardedForHandler(true,
143+
false,
144+
"",
145+
true, Set.of(host192, net10_0_0))
146+
);
147+
final FullHttpRequest httpRequest = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/raw");
148+
// there's a proxy that we don't trust
149+
httpRequest.headers().add("X-Forwarded-For", "6.6.6.6, 10.1.0.24, 192.168.1.1");
150+
// payload is irrelevant for this test
151+
httpRequest.content().writeBytes(EMPTY_PAYLOAD);
152+
153+
channel.writeInbound(httpRequest);
154+
channel.finish();
155+
assertThat(channel.hasAttr(RawMessageHandler.ORIGINAL_IP_KEY)).isFalse();
156+
}
157+
158+
159+
@Test
160+
void xForwardedForHonorsTrustedProxiesNoneTrusted() throws UnknownHostException {
161+
final IpSubnet host192 = new IpSubnet("192.168.1.1/32");
162+
final IpSubnet net10_0_0 = new IpSubnet("10.0.0.1/24");
163+
final EmbeddedChannel channel = new EmbeddedChannel(
164+
new HttpForwardedForHandler(true,
165+
false,
166+
"",
167+
true, Set.of(host192, net10_0_0))
168+
);
169+
final FullHttpRequest httpRequest = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/raw");
170+
// there's a proxy that we don't trust
171+
httpRequest.headers().add("X-Forwarded-For", "6.6.6.6, 10.1.0.24, 192.168.2.1");
172+
// payload is irrelevant for this test
173+
httpRequest.content().writeBytes(EMPTY_PAYLOAD);
174+
175+
channel.writeInbound(httpRequest);
176+
channel.finish();
177+
assertThat(channel.hasAttr(RawMessageHandler.ORIGINAL_IP_KEY)).isFalse();
178+
}
179+
180+
@Test
181+
void forwardedHonorsTrustedProxies() throws UnknownHostException {
182+
final IpSubnet host192 = new IpSubnet("192.168.1.1/32");
183+
final IpSubnet net10_0_0 = new IpSubnet("10.0.0.1/24");
184+
final EmbeddedChannel channel = new EmbeddedChannel(
185+
new HttpForwardedForHandler(true,
186+
false,
187+
"",
188+
true, Set.of(host192, net10_0_0))
189+
);
190+
final FullHttpRequest httpRequest = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/raw");
191+
httpRequest.headers().add("Forwarded", "for=6.6.6.6, for=192.168.1.1");
192+
httpRequest.headers().add("Forwarded", "for=10.0.0.24");
193+
// payload is irrelevant for this test
194+
httpRequest.content().writeBytes(EMPTY_PAYLOAD);
195+
196+
channel.writeInbound(httpRequest);
197+
channel.finish();
198+
assertThat(channel.hasAttr(RawMessageHandler.ORIGINAL_IP_KEY)).isTrue();
199+
assertThat(channel.attr(RawMessageHandler.ORIGINAL_IP_KEY).get()).isEqualTo(new InetSocketAddress("6.6.6.6", 0));
200+
}
201+
202+
203+
@Test
204+
void forwardedHonorsTrustedProxiesOneUntrusted() throws UnknownHostException {
205+
final IpSubnet host192 = new IpSubnet("192.168.1.1/32");
206+
final IpSubnet net10_0_0 = new IpSubnet("10.0.0.1/24");
207+
final EmbeddedChannel channel = new EmbeddedChannel(
208+
new HttpForwardedForHandler(true,
209+
false,
210+
"",
211+
true, Set.of(host192, net10_0_0))
212+
);
213+
final FullHttpRequest httpRequest = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/raw");
214+
// there's a proxy that we don't trust
215+
httpRequest.headers().add("Forwarded", "for=6.6.6.6, for=192.168.2.1");
216+
httpRequest.headers().add("Forwarded", "for=10.0.0.24");
217+
// payload is irrelevant for this test
218+
httpRequest.content().writeBytes(EMPTY_PAYLOAD);
219+
220+
channel.writeInbound(httpRequest);
221+
channel.finish();
222+
assertThat(channel.hasAttr(RawMessageHandler.ORIGINAL_IP_KEY)).isFalse();
223+
}
224+
225+
226+
@Test
227+
void forwardedHonorsTrustedProxiesNoneTrusted() throws UnknownHostException {
228+
final IpSubnet host192 = new IpSubnet("192.168.1.1/32");
229+
final IpSubnet net10_0_0 = new IpSubnet("10.0.0.1/24");
230+
final EmbeddedChannel channel = new EmbeddedChannel(
231+
new HttpForwardedForHandler(true,
232+
false,
233+
"",
234+
true, Set.of(host192, net10_0_0))
235+
);
236+
final FullHttpRequest httpRequest = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/raw");
237+
// there's a proxy that we don't trust
238+
httpRequest.headers().add("Forwarded", "for=6.6.6.6, for=192.168.2.1");
239+
httpRequest.headers().add("Forwarded", "for=10.1.0.24");
240+
// payload is irrelevant for this test
241+
httpRequest.content().writeBytes(EMPTY_PAYLOAD);
242+
243+
channel.writeInbound(httpRequest);
244+
channel.finish();
245+
assertThat(channel.hasAttr(RawMessageHandler.ORIGINAL_IP_KEY)).isFalse();
246+
}
247+
248+
}

‎graylog2-server/src/test/java/org/graylog2/inputs/transports/netty/RawMessageHandlerTest.java‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
package org.graylog2.inputs.transports.netty;
2+
3+
import io.netty.buffer.Unpooled;
4+
import io.netty.channel.embedded.EmbeddedChannel;
5+
import org.graylog2.plugin.ResolvableInetSocketAddress;
6+
import org.graylog2.plugin.inputs.MessageInput;
7+
import org.graylog2.plugin.journal.RawMessage;
8+
import org.junit.jupiter.api.Test;
9+
import org.mockito.ArgumentCaptor;
10+
11+
import java.net.InetSocketAddress;
12+
import java.net.SocketAddress;
13+
14+
import static org.assertj.core.api.Assertions.assertThat;
15+
import static org.mockito.Mockito.mock;
16+
import static org.mockito.Mockito.verify;
17+
18+
public class RawMessageHandlerTest {
19+
20+
@Test
21+
void channelAttributeNotSet() {
22+
// catch the raw message written out by the handler
23+
final MessageInput input = mock(MessageInput.class);
24+
25+
// we have to override this, because EmbeddedChannel uses a different class than RawMessageHandler expects for
26+
// the remote address.
27+
final EmbeddedChannel channel = new EmbeddedChannel(new RawMessageHandler(input)) {
28+
@Override
29+
protected SocketAddress remoteAddress0() {
30+
return new InetSocketAddress("6.6.6.6", 0);
31+
}
32+
};
33+
34+
channel.writeInbound(Unpooled.wrappedBuffer(new byte[]{1, 2, 3}));
35+
channel.finish();
36+
ArgumentCaptor<RawMessage> captor = ArgumentCaptor.forClass(RawMessage.class);
37+
verify(input).processRawMessage(captor.capture());
38+
final ResolvableInetSocketAddress actual = captor.getValue().getRemoteAddress();
39+
final ResolvableInetSocketAddress expected = ResolvableInetSocketAddress.wrap(new InetSocketAddress("6.6.6.6", 0));
40+
assertThat(actual).isEqualTo(expected);
41+
}
42+
43+
@Test
44+
void remoteAddressOverriddenByChannelAttribute() {
45+
// catch the raw message written out by the handler
46+
final MessageInput input = mock(MessageInput.class);
47+
48+
// we have to override this, because EmbeddedChannel uses a different class than RawMessageHandler expects for
49+
// the remote address.
50+
final EmbeddedChannel channel = new EmbeddedChannel(new RawMessageHandler(input)) {
51+
@Override
52+
protected SocketAddress remoteAddress0() {
53+
return new InetSocketAddress("6.6.6.6", 0);
54+
}
55+
};
56+
57+
channel.attr(RawMessageHandler.ORIGINAL_IP_KEY).set(new InetSocketAddress("3.3.3.3", 0));
58+
channel.writeInbound(Unpooled.wrappedBuffer(new byte[]{1, 2, 3}));
59+
channel.finish();
60+
ArgumentCaptor<RawMessage> captor = ArgumentCaptor.forClass(RawMessage.class);
61+
verify(input).processRawMessage(captor.capture());
62+
final ResolvableInetSocketAddress actual = captor.getValue().getRemoteAddress();
63+
final ResolvableInetSocketAddress expected = ResolvableInetSocketAddress.wrap(new InetSocketAddress("3.3.3.3", 0));
64+
assertThat(actual).isEqualTo(expected);
65+
}
66+
}

0 commit comments

Comments
 (0)