diff --git a/java/google/registry/config/RegistryConfig.java b/java/google/registry/config/RegistryConfig.java index 01ba8fcfe..5d31c485c 100644 --- a/java/google/registry/config/RegistryConfig.java +++ b/java/google/registry/config/RegistryConfig.java @@ -573,6 +573,17 @@ public final class RegistryConfig { return beamBucketUrl + "/templates/spec11"; } + /** + * Returns whether an SSL certificate hash is required to log in via EPP and run flows. + * + * @see google.registry.flows.TlsCredentials + */ + @Provides + @Config("requireSslCertificates") + public static boolean provideRequireSslCertificates(RegistryConfigSettings config) { + return config.registryPolicy.requireSslCertificates; + } + /** * Returns the default job zone to run Apache Beam (Cloud Dataflow) jobs in. * diff --git a/java/google/registry/config/RegistryConfigSettings.java b/java/google/registry/config/RegistryConfigSettings.java index 2b7c9b4fe..a60301e59 100644 --- a/java/google/registry/config/RegistryConfigSettings.java +++ b/java/google/registry/config/RegistryConfigSettings.java @@ -92,6 +92,7 @@ public class RegistryConfigSettings { public String rdapTos; public String rdapTosStaticUrl; public String spec11EmailBodyTemplate; + public boolean requireSslCertificates; } /** Configuration for Cloud Datastore. */ diff --git a/java/google/registry/config/files/default-config.yaml b/java/google/registry/config/files/default-config.yaml index c884d225b..e5a9de33e 100644 --- a/java/google/registry/config/files/default-config.yaml +++ b/java/google/registry/config/files/default-config.yaml @@ -183,6 +183,11 @@ registryPolicy: If you have any questions regarding this notice, please contact {REPLY_TO_EMAIL}. + # Whether to require an SSL certificate hash in order to be able to log in + # via EPP and run commands. This can be false for testing environments but + # should generally be true for production environments, for added security. + requireSslCertificates: true + datastore: # Number of commit log buckets in Datastore. Lowering this after initial # install risks losing up to a days' worth of differential backups. diff --git a/java/google/registry/flows/TlsCredentials.java b/java/google/registry/flows/TlsCredentials.java index f4119925c..455123e27 100644 --- a/java/google/registry/flows/TlsCredentials.java +++ b/java/google/registry/flows/TlsCredentials.java @@ -26,6 +26,7 @@ import com.google.common.net.HostAndPort; import com.google.common.net.InetAddresses; import dagger.Module; import dagger.Provides; +import google.registry.config.RegistryConfig.Config; import google.registry.flows.EppException.AuthenticationErrorException; import google.registry.model.registrar.Registrar; import google.registry.request.Header; @@ -54,14 +55,17 @@ public class TlsCredentials implements TransportCredentials { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + private final boolean requireSslCertificates; private final String clientCertificateHash; private final InetAddress clientInetAddr; @Inject @VisibleForTesting public TlsCredentials( + @Config("requireSslCertificates") boolean requireSslCertificates, @Header("X-SSL-Certificate") String clientCertificateHash, @Header("X-Forwarded-For") Optional clientAddress) { + this.requireSslCertificates = requireSslCertificates; this.clientCertificateHash = clientCertificateHash; this.clientInetAddr = clientAddress.isPresent() ? parseInetAddress(clientAddress.get()) : null; } @@ -112,13 +116,17 @@ public class TlsCredentials implements TransportCredentials { * @throws MissingRegistrarCertificateException if frontend didn't send certificate hash header * @throws BadRegistrarCertificateException if registrar requires certificate and it didn't match */ - private void validateCertificate(Registrar registrar) throws AuthenticationErrorException { + @VisibleForTesting + void validateCertificate(Registrar registrar) throws AuthenticationErrorException { if (isNullOrEmpty(registrar.getClientCertificateHash()) && isNullOrEmpty(registrar.getFailoverClientCertificateHash())) { - logger.atInfo().log( - "Skipping SSL certificate check because %s doesn't have any certificate hashes on file", - registrar.getClientId()); - return; + if (requireSslCertificates) { + throw new RegistrarCertificateNotConfiguredException(); + } else { + // If the environment is configured to allow missing SSL certificate hashes and this hash is + // missing, then bypass the certificate hash checks. + return; + } } if (isNullOrEmpty(clientCertificateHash)) { logger.atInfo().log("Request did not include X-SSL-Certificate"); @@ -165,6 +173,14 @@ public class TlsCredentials implements TransportCredentials { } } + /** Registrar certificate is not configured. */ + public static class RegistrarCertificateNotConfiguredException + extends AuthenticationErrorException { + public RegistrarCertificateNotConfiguredException() { + super("Registrar certificate is not configured"); + } + } + /** Registrar IP address is not in stored whitelist. */ public static class BadRegistrarIpAddressException extends AuthenticationErrorException { public BadRegistrarIpAddressException() { diff --git a/java/google/registry/tools/ValidateLoginCredentialsCommand.java b/java/google/registry/tools/ValidateLoginCredentialsCommand.java index a4ac79904..b4c771217 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)) + new TlsCredentials(true, 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 121285f8f..a461946cc 100644 --- a/javatests/google/registry/flows/EppLoginTlsTest.java +++ b/javatests/google/registry/flows/EppLoginTlsTest.java @@ -33,15 +33,11 @@ import org.junit.runners.JUnit4; @RunWith(JUnit4.class) public class EppLoginTlsTest extends EppTestCase { - @Rule - public final AppEngineRule appEngine = AppEngineRule.builder() - .withDatastore() - .build(); - + @Rule public final AppEngineRule appEngine = AppEngineRule.builder().withDatastore().build(); void setClientCertificateHash(String clientCertificateHash) { setTransportCredentials( - new TlsCredentials(clientCertificateHash, Optional.of("192.168.1.100:54321"))); + new TlsCredentials(true, clientCertificateHash, Optional.of("192.168.1.100:54321"))); } @Before @@ -160,7 +156,7 @@ public class EppLoginTlsTest extends EppTestCase { } @Test - public void testRegistrarHasNoCertificatesOnFile_disablesCertChecking() throws Exception { + public void testRegistrarHasNoCertificatesOnFile_fails() throws Exception { setClientCertificateHash("laffo"); DateTime now = DateTime.now(UTC); persistResource( @@ -169,6 +165,9 @@ public class EppLoginTlsTest extends EppTestCase { .setClientCertificate(null, now) .setFailoverClientCertificate(null, now) .build()); - assertThatLoginSucceeds("NewRegistrar", "foo-BAR2"); + assertThatLogin("NewRegistrar", "foo-BAR2") + .hasResponse( + "response_error.xml", + ImmutableMap.of("CODE", "2200", "MSG", "Registrar certificate is not configured")); } } diff --git a/javatests/google/registry/flows/FlowRunnerTest.java b/javatests/google/registry/flows/FlowRunnerTest.java index ac0d6ea72..c66cc5d78 100644 --- a/javatests/google/registry/flows/FlowRunnerTest.java +++ b/javatests/google/registry/flows/FlowRunnerTest.java @@ -152,7 +152,7 @@ public class FlowRunnerTest extends ShardableTestCase { @Test public void testRun_loggingStatement_tlsCredentials() throws Exception { - flowRunner.credentials = new TlsCredentials("abc123def", Optional.of("127.0.0.1")); + flowRunner.credentials = new TlsCredentials(true, "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}"); diff --git a/javatests/google/registry/flows/TlsCredentialsTest.java b/javatests/google/registry/flows/TlsCredentialsTest.java index 7e5fb1d1a..bf7efc49a 100644 --- a/javatests/google/registry/flows/TlsCredentialsTest.java +++ b/javatests/google/registry/flows/TlsCredentialsTest.java @@ -15,19 +15,31 @@ package google.registry.flows; import static com.google.common.truth.Truth.assertThat; +import static google.registry.testing.DatastoreHelper.loadRegistrar; +import static google.registry.testing.DatastoreHelper.persistResource; import static google.registry.testing.JUnitBackports.assertThrows; +import static org.joda.time.DateTimeZone.UTC; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import google.registry.model.registrar.Registrar; import google.registry.request.HttpException.BadRequestException; +import google.registry.testing.AppEngineRule; +import google.registry.testing.ShardableTestCase; +import java.util.Optional; import javax.servlet.http.HttpServletRequest; +import org.joda.time.DateTime; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; /** Unit tests for {@link TlsCredentials}. */ @RunWith(JUnit4.class) -public final class TlsCredentialsTest { +public final class TlsCredentialsTest extends ShardableTestCase { + + @Rule public final AppEngineRule appEngine = AppEngineRule.builder().withDatastore().build(); + @Test public void testProvideClientCertificateHash() { HttpServletRequest req = mock(HttpServletRequest.class); @@ -46,8 +58,15 @@ public final class TlsCredentialsTest { } @Test - public void testNothing1() {} - - @Test - public void testNothing2() {} + public void test_validateCertificate_canBeConfiguredToBypassCertHashes() throws Exception { + TlsCredentials tls = new TlsCredentials(false, "certHash", Optional.of("192.168.1.1")); + persistResource( + loadRegistrar("TheRegistrar") + .asBuilder() + .setClientCertificate(null, DateTime.now(UTC)) + .setFailoverClientCertificate(null, DateTime.now(UTC)) + .build()); + // This would throw a RegistrarCertificateNotConfiguredException if cert hashes wren't bypassed. + tls.validateCertificate(Registrar.loadByClientId("TheRegistrar").get()); + } } diff --git a/javatests/google/registry/flows/session/LoginFlowViaTlsTest.java b/javatests/google/registry/flows/session/LoginFlowViaTlsTest.java index 096b1535f..4aaaba315 100644 --- a/javatests/google/registry/flows/session/LoginFlowViaTlsTest.java +++ b/javatests/google/registry/flows/session/LoginFlowViaTlsTest.java @@ -49,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); + credentials = new TlsCredentials(true, GOOD_CERT, GOOD_IP); doSuccessfulTest("login_valid.xml"); } @@ -60,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); + credentials = new TlsCredentials(true, GOOD_CERT, GOOD_IPV6); doSuccessfulTest("login_valid.xml"); } @@ -71,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); + credentials = new TlsCredentials(true, GOOD_CERT, GOOD_IPV6); doSuccessfulTest("login_valid.xml"); } @@ -82,21 +82,21 @@ public class LoginFlowViaTlsTest extends LoginFlowTestCase { .setIpAddressWhitelist(ImmutableList.of( CidrAddressBlock.create("192.168.1.255/24"))) .build()); - credentials = new TlsCredentials(GOOD_CERT, GOOD_IP); + credentials = new TlsCredentials(true, GOOD_CERT, GOOD_IP); doSuccessfulTest("login_valid.xml"); } @Test public void testFailure_incorrectClientCertificateHash() { persistResource(getRegistrarBuilder().build()); - credentials = new TlsCredentials(BAD_CERT, GOOD_IP); + credentials = new TlsCredentials(true, BAD_CERT, GOOD_IP); doFailingTest("login_valid.xml", BadRegistrarCertificateException.class); } @Test public void testFailure_missingClientCertificateHash() { persistResource(getRegistrarBuilder().build()); - credentials = new TlsCredentials(null, GOOD_IP); + credentials = new TlsCredentials(true, null, GOOD_IP); doFailingTest("login_valid.xml", MissingRegistrarCertificateException.class); } @@ -108,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()); + credentials = new TlsCredentials(true, GOOD_CERT, Optional.empty()); doFailingTest("login_valid.xml", BadRegistrarIpAddressException.class); } @@ -120,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); + credentials = new TlsCredentials(true, GOOD_CERT, BAD_IP); doFailingTest("login_valid.xml", BadRegistrarIpAddressException.class); } @@ -132,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); + credentials = new TlsCredentials(true, GOOD_CERT, BAD_IPV6); doFailingTest("login_valid.xml", BadRegistrarIpAddressException.class); } }