Fix a bug where a tailing dot is not stripped for SNI

Motivation: A trailing dot specified in an hostname can be used to make DNS queries to avoid using search domains. However, the trailing dot should be removed for SNI. https://datatracker.ietf.org/doc/html/rfc6066#section-3 > The hostname is represented as a byte string using ASCII encoding > without a trailing dot. Modifications: - Add `Endpoint.withTrailingDot()` to remove a trailing dot if exists. - Use an `Endpoint` without a trailing dot create an remote address which is used for SNI. Result: - Fixed a bug where a trailing dot was included in the hostname used by SNI. - Closes #6044
line · Dec 23, 2024 · 21c26dc · 21c26dc
1 parent 82a1f94
commit 21c26dc
Show file tree

Hide file tree

Showing 4 changed files with 102 additions and 1 deletion.
diff --git a/core/src/main/java/com/linecorp/armeria/client/Endpoint.java b/core/src/main/java/com/linecorp/armeria/client/Endpoint.java
@@ -639,6 +639,22 @@ private Endpoint withoutIpAddr() {
         return new Endpoint(Type.HOSTNAME_ONLY, host, null, port, weight, attributes);
     }
 
+    /**
+     * Returns a new {@link Endpoint} with a host whose trailing dot is removed.
+     *
+     * @return the new endpoint with the new host whose trailing dot is removed.
+     *         {@code this} if the {@link #host()} does not end with a dot ({@code .}).
+     */
+    @UnstableApi
+    public Endpoint withoutTrailingDot() {
+        if (!hasTrailingDot(host)) {
+            return this;
+        }
+
+        final String stripped = host.substring(0, host.length() - 1);
+        return new Endpoint(type, stripped, ipAddr, port, weight, attributes);
+    }
+
     /**
      * Returns a new host endpoint with the specified weight.
      *

diff --git a/core/src/main/java/com/linecorp/armeria/client/HttpChannelPool.java b/core/src/main/java/com/linecorp/armeria/client/HttpChannelPool.java
@@ -619,7 +619,9 @@ static final class PoolKey {
         private final int hashCode;
 
         PoolKey(Endpoint endpoint, ProxyConfig proxyConfig) {
-            this.endpoint = endpoint;
+            // Remove the trailing dot of the host name because SNI does not allow it.
+            // https://lists.w3.org/Archives/Public/ietf-http-wg/2016JanMar/0430.html
+            this.endpoint = endpoint.withoutTrailingDot();
             this.proxyConfig = proxyConfig;
             hashCode = endpoint.hashCode() * 31 + proxyConfig.hashCode();
         }

diff --git a/core/src/test/java/com/linecorp/armeria/client/EndpointTest.java b/core/src/test/java/com/linecorp/armeria/client/EndpointTest.java
@@ -148,6 +148,13 @@ void hostWithTrailingDot() {
         // The trailing dot should be removed for the authority.
         assertThat(endpoint.authority()).isEqualTo("foo.com");
         assertThat(endpoint.toSocketAddress(80).getHostString()).isEqualTo("foo.com.");
+
+        final Endpoint withoutTrailingDot = endpoint.withoutTrailingDot();
+        assertThat(withoutTrailingDot.host()).isEqualTo("foo.com");
+        assertThat(withoutTrailingDot.authority()).isEqualTo("foo.com");
+        assertThat(withoutTrailingDot.toSocketAddress(80).getHostString()).isEqualTo("foo.com");
+        assertThat(withoutTrailingDot.withoutTrailingDot()).isSameAs(withoutTrailingDot);
+
         final Endpoint barEndpoint = endpoint.withHost("bar.com");
         assertThat(barEndpoint.host()).isEqualTo("bar.com");
         assertThat(barEndpoint.authority()).isEqualTo("bar.com");

diff --git a/core/src/test/java/com/linecorp/armeria/client/TrailingDotSniTest.java b/core/src/test/java/com/linecorp/armeria/client/TrailingDotSniTest.java
@@ -0,0 +1,76 @@
+/*
+ * Copyright 2024 LINE Corporation
+ *
+ * LINE Corporation licenses this file to you under the Apache License,
+ * version 2.0 (the "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at:
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+package com.linecorp.armeria.client;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.junit.jupiter.api.Order;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import com.linecorp.armeria.common.AggregatedHttpResponse;
+import com.linecorp.armeria.common.HttpResponse;
+import com.linecorp.armeria.common.HttpStatus;
+import com.linecorp.armeria.common.TlsProvider;
+import com.linecorp.armeria.internal.testing.MockAddressResolverGroup;
+import com.linecorp.armeria.server.ServerBuilder;
+import com.linecorp.armeria.testing.junit5.server.SelfSignedCertificateExtension;
+import com.linecorp.armeria.testing.junit5.server.ServerExtension;
+
+class TrailingDotSniTest {
+
+    @Order(0)
+    @RegisterExtension
+    static SelfSignedCertificateExtension ssc = new SelfSignedCertificateExtension("example.com");
+
+    @Order(1)
+    @RegisterExtension
+    static ServerExtension server = new ServerExtension() {
+        @Override
+        protected void configure(ServerBuilder sb) {
+            sb.https(0);
+            sb.tlsProvider(TlsProvider.builder()
+                                      .keyPair("example.com", ssc.tlsKeyPair())
+                                      .build());
+            sb.service("/", (ctx, req) -> {
+                return HttpResponse.of(200);
+            });
+        }
+    };
+
+    @Test
+    void shouldStripTrailingDotForSni() {
+        try (ClientFactory factory =
+                     ClientFactory.builder()
+                                  .addressResolverGroupFactory(unused -> {
+                                      return MockAddressResolverGroup.localhost();
+                                  })
+                                  .tlsCustomizer(b -> {
+                                      b.trustManager(ssc.certificate());
+                                  })
+                                  .build()) {
+
+            final BlockingWebClient client = WebClient.builder("https://example.com.:" + server.httpsPort())
+                                                      .factory(factory)
+                                                      .build()
+                                                      .blocking();
+            final AggregatedHttpResponse response = client.get("/");
+            assertThat(response.status()).isEqualTo(HttpStatus.OK);
+        }
+    }
+}