Remove support for X-Forwarded in client IP resolution (#7946)

smola · web-flow · commit 508c314d67b4 · 2024-11-28T15:55:07.000+01:00
diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/http/ClientIpAddressResolver.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/http/ClientIpAddressResolver.java
@@ -92,14 +92,6 @@ private static InetAddress doResolve(AgentSpan.Context.Extracted context, Mutabl
       result = coalesce(result, addr);
     }
 
-    addr = tryHeader(context.getXForwarded(), FORWARDED_PARSER);
-    if (addr != null) {
-      if (!isIpAddrPrivate(addr)) {
-        return addr;
-      }
-      result = coalesce(result, addr);
-    }
-
     addr = tryHeader(context.getForwardedFor(), PLAIN_IP_ADDRESS_PARSER);
     if (addr != null) {
       if (!isIpAddrPrivate(addr)) {
diff --git a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/http/ClientIpAddressResolverSpecification.groovy b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/http/ClientIpAddressResolverSpecification.groovy
@@ -60,15 +60,6 @@ class ClientIpAddressResolverSpecification extends Specification {
     'x-real-ip' | '42' | '0.0.0.42'
 
     'x-client-ip' | '2.2.2.2' | '2.2.2.2'
-    'x-forwarded' | 'for="[2001::1]:1111"' | '2001::1'
-    'x-forwarded' | 'fOr="[2001::1]:1111"' | '2001::1'
-    'x-forwarded' | 'for=some_host' | null
-    'x-forwarded' | 'for=127.0.0.1, FOR=1.1.1.1' | '1.1.1.1'
-    'x-forwarded' |'for="\"foobar";proto=http,FOR="1.1.1.1"' | '1.1.1.1'
-    'x-forwarded' | 'for="8.8.8.8:2222",' | '8.8.8.8'
-    'x-forwarded' | 'for="8.8.8.8' | null  // quote not closed
-    'x-forwarded' | 'far="8.8.8.8",for=4.4.4.4;' | '4.4.4.4'
-    'x-forwarded' | '   for=127.0.0.1,for= for=,for=;"for = for="" ,; for=8.8.8.8;' | '8.8.8.8'
 
     'x-cluster-client-ip' | '2.2.2.2' | '2.2.2.2'
 
@@ -119,9 +110,6 @@ class ClientIpAddressResolverSpecification extends Specification {
     then:
     1 * context.getXClientIp() >> null
 
-    then:
-    1 * context.getXForwarded() >> null
-
     then:
     1 * context.getForwardedFor() >> null
 
@@ -174,7 +162,6 @@ class ClientIpAddressResolverSpecification extends Specification {
     1 * context.getXForwardedFor() >> '127.0.0.1'
     1 * context.getXRealIp() >> '127.0.0.2'
     1 * context.getXClientIp() >> '127.0.0.3'
-    1 * context.getXForwarded() >> 'for=127.0.0.4'
     1 * context.getXClusterClientIp() >> '127.0.0.5'
     1 * context.getForwardedFor() >> '127.0.0.6'
     1 * context.getTrueClientIp() >> '127.0.0.9'
diff --git a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/SpringBootSmokeTest.groovy
@@ -177,7 +177,7 @@ class SpringBootSmokeTest extends AbstractAppSecServerSmokeTest {
     def request = new Request.Builder()
       .url(url)
       .addHeader("User-Agent", "Arachni/v1")
-      .addHeader("X-Forwarded", 'for="[::ffff:1.2.3.4]"')
+      .addHeader("X-Client-Ip", '::ffff:1.2.3.4')
       .build()
     def response = client.newCall(request).execute()
     def responseBodyStr = response.body().string()
diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ContextInterpreter.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ContextInterpreter.java
@@ -11,7 +11,6 @@
 import static datadog.trace.core.propagation.HttpCodec.X_CLUSTER_CLIENT_IP_KEY;
 import static datadog.trace.core.propagation.HttpCodec.X_FORWARDED_FOR_KEY;
 import static datadog.trace.core.propagation.HttpCodec.X_FORWARDED_HOST_KEY;
-import static datadog.trace.core.propagation.HttpCodec.X_FORWARDED_KEY;
 import static datadog.trace.core.propagation.HttpCodec.X_FORWARDED_PORT_KEY;
 import static datadog.trace.core.propagation.HttpCodec.X_FORWARDED_PROTO_KEY;
 import static datadog.trace.core.propagation.HttpCodec.X_REAL_IP_KEY;
@@ -122,10 +121,6 @@ protected final boolean handledXForwarding(String key, String value) {
       getHeaders().xForwardedPort = value;
       return true;
     }
-    if (X_FORWARDED_KEY.equalsIgnoreCase(key)) {
-      getHeaders().xForwarded = value;
-      return true;
-    }
     return false;
   }
 
diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/HttpCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/HttpCodec.java
@@ -37,7 +37,6 @@ public class HttpCodec {
   static final String FORWARDED_FOR_KEY = "forwarded-for";
   static final String X_FORWARDED_PROTO_KEY = "x-forwarded-proto";
   static final String X_FORWARDED_HOST_KEY = "x-forwarded-host";
-  static final String X_FORWARDED_KEY = "x-forwarded";
   static final String X_FORWARDED_FOR_KEY = "x-forwarded-for";
   static final String X_FORWARDED_PORT_KEY = "x-forwarded-port";
 
diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/B3HttpExtractorTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/B3HttpExtractorTest.groovy
@@ -367,7 +367,6 @@ class B3HttpExtractorTest extends DDSpecification {
       (HttpCodec.X_CLIENT_IP_KEY): '3.3.3.3',
       (HttpCodec.TRUE_CLIENT_IP_KEY): '4.4.4.4',
       (HttpCodec.FORWARDED_FOR_KEY): '5.5.5.5',
-      (HttpCodec.X_FORWARDED_KEY): '6.6.6.6',
       (HttpCodec.FASTLY_CLIENT_IP_KEY): '7.7.7.7',
       (HttpCodec.CF_CONNECTING_IP_KEY): '8.8.8.8',
       (HttpCodec.CF_CONNECTING_IP_V6_KEY): '9.9.9.9',
@@ -383,7 +382,6 @@ class B3HttpExtractorTest extends DDSpecification {
     assert context.XClientIp == '3.3.3.3'
     assert context.trueClientIp == '4.4.4.4'
     assert context.forwardedFor == '5.5.5.5'
-    assert context.XForwarded == '6.6.6.6'
     assert context.fastlyClientIp == '7.7.7.7'
     assert context.cfConnectingIp == '8.8.8.8'
     assert context.cfConnectingIpv6 == '9.9.9.9'
diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogHttpExtractorTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogHttpExtractorTest.groovy
@@ -438,7 +438,6 @@ class DatadogHttpExtractorTest extends DDSpecification {
       (HttpCodec.X_CLIENT_IP_KEY): '3.3.3.3',
       (HttpCodec.TRUE_CLIENT_IP_KEY): '4.4.4.4',
       (HttpCodec.FORWARDED_FOR_KEY): '5.5.5.5',
-      (HttpCodec.X_FORWARDED_KEY): '6.6.6.6',
       (HttpCodec.FASTLY_CLIENT_IP_KEY): '7.7.7.7',
       (HttpCodec.CF_CONNECTING_IP_KEY): '8.8.8.8',
       (HttpCodec.CF_CONNECTING_IP_V6_KEY): '9.9.9.9',
@@ -454,7 +453,6 @@ class DatadogHttpExtractorTest extends DDSpecification {
     assert context.XClientIp == '3.3.3.3'
     assert context.trueClientIp == '4.4.4.4'
     assert context.forwardedFor == '5.5.5.5'
-    assert context.XForwarded == '6.6.6.6'
     assert context.fastlyClientIp == '7.7.7.7'
     assert context.cfConnectingIp == '8.8.8.8'
     assert context.cfConnectingIpv6 == '9.9.9.9'
diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/HaystackHttpExtractorTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/HaystackHttpExtractorTest.groovy
@@ -300,7 +300,6 @@ class HaystackHttpExtractorTest extends DDSpecification {
       (HttpCodec.X_CLIENT_IP_KEY): '3.3.3.3',
       (HttpCodec.TRUE_CLIENT_IP_KEY): '4.4.4.4',
       (HttpCodec.FORWARDED_FOR_KEY): '5.5.5.5',
-      (HttpCodec.X_FORWARDED_KEY): '6.6.6.6',
       (HttpCodec.FASTLY_CLIENT_IP_KEY): '7.7.7.7',
       (HttpCodec.CF_CONNECTING_IP_KEY): '8.8.8.8',
       (HttpCodec.CF_CONNECTING_IP_V6_KEY): '9.9.9.9',
@@ -316,7 +315,6 @@ class HaystackHttpExtractorTest extends DDSpecification {
     assert context.XClientIp == '3.3.3.3'
     assert context.trueClientIp == '4.4.4.4'
     assert context.forwardedFor == '5.5.5.5'
-    assert context.XForwarded == '6.6.6.6'
     assert context.fastlyClientIp == '7.7.7.7'
     assert context.cfConnectingIp == '8.8.8.8'
     assert context.cfConnectingIpv6 == '9.9.9.9'
diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/NoneHttpExtractorTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/NoneHttpExtractorTest.groovy
@@ -303,7 +303,6 @@ class NoneHttpExtractorTest extends DDSpecification {
       (HttpCodec.X_CLIENT_IP_KEY): '3.3.3.3',
       (HttpCodec.TRUE_CLIENT_IP_KEY): '4.4.4.4',
       (HttpCodec.FORWARDED_FOR_KEY): '5.5.5.5',
-      (HttpCodec.X_FORWARDED_KEY): '6.6.6.6',
       (HttpCodec.FASTLY_CLIENT_IP_KEY): '7.7.7.7',
       (HttpCodec.CF_CONNECTING_IP_KEY): '8.8.8.8',
       (HttpCodec.CF_CONNECTING_IP_V6_KEY): '9.9.9.9',
@@ -319,7 +318,6 @@ class NoneHttpExtractorTest extends DDSpecification {
     assert context.XClientIp == '3.3.3.3'
     assert context.trueClientIp == '4.4.4.4'
     assert context.forwardedFor == '5.5.5.5'
-    assert context.XForwarded == '6.6.6.6'
     assert context.fastlyClientIp == '7.7.7.7'
     assert context.cfConnectingIp == '8.8.8.8'
     assert context.cfConnectingIpv6 == '9.9.9.9'
diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CHttpExtractorTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CHttpExtractorTest.groovy
@@ -365,7 +365,6 @@ class W3CHttpExtractorTest extends DDSpecification {
       (HttpCodec.X_CLIENT_IP_KEY): '3.3.3.3',
       (HttpCodec.TRUE_CLIENT_IP_KEY): '4.4.4.4',
       (HttpCodec.FORWARDED_FOR_KEY): '5.5.5.5',
-      (HttpCodec.X_FORWARDED_KEY): '6.6.6.6',
       (HttpCodec.FASTLY_CLIENT_IP_KEY): '7.7.7.7',
       (HttpCodec.CF_CONNECTING_IP_KEY): '8.8.8.8',
       (HttpCodec.CF_CONNECTING_IP_V6_KEY): '9.9.9.9',
@@ -381,7 +380,6 @@ class W3CHttpExtractorTest extends DDSpecification {
     assert context.XClientIp == '3.3.3.3'
     assert context.trueClientIp == '4.4.4.4'
     assert context.forwardedFor == '5.5.5.5'
-    assert context.XForwarded == '6.6.6.6'
     assert context.fastlyClientIp == '7.7.7.7'
     assert context.cfConnectingIp == '8.8.8.8'
     assert context.cfConnectingIpv6 == '9.9.9.9'
diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/XRayHttpExtractorTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/XRayHttpExtractorTest.groovy
@@ -256,7 +256,6 @@ class XRayHttpExtractorTest extends DDSpecification {
       (HttpCodec.X_CLIENT_IP_KEY): '3.3.3.3',
       (HttpCodec.TRUE_CLIENT_IP_KEY): '4.4.4.4',
       (HttpCodec.FORWARDED_FOR_KEY): '5.5.5.5',
-      (HttpCodec.X_FORWARDED_KEY): '6.6.6.6',
       (HttpCodec.FASTLY_CLIENT_IP_KEY): '7.7.7.7',
       (HttpCodec.CF_CONNECTING_IP_KEY): '8.8.8.8',
       (HttpCodec.CF_CONNECTING_IP_V6_KEY): '9.9.9.9',
@@ -272,7 +271,6 @@ class XRayHttpExtractorTest extends DDSpecification {
     assert context.XClientIp == '3.3.3.3'
     assert context.trueClientIp == '4.4.4.4'
     assert context.forwardedFor == '5.5.5.5'
-    assert context.XForwarded == '6.6.6.6'
     assert context.fastlyClientIp == '7.7.7.7'
     assert context.cfConnectingIp == '8.8.8.8'
     assert context.cfConnectingIpv6 == '9.9.9.9'
diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java
@@ -219,8 +219,6 @@ interface Extracted extends Context {
 
       String getForwardedFor();
 
-      String getXForwarded();
-
       String getXForwardedFor();
 
       String getXClusterClientIp();
diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java
@@ -1043,11 +1043,6 @@ public String getForwardedFor() {
       return null;
     }
 
-    @Override
-    public String getXForwarded() {
-      return null;
-    }
-
     @Override
     public String getXForwardedFor() {
       return null;
diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/TagContext.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/TagContext.java
@@ -125,11 +125,6 @@ public String getForwardedFor() {
     return httpHeaders.forwardedFor;
   }
 
-  @Override
-  public String getXForwarded() {
-    return httpHeaders.xForwarded;
-  }
-
   @Override
   public String getXForwardedFor() {
     return httpHeaders.xForwardedFor;
@@ -264,12 +259,11 @@ public static class HttpHeaders {
     public String fastlyClientIp;
     public String cfConnectingIp;
     public String cfConnectingIpv6;
-    public String xForwarded;
-    public String forwarded;
     public String xForwardedProto;
     public String xForwardedHost;
     public String xForwardedPort;
     public String xForwardedFor;
+    public String forwarded;
     public String forwardedFor;
     public String xClusterClientIp;
     public String xRealIp;
