From 76bd13ddf0d27615a87ce913b66dc5e6e223eab6 Mon Sep 17 00:00:00 2001 From: Pavlo Tkach <3469726+ptkach@users.noreply.github.com> Date: Thu, 6 Aug 2026 19:53:11 -0400 Subject: [PATCH] Remove EppProxyProtocolHandler from the pipeline (#3202) --- .../registry/eppserver/EppProtocolModule.java | 3 - .../handler/EppProxyProtocolHandler.java | 208 ------------------ .../eppserver/handler/EppServiceHandler.java | 22 +- .../handler/EppProxyProtocolHandlerTest.java | 101 --------- .../handler/EppServiceHandlerTest.java | 73 +++++- 5 files changed, 90 insertions(+), 317 deletions(-) delete mode 100644 core/src/main/java/google/registry/eppserver/handler/EppProxyProtocolHandler.java delete mode 100644 core/src/test/java/google/registry/eppserver/handler/EppProxyProtocolHandlerTest.java diff --git a/core/src/main/java/google/registry/eppserver/EppProtocolModule.java b/core/src/main/java/google/registry/eppserver/EppProtocolModule.java index 97c09d098..9529117ec 100644 --- a/core/src/main/java/google/registry/eppserver/EppProtocolModule.java +++ b/core/src/main/java/google/registry/eppserver/EppProtocolModule.java @@ -23,7 +23,6 @@ import dagger.multibindings.IntoSet; import google.registry.config.RegistryConfig.Config; import google.registry.config.RegistryConfigSettings; import google.registry.eppserver.Protocol.FrontendProtocol; -import google.registry.eppserver.handler.EppProxyProtocolHandler; import google.registry.eppserver.handler.EppServiceHandler; import google.registry.eppserver.quota.QuotaManager; import google.registry.networking.handler.SslServerInitializer; @@ -78,14 +77,12 @@ public final class EppProtocolModule { @Provides @EppProtocol static ImmutableList> provideHandlerProviders( - Provider proxyProtocolHandlerProvider, @EppProtocol Provider> sslServerInitializerProvider, @EppProtocol Provider readTimeoutHandlerProvider, Provider lengthFieldBasedFrameDecoderProvider, Provider lengthFieldPrependerProvider, Provider eppServiceHandlerProvider) { return ImmutableList.of( - proxyProtocolHandlerProvider, sslServerInitializerProvider, readTimeoutHandlerProvider, lengthFieldBasedFrameDecoderProvider, diff --git a/core/src/main/java/google/registry/eppserver/handler/EppProxyProtocolHandler.java b/core/src/main/java/google/registry/eppserver/handler/EppProxyProtocolHandler.java deleted file mode 100644 index b1b5f86d8..000000000 --- a/core/src/main/java/google/registry/eppserver/handler/EppProxyProtocolHandler.java +++ /dev/null @@ -1,208 +0,0 @@ -// Copyright 2017 The Nomulus Authors. All Rights Reserved. -// -// Licensed 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 -// -// http://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 google.registry.eppserver.handler; - -import static com.google.common.base.Preconditions.checkState; -import static java.nio.charset.StandardCharsets.US_ASCII; - -import com.google.common.flogger.FluentLogger; -import com.google.common.net.InetAddresses; -import io.netty.buffer.ByteBuf; -import io.netty.channel.ChannelHandlerContext; -import io.netty.handler.codec.ByteToMessageDecoder; -import io.netty.util.AttributeKey; -import jakarta.inject.Inject; -import java.net.InetSocketAddress; -import java.net.SocketAddress; -import java.util.List; - -/** - * Handler that processes possible existence of a PROXY protocol v1 header. - * - *

When an EPP client connects to the registry (through the proxy), the registry performs two - * validations to ensure that only known registrars are allowed. First it checks the sha265 hash of - * the client SSL certificate and match it to the hash stored in the database for the registrar. It - * then checks if the connection is from an allow-listed IP address that belongs to that registrar. - * - *

The proxy receives client connects via the GCP load balancer, which results in the loss of - * original client IP from the channel. Luckily, the load balancer supports the PROXY protocol v1, - * which adds a header with source IP information, among other things, to the TCP request at the - * start of the connection. - * - *

This handler determines if a connection is proxied (PROXY protocol v1 header present) and - * correctly sets the source IP address to the channel's attribute regardless of whether it is - * proxied. After that it removes itself from the channel pipeline because the proxy header is only - * present at the beginning of the connection. - * - *

This handler must be the very first handler in a protocol, even before SSL handlers, because - * PROXY protocol header comes as the very first thing, even before SSL handshake request. - * - * @see The PROXY protocol - */ -public class EppProxyProtocolHandler extends ByteToMessageDecoder { - - /** Key used to retrieve origin IP address from a channel's attribute. */ - public static final AttributeKey REMOTE_ADDRESS_KEY = - AttributeKey.valueOf("REMOTE_ADDRESS_KEY"); - - private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - - // The proxy header must start with this prefix. - // Sample header: "PROXY TCP4 255.255.255.255 255.255.255.255 65535 65535\r\n". - private static final byte[] HEADER_PREFIX = "PROXY".getBytes(US_ASCII); - - private boolean finished = false; - private String proxyHeader = null; - - @Inject - EppProxyProtocolHandler() {} - - @Override - public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception { - super.channelRead(ctx, msg); - if (finished) { - String remoteIP; - if (proxyHeader != null) { - logger.atFine().log("PROXIED CONNECTION: %s", ctx.channel()); - logger.atFine().log("PROXY HEADER for channel %s: %s", ctx.channel(), proxyHeader); - String[] headerArray = proxyHeader.split(" ", -1); - if (headerArray.length == 6) { - String parsedIP = headerArray[2]; - if (InetAddresses.isInetAddress(parsedIP)) { - remoteIP = parsedIP; - logger.atFine().log( - "Header parsed, using %s as remote IP for channel %s", remoteIP, ctx.channel()); - } else { - logger.atWarning().log( - "Invalid IP address in PROXY header: %s, falling back to source IP for channel %s", - parsedIP, ctx.channel()); - remoteIP = getSourceIP(ctx); - } - // If the header is "PROXY UNKNOWN" - // (see https://www.haproxy.org/download/1.8/doc/proxy-protocol.txt), likely when the - // remote connection to the external load balancer is through special means, make it - // 0.0.0.0 so that it can be treated accordingly by the relevant quota configs. - } else if (headerArray.length == 2 && headerArray[1].equals("UNKNOWN")) { - logger.atFine().log( - "Header parsed, source IP unknown, using 0.0.0.0 as remote IP for channel %s", - ctx.channel()); - remoteIP = "0.0.0.0"; - } else { - logger.atFine().log( - "Cannot parse the header, using source IP as remote IP for channel %s", - ctx.channel()); - remoteIP = getSourceIP(ctx); - } - } else { - logger.atFine().log( - "No header present, using source IP directly for channel %s", ctx.channel()); - remoteIP = getSourceIP(ctx); - } - if (remoteIP != null) { - ctx.channel().attr(REMOTE_ADDRESS_KEY).set(remoteIP); - } else { - logger.atWarning().log("Not able to obtain remote IP for channel %s", ctx.channel()); - } - // ByteToMessageDecoder automatically flushes unread bytes in the ByteBuf to the next handler - // when itself is being removed. - ctx.pipeline().remove(this); - } - } - - private static String getSourceIP(ChannelHandlerContext ctx) { - SocketAddress remoteAddress = ctx.channel().remoteAddress(); - return (remoteAddress instanceof InetSocketAddress inetSocketAddress) - ? inetSocketAddress.getAddress().getHostAddress() - : null; - } - - /** - * Attempts to decode an internally accumulated buffer and find the proxy protocol header. - * - *

When the connection is not proxied (i. e. the initial bytes are not "PROXY"), simply set - * {@link #finished} to true and allow the handler to be removed. Otherwise the handler waits - * until there's enough bytes to parse the header, save the parsed header to {@link #proxyHeader}, - * and then mark {@link #finished}. - * - * @param in internally accumulated buffer, newly arrived bytes are appended to it. - * @param out objects passed to the next handler, in this case nothing is ever passed because the - * header itself is processed and written to the attribute of the proxy, and the handler is - * then removed from the pipeline. - */ - @Override - protected void decode(ChannelHandlerContext ctx, ByteBuf in, List out) { - // Wait until there are more bytes available than the header's length before processing. - if (in.readableBytes() >= HEADER_PREFIX.length) { - if (containsHeader(in)) { - // The inbound message contains the header, it must be a proxied connection. Note that - // currently proxied connection is only used for EPP protocol, which requires the connection - // to be SSL enabled. So the beginning of the inbound message upon connection can only be - // either the proxy header (when proxied), or SSL handshake request (when not proxied), - // which does not start with "PROXY". Therefore it is safe to assume that if the beginning - // of the message contains "PROXY", it must be proxied, and must contain \r\n. - int eol = findEndOfLine(in); - // If eol is not found, that is because that we do not yet have enough inbound message, do - // nothing and wait for more bytes to be readable. eol will eventually be positive because - // of the reasoning above: The connection starts with "PROXY", so it must be a proxied - // connection and contain \r\n. - if (eol >= 0) { - // ByteBuf.readBytes is called so that the header is processed and not passed to handlers - // further in the pipeline. - byte[] headerBytes = new byte[eol]; - in.readBytes(headerBytes); - proxyHeader = new String(headerBytes, US_ASCII); - // Skip \r\n. - in.skipBytes(2); - // Proxy header processed, mark finished so that this handler is removed. - finished = true; - } - } else { - // The inbound message does not contain a proxy header, mark finished so that this handler - // is removed. Note that no inbound bytes are actually processed by this handler because we - // did not call ByteBuf.readBytes(), but ByteBuf.getByte(), which does not change reader - // index of the ByteBuf. So any inbound byte is then passed to the next handler to process. - finished = true; - } - } - } - - /** - * Returns the index in the buffer of the end of line found. Returns -1 if no end of line was - * found in the buffer. - */ - private static int findEndOfLine(final ByteBuf buffer) { - final int n = buffer.writerIndex(); - for (int i = buffer.readerIndex(); i < n; i++) { - final byte b = buffer.getByte(i); - if (b == '\r' && i < n - 1 && buffer.getByte(i + 1) == '\n') { - return i; // \r\n - } - } - return -1; // Not found. - } - - /** Checks if the given buffer contains the proxy header prefix. */ - private boolean containsHeader(ByteBuf buffer) { - // The readable bytes is always more or equal to the size of the header prefix because this - // method is only called when this condition is true. - checkState(buffer.readableBytes() >= HEADER_PREFIX.length); - for (int i = 0; i < HEADER_PREFIX.length; ++i) { - if (buffer.getByte(buffer.readerIndex() + i) != HEADER_PREFIX[i]) { - return false; - } - } - return true; - } -} diff --git a/core/src/main/java/google/registry/eppserver/handler/EppServiceHandler.java b/core/src/main/java/google/registry/eppserver/handler/EppServiceHandler.java index e9cfd3b1b..b4e51915a 100644 --- a/core/src/main/java/google/registry/eppserver/handler/EppServiceHandler.java +++ b/core/src/main/java/google/registry/eppserver/handler/EppServiceHandler.java @@ -14,7 +14,6 @@ package google.registry.eppserver.handler; -import static google.registry.eppserver.handler.EppProxyProtocolHandler.REMOTE_ADDRESS_KEY; import static google.registry.networking.handler.SslServerInitializer.CLIENT_CERTIFICATE_PROMISE_KEY; import static google.registry.util.GcpJsonFormatter.setCurrentRequest; import static google.registry.util.GcpJsonFormatter.setCurrentTraceId; @@ -45,6 +44,8 @@ import io.netty.util.concurrent.Promise; import io.netty.util.concurrent.ScheduledFuture; import jakarta.inject.Inject; import jakarta.inject.Named; +import java.net.InetSocketAddress; +import java.net.SocketAddress; import java.security.cert.X509Certificate; import java.util.UUID; import java.util.concurrent.TimeUnit; @@ -69,6 +70,9 @@ public class EppServiceHandler extends SimpleChannelInboundHandler { public static final AttributeKey CLIENT_CERTIFICATE_HASH_KEY = AttributeKey.valueOf("CLIENT_CERTIFICATE_HASH_KEY"); + public static final AttributeKey REMOTE_ADDRESS_KEY = + AttributeKey.valueOf("REMOTE_ADDRESS_KEY"); + private final byte[] helloBytes; private final FrontendMetrics metrics; private final LocalConnectionLimiter localConnectionLimiter; @@ -128,8 +132,22 @@ public class EppServiceHandler extends SimpleChannelInboundHandler { if (!ctx.channel().isActive()) { return; } - sslClientCertificateHash = getCertificateHash(cert); + clientAddress = ctx.channel().attr(REMOTE_ADDRESS_KEY).get(); + if (clientAddress == null) { + SocketAddress remoteAddress = ctx.channel().remoteAddress(); + if (remoteAddress instanceof InetSocketAddress inetSocketAddress) { + clientAddress = inetSocketAddress.getAddress().getHostAddress(); + } + } + + if (clientAddress == null || clientAddress.isEmpty()) { + logger.atSevere().log("Failed to resolve client IP address, closing connection"); + closeConnection(ctx); + return; + } + + sslClientCertificateHash = getCertificateHash(cert); ctx.channel().attr(CLIENT_CERTIFICATE_HASH_KEY).set(sslClientCertificateHash); // 1. Connection throttling (IP only pre-login) diff --git a/core/src/test/java/google/registry/eppserver/handler/EppProxyProtocolHandlerTest.java b/core/src/test/java/google/registry/eppserver/handler/EppProxyProtocolHandlerTest.java deleted file mode 100644 index ef6e64be2..000000000 --- a/core/src/test/java/google/registry/eppserver/handler/EppProxyProtocolHandlerTest.java +++ /dev/null @@ -1,101 +0,0 @@ -// Copyright 2024 The Nomulus Authors. All Rights Reserved. -// -// Licensed 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 -// -// http://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 google.registry.eppserver.handler; - -import static com.google.common.truth.Truth.assertThat; - -import io.netty.buffer.ByteBuf; -import io.netty.buffer.Unpooled; -import io.netty.channel.embedded.EmbeddedChannel; -import java.net.InetAddress; -import java.net.InetSocketAddress; -import java.net.SocketAddress; -import java.nio.charset.StandardCharsets; -import org.junit.jupiter.api.Test; - -class EppProxyProtocolHandlerTest { - - private EmbeddedChannel createChannel(EppProxyProtocolHandler handler) { - return new EmbeddedChannel(handler) { - @Override - public SocketAddress remoteAddress() { - return new InetSocketAddress(InetAddress.getLoopbackAddress(), 12345); - } - }; - } - - @Test - void testProxyProtocol_parsesValidHeader() { - EppProxyProtocolHandler handler = new EppProxyProtocolHandler(); - EmbeddedChannel channel = new EmbeddedChannel(handler); - - String proxyHeader = "PROXY TCP4 192.168.1.1 10.0.0.1 50000 443\r\n"; - ByteBuf buffer = Unpooled.wrappedBuffer(proxyHeader.getBytes(StandardCharsets.US_ASCII)); - - channel.writeInbound(buffer); - - String remoteAddress = channel.attr(EppProxyProtocolHandler.REMOTE_ADDRESS_KEY).get(); - assertThat(remoteAddress).isEqualTo("192.168.1.1"); - assertThat(channel.pipeline().get(EppProxyProtocolHandler.class)).isNull(); - } - - @Test - void testProxyProtocol_invalidIP_fallsBackToSource() { - EppProxyProtocolHandler handler = new EppProxyProtocolHandler(); - EmbeddedChannel channel = createChannel(handler); - - String proxyHeader = "PROXY TCP4 invalid_ip_address 10.0.0.1 50000 443\r\n"; - ByteBuf buffer = Unpooled.wrappedBuffer(proxyHeader.getBytes(StandardCharsets.US_ASCII)); - - channel.writeInbound(buffer); - - String remoteAddress = channel.attr(EppProxyProtocolHandler.REMOTE_ADDRESS_KEY).get(); - assertThat(remoteAddress).isEqualTo("127.0.0.1"); // Falls back to mocked remoteAddress - assertThat(channel.pipeline().get(EppProxyProtocolHandler.class)).isNull(); - } - - @Test - void testProxyProtocol_unknownHeader() { - EppProxyProtocolHandler handler = new EppProxyProtocolHandler(); - EmbeddedChannel channel = new EmbeddedChannel(handler); - - String proxyHeader = "PROXY UNKNOWN\r\n"; - ByteBuf buffer = Unpooled.wrappedBuffer(proxyHeader.getBytes(StandardCharsets.US_ASCII)); - - channel.writeInbound(buffer); - - String remoteAddress = channel.attr(EppProxyProtocolHandler.REMOTE_ADDRESS_KEY).get(); - assertThat(remoteAddress).isEqualTo("0.0.0.0"); - assertThat(channel.pipeline().get(EppProxyProtocolHandler.class)).isNull(); - } - - @Test - void testProxyProtocol_noHeader_notProxied() { - EppProxyProtocolHandler handler = new EppProxyProtocolHandler(); - EmbeddedChannel channel = createChannel(handler); - - String normalData = "NOT_A_PROXY_HEADER"; - ByteBuf buffer = Unpooled.wrappedBuffer(normalData.getBytes(StandardCharsets.US_ASCII)); - - channel.writeInbound(buffer); - - String remoteAddress = channel.attr(EppProxyProtocolHandler.REMOTE_ADDRESS_KEY).get(); - assertThat(remoteAddress).isEqualTo("127.0.0.1"); // Falls back to mocked remoteAddress - assertThat(channel.pipeline().get(EppProxyProtocolHandler.class)).isNull(); - - ByteBuf passedOn = channel.readInbound(); - assertThat(passedOn.toString(StandardCharsets.US_ASCII)).isEqualTo("NOT_A_PROXY_HEADER"); - } -} diff --git a/core/src/test/java/google/registry/eppserver/handler/EppServiceHandlerTest.java b/core/src/test/java/google/registry/eppserver/handler/EppServiceHandlerTest.java index 97e401dc0..520e9e5f6 100644 --- a/core/src/test/java/google/registry/eppserver/handler/EppServiceHandlerTest.java +++ b/core/src/test/java/google/registry/eppserver/handler/EppServiceHandlerTest.java @@ -14,10 +14,10 @@ package google.registry.eppserver.handler; -import static google.registry.eppserver.handler.EppProxyProtocolHandler.REMOTE_ADDRESS_KEY; +import static com.google.common.truth.Truth.assertThat; +import static google.registry.eppserver.handler.EppServiceHandler.REMOTE_ADDRESS_KEY; import static google.registry.networking.handler.SslServerInitializer.CLIENT_CERTIFICATE_PROMISE_KEY; import static java.nio.charset.StandardCharsets.UTF_8; -import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.argThat; @@ -51,6 +51,8 @@ import io.netty.util.concurrent.EventExecutor; import io.netty.util.concurrent.ImmediateEventExecutor; import io.netty.util.concurrent.Promise; import io.netty.util.concurrent.ScheduledFuture; +import java.net.InetSocketAddress; +import java.net.SocketAddress; import java.security.cert.X509Certificate; import java.util.concurrent.TimeUnit; import java.util.function.Supplier; @@ -382,7 +384,7 @@ class EppServiceHandlerTest { setUpSuccessfulHandshake(); Runnable timeoutTask = timeoutTaskCaptor.getValue(); - assertNotNull(timeoutTask); + assertThat(timeoutTask).isNotNull(); ChannelFuture closeFuture = mock(ChannelFuture.class); when(ctx.close()).thenReturn(closeFuture); @@ -392,4 +394,69 @@ class EppServiceHandlerTest { verify(metrics).registerQuotaRejection("epp_login_timeout", "192.168.1.1"); verify(ctx).close(); } + + @Test + void testFallbackToSocketIpAddress_whenRemoteAddressKeyIsNull() throws Exception { + certPromise = new DefaultPromise<>(ImmediateEventExecutor.INSTANCE); + when(channel.attr(CLIENT_CERTIFICATE_PROMISE_KEY)).thenReturn(certPromiseAttr); + when(certPromiseAttr.get()).thenReturn(certPromise); + + handler.channelActive(ctx); + + // Mock REMOTE_ADDRESS_KEY to return null + when(channel.attr(REMOTE_ADDRESS_KEY)).thenReturn(remoteAddressAttr); + when(remoteAddressAttr.get()).thenReturn(null); + + // Mock channel.remoteAddress() to return a socket address + InetSocketAddress socketAddress = new InetSocketAddress("10.0.0.1", 12345); + when(channel.remoteAddress()).thenReturn(socketAddress); + + when(channel.attr(EppServiceHandler.CLIENT_CERTIFICATE_HASH_KEY)).thenReturn(certHashAttr); + when(certificate.getEncoded()).thenReturn(new byte[] {1, 2, 3}); + + when(localConnectionLimiter.acquireIp("10.0.0.1")).thenReturn(true); + when(idTokenSupplier.get()).thenReturn("fake_id_token"); + + // Stub request handler to capture the request + ArgumentCaptor requestCaptor = + ArgumentCaptor.forClass(FakeHttpServletRequest.class); + doAnswer( + invocation -> { + FakeHttpServletResponse rsp = invocation.getArgument(1); + rsp.getWriter().write(""); + return null; + }) + .when(requestHandler) + .handleRequest(requestCaptor.capture(), any(FakeHttpServletResponse.class)); + + certPromise.setSuccess(certificate); + + // Verify it resolved IP to 10.0.0.1 + verify(localConnectionLimiter).acquireIp("10.0.0.1"); + FakeHttpServletRequest capturedRequest = requestCaptor.getValue(); + assertThat(capturedRequest).isNotNull(); + assertThat(capturedRequest.getHeader(ProxyHttpHeaders.IP_ADDRESS)).isEqualTo("10.0.0.1"); + } + + @Test + void testFallbackToSocketIpAddress_fails_closesConnection() throws Exception { + certPromise = new DefaultPromise<>(ImmediateEventExecutor.INSTANCE); + when(channel.attr(CLIENT_CERTIFICATE_PROMISE_KEY)).thenReturn(certPromiseAttr); + when(certPromiseAttr.get()).thenReturn(certPromise); + + handler.channelActive(ctx); + + // Mock REMOTE_ADDRESS_KEY to return null + when(channel.attr(REMOTE_ADDRESS_KEY)).thenReturn(remoteAddressAttr); + when(remoteAddressAttr.get()).thenReturn(null); + + // Mock channel.remoteAddress() to return a non-InetSocketAddress (e.g. mock SocketAddress) + SocketAddress mockSocketAddress = mock(SocketAddress.class); + when(channel.remoteAddress()).thenReturn(mockSocketAddress); + + certPromise.setSuccess(certificate); + + // Verify it logged error and closed connection + verify(ctx).close(); + } }