Remove checking of SNI headers

This is only useful when we used the [] proxy because the GFE requires SNI during handshake in order to request the client certificate. The GCP proxy does not need this (it always requests the client certificate). We do not need to check for its existence.

Also removed the checking of internal headers for ssl cert hash used only by the [] proxy.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=213059027

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

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,

java/google/registry/flows/TlsCredentials.java

@@ -48,9 +48,6 @@ import javax.servlet.http.HttpServletRequest;
  *   <dt>X-Forwarded-For
  *   <dd>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.
- *   <dt>X-GFE-Requested-Servername-SNI
- *   <dd>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.
  * </dl>
  */
 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<String> clientAddress,
+      @Header("X-Forwarded-For") Optional<String> clientAddress) {
-      @Header("X-Requested-Servername-SNI") String sni) {
     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<String> 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");
-    }
   }
 }

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}

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");
   }

javatests/google/registry/flows/EppLoginTlsTest.java

@@ -40,8 +40,8 @@ public class EppLoginTlsTest extends EppTestCase {
 
 
   void setClientCertificateHash(String clientCertificateHash) {
-    setTransportCredentials(new TlsCredentials(
+    setTransportCredentials(
-        clientCertificateHash, Optional.of("192.168.1.100:54321"), "test.example"));
+        new TlsCredentials(clientCertificateHash, Optional.of("192.168.1.100:54321")));
   }
 
   @Before

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);

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(
+        .contains("TlsCredentials{clientCertificateHash=abc123def, clientAddress=/127.0.0.1}");
-            "TlsCredentials{clientCertificateHash=abc123def, clientAddress=/127.0.0.1, sni=sni}");
   }
 
   @Test

javatests/google/registry/flows/TlsCredentialsTest.java

@@ -46,21 +46,8 @@ public final class TlsCredentialsTest {
   }
 
   @Test
-  public void testProvideRequestedServername() {
+  public void testNothing1() {}
-    HttpServletRequest req = mock(HttpServletRequest.class);
-    when(req.getHeader("X-Requested-Servername-SNI")).thenReturn("data");
-    assertThat(TlsCredentials.EppTlsModule.provideRequestedServername(req))
-        .isEqualTo("data");
-  }
 
   @Test
-  public void testProvideRequestedServername_missing() {
+  public void testNothing2() {}
-    HttpServletRequest req = mock(HttpServletRequest.class);
-    BadRequestException thrown =
-        assertThrows(
-            BadRequestException.class,
-            () -> TlsCredentials.EppTlsModule.provideRequestedServername(req));
-    assertThat(thrown).hasMessageThat().contains("Missing header: X-Requested-Servername-SNI");
-  }
-
 }

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);
   }
 }