ISSUE-2674 Strip sensitive data from the url (open-telemetry#6417)

Fixes open-telemetry#2674 by replacing basic auth information as part of the URL with `username:password`. Co-authored-by: Malte <[email protected]> Co-authored-by: Mateusz Rzeszutek <[email protected]> Co-authored-by: Trask Stalnaker <[email protected]>
helios · Oct 31, 2022 · b7a5ca1 · b7a5ca1
1 parent 88bbae9
commit b7a5ca1
Show file tree

Hide file tree

Showing 2 changed files with 82 additions and 1 deletion.
diff --git a/...io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java b/...io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java
@@ -54,7 +54,50 @@ public static <REQUEST, RESPONSE> HttpClientAttributesExtractorBuilder<REQUEST,
   @Override
   public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST request) {
     super.onStart(attributes, parentContext, request);
-    internalSet(attributes, SemanticAttributes.HTTP_URL, getter.url(request));
+    internalSet(attributes, SemanticAttributes.HTTP_URL, stripSensitiveData(getter.url(request)));
+  }
+
+  @Nullable
+  private static String stripSensitiveData(@Nullable String url) {
+    if (url == null || url.isEmpty()) {
+      return url;
+    }
+
+    int schemeEndIndex = url.indexOf(':');
+
+    if (schemeEndIndex == -1) {
+      // not a valid url
+      return url;
+    }
+
+    int len = url.length();
+    if (len <= schemeEndIndex + 2
+        || url.charAt(schemeEndIndex + 1) != '/'
+        || url.charAt(schemeEndIndex + 2) != '/') {
+      // has no authority component
+      return url;
+    }
+
+    // look for the end of the authority component:
+    //   '/', '?', '#' ==> start of path
+    int index;
+    int atIndex = -1;
+    for (index = schemeEndIndex + 3; index < len; index++) {
+      char c = url.charAt(index);
+
+      if (c == '@') {
+        atIndex = index;
+      }
+
+      if (c == '/' || c == '?' || c == '#') {
+        break;
+      }
+    }
+
+    if (atIndex == -1 || atIndex == len - 1) {
+      return url;
+    }
+    return url.substring(0, schemeEndIndex + 3) + url.substring(atIndex + 1);
   }
 
   @Override

diff --git a/...pentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java b/...pentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java
@@ -10,6 +10,7 @@
 import static java.util.Collections.emptyList;
 import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.entry;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
 
 import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.api.common.Attributes;
@@ -19,8 +20,12 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.stream.Stream;
 import javax.annotation.Nullable;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 
 class HttpClientAttributesExtractorTest {
 
@@ -112,6 +117,39 @@ void normal() {
                 asList("654", "321")));
   }
 
+  @ParameterizedTest
+  @MethodSource("stripUrlArguments")
+  void stripBasicAuthTest(String url, String expectedResult) {
+    Map<String, String> request = new HashMap<>();
+    request.put("url", url);
+
+    stripRequestTest(request, expectedResult);
+  }
+
+  private static Stream<Arguments> stripUrlArguments() {
+    return Stream.of(
+        arguments("https://user1:[email protected]", "https://github.com"),
+        arguments("https://user1:[email protected]/path/", "https://github.com/path/"),
+        arguments("https://user1:[email protected]#test.html", "https://github.com#test.html"),
+        arguments("https://user1:[email protected]?foo=b@r", "https://github.com?foo=b@r"),
+        arguments(
+            "https://user1:[email protected]/p@th?foo=b@r", "https://github.com/p@th?foo=b@r"),
+        arguments("https://github.com/p@th?foo=b@r", "https://github.com/p@th?foo=b@r"),
+        arguments("https://github.com#[email protected]", "https://github.com#[email protected]"),
+        arguments("user1:[email protected]", "user1:[email protected]"),
+        arguments("https://github.com@", "https://github.com@"));
+  }
+
+  private static void stripRequestTest(Map<String, String> request, String expected) {
+    HttpClientAttributesExtractor<Map<String, String>, Map<String, String>> extractor =
+        HttpClientAttributesExtractor.builder(new TestHttpClientAttributesGetter()).build();
+
+    AttributesBuilder attributes = Attributes.builder();
+    extractor.onStart(attributes, Context.root(), request);
+
+    assertThat(attributes.build()).containsOnly(entry(SemanticAttributes.HTTP_URL, expected));
+  }
+
   @Test
   void invalidStatusCode() {
     Map<String, String> request = new HashMap<>();