[AzureMonitorAutoConfigure] Validate ingestion and Live Metrics redirect targets

sdk/monitor/azure-monitor-opentelemetry-autoconfigure/CHANGELOG.md

@@ -8,6 +8,8 @@
 
 ### Bugs Fixed
 
+- Validate ingestion and Live Metrics redirect targets before following them ([#50117](https://github.com/Azure/azure-sdk-for-java/pull/50117))
+
 ### Other Changes
 
 - Align customer-facing SDKStats configuration and custom dimension names with the stable specification.

sdk/monitor/azure-monitor-opentelemetry-autoconfigure/src/main/java/com/azure/monitor/opentelemetry/autoconfigure/implementation/pipeline/TelemetryPipeline.java

@@ -11,6 +11,7 @@
 import com.azure.core.util.tracing.Tracer;
 import com.azure.monitor.opentelemetry.autoconfigure.implementation.configuration.ConnectionString;
 import com.azure.monitor.opentelemetry.autoconfigure.implementation.statsbeat.TelemetryBatchMetadata;
+import com.azure.monitor.opentelemetry.autoconfigure.implementation.utils.RedirectPolicyHelper;
 import com.azure.monitor.opentelemetry.autoconfigure.implementation.utils.StatusCode;
 import io.opentelemetry.sdk.common.CompletableResultCode;
 import reactor.core.publisher.Mono;
@@ -114,6 +115,12 @@ private void onResponseBody(TelemetryPipelineRequest request, HttpResponse respo
                 listener.onException(request, "Invalid redirect: " + location, e);
                 return;
             }
+            if (!RedirectPolicyHelper.isTrustedRedirect(request.getUrl(), locationUrl)) {
+                String errorMessage = "Refused cross-origin redirect: " + location;
+                listener.onException(request, errorMessage, new MalformedURLException(errorMessage));
+                result.fail();
+                return;
+            }
             redirectCache.put(request.getConnectionString(), locationUrl);
             request.setUrl(locationUrl);
             sendInternal(request, listener, result, remainingRedirects - 1);

sdk/monitor/azure-monitor-opentelemetry-autoconfigure/src/main/java/com/azure/monitor/opentelemetry/autoconfigure/implementation/quickpulse/QuickPulseCoordinator.java

@@ -10,7 +10,6 @@
 import reactor.util.annotation.Nullable;
 
 import java.net.MalformedURLException;
-import java.net.URL;
 import java.util.concurrent.TimeUnit;
 
 import static com.azure.monitor.opentelemetry.autoconfigure.implementation.utils.AzureMonitorMsgId.QUICK_PULSE_PING_ERROR;
@@ -150,17 +149,16 @@ private long ping() {
         return 0;
     }
 
-    private QuickPulseStatus handleReceivedPingHeaders(IsSubscribedHeaders pingHeaders) {
+    QuickPulseStatus handleReceivedPingHeaders(IsSubscribedHeaders pingHeaders) {
         String redirectLink = pingHeaders.getXMsQpsServiceEndpointRedirectV2();
         if (!Strings.isNullOrEmpty(redirectLink)) {
             try {
-                URL redirectUrl = new URL(redirectLink);
-                // Taking the QuickPulseService.svc part out if present because the swagger will add that on.
-                qpsServiceRedirectedEndpoint = redirectUrl.getProtocol() + "://" + redirectUrl.getHost() + "/";
+                qpsServiceRedirectedEndpoint = QuickPulseRedirectValidator
+                    .validateAndGetEndpointPrefix(pingSender.getQuickPulseEndpoint(), redirectLink);
                 logger.verbose("Handling ping header to redirect to {}", qpsServiceRedirectedEndpoint);
                 dataSender.setRedirectEndpointPrefix(qpsServiceRedirectedEndpoint);
             } catch (MalformedURLException e) {
-                logger.error("The service returned a malformed URL in the redirect header: {}. Exception message: {}",
+                logger.error("The service returned an invalid URL in the redirect header: {}. Exception message: {}",
                     redirectLink, e.getMessage());
             }
         }

sdk/monitor/azure-monitor-opentelemetry-autoconfigure/src/main/java/com/azure/monitor/opentelemetry/autoconfigure/implementation/quickpulse/QuickPulseRedirectValidator.java

@@ -0,0 +1,27 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+package com.azure.monitor.opentelemetry.autoconfigure.implementation.quickpulse;
+
+import com.azure.monitor.opentelemetry.autoconfigure.implementation.utils.RedirectPolicyHelper;
+
+import java.net.MalformedURLException;
+import java.net.URL;
+
+final class QuickPulseRedirectValidator {
+
+    private QuickPulseRedirectValidator() {
+    }
+
+    static String validateAndGetEndpointPrefix(String configuredEndpoint, String redirectLink)
+        throws MalformedURLException {
+        URL configuredUrl = new URL(configuredEndpoint);
+        URL redirectUrl = new URL(redirectLink);
+
+        if (!RedirectPolicyHelper.isTrustedRedirect(configuredUrl, redirectUrl)) {
+            throw new MalformedURLException("Redirect host is outside the configured Live Metrics endpoint boundary");
+        }
+
+        return redirectUrl.getProtocol() + "://" + redirectUrl.getAuthority() + "/";
+    }
+}

sdk/monitor/azure-monitor-opentelemetry-autoconfigure/src/main/java/com/azure/monitor/opentelemetry/autoconfigure/implementation/utils/RedirectPolicyHelper.java

@@ -0,0 +1,80 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+package com.azure.monitor.opentelemetry.autoconfigure.implementation.utils;
+
+import java.net.URL;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.Locale;
+
+/**
+ * Trust boundary checks for server-issued redirects. Following an attacker-controlled redirect would cause the
+ * pipeline to attach a freshly signed credential (and the telemetry payload) to a foreign host.
+ */
+public final class RedirectPolicyHelper {
+
+    private static final String HTTPS = "https";
+
+    private static final List<String> ALLOWED_REDIRECT_DOMAIN_SUFFIXES = Collections.unmodifiableList(
+        Arrays.asList(".livediagnostics.monitor.azure.com", ".monitor.azure.com", ".services.visualstudio.com",
+            ".applicationinsights.azure.com", ".applicationinsights.microsoft.com", ".monitor.azure.us",
+            ".applicationinsights.azure.us", ".monitor.azure.cn", ".applicationinsights.azure.cn"));
+
+    /**
+     * Returns whether a redirect target is safe to follow.
+     * <p>
+     * Stamp reassignment moves between suffixes (for example {@code rt.services.visualstudio.com} to
+     * {@code &lt;region&gt;.livediagnostics.monitor.azure.com}), so the target host is checked on its own rather than
+     * being required to share a suffix with the current host.
+     *
+     * @param currentUrl the endpoint the request is currently targeting
+     * @param redirectUrl the redirect target
+     * @return true if the redirect target is trusted
+     */
+    public static boolean isTrustedRedirect(URL currentUrl, URL redirectUrl) {
+        if (!HTTPS.equalsIgnoreCase(redirectUrl.getProtocol())
+            || redirectUrl.getUserInfo() != null
+            || !isDefaultPort(redirectUrl)) {
+            return false;
+        }
+
+        String redirectHost = canonicalHost(redirectUrl);
+        if (redirectHost.isEmpty()) {
+            return false;
+        }
+
+        // A redirect back to the current host stays inside the boundary the customer already chose, which keeps
+        // custom endpoints and reverse proxies working.
+        return redirectHost.equals(canonicalHost(currentUrl)) || hasAllowedSuffix(redirectHost);
+    }
+
+    private static boolean hasAllowedSuffix(String host) {
+        for (String suffix : ALLOWED_REDIRECT_DOMAIN_SUFFIXES) {
+            if (host.endsWith(suffix)) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    private static boolean isDefaultPort(URL url) {
+        return url.getPort() == -1 || url.getPort() == url.getDefaultPort();
+    }
+
+    private static String canonicalHost(URL url) {
+        String host = url.getHost();
+        if (host == null) {
+            return "";
+        }
+        String canonicalHost = host.toLowerCase(Locale.ROOT);
+        while (canonicalHost.endsWith(".")) {
+            canonicalHost = canonicalHost.substring(0, canonicalHost.length() - 1);
+        }
+        return canonicalHost;
+    }
+
+    private RedirectPolicyHelper() {
+    }
+}

sdk/monitor/azure-monitor-opentelemetry-autoconfigure/src/test/java/com/azure/monitor/opentelemetry/autoconfigure/implementation/pipeline/TelemetryItemExporterTest.java

@@ -47,13 +47,15 @@
 
 public class TelemetryItemExporterTest {
 
-    private static final String CONNECTION_STRING
-        = "InstrumentationKey=00000000-0000-0000-0000-0FEEDDADBEEF;IngestionEndpoint=http://foo.bar";
-    private static final String REDIRECT_CONNECTION_STRING
-        = "InstrumentationKey=11111111-0000-0000-0000-0FEEDDADBEEF;IngestionEndpoint=http://foo.bar";
+    private static final String CONNECTION_STRING = "InstrumentationKey=00000000-0000-0000-0000-0FEEDDADBEEF;"
+        + "IngestionEndpoint=https://dc.services.visualstudio.com";
+    private static final String REDIRECT_CONNECTION_STRING = "InstrumentationKey=11111111-0000-0000-0000-0FEEDDADBEEF;"
+        + "IngestionEndpoint=https://dc.services.visualstudio.com";
 
     private static final String INSTRUMENTATION_KEY = "00000000-0000-0000-0000-0FEEDDADBEEF";
-    private static final String REDIRECT_URL = "http://foo.bar.redirect";
+    // stamp reassignment moves from the global host to a regional host under a different suffix
+    private static final String REDIRECT_URL = "https://westus-0.in.applicationinsights.azure.com";
+    private static final String UNTRUSTED_REDIRECT_URL = "https://attacker.invalid";
 
     RecordingHttpClient recordingHttpClient;
 
@@ -311,6 +313,27 @@ public void groupTelemetryItemsByConnectionStringAndRoleNameTest() {
         assertThat(group3.get(1).getConnectionString()).isEqualTo(REDIRECT_CONNECTION_STRING);
     }
 
+    @Test
+    public void untrustedRedirectIsNotFollowedOrCachedTest() {
+        // given
+        recordingHttpClient = new RecordingHttpClient(request -> {
+            Map<String, String> headers = new HashMap<>();
+            headers.put("Location", UNTRUSTED_REDIRECT_URL);
+            return Mono.just(new MockHttpResponse(request, 307, new HttpHeaders(headers)));
+        });
+        List<TelemetryItem> telemetryItems = new ArrayList<>();
+        telemetryItems.add(TestUtils.createMetricTelemetry("metric" + 1, 1, REDIRECT_CONNECTION_STRING));
+        TelemetryItemExporter exporter = getExporter();
+
+        // when
+        exporter.send(telemetryItems);
+        exporter.send(telemetryItems);
+
+        // then
+        // each send stops at the original endpoint instead of replaying against the untrusted host
+        assertThat(recordingHttpClient.getCount()).isEqualTo(2);
+    }
+
     static class RecordingHttpClient implements HttpClient {
 
         private final AtomicInteger count = new AtomicInteger();

sdk/monitor/azure-monitor-opentelemetry-autoconfigure/src/test/java/com/azure/monitor/opentelemetry/autoconfigure/implementation/quickpulse/QuickPulseCoordinatorTest.java

@@ -16,6 +16,7 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
 
 class QuickPulseCoordinatorTest {
     private static final long VERIFY_TIMEOUT_MILLIS = 10000;
@@ -120,6 +121,105 @@ private static void stopAndJoin(QuickPulseCoordinator coordinator, Thread thread
         assertThat(thread.isAlive()).isFalse();
     }
 
+    @Test
+    void acceptsSameLiveMetricsDomainRedirect() {
+        QuickPulseDataSender mockSender = Mockito.mock(QuickPulseDataSender.class);
+        QuickPulsePingSender mockPingSender = Mockito.mock(QuickPulsePingSender.class);
+        Mockito.doReturn("https://westus.livediagnostics.monitor.azure.com/")
+            .when(mockPingSender)
+            .getQuickPulseEndpoint();
+
+        QuickPulseCoordinator coordinator = createCoordinator(mockSender, mockPingSender);
+
+        HttpHeaders rawPingHeaders = new HttpHeaders();
+        rawPingHeaders.add(QPS_STATUS_HEADER, "true");
+        rawPingHeaders.add(QPS_SERVICE_ENDPOINT_REDIRECT,
+            "https://eastus.livediagnostics.monitor.azure.com/QuickPulseService.svc/");
+
+        assertThat(coordinator.handleReceivedPingHeaders(new IsSubscribedHeaders(rawPingHeaders)))
+            .isEqualTo(QuickPulseStatus.QP_IS_ON);
+        verify(mockSender).setRedirectEndpointPrefix("https://eastus.livediagnostics.monitor.azure.com/");
+    }
+
+    @Test
+    void acceptsSameHostRedirect() {
+        QuickPulseDataSender mockSender = Mockito.mock(QuickPulseDataSender.class);
+        QuickPulsePingSender mockPingSender = Mockito.mock(QuickPulsePingSender.class);
+        Mockito.doReturn("https://live.example.com/").when(mockPingSender).getQuickPulseEndpoint();
+
+        QuickPulseCoordinator coordinator = createCoordinator(mockSender, mockPingSender);
+
+        HttpHeaders rawPingHeaders = new HttpHeaders();
+        rawPingHeaders.add(QPS_STATUS_HEADER, "true");
+        rawPingHeaders.add(QPS_SERVICE_ENDPOINT_REDIRECT, "https://live.example.com/QuickPulseService.svc/");
+
+        assertThat(coordinator.handleReceivedPingHeaders(new IsSubscribedHeaders(rawPingHeaders)))
+            .isEqualTo(QuickPulseStatus.QP_IS_ON);
+        verify(mockSender).setRedirectEndpointPrefix("https://live.example.com/");
+    }
+
+    @Test
+    void rejectsCrossOriginRedirect() {
+        QuickPulseDataSender mockSender = Mockito.mock(QuickPulseDataSender.class);
+        QuickPulsePingSender mockPingSender = Mockito.mock(QuickPulsePingSender.class);
+        Mockito.doReturn("https://westus.livediagnostics.monitor.azure.com/")
+            .when(mockPingSender)
+            .getQuickPulseEndpoint();
+
+        QuickPulseCoordinator coordinator = createCoordinator(mockSender, mockPingSender);
+
+        HttpHeaders rawPingHeaders = new HttpHeaders();
+        rawPingHeaders.add(QPS_STATUS_HEADER, "true");
+        rawPingHeaders.add(QPS_SERVICE_ENDPOINT_REDIRECT, "https://attacker.invalid/QuickPulseService.svc/");
+
+        assertThat(coordinator.handleReceivedPingHeaders(new IsSubscribedHeaders(rawPingHeaders)))
+            .isEqualTo(QuickPulseStatus.QP_IS_ON);
+        Mockito.verify(mockSender, Mockito.never()).setRedirectEndpointPrefix(any());
+    }
+
+    @Test
+    void rejectsInvalidRedirects() {
+        assertRedirectRejected("http://eastus.livediagnostics.monitor.azure.com/QuickPulseService.svc/");
+        assertRedirectRejected("https://user@eastus.livediagnostics.monitor.azure.com/QuickPulseService.svc/");
+        assertRedirectRejected("https://eastus.livediagnostics.monitor.azure.com:444/QuickPulseService.svc/");
+        assertRedirectRejected(
+            "https://evil.livediagnostics.monitor.azure.com.attacker.invalid/QuickPulseService.svc/");
+        assertRedirectRejected("https://evil.live.example.com/QuickPulseService.svc/");
+    }
+
+    private static void assertRedirectRejected(String redirectLink) {
+        QuickPulseDataSender mockSender = Mockito.mock(QuickPulseDataSender.class);
+        QuickPulsePingSender mockPingSender = Mockito.mock(QuickPulsePingSender.class);
+        Mockito.doReturn("https://westus.livediagnostics.monitor.azure.com/")
+            .when(mockPingSender)
+            .getQuickPulseEndpoint();
+
+        QuickPulseCoordinator coordinator = createCoordinator(mockSender, mockPingSender);
+
+        HttpHeaders rawPingHeaders = new HttpHeaders();
+        rawPingHeaders.add(QPS_STATUS_HEADER, "true");
+        rawPingHeaders.add(QPS_SERVICE_ENDPOINT_REDIRECT, redirectLink);
+
+        assertThat(coordinator.handleReceivedPingHeaders(new IsSubscribedHeaders(rawPingHeaders)))
+            .isEqualTo(QuickPulseStatus.QP_IS_ON);
+        Mockito.verify(mockSender, Mockito.never()).setRedirectEndpointPrefix(any());
+    }
+
+    private static QuickPulseCoordinator createCoordinator(QuickPulseDataSender mockSender,
+        QuickPulsePingSender mockPingSender) {
+        AtomicReference<FilteringConfiguration> configuration = new AtomicReference<>(new FilteringConfiguration());
+        QuickPulseCoordinatorInitData initData
+            = new QuickPulseCoordinatorInitDataBuilder().withDataFetcher(mock(QuickPulseDataFetcher.class))
+                .withDataSender(mockSender)
+                .withPingSender(mockPingSender)
+                .withCollector(new QuickPulseDataCollector(configuration))
+                .withWaitBetweenPingsInMillis(10L)
+                .withWaitBetweenPostsInMillis(10L)
+                .withWaitOnErrorInMillis(10L)
+                .build();
+        return new QuickPulseCoordinator(initData);
+    }
+
     @Disabled("sporadically failing on CI")
     @Test
     void testOnePingAndThenOnePostWithRedirectedLink() throws InterruptedException {