diff --git a/core/src/main/java/google/registry/model/OteAccountBuilder.java b/core/src/main/java/google/registry/model/OteAccountBuilder.java index 325c84d15..f34ee455c 100644 --- a/core/src/main/java/google/registry/model/OteAccountBuilder.java +++ b/core/src/main/java/google/registry/model/OteAccountBuilder.java @@ -211,11 +211,6 @@ public final class OteAccountBuilder { return transformRegistrars(builder -> builder.setPassword(password)); } - /** Sets the client certificate hash to all the OT&E Registrars. */ - public OteAccountBuilder setCertificateHash(String certHash) { - return transformRegistrars(builder -> builder.setClientCertificateHash(certHash)); - } - /** Sets the client certificate to all the OT&E Registrars. */ public OteAccountBuilder setCertificate(String asciiCert, DateTime now) { return transformRegistrars(builder -> builder.setClientCertificate(asciiCert, now)); diff --git a/core/src/main/java/google/registry/model/registrar/Registrar.java b/core/src/main/java/google/registry/model/registrar/Registrar.java index f00f4be1e..a2dcd4ee8 100644 --- a/core/src/main/java/google/registry/model/registrar/Registrar.java +++ b/core/src/main/java/google/registry/model/registrar/Registrar.java @@ -856,26 +856,6 @@ public class Registrar extends ImmutableObject } } - /** - * Sets client certificate hash, but not the certificate. - * - *

Warning: {@link #setClientCertificate(String, DateTime)} sets the hash for you and - * is preferred. Calling this method will nullify the {@code clientCertificate} field. - */ - public Builder setClientCertificateHash(String clientCertificateHash) { - if (clientCertificateHash != null) { - checkArgument( - Pattern.matches("[A-Za-z0-9+/]+", clientCertificateHash), - "--cert_hash not a valid base64 (no padding) value"); - checkArgument( - base64().decode(clientCertificateHash).length == 256 / 8, - "--cert_hash base64 does not decode to 256 bits"); - } - getInstance().clientCertificate = null; - getInstance().clientCertificateHash = clientCertificateHash; - return this; - } - public Builder setContactsRequireSyncing(boolean contactsRequireSyncing) { getInstance().contactsRequireSyncing = contactsRequireSyncing; return this; diff --git a/core/src/main/java/google/registry/tools/CreateOrUpdateRegistrarCommand.java b/core/src/main/java/google/registry/tools/CreateOrUpdateRegistrarCommand.java index b1a7925f4..39dda3776 100644 --- a/core/src/main/java/google/registry/tools/CreateOrUpdateRegistrarCommand.java +++ b/core/src/main/java/google/registry/tools/CreateOrUpdateRegistrarCommand.java @@ -138,15 +138,6 @@ abstract class CreateOrUpdateRegistrarCommand extends MutatingCommand { validateWith = PathParameter.InputFile.class) Path clientCertificateFilename; - @Nullable - @Parameter( - names = "--cert_hash", - description = - "Hash of client certificate (SHA256 base64 no padding). Do not use this unless " - + "you want to store ONLY the hash and not the full certificate" - ) - String clientCertificateHash; - @Nullable @Parameter( names = "--failover_cert_file", @@ -375,14 +366,6 @@ abstract class CreateOrUpdateRegistrarCommand extends MutatingCommand { } builder.setFailoverClientCertificate(asciiCert, now); } - if (!isNullOrEmpty(clientCertificateHash)) { - checkArgument(clientCertificateFilename == null, - "Can't specify both --cert_hash and --cert_file"); - if ("null".equals(clientCertificateHash)) { - clientCertificateHash = null; - } - builder.setClientCertificateHash(clientCertificateHash); - } if (ianaId != null) { builder.setIanaIdentifier(ianaId.orElse(null)); } diff --git a/core/src/main/java/google/registry/tools/SetupOteCommand.java b/core/src/main/java/google/registry/tools/SetupOteCommand.java index a61af79df..a564c1fdf 100644 --- a/core/src/main/java/google/registry/tools/SetupOteCommand.java +++ b/core/src/main/java/google/registry/tools/SetupOteCommand.java @@ -65,13 +65,6 @@ final class SetupOteCommand extends ConfirmingCommand implements CommandWithRemo validateWith = PathParameter.InputFile.class) private Path certFile; - @Parameter( - names = {"-h", "--certhash"}, - description = - "Hash of client certificate (SHA256 base64 no padding). Do not use this unless " - + "you want to store ONLY the hash and not the full certificate.") - private String certHash; - @Parameter( names = {"--overwrite"}, description = "Whether to replace existing entities if we encounter any, instead of failing.") @@ -89,9 +82,7 @@ final class SetupOteCommand extends ConfirmingCommand implements CommandWithRemo /** Run any pre-execute command checks */ @Override protected void init() throws Exception { - checkArgument( - certFile == null ^ certHash == null, - "Must specify exactly one of client certificate file or client certificate hash."); + checkArgument(certFile != null, "Must specify exactly one client certificate file."); password = passwordGenerator.createString(PASSWORD_LENGTH); oteAccountBuilder = @@ -101,16 +92,10 @@ final class SetupOteCommand extends ConfirmingCommand implements CommandWithRemo .setIpAllowList(ipAllowList) .setReplaceExisting(overwrite); - if (certFile != null) { String asciiCert = MoreFiles.asCharSource(certFile, US_ASCII).read(); // Don't wait for create_registrar to fail if it's a bad certificate file. loadCertificate(asciiCert); oteAccountBuilder.setCertificate(asciiCert, clock.nowUtc()); - } - - if (certHash != null) { - oteAccountBuilder.setCertificateHash(certHash); - } } @Override diff --git a/core/src/test/java/google/registry/flows/EppLoginTlsTest.java b/core/src/test/java/google/registry/flows/EppLoginTlsTest.java index 23defb91d..9e724ed39 100644 --- a/core/src/test/java/google/registry/flows/EppLoginTlsTest.java +++ b/core/src/test/java/google/registry/flows/EppLoginTlsTest.java @@ -44,13 +44,13 @@ class EppLoginTlsTest extends EppTestCase { persistResource( loadRegistrar("NewRegistrar") .asBuilder() - .setClientCertificateHash(CertificateSamples.SAMPLE_CERT_HASH) + .setClientCertificate(CertificateSamples.SAMPLE_CERT, DateTime.now(UTC)) .build()); // Set a cert for the second registrar, or else any cert will be allowed for login. persistResource( loadRegistrar("TheRegistrar") .asBuilder() - .setClientCertificateHash(CertificateSamples.SAMPLE_CERT2_HASH) + .setClientCertificate(CertificateSamples.SAMPLE_CERT2, DateTime.now(UTC)) .build()); } diff --git a/core/src/test/java/google/registry/flows/session/LoginFlowViaTlsTest.java b/core/src/test/java/google/registry/flows/session/LoginFlowViaTlsTest.java index dae577cb7..0e4ef5888 100644 --- a/core/src/test/java/google/registry/flows/session/LoginFlowViaTlsTest.java +++ b/core/src/test/java/google/registry/flows/session/LoginFlowViaTlsTest.java @@ -15,6 +15,7 @@ package google.registry.flows.session; import static google.registry.testing.DatabaseHelper.persistResource; +import static org.joda.time.DateTimeZone.UTC; import com.google.common.collect.ImmutableList; import com.google.common.net.InetAddresses; @@ -26,6 +27,7 @@ import google.registry.model.registrar.Registrar; import google.registry.testing.CertificateSamples; import google.registry.util.CidrAddressBlock; import java.util.Optional; +import org.joda.time.DateTime; import org.junit.jupiter.api.Test; /** Unit tests for {@link LoginFlow} when accessed via a TLS transport. */ @@ -41,7 +43,7 @@ public class LoginFlowViaTlsTest extends LoginFlowTestCase { @Override protected Registrar.Builder getRegistrarBuilder() { return super.getRegistrarBuilder() - .setClientCertificateHash(GOOD_CERT) + .setClientCertificate(CertificateSamples.SAMPLE_CERT, DateTime.now(UTC)) .setIpAddressAllowList( ImmutableList.of(CidrAddressBlock.create(InetAddresses.forString(GOOD_IP.get()), 32))); } diff --git a/core/src/test/java/google/registry/model/OteAccountBuilderTest.java b/core/src/test/java/google/registry/model/OteAccountBuilderTest.java index ae1643b10..218126bf6 100644 --- a/core/src/test/java/google/registry/model/OteAccountBuilderTest.java +++ b/core/src/test/java/google/registry/model/OteAccountBuilderTest.java @@ -156,16 +156,6 @@ public final class OteAccountBuilderTest { .isTrue(); } - @Test - void testCreateOteEntities_setCertificateHash() { - OteAccountBuilder.forClientId("myclientid") - .setCertificateHash(SAMPLE_CERT_HASH) - .buildAndPersist(); - - assertThat(Registrar.loadByClientId("myclientid-3").get().getClientCertificateHash()) - .isEqualTo(SAMPLE_CERT_HASH); - } - @Test void testCreateOteEntities_setCertificate() { OteAccountBuilder.forClientId("myclientid") diff --git a/core/src/test/java/google/registry/tools/CreateRegistrarCommandTest.java b/core/src/test/java/google/registry/tools/CreateRegistrarCommandTest.java index e01f75718..fa8f65e6a 100644 --- a/core/src/test/java/google/registry/tools/CreateRegistrarCommandTest.java +++ b/core/src/test/java/google/registry/tools/CreateRegistrarCommandTest.java @@ -21,7 +21,6 @@ import static google.registry.persistence.transaction.TransactionManagerFactory. import static google.registry.testing.CertificateSamples.SAMPLE_CERT; import static google.registry.testing.CertificateSamples.SAMPLE_CERT3; import static google.registry.testing.CertificateSamples.SAMPLE_CERT3_HASH; -import static google.registry.testing.CertificateSamples.SAMPLE_CERT_HASH; import static google.registry.testing.DatabaseHelper.createTlds; import static google.registry.testing.DatabaseHelper.persistNewRegistrar; import static google.registry.util.DateTimeUtils.START_OF_TIME; @@ -447,29 +446,6 @@ class CreateRegistrarCommandTest extends CommandTestCase assertThat(registrar).isEmpty(); } - @Test - void testSuccess_clientCertHashFlag() throws Exception { - runCommandForced( - "--name=blobio", - "--password=some_password", - "--registrar_type=REAL", - "--iana_id=8", - "--cert_hash=" + SAMPLE_CERT_HASH, - "--passcode=01234", - "--icann_referral_email=foo@bar.test", - "--street=\"123 Fake St\"", - "--city Fakington", - "--state MA", - "--zip 00351", - "--cc US", - "clientz"); - - Optional registrar = Registrar.loadByClientId("clientz"); - assertThat(registrar).isPresent(); - assertThat(registrar.get().getClientCertificate()).isNull(); - assertThat(registrar.get().getClientCertificateHash()).isEqualTo(SAMPLE_CERT_HASH); - } - @Test void testSuccess_failoverClientCertFileFlag() throws Exception { fakeClock.setTo(DateTime.parse("2020-11-01T00:00:00Z")); @@ -1182,74 +1158,6 @@ class CreateRegistrarCommandTest extends CommandTestCase "clientz")); } - @Test - void testFailure_certHashAndCertFile() { - assertThrows( - IllegalArgumentException.class, - () -> - runCommandForced( - "--name=blobio", - "--password=some_password", - "--registrar_type=REAL", - "--iana_id=8", - "--cert_file=" + getCertFilename(), - "--cert_hash=ABCDEF", - "--passcode=01234", - "--icann_referral_email=foo@bar.test", - "--street=\"123 Fake St\"", - "--city Fakington", - "--state MA", - "--zip 00351", - "--cc US", - "clientz")); - } - - @Test - void testFailure_certHashNotBase64() { - IllegalArgumentException thrown = - assertThrows( - IllegalArgumentException.class, - () -> - runCommandForced( - "--name=blobio", - "--password=some_password", - "--registrar_type=REAL", - "--iana_id=8", - "--cert_hash=!", - "--passcode=01234", - "--icann_referral_email=foo@bar.test", - "--street=\"123 Fake St\"", - "--city Fakington", - "--state MA", - "--zip 00351", - "--cc US", - "clientz")); - assertThat(thrown).hasMessageThat().contains("base64"); - } - - @Test - void testFailure_certHashNotA256BitValue() { - IllegalArgumentException thrown = - assertThrows( - IllegalArgumentException.class, - () -> - runCommandForced( - "--name=blobio", - "--password=some_password", - "--registrar_type=REAL", - "--iana_id=8", - "--cert_hash=abc", - "--passcode=01234", - "--icann_referral_email=foo@bar.test", - "--street=\"123 Fake St\"", - "--city Fakington", - "--state MA", - "--zip 00351", - "--cc US", - "clientz")); - assertThat(thrown).hasMessageThat().contains("256"); - } - @Test void testFailure_missingName() { IllegalArgumentException thrown = diff --git a/core/src/test/java/google/registry/tools/SetupOteCommandTest.java b/core/src/test/java/google/registry/tools/SetupOteCommandTest.java index 50053563f..09630fa37 100644 --- a/core/src/test/java/google/registry/tools/SetupOteCommandTest.java +++ b/core/src/test/java/google/registry/tools/SetupOteCommandTest.java @@ -18,7 +18,6 @@ import static com.google.common.truth.Truth.assertThat; import static google.registry.model.registrar.Registrar.State.ACTIVE; import static google.registry.model.registry.Registry.TldState.GENERAL_AVAILABILITY; import static google.registry.model.registry.Registry.TldState.START_DATE_SUNRISE; -import static google.registry.testing.CertificateSamples.SAMPLE_CERT; import static google.registry.testing.CertificateSamples.SAMPLE_CERT_HASH; import static google.registry.testing.DatabaseHelper.createTld; import static google.registry.testing.DatabaseHelper.loadRegistrar; @@ -98,8 +97,7 @@ class SetupOteCommandTest extends CommandTestCase { String registrarName, String allowedTld, String password, - ImmutableList ipAllowList, - boolean hashOnly) { + ImmutableList ipAllowList) { Registrar registrar = loadRegistrar(registrarName); assertThat(registrar).isNotNull(); assertThat(registrar.getAllowedTlds()).containsExactlyElementsIn(ImmutableSet.of(allowedTld)); @@ -108,18 +106,6 @@ class SetupOteCommandTest extends CommandTestCase { assertThat(registrar.verifyPassword(password)).isTrue(); assertThat(registrar.getIpAddressAllowList()).isEqualTo(ipAllowList); assertThat(registrar.getClientCertificateHash()).isEqualTo(SAMPLE_CERT_HASH); - // If certificate hash is provided, there's no certificate file stored with the registrar. - if (!hashOnly) { - assertThat(registrar.getClientCertificate()).isEqualTo(SAMPLE_CERT); - } - } - - private void verifyRegistrarCreation( - String registrarName, - String allowedTld, - String password, - ImmutableList ipAllowList) { - verifyRegistrarCreation(registrarName, allowedTld, password, ipAllowList, false); } private void verifyRegistrarContactCreation(String registrarName, String email) { @@ -184,24 +170,6 @@ class SetupOteCommandTest extends CommandTestCase { verifyRegistrarContactCreation("abc-5", "abc@email.com"); } - @Test - void testSuccess_certificateHash() throws Exception { - runCommandForced( - "--ip_allow_list=1.1.1.1", - "--registrar=blobio", - "--email=contact@email.com", - "--certhash=" + SAMPLE_CERT_HASH); - - verifyTldCreation("blobio-eap", "BLOBIOE3", GENERAL_AVAILABILITY, true); - - ImmutableList ipAddress = - ImmutableList.of(CidrAddressBlock.create("1.1.1.1")); - - verifyRegistrarCreation("blobio-5", "blobio-eap", PASSWORD, ipAddress, true); - - verifyRegistrarContactCreation("blobio-5", "contact@email.com"); - } - @Test void testSuccess_multipleIps() throws Exception { runCommandForced( @@ -256,7 +224,7 @@ class SetupOteCommandTest extends CommandTestCase { } @Test - void testFailure_missingCertificateFileAndCertificateHash() { + void testFailure_missingCertificateFile() { IllegalArgumentException thrown = assertThrows( IllegalArgumentException.class, @@ -265,26 +233,7 @@ class SetupOteCommandTest extends CommandTestCase { "--ip_allow_list=1.1.1.1", "--email=contact@email.com", "--registrar=blobio")); assertThat(thrown) .hasMessageThat() - .contains( - "Must specify exactly one of client certificate file or client certificate hash."); - } - - @Test - void testFailure_suppliedCertificateFileAndCertificateHash() { - IllegalArgumentException thrown = - assertThrows( - IllegalArgumentException.class, - () -> - runCommandForced( - "--ip_allow_list=1.1.1.1", - "--email=contact@email.com", - "--registrar=blobio", - "--certfile=" + getCertFilename(), - "--certhash=" + SAMPLE_CERT_HASH)); - assertThat(thrown) - .hasMessageThat() - .contains( - "Must specify exactly one of client certificate file or client certificate hash."); + .contains("Must specify exactly one client certificate file."); } @Test diff --git a/core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java b/core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java index 89f4c3f54..683dc39c4 100644 --- a/core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java +++ b/core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java @@ -21,7 +21,6 @@ import static google.registry.persistence.transaction.TransactionManagerFactory. import static google.registry.testing.CertificateSamples.SAMPLE_CERT; import static google.registry.testing.CertificateSamples.SAMPLE_CERT3; import static google.registry.testing.CertificateSamples.SAMPLE_CERT3_HASH; -import static google.registry.testing.CertificateSamples.SAMPLE_CERT_HASH; import static google.registry.testing.DatabaseHelper.createTlds; import static google.registry.testing.DatabaseHelper.loadRegistrar; import static google.registry.testing.DatabaseHelper.persistResource; @@ -339,14 +338,6 @@ class UpdateRegistrarCommandTest extends CommandTestCase assertThat(registrar.getFailoverClientCertificate()).isEqualTo(SAMPLE_CERT3); } - @Test - void testSuccess_certHash() throws Exception { - assertThat(loadRegistrar("NewRegistrar").getClientCertificateHash()).isNull(); - runCommand("--cert_hash=" + SAMPLE_CERT_HASH, "--force", "NewRegistrar"); - assertThat(loadRegistrar("NewRegistrar").getClientCertificateHash()) - .isEqualTo(SAMPLE_CERT_HASH); - } - @Test void testSuccess_clearCert() throws Exception { persistResource( @@ -359,18 +350,6 @@ class UpdateRegistrarCommandTest extends CommandTestCase assertThat(loadRegistrar("NewRegistrar").getClientCertificate()).isNull(); } - @Test - void testSuccess_clearCertHash() throws Exception { - persistResource( - loadRegistrar("NewRegistrar") - .asBuilder() - .setClientCertificateHash(SAMPLE_CERT_HASH) - .build()); - assertThat(isNullOrEmpty(loadRegistrar("NewRegistrar").getClientCertificateHash())).isFalse(); - runCommand("--cert_hash=null", "--force", "NewRegistrar"); - assertThat(loadRegistrar("NewRegistrar").getClientCertificateHash()).isNull(); - } - @Test void testSuccess_ianaId() throws Exception { assertThat(loadRegistrar("NewRegistrar").getIanaIdentifier()).isEqualTo(8); @@ -762,18 +741,6 @@ class UpdateRegistrarCommandTest extends CommandTestCase () -> runCommand("--cert_file=" + writeToTmpFile("ABCDEF"), "--force", "NewRegistrar")); } - @Test - void testFailure_certHashAndCertFile() { - assertThrows( - IllegalArgumentException.class, - () -> - runCommand( - "--cert_file=" + getCertFilename(SAMPLE_CERT3), - "--cert_hash=ABCDEF", - "--force", - "NewRegistrar")); - } - @Test void testFailure_missingClientId() { assertThrows(ParameterException.class, () -> runCommand("--force")); diff --git a/core/src/test/java/google/registry/tools/ValidateLoginCredentialsCommandTest.java b/core/src/test/java/google/registry/tools/ValidateLoginCredentialsCommandTest.java index 0c27237e9..6a2e7e039 100644 --- a/core/src/test/java/google/registry/tools/ValidateLoginCredentialsCommandTest.java +++ b/core/src/test/java/google/registry/tools/ValidateLoginCredentialsCommandTest.java @@ -20,6 +20,7 @@ import static google.registry.testing.DatabaseHelper.createTld; import static google.registry.testing.DatabaseHelper.loadRegistrar; import static google.registry.testing.DatabaseHelper.persistResource; import static google.registry.testing.EppExceptionSubject.assertAboutEppExceptions; +import static org.joda.time.DateTimeZone.UTC; import static org.junit.jupiter.api.Assertions.assertThrows; import com.beust.jcommander.ParameterException; @@ -33,6 +34,7 @@ import google.registry.testing.CertificateSamples; import google.registry.util.CidrAddressBlock; import java.nio.file.Files; import java.nio.file.Path; +import org.joda.time.DateTime; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -50,7 +52,7 @@ class ValidateLoginCredentialsCommandTest extends CommandTestCase { - persistResource( - loadRegistrar("TheRegistrar") - .asBuilder() - .setClientCertificateHash(CertificateSamples.SAMPLE_CERT_HASH) - .build()); - return null; - }); - driver.manage().window().setSize(new Dimension(1050, 2000)); - driver.get(server.getUrl("/registrar#security-settings")); - driver.waitForDisplayedElement(By.tagName("h1")); - driver.diffPage("view"); - driver.waitForDisplayedElement(By.id("reg-app-btn-edit")).click(); - driver.waitForDisplayedElement(By.tagName("h1")); - driver.diffPage("edit"); - } - @RetryingTest(3) void index_registrarDisabled() throws Throwable { server.runInAppEngineEnvironment(