From 7acfa9452bc1747e2df73cd747ad96103c61fde3 Mon Sep 17 00:00:00 2001 From: Ikhun Um Date: Thu, 2 Jan 2025 10:26:22 +0900 Subject: [PATCH] Fix a bug where a tailing dot is not stripped for SNI (#6046) Motivation: A trailing dot specified in a 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 present. - Use an `Endpoint` without a trailing dot to create a 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 --- .../com/linecorp/armeria/client/Endpoint.java | 16 ++++ .../armeria/client/HttpChannelPool.java | 4 +- .../linecorp/armeria/client/EndpointTest.java | 7 ++ .../armeria/client/TrailingDotSniTest.java | 76 +++++++++++++++++++ 4 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 core/src/test/java/com/linecorp/armeria/client/TrailingDotSniTest.java diff --git a/core/src/main/java/com/linecorp/armeria/client/Endpoint.java b/core/src/main/java/com/linecorp/armeria/client/Endpoint.java index 500e8622ab9..c33418f6d9f 100644 --- 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 index 57e9fa13261..43a6afb0f9b 100644 --- a/core/src/main/java/com/linecorp/armeria/client/HttpChannelPool.java +++ b/core/src/main/java/com/linecorp/armeria/client/HttpChannelPool.java @@ -613,7 +613,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 index e1b6a04633f..31c815d1f80 100644 --- 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 new file mode 100644 index 00000000000..342400a0227 --- /dev/null +++ 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); + } + } +}