From 3f6a796aaf815abe28f8adc013751bcdab419958 Mon Sep 17 00:00:00 2001 From: sarahcaseybot Date: Thu, 11 Feb 2021 12:02:51 -0500 Subject: [PATCH] Add string constants for HTTP header names (#956) * Add string constants for HTTP header names * revert package-lock changes * Clarify names * add CONTENT_TYPE * Fix formatting * Move X-FORWARDED-FOR to ProxyHttpHeaders --- .../registry/flows/EppRequestHandler.java | 5 +- .../google/registry/flows/TlsCredentials.java | 21 ++++---- .../google/registry/flows/EppTestCase.java | 6 ++- .../registry/flows/TlsCredentialsTest.java | 5 +- .../proxy/handler/EppServiceHandler.java | 26 +++------- .../java/google/registry/proxy/TestUtils.java | 14 +++--- .../proxy/handler/EppServiceHandlerTest.java | 9 ++-- .../registry/util/ProxyHttpHeaders.java | 48 +++++++++++++++++++ 8 files changed, 88 insertions(+), 46 deletions(-) create mode 100644 util/src/main/java/google/registry/util/ProxyHttpHeaders.java diff --git a/core/src/main/java/google/registry/flows/EppRequestHandler.java b/core/src/main/java/google/registry/flows/EppRequestHandler.java index 1bcae2f28..08ff98e2a 100644 --- a/core/src/main/java/google/registry/flows/EppRequestHandler.java +++ b/core/src/main/java/google/registry/flows/EppRequestHandler.java @@ -25,6 +25,7 @@ import com.google.common.flogger.FluentLogger; import com.google.common.net.MediaType; import google.registry.model.eppoutput.EppOutput; import google.registry.request.Response; +import google.registry.util.ProxyHttpHeaders; import javax.inject.Inject; /** Handle an EPP request and response. */ @@ -72,7 +73,7 @@ public class EppRequestHandler { // See: https://tools.ietf.org/html/rfc5734#section-2 if (eppOutput.isResponse() && eppOutput.getResponse().getResult().getCode() == SUCCESS_AND_CLOSE) { - response.setHeader("Epp-Session", "close"); + response.setHeader(ProxyHttpHeaders.EPP_SESSION, "close"); } // If a login request returns a success, a logged-in header is added to the response to inform // the proxy that it is no longer necessary to send the full client certificate to the backend @@ -80,7 +81,7 @@ public class EppRequestHandler { if (eppOutput.isResponse() && eppOutput.getResponse().isLoginResponse() && eppOutput.isSuccess()) { - response.setHeader("Logged-In", "true"); + response.setHeader(ProxyHttpHeaders.LOGGED_IN, "true"); } } catch (Exception e) { logger.atWarning().withCause(e).log("handleEppCommand general exception"); diff --git a/core/src/main/java/google/registry/flows/TlsCredentials.java b/core/src/main/java/google/registry/flows/TlsCredentials.java index 180534761..dcd573faa 100644 --- a/core/src/main/java/google/registry/flows/TlsCredentials.java +++ b/core/src/main/java/google/registry/flows/TlsCredentials.java @@ -34,6 +34,7 @@ import google.registry.model.registrar.Registrar; import google.registry.request.Header; import google.registry.util.CidrAddressBlock; import google.registry.util.Clock; +import google.registry.util.ProxyHttpHeaders; import java.io.ByteArrayInputStream; import java.net.InetAddress; import java.security.cert.CertificateException; @@ -78,9 +79,9 @@ public class TlsCredentials implements TransportCredentials { @Inject public TlsCredentials( @Config("requireSslCertificates") boolean requireSslCertificates, - @Header("X-SSL-Certificate") Optional clientCertificateHash, - @Header("X-SSL-Full-Certificate") Optional clientCertificate, - @Header("X-Forwarded-For") Optional clientAddress, + @Header(ProxyHttpHeaders.CERTIFICATE_HASH) Optional clientCertificateHash, + @Header(ProxyHttpHeaders.FULL_CERTIFICATE) Optional clientCertificate, + @Header(ProxyHttpHeaders.IP_ADDRESS) Optional clientAddress, CertificateChecker certificateChecker, Clock clock) { this.requireSslCertificates = requireSslCertificates; @@ -328,25 +329,25 @@ public class TlsCredentials implements TransportCredentials { public static final class EppTlsModule { @Provides - @Header("X-SSL-Certificate") + @Header(ProxyHttpHeaders.CERTIFICATE_HASH) static Optional provideClientCertificateHash(HttpServletRequest req) { // Note: This header is actually required, we just want to handle its absence explicitly // by throwing an EPP exception rather than a generic Bad Request exception. - return extractOptionalHeader(req, "X-SSL-Certificate"); + return extractOptionalHeader(req, ProxyHttpHeaders.CERTIFICATE_HASH); } @Provides - @Header("X-SSL-Full-Certificate") + @Header(ProxyHttpHeaders.FULL_CERTIFICATE) static Optional provideClientCertificate(HttpServletRequest req) { // Note: This header is actually required, we just want to handle its absence explicitly // by throwing an EPP exception rather than a generic Bad Request exception. - return extractOptionalHeader(req, "X-SSL-Full-Certificate"); + return extractOptionalHeader(req, ProxyHttpHeaders.FULL_CERTIFICATE); } @Provides - @Header("X-Forwarded-For") - static Optional provideForwardedFor(HttpServletRequest req) { - return extractOptionalHeader(req, "X-Forwarded-For"); + @Header(ProxyHttpHeaders.IP_ADDRESS) + static Optional provideIpAddress(HttpServletRequest req) { + return extractOptionalHeader(req, ProxyHttpHeaders.IP_ADDRESS); } } } diff --git a/core/src/test/java/google/registry/flows/EppTestCase.java b/core/src/test/java/google/registry/flows/EppTestCase.java index a32344f6d..9da13d4dc 100644 --- a/core/src/test/java/google/registry/flows/EppTestCase.java +++ b/core/src/test/java/google/registry/flows/EppTestCase.java @@ -46,6 +46,7 @@ import google.registry.testing.FakeClock; import google.registry.testing.FakeHttpSession; import google.registry.testing.FakeResponse; import google.registry.testing.InjectExtension; +import google.registry.util.ProxyHttpHeaders; import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -162,7 +163,8 @@ public class EppTestCase { FakeResponse response = executeXmlCommand(input); // Check that the logged-in header was added to the response - assertThat(response.getHeaders()).isEqualTo(ImmutableMap.of("Logged-In", "true")); + assertThat(response.getHeaders()) + .isEqualTo(ImmutableMap.of(ProxyHttpHeaders.LOGGED_IN, "true")); return verifyAndReturnOutput( response.getPayload(), expectedOutput, inputFilename, outputFilename); @@ -183,7 +185,7 @@ public class EppTestCase { // Checks that the Logged-In header is not in the response. If testing the login command, use // assertLoginCommandAndResponse instead of this method. - assertThat(response.getHeaders()).doesNotContainEntry("Logged-In", "true"); + assertThat(response.getHeaders()).doesNotContainEntry(ProxyHttpHeaders.LOGGED_IN, "true"); return verifyAndReturnOutput( response.getPayload(), expectedOutput, inputFilename, outputFilename); diff --git a/core/src/test/java/google/registry/flows/TlsCredentialsTest.java b/core/src/test/java/google/registry/flows/TlsCredentialsTest.java index 720fba483..d69eeece4 100644 --- a/core/src/test/java/google/registry/flows/TlsCredentialsTest.java +++ b/core/src/test/java/google/registry/flows/TlsCredentialsTest.java @@ -33,6 +33,7 @@ import google.registry.model.registrar.Registrar; import google.registry.testing.AppEngineExtension; import google.registry.testing.FakeClock; import google.registry.util.CidrAddressBlock; +import google.registry.util.ProxyHttpHeaders; import java.util.Optional; import javax.servlet.http.HttpServletRequest; import org.joda.time.DateTime; @@ -59,7 +60,7 @@ final class TlsCredentialsTest { @Test void testProvideClientCertificateHash() { HttpServletRequest req = mock(HttpServletRequest.class); - when(req.getHeader("X-SSL-Certificate")).thenReturn("data"); + when(req.getHeader(ProxyHttpHeaders.CERTIFICATE_HASH)).thenReturn("data"); assertThat(TlsCredentials.EppTlsModule.provideClientCertificateHash(req)).hasValue("data"); } @@ -128,7 +129,7 @@ final class TlsCredentialsTest { @Test void testProvideClientCertificate() { HttpServletRequest req = mock(HttpServletRequest.class); - when(req.getHeader("X-SSL-Full-Certificate")).thenReturn("data"); + when(req.getHeader(ProxyHttpHeaders.FULL_CERTIFICATE)).thenReturn("data"); assertThat(TlsCredentials.EppTlsModule.provideClientCertificate(req)).hasValue("data"); } diff --git a/proxy/src/main/java/google/registry/proxy/handler/EppServiceHandler.java b/proxy/src/main/java/google/registry/proxy/handler/EppServiceHandler.java index ebb624800..1a7d7da1b 100644 --- a/proxy/src/main/java/google/registry/proxy/handler/EppServiceHandler.java +++ b/proxy/src/main/java/google/registry/proxy/handler/EppServiceHandler.java @@ -22,6 +22,7 @@ import static google.registry.util.X509Utils.getCertificateHash; import com.google.common.flogger.FluentLogger; import google.registry.proxy.metric.FrontendMetrics; +import google.registry.util.ProxyHttpHeaders; import io.netty.buffer.ByteBuf; import io.netty.buffer.Unpooled; import io.netty.channel.ChannelFuture; @@ -51,21 +52,6 @@ public class EppServiceHandler extends HttpsRelayServiceHandler { public static final AttributeKey CLIENT_CERTIFICATE_HASH_KEY = AttributeKey.valueOf("CLIENT_CERTIFICATE_HASH_KEY"); - /** Name of the HTTP header that stores the client certificate hash. */ - public static final String SSL_CLIENT_CERTIFICATE_HASH_FIELD = "X-SSL-Certificate"; - - /** Name of the HTTP header that stores the full client certificate. */ - public static final String SSL_CLIENT_FULL_CERTIFICATE_FIELD = "X-SSL-Full-Certificate"; - - /** Name of the HTTP header that stores the client IP address. */ - public static final String FORWARDED_FOR_FIELD = "X-Forwarded-For"; - - /** Name of the HTTP header that indicates if the EPP session should be closed. */ - public static final String EPP_SESSION_FIELD = "Epp-Session"; - - /** Name of the HTTP header that indicates a successful login has occurred. */ - public static final String EPP_LOGGED_IN_FIELD = "Logged-In"; - public static final String EPP_CONTENT_TYPE = "application/epp+xml"; private final byte[] helloBytes; @@ -139,8 +125,8 @@ public class EppServiceHandler extends HttpsRelayServiceHandler { FullHttpRequest request = super.decodeFullHttpRequest(byteBuf); request .headers() - .set(SSL_CLIENT_CERTIFICATE_HASH_FIELD, sslClientCertificateHash) - .set(FORWARDED_FOR_FIELD, clientAddress) + .set(ProxyHttpHeaders.CERTIFICATE_HASH, sslClientCertificateHash) + .set(ProxyHttpHeaders.IP_ADDRESS, clientAddress) .set(HttpHeaderNames.CONTENT_TYPE, EPP_CONTENT_TYPE) .set(HttpHeaderNames.ACCEPT, EPP_CONTENT_TYPE); if (!isLoggedIn) { @@ -148,7 +134,7 @@ public class EppServiceHandler extends HttpsRelayServiceHandler { request .headers() .set( - SSL_CLIENT_FULL_CERTIFICATE_FIELD, + ProxyHttpHeaders.FULL_CERTIFICATE, Base64.getEncoder().encodeToString(sslClientCertificate.getEncoded())); } catch (CertificateEncodingException e) { throw new RuntimeException("Cannot encode client certificate", e); @@ -162,8 +148,8 @@ public class EppServiceHandler extends HttpsRelayServiceHandler { throws Exception { checkArgument(msg instanceof HttpResponse); HttpResponse response = (HttpResponse) msg; - String sessionAliveValue = response.headers().get(EPP_SESSION_FIELD); - String loginValue = response.headers().get(EPP_LOGGED_IN_FIELD); + String sessionAliveValue = response.headers().get(ProxyHttpHeaders.EPP_SESSION); + String loginValue = response.headers().get(ProxyHttpHeaders.LOGGED_IN); if (sessionAliveValue != null && sessionAliveValue.equals("close")) { promise.addListener(ChannelFutureListener.CLOSE); } diff --git a/proxy/src/test/java/google/registry/proxy/TestUtils.java b/proxy/src/test/java/google/registry/proxy/TestUtils.java index 831f7670a..036b24747 100644 --- a/proxy/src/test/java/google/registry/proxy/TestUtils.java +++ b/proxy/src/test/java/google/registry/proxy/TestUtils.java @@ -17,6 +17,7 @@ package google.registry.proxy; import static com.google.common.truth.Truth.assertThat; import static java.nio.charset.StandardCharsets.US_ASCII; +import google.registry.util.ProxyHttpHeaders; import io.netty.buffer.ByteBuf; import io.netty.buffer.Unpooled; import io.netty.handler.codec.http.DefaultFullHttpRequest; @@ -24,6 +25,7 @@ import io.netty.handler.codec.http.DefaultFullHttpResponse; import io.netty.handler.codec.http.FullHttpMessage; import io.netty.handler.codec.http.FullHttpRequest; import io.netty.handler.codec.http.FullHttpResponse; +import io.netty.handler.codec.http.HttpHeaderNames; import io.netty.handler.codec.http.HttpMessage; import io.netty.handler.codec.http.HttpMethod; import io.netty.handler.codec.http.HttpRequest; @@ -122,7 +124,7 @@ public class TestUtils { request .headers() .set("authorization", "Bearer " + accessToken) - .set("content-type", "text/plain") + .set(HttpHeaderNames.CONTENT_TYPE, "text/plain") .set("accept", "text/plain"); return request; } @@ -139,10 +141,10 @@ public class TestUtils { request .headers() .set("authorization", "Bearer " + accessToken) - .set("content-type", "application/epp+xml") + .set(HttpHeaderNames.CONTENT_TYPE, "application/epp+xml") .set("accept", "application/epp+xml") - .set("X-SSL-Certificate", sslClientCertificateHash) - .set("X-Forwarded-For", clientAddress); + .set(ProxyHttpHeaders.CERTIFICATE_HASH, sslClientCertificateHash) + .set(ProxyHttpHeaders.IP_ADDRESS, clientAddress); if (cookies.length != 0) { request.headers().set("cookie", ClientCookieEncoder.STRICT.encode(cookies)); } @@ -166,14 +168,14 @@ public class TestUtils { public static FullHttpResponse makeWhoisHttpResponse(String content, HttpResponseStatus status) { FullHttpResponse response = makeHttpResponse(content, status); - response.headers().set("content-type", "text/plain"); + response.headers().set(HttpHeaderNames.CONTENT_TYPE, "text/plain"); return response; } public static FullHttpResponse makeEppHttpResponse( String content, HttpResponseStatus status, Cookie... cookies) { FullHttpResponse response = makeHttpResponse(content, status); - response.headers().set("content-type", "application/epp+xml"); + response.headers().set(HttpHeaderNames.CONTENT_TYPE, "application/epp+xml"); for (Cookie cookie : cookies) { response.headers().add("set-cookie", ServerCookieEncoder.STRICT.encode(cookie)); } diff --git a/proxy/src/test/java/google/registry/proxy/handler/EppServiceHandlerTest.java b/proxy/src/test/java/google/registry/proxy/handler/EppServiceHandlerTest.java index 1869cafe5..9f9122f69 100644 --- a/proxy/src/test/java/google/registry/proxy/handler/EppServiceHandlerTest.java +++ b/proxy/src/test/java/google/registry/proxy/handler/EppServiceHandlerTest.java @@ -32,6 +32,7 @@ import com.google.common.base.Throwables; import google.registry.proxy.TestUtils; import google.registry.proxy.handler.HttpsRelayServiceHandler.NonOkHttpResponseException; import google.registry.proxy.metric.FrontendMetrics; +import google.registry.util.ProxyHttpHeaders; import google.registry.util.SelfSignedCaCertificate; import io.netty.buffer.ByteBuf; import io.netty.buffer.Unpooled; @@ -250,7 +251,7 @@ class EppServiceHandlerTest { channel.writeInbound(Unpooled.wrappedBuffer(content.getBytes(UTF_8))); FullHttpRequest request = channel.readInbound(); assertThat(request).isEqualTo(makeEppHttpRequestWithCertificate(content)); - String encodedCert = request.headers().get("X-SSL-Full-Certificate"); + String encodedCert = request.headers().get(ProxyHttpHeaders.FULL_CERTIFICATE); assertThat(encodedCert).isNotEqualTo(SAMPLE_CERT); X509Certificate decodedCert = loadCertificate(new ByteArrayInputStream(Base64.getDecoder().decode(encodedCert))); @@ -269,7 +270,7 @@ class EppServiceHandlerTest { assertThat(request).isEqualTo(makeEppHttpRequestWithCertificate(content)); // Receive response indicating session is logged in HttpResponse response = makeEppHttpResponse(content, HttpResponseStatus.OK); - response.headers().set("Logged-In", "true"); + response.headers().set(ProxyHttpHeaders.LOGGED_IN, "true"); // Send another inbound message after login channel.writeOutbound(response); channel.writeInbound(Unpooled.wrappedBuffer(content.getBytes(UTF_8))); @@ -297,7 +298,7 @@ class EppServiceHandlerTest { setHandshakeSuccess(); String content = "stuff"; HttpResponse response = makeEppHttpResponse(content, HttpResponseStatus.OK); - response.headers().set("Epp-Session", "close"); + response.headers().set(ProxyHttpHeaders.EPP_SESSION, "close"); channel.writeOutbound(response); ByteBuf expectedResponse = channel.readOutbound(); assertThat(Unpooled.wrappedBuffer(content.getBytes(UTF_8))).isEqualTo(expectedResponse); @@ -384,7 +385,7 @@ class EppServiceHandlerTest { // Second response written. HttpResponse response = makeEppHttpResponse(responseContent2, HttpResponseStatus.OK, cookie3, newCookie2); - response.headers().set("Logged-In", "true"); + response.headers().set(ProxyHttpHeaders.LOGGED_IN, "true"); channel.writeOutbound(response); channel.readOutbound(); String requestContent2 = "request2"; diff --git a/util/src/main/java/google/registry/util/ProxyHttpHeaders.java b/util/src/main/java/google/registry/util/ProxyHttpHeaders.java new file mode 100644 index 000000000..202c10fd8 --- /dev/null +++ b/util/src/main/java/google/registry/util/ProxyHttpHeaders.java @@ -0,0 +1,48 @@ +// Copyright 2021 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.util; + +import com.google.common.net.HttpHeaders; + +/** Utility class of HTTP header names used for HTTP calls between Nomulus and the proxy. */ +public final class ProxyHttpHeaders { + + /** + * HTTP header name used to pass a full SSL certificate from the proxy to Nomulus. + * + *

This header contains the SSL certificate encoded to a string. It is used to pass the client + * certificate used for login to Nomulus for validation. + */ + public static final String FULL_CERTIFICATE = "X-SSL-Full-Certificate"; + + /** HTTP header name used to pass the certificate hash from the proxy to Nomulus. */ + public static final String CERTIFICATE_HASH = "X-SSL-Certificate"; + + /** + * HTTP header name passed from Nomulus to proxy to indicate that a client has successfully logged + * in. + */ + public static final String LOGGED_IN = "Logged-In"; + + /** + * HTTP header name passed from Nomulus to proxy to indicate that an EPP session should be closed. + */ + public static final String EPP_SESSION = "Epp-Session"; + + /** HTTP header name used to pass the client IP address from the proxy to Nomulus. */ + public static final String IP_ADDRESS = HttpHeaders.X_FORWARDED_FOR; + + private ProxyHttpHeaders() {} +}