diff --git a/docs/flows.md b/docs/flows.md index c83484dc1..79bf91276 100644 --- a/docs/flows.md +++ b/docs/flows.md @@ -1174,7 +1174,6 @@ An EPP flow for login. * Registrar certificate does not match stored certificate. * Registrar IP address is not in stored whitelist. * Registrar certificate not present. - * SNI header is required. * Registrar password is incorrect. * Registrar with this client ID could not be found. * 2201 diff --git a/java/google/registry/flows/EppTlsAction.java b/java/google/registry/flows/EppTlsAction.java index bf7607877..5537d667b 100644 --- a/java/google/registry/flows/EppTlsAction.java +++ b/java/google/registry/flows/EppTlsAction.java @@ -43,11 +43,6 @@ public class EppTlsAction implements Runnable { @Override public void run() { - // Check that SNI header is present. This is a signal that we're receiving traffic proxied by a - // GFE, which is the expectation of this servlet. The value is unused. - if (!tlsCredentials.hasSni()) { - logger.atWarning().log("Request did not include required SNI header."); - } eppRequestHandler.executeEpp( new HttpSessionMetadata(session), tlsCredentials, diff --git a/java/google/registry/flows/TlsCredentials.java b/java/google/registry/flows/TlsCredentials.java index 04566cb77..f4119925c 100644 --- a/java/google/registry/flows/TlsCredentials.java +++ b/java/google/registry/flows/TlsCredentials.java @@ -48,9 +48,6 @@ import javax.servlet.http.HttpServletRequest; *
X-Forwarded-For *
This field should contain the host and port of the connecting client. It is validated * during an EPP login command against an IP whitelist that is transmitted out of band. - *
X-GFE-Requested-Servername-SNI - *
This field should contain the servername that the client requested during the TLS - * handshake. It is unused, but expected to be present in the GFE-proxied configuration. * */ public class TlsCredentials implements TransportCredentials { @@ -58,18 +55,15 @@ public class TlsCredentials implements TransportCredentials { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private final String clientCertificateHash; - private final String sni; private final InetAddress clientInetAddr; @Inject @VisibleForTesting public TlsCredentials( @Header("X-SSL-Certificate") String clientCertificateHash, - @Header("X-Forwarded-For") Optional clientAddress, - @Header("X-Requested-Servername-SNI") String sni) { + @Header("X-Forwarded-For") Optional clientAddress) { this.clientCertificateHash = clientCertificateHash; this.clientInetAddr = clientAddress.isPresent() ? parseInetAddress(clientAddress.get()) : null; - this.sni = sni; } static InetAddress parseInetAddress(String asciiAddr) { @@ -80,11 +74,6 @@ public class TlsCredentials implements TransportCredentials { } } - /** Returns {@code true} if frontend passed us the requested server name. */ - boolean hasSni() { - return !isNullOrEmpty(sni); - } - @Override public void validate(Registrar registrar, String password) throws AuthenticationErrorException { validateIp(registrar); @@ -120,7 +109,6 @@ public class TlsCredentials implements TransportCredentials { /** * Verifies client SSL certificate is permitted to issue commands as {@code registrar}. * - * @throws NoSniException if frontend didn't send host or certificate hash headers * @throws MissingRegistrarCertificateException if frontend didn't send certificate hash header * @throws BadRegistrarCertificateException if registrar requires certificate and it didn't match */ @@ -133,11 +121,6 @@ public class TlsCredentials implements TransportCredentials { return; } if (isNullOrEmpty(clientCertificateHash)) { - // If there's no SNI header that's probably why we don't have a cert, so send a specific - // message. Otherwise, send a missing certificate message. - if (!hasSni()) { - throw new NoSniException(); - } logger.atInfo().log("Request did not include X-SSL-Certificate"); throw new MissingRegistrarCertificateException(); } @@ -165,7 +148,6 @@ public class TlsCredentials implements TransportCredentials { return toStringHelper(getClass()) .add("clientCertificateHash", clientCertificateHash) .add("clientAddress", clientInetAddr) - .add("sni", sni) .toString(); } @@ -183,13 +165,6 @@ public class TlsCredentials implements TransportCredentials { } } - /** SNI header is required. */ - public static class NoSniException extends AuthenticationErrorException { - public NoSniException() { - super("SNI header is required"); - } - } - /** Registrar IP address is not in stored whitelist. */ public static class BadRegistrarIpAddressException extends AuthenticationErrorException { public BadRegistrarIpAddressException() { @@ -211,11 +186,5 @@ public class TlsCredentials implements TransportCredentials { static Optional provideForwardedFor(HttpServletRequest req) { return extractOptionalHeader(req, "X-Forwarded-For"); } - - @Provides - @Header("X-Requested-Servername-SNI") - static String provideRequestedServername(HttpServletRequest req) { - return extractRequiredHeader(req, "X-Requested-Servername-SNI"); - } } } diff --git a/java/google/registry/flows/session/LoginFlow.java b/java/google/registry/flows/session/LoginFlow.java index 08ce41da4..bebd4603c 100644 --- a/java/google/registry/flows/session/LoginFlow.java +++ b/java/google/registry/flows/session/LoginFlow.java @@ -56,7 +56,6 @@ import javax.inject.Inject; * @error {@link google.registry.flows.TlsCredentials.BadRegistrarCertificateException} * @error {@link google.registry.flows.TlsCredentials.BadRegistrarIpAddressException} * @error {@link google.registry.flows.TlsCredentials.MissingRegistrarCertificateException} - * @error {@link google.registry.flows.TlsCredentials.NoSniException} * @error {@link google.registry.flows.TransportCredentials.BadRegistrarPasswordException} * @error {@link LoginFlow.AlreadyLoggedInException} * @error {@link LoginFlow.BadRegistrarClientIdException} diff --git a/java/google/registry/tools/ValidateLoginCredentialsCommand.java b/java/google/registry/tools/ValidateLoginCredentialsCommand.java index af882b78c..a4ac79904 100644 --- a/java/google/registry/tools/ValidateLoginCredentialsCommand.java +++ b/java/google/registry/tools/ValidateLoginCredentialsCommand.java @@ -78,7 +78,7 @@ final class ValidateLoginCredentialsCommand implements CommandWithRemoteApi { Registrar registrar = checkArgumentPresent( Registrar.loadByClientId(clientId), "Registrar %s not found", clientId); - new TlsCredentials(clientCertificateHash, Optional.of(clientIpAddress), null) + new TlsCredentials(clientCertificateHash, Optional.of(clientIpAddress)) .validate(registrar, password); checkState(!registrar.getState().equals(Registrar.State.PENDING), "Account pending"); } diff --git a/javatests/google/registry/flows/EppLoginTlsTest.java b/javatests/google/registry/flows/EppLoginTlsTest.java index 30b9b1685..121285f8f 100644 --- a/javatests/google/registry/flows/EppLoginTlsTest.java +++ b/javatests/google/registry/flows/EppLoginTlsTest.java @@ -40,8 +40,8 @@ public class EppLoginTlsTest extends EppTestCase { void setClientCertificateHash(String clientCertificateHash) { - setTransportCredentials(new TlsCredentials( - clientCertificateHash, Optional.of("192.168.1.100:54321"), "test.example")); + setTransportCredentials( + new TlsCredentials(clientCertificateHash, Optional.of("192.168.1.100:54321"))); } @Before diff --git a/javatests/google/registry/flows/EppTlsActionTest.java b/javatests/google/registry/flows/EppTlsActionTest.java index b911f28a4..a1dabc5aa 100644 --- a/javatests/google/registry/flows/EppTlsActionTest.java +++ b/javatests/google/registry/flows/EppTlsActionTest.java @@ -21,7 +21,6 @@ import static org.mockito.Matchers.eq; import static org.mockito.Matchers.same; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; import google.registry.testing.FakeHttpSession; import google.registry.testing.ShardableTestCase; @@ -41,7 +40,6 @@ public class EppTlsActionTest extends ShardableTestCase { EppTlsAction action = new EppTlsAction(); action.inputXmlBytes = INPUT_XML_BYTES; action.tlsCredentials = mock(TlsCredentials.class); - when(action.tlsCredentials.hasSni()).thenReturn(true); action.session = new FakeHttpSession(); action.session.setAttribute("CLIENT_ID", "ClientIdentifier"); action.eppRequestHandler = mock(EppRequestHandler.class); diff --git a/javatests/google/registry/flows/FlowRunnerTest.java b/javatests/google/registry/flows/FlowRunnerTest.java index 11bba256d..82a3362fa 100644 --- a/javatests/google/registry/flows/FlowRunnerTest.java +++ b/javatests/google/registry/flows/FlowRunnerTest.java @@ -164,11 +164,10 @@ public class FlowRunnerTest extends ShardableTestCase { @Test public void testRun_loggingStatement_tlsCredentials() throws Exception { - flowRunner.credentials = new TlsCredentials("abc123def", Optional.of("127.0.0.1"), "sni"); + flowRunner.credentials = new TlsCredentials("abc123def", Optional.of("127.0.0.1")); flowRunner.run(eppMetricBuilder); assertThat(Splitter.on("\n\t").split(findFirstLogMessageByPrefix(handler, "EPP Command\n\t"))) - .contains( - "TlsCredentials{clientCertificateHash=abc123def, clientAddress=/127.0.0.1, sni=sni}"); + .contains("TlsCredentials{clientCertificateHash=abc123def, clientAddress=/127.0.0.1}"); } @Test diff --git a/javatests/google/registry/flows/TlsCredentialsTest.java b/javatests/google/registry/flows/TlsCredentialsTest.java index 065163295..7e5fb1d1a 100644 --- a/javatests/google/registry/flows/TlsCredentialsTest.java +++ b/javatests/google/registry/flows/TlsCredentialsTest.java @@ -46,21 +46,8 @@ public final class TlsCredentialsTest { } @Test - public void testProvideRequestedServername() { - HttpServletRequest req = mock(HttpServletRequest.class); - when(req.getHeader("X-Requested-Servername-SNI")).thenReturn("data"); - assertThat(TlsCredentials.EppTlsModule.provideRequestedServername(req)) - .isEqualTo("data"); - } + public void testNothing1() {} @Test - public void testProvideRequestedServername_missing() { - HttpServletRequest req = mock(HttpServletRequest.class); - BadRequestException thrown = - assertThrows( - BadRequestException.class, - () -> TlsCredentials.EppTlsModule.provideRequestedServername(req)); - assertThat(thrown).hasMessageThat().contains("Missing header: X-Requested-Servername-SNI"); - } - + public void testNothing2() {} } diff --git a/javatests/google/registry/flows/session/LoginFlowViaTlsTest.java b/javatests/google/registry/flows/session/LoginFlowViaTlsTest.java index f6010e359..096b1535f 100644 --- a/javatests/google/registry/flows/session/LoginFlowViaTlsTest.java +++ b/javatests/google/registry/flows/session/LoginFlowViaTlsTest.java @@ -22,7 +22,6 @@ import google.registry.flows.TlsCredentials; import google.registry.flows.TlsCredentials.BadRegistrarCertificateException; import google.registry.flows.TlsCredentials.BadRegistrarIpAddressException; import google.registry.flows.TlsCredentials.MissingRegistrarCertificateException; -import google.registry.flows.TlsCredentials.NoSniException; import google.registry.model.registrar.Registrar; import google.registry.testing.CertificateSamples; import google.registry.util.CidrAddressBlock; @@ -50,7 +49,7 @@ public class LoginFlowViaTlsTest extends LoginFlowTestCase { @Test public void testSuccess_withGoodCredentials() throws Exception { persistResource(getRegistrarBuilder().build()); - credentials = new TlsCredentials(GOOD_CERT, GOOD_IP, "goo.example"); + credentials = new TlsCredentials(GOOD_CERT, GOOD_IP); doSuccessfulTest("login_valid.xml"); } @@ -61,7 +60,7 @@ public class LoginFlowViaTlsTest extends LoginFlowTestCase { .setIpAddressWhitelist(ImmutableList.of( CidrAddressBlock.create("2001:db8:0:0:0:0:1:1/32"))) .build()); - credentials = new TlsCredentials(GOOD_CERT, GOOD_IPV6, "goo.example"); + credentials = new TlsCredentials(GOOD_CERT, GOOD_IPV6); doSuccessfulTest("login_valid.xml"); } @@ -72,7 +71,7 @@ public class LoginFlowViaTlsTest extends LoginFlowTestCase { .setIpAddressWhitelist(ImmutableList.of( CidrAddressBlock.create("2001:db8:0:0:0:0:1:1/32"))) .build()); - credentials = new TlsCredentials(GOOD_CERT, GOOD_IPV6, "goo.example"); + credentials = new TlsCredentials(GOOD_CERT, GOOD_IPV6); doSuccessfulTest("login_valid.xml"); } @@ -83,31 +82,24 @@ public class LoginFlowViaTlsTest extends LoginFlowTestCase { .setIpAddressWhitelist(ImmutableList.of( CidrAddressBlock.create("192.168.1.255/24"))) .build()); - credentials = new TlsCredentials(GOOD_CERT, GOOD_IP, "goo.example"); + credentials = new TlsCredentials(GOOD_CERT, GOOD_IP); doSuccessfulTest("login_valid.xml"); } @Test public void testFailure_incorrectClientCertificateHash() { persistResource(getRegistrarBuilder().build()); - credentials = new TlsCredentials(BAD_CERT, GOOD_IP, "goo.example"); + credentials = new TlsCredentials(BAD_CERT, GOOD_IP); doFailingTest("login_valid.xml", BadRegistrarCertificateException.class); } @Test public void testFailure_missingClientCertificateHash() { persistResource(getRegistrarBuilder().build()); - credentials = new TlsCredentials(null, GOOD_IP, "goo.example"); + credentials = new TlsCredentials(null, GOOD_IP); doFailingTest("login_valid.xml", MissingRegistrarCertificateException.class); } - @Test - public void testFailure_noSniAndCertRequired() { - persistResource(getRegistrarBuilder().build()); - credentials = new TlsCredentials(null, GOOD_IP, null); - doFailingTest("login_valid.xml", NoSniException.class); - } - @Test public void testFailure_missingClientIpAddress() { persistResource( @@ -116,7 +108,7 @@ public class LoginFlowViaTlsTest extends LoginFlowTestCase { CidrAddressBlock.create(InetAddresses.forString("192.168.1.1"), 32), CidrAddressBlock.create(InetAddresses.forString("2001:db8::1"), 128))) .build()); - credentials = new TlsCredentials(GOOD_CERT, Optional.empty(), "goo.example"); + credentials = new TlsCredentials(GOOD_CERT, Optional.empty()); doFailingTest("login_valid.xml", BadRegistrarIpAddressException.class); } @@ -128,7 +120,7 @@ public class LoginFlowViaTlsTest extends LoginFlowTestCase { CidrAddressBlock.create(InetAddresses.forString("192.168.1.1"), 32), CidrAddressBlock.create(InetAddresses.forString("2001:db8::1"), 128))) .build()); - credentials = new TlsCredentials(GOOD_CERT, BAD_IP, "goo.example"); + credentials = new TlsCredentials(GOOD_CERT, BAD_IP); doFailingTest("login_valid.xml", BadRegistrarIpAddressException.class); } @@ -140,7 +132,7 @@ public class LoginFlowViaTlsTest extends LoginFlowTestCase { CidrAddressBlock.create(InetAddresses.forString("192.168.1.1"), 32), CidrAddressBlock.create(InetAddresses.forString("2001:db8::1"), 128))) .build()); - credentials = new TlsCredentials(GOOD_CERT, BAD_IPV6, "goo.example"); + credentials = new TlsCredentials(GOOD_CERT, BAD_IPV6); doFailingTest("login_valid.xml", BadRegistrarIpAddressException.class); } }