diff --git a/core/src/main/java/google/registry/config/CertificateCheckerModule.java b/core/src/main/java/google/registry/config/CertificateCheckerModule.java new file mode 100644 index 000000000..8644bde7c --- /dev/null +++ b/core/src/main/java/google/registry/config/CertificateCheckerModule.java @@ -0,0 +1,44 @@ +// Copyright 2020 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.config; + +import com.google.common.collect.ImmutableSortedMap; +import dagger.Module; +import dagger.Provides; +import google.registry.config.RegistryConfig.Config; +import google.registry.util.CertificateChecker; +import google.registry.util.Clock; +import javax.inject.Singleton; +import org.joda.time.DateTime; + +/** Dagger module that provides the {@link CertificateChecker} used in the application. */ +// TODO(sarahbot@): Move this module to a better location. Possibly flows/. If we decide to move +// CertificateChecker.java to core/ delete this file and inject the CertificateChecker constructor +// instead. +@Module +public abstract class CertificateCheckerModule { + + @Provides + @Singleton + static CertificateChecker provideCertificateChecker( + @Config("maxValidityDaysSchedule") ImmutableSortedMap validityDaysMap, + @Config("expirationWarningDays") int daysToExpiration, + @Config("minimumRsaKeyLength") int minimumRsaKeyLength, + Clock clock) { + return new CertificateChecker(validityDaysMap, daysToExpiration, minimumRsaKeyLength, clock); + } + + private CertificateCheckerModule() {} +} diff --git a/core/src/main/java/google/registry/config/RegistryConfig.java b/core/src/main/java/google/registry/config/RegistryConfig.java index 4aa8373b2..3807129e1 100644 --- a/core/src/main/java/google/registry/config/RegistryConfig.java +++ b/core/src/main/java/google/registry/config/RegistryConfig.java @@ -16,9 +16,12 @@ package google.registry.config; import static com.google.common.base.Suppliers.memoize; import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.common.collect.ImmutableSortedMap.toImmutableSortedMap; import static google.registry.config.ConfigUtils.makeUrl; +import static google.registry.util.DateTimeUtils.START_OF_TIME; import static google.registry.util.ResourceUtils.readResourceUtf8; import static java.lang.annotation.RetentionPolicy.RUNTIME; +import static java.util.Comparator.naturalOrder; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Ascii; @@ -27,6 +30,7 @@ import com.google.common.base.Strings; import com.google.common.base.Supplier; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.ImmutableSortedMap; import dagger.Module; import dagger.Provides; import google.registry.util.TaskQueueUtils; @@ -46,6 +50,7 @@ import javax.inject.Qualifier; import javax.inject.Singleton; import javax.mail.internet.AddressException; import javax.mail.internet.InternetAddress; +import org.joda.time.DateTime; import org.joda.time.DateTimeConstants; import org.joda.time.Duration; @@ -1345,6 +1350,33 @@ public final class RegistryConfig { public static String provideRdapTosStaticUrl(RegistryConfigSettings config) { return config.registryPolicy.rdapTosStaticUrl; } + + @Provides + @Config("maxValidityDaysSchedule") + public static ImmutableSortedMap provideValidityDaysMap( + RegistryConfigSettings config) { + return config.sslCertificateValidation.maxValidityDaysSchedule.entrySet().stream() + .collect( + toImmutableSortedMap( + naturalOrder(), + e -> + e.getKey().equals("START_OF_TIME") + ? START_OF_TIME + : DateTime.parse(e.getKey()), + e -> e.getValue())); + } + + @Provides + @Config("expirationWarningDays") + public static int provideDaysToExpiration(RegistryConfigSettings config) { + return config.sslCertificateValidation.expirationWarningDays; + } + + @Provides + @Config("minimumRsaKeyLength") + public static int provideMinimumRsaKeyLength(RegistryConfigSettings config) { + return config.sslCertificateValidation.minimumRsaKeyLength; + } } /** Returns the App Engine project ID, which is based off the environment name. */ diff --git a/core/src/main/java/google/registry/config/RegistryConfigSettings.java b/core/src/main/java/google/registry/config/RegistryConfigSettings.java index 81ff02e3f..88a38eb29 100644 --- a/core/src/main/java/google/registry/config/RegistryConfigSettings.java +++ b/core/src/main/java/google/registry/config/RegistryConfigSettings.java @@ -15,6 +15,7 @@ package google.registry.config; import java.util.List; +import java.util.Map; /** The POJO that YAML config files are deserialized into. */ public class RegistryConfigSettings { @@ -38,6 +39,7 @@ public class RegistryConfigSettings { public Beam beam; public Keyring keyring; public RegistryTool registryTool; + public SslCertificateValidation sslCertificateValidation; /** Configuration options that apply to the entire App Engine project. */ public static class AppEngine { @@ -218,4 +220,11 @@ public class RegistryConfigSettings { public String clientSecret; public String username; } + + /** Configuration for the certificate checker. */ + public static class SslCertificateValidation { + public Map maxValidityDaysSchedule; + public int expirationWarningDays; + public int minimumRsaKeyLength; + } } diff --git a/core/src/main/java/google/registry/config/files/default-config.yaml b/core/src/main/java/google/registry/config/files/default-config.yaml index 3e3bc792b..ed51c345a 100644 --- a/core/src/main/java/google/registry/config/files/default-config.yaml +++ b/core/src/main/java/google/registry/config/files/default-config.yaml @@ -446,3 +446,17 @@ registryTool: # OAuth client secret used by the tool. clientSecret: YOUR_CLIENT_SECRET username: toolusername + +# Configuration options for checking SSL certificates. +sslCertificateValidation: + # A map specifying the maximum amount of days the certificate can be valid. + # The entry key is the date closest before the date the certificate was issued + # and the entry value is the applicable maximum validity days for that certificate. + maxValidityDaysSchedule: + "START_OF_TIME": 825 + "2020-09-01T00:00:00Z": 398 + # The number of days before a certificate expires that indicates the + # certificate is nearing expiration and warnings should be sent. + expirationWarningDays: 30 + # The minimum number of bits an RSA key must contain + minimumRsaKeyLength: 2048 diff --git a/core/src/main/java/google/registry/tools/CreateOrUpdateRegistrarCommand.java b/core/src/main/java/google/registry/tools/CreateOrUpdateRegistrarCommand.java index 091ddac5e..2103c3867 100644 --- a/core/src/main/java/google/registry/tools/CreateOrUpdateRegistrarCommand.java +++ b/core/src/main/java/google/registry/tools/CreateOrUpdateRegistrarCommand.java @@ -22,6 +22,7 @@ import static google.registry.persistence.transaction.TransactionManagerFactory. import static google.registry.util.DomainNameUtils.canonicalizeDomainName; import static google.registry.util.RegistrarUtils.normalizeRegistrarName; import static java.nio.charset.StandardCharsets.US_ASCII; +import static java.nio.charset.StandardCharsets.UTF_8; import static org.joda.time.DateTimeZone.UTC; import com.beust.jcommander.Parameter; @@ -38,9 +39,15 @@ import google.registry.tools.params.OptionalLongParameter; import google.registry.tools.params.OptionalPhoneNumberParameter; import google.registry.tools.params.OptionalStringParameter; import google.registry.tools.params.PathParameter; +import google.registry.util.CertificateChecker; +import google.registry.util.CertificateChecker.CertificateViolation; import google.registry.util.CidrAddressBlock; +import java.io.ByteArrayInputStream; import java.nio.file.Files; import java.nio.file.Path; +import java.security.cert.CertificateException; +import java.security.cert.CertificateFactory; +import java.security.cert.X509Certificate; import java.util.ArrayList; import java.util.Arrays; import java.util.LinkedHashMap; @@ -48,7 +55,9 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.stream.Collectors; import javax.annotation.Nullable; +import javax.inject.Inject; import org.joda.money.CurrencyUnit; import org.joda.time.DateTime; @@ -57,9 +66,9 @@ abstract class CreateOrUpdateRegistrarCommand extends MutatingCommand { static final FluentLogger logger = FluentLogger.forEnclosingClass(); - @Parameter( - description = "Client identifier of the registrar account", - required = true) + @Inject CertificateChecker certificateChecker; + + @Parameter(description = "Client identifier of the registrar account", required = true) List mainParameters; @Parameter( @@ -356,11 +365,21 @@ abstract class CreateOrUpdateRegistrarCommand extends MutatingCommand { } if (clientCertificateFilename != null) { String asciiCert = new String(Files.readAllBytes(clientCertificateFilename), US_ASCII); + // An empty certificate file is allowed in order to provide a functionality for removing an + // existing certificate without providing a replacement. An uploaded empty certificate file + // will prevent the registrar from being able to establish EPP connections. + if (!asciiCert.equals("")) { + verifyCertificate(asciiCert); + } builder.setClientCertificate(asciiCert, now); } + if (failoverClientCertificateFilename != null) { String asciiCert = new String(Files.readAllBytes(failoverClientCertificateFilename), US_ASCII); + if (!asciiCert.equals("")) { + verifyCertificate(asciiCert); + } builder.setFailoverClientCertificate(asciiCert, now); } if (!isNullOrEmpty(clientCertificateHash)) { @@ -463,6 +482,22 @@ abstract class CreateOrUpdateRegistrarCommand extends MutatingCommand { } } + private void verifyCertificate(String certificateString) throws CertificateException { + X509Certificate certificate = + (X509Certificate) + CertificateFactory.getInstance("X509") + .generateCertificate(new ByteArrayInputStream(certificateString.getBytes(UTF_8))); + ImmutableSet violations = + certificateChecker.checkCertificate(certificate); + if (!violations.isEmpty()) { + String displayMessages = + violations.stream() + .map(violation -> violation.getDisplayMessage(certificateChecker)) + .collect(Collectors.joining("\n")); + throw new CertificateException(displayMessages); + } + } + @Override protected String execute() throws Exception { // Save registrar to Datastore and output its response diff --git a/core/src/main/java/google/registry/tools/RegistryToolComponent.java b/core/src/main/java/google/registry/tools/RegistryToolComponent.java index 9a95f73d5..cec473997 100644 --- a/core/src/main/java/google/registry/tools/RegistryToolComponent.java +++ b/core/src/main/java/google/registry/tools/RegistryToolComponent.java @@ -20,6 +20,7 @@ import dagger.Lazy; import google.registry.batch.BatchModule; import google.registry.beam.initsql.BeamJpaModule; import google.registry.bigquery.BigqueryModule; +import google.registry.config.CertificateCheckerModule; import google.registry.config.CredentialModule.LocalCredentialJson; import google.registry.config.RegistryConfig.Config; import google.registry.config.RegistryConfig.ConfigModule; @@ -60,6 +61,7 @@ import javax.inject.Singleton; BatchModule.class, BeamJpaModule.class, BigqueryModule.class, + CertificateCheckerModule.class, ConfigModule.class, CloudDnsWriterModule.class, DatastoreAdminModule.class, @@ -83,42 +85,83 @@ import javax.inject.Singleton; }) interface RegistryToolComponent { void inject(AckPollMessagesCommand command); + void inject(CheckDomainClaimsCommand command); + void inject(CheckDomainCommand command); + void inject(CountDomainsCommand command); + void inject(CreateAnchorTenantCommand command); + void inject(CreateCdnsTld command); + void inject(CreateContactCommand command); + void inject(CreateDomainCommand command); + + void inject(CreateRegistrarCommand command); + void inject(CreateTldCommand command); + void inject(DeployInvoicingPipelineCommand command); + void inject(DeploySpec11PipelineCommand command); + void inject(EncryptEscrowDepositCommand command); + void inject(GenerateAllocationTokensCommand command); + void inject(GenerateDnsReportCommand command); + void inject(GenerateEscrowDepositCommand command); + void inject(GetKeyringSecretCommand command); + void inject(GetOperationStatusCommand command); + void inject(GhostrydeCommand command); + void inject(ImportDatastoreCommand command); + void inject(ListCursorsCommand command); + void inject(ListDatastoreOperationsCommand command); + void inject(LoadSnapshotCommand command); + void inject(LockDomainCommand command); + void inject(LoginCommand command); + void inject(LogoutCommand command); + void inject(PendingEscrowCommand command); + void inject(RenewDomainCommand command); + void inject(SendEscrowReportToIcannCommand command); + void inject(SetNumInstancesCommand command); + void inject(SetupOteCommand command); + void inject(UnlockDomainCommand command); + void inject(UnrenewDomainCommand command); + void inject(UpdateCursorsCommand command); + void inject(UpdateDomainCommand command); + void inject(UpdateKmsKeyringCommand command); + + void inject(UpdateRegistrarCommand command); + void inject(UpdateTldCommand command); + void inject(ValidateEscrowDepositCommand command); + void inject(WhoisQueryCommand command); AppEngineConnection appEngineConnection(); diff --git a/core/src/test/java/google/registry/testing/CertificateSamples.java b/core/src/test/java/google/registry/testing/CertificateSamples.java index 01ede93c6..dcd552c6f 100644 --- a/core/src/test/java/google/registry/testing/CertificateSamples.java +++ b/core/src/test/java/google/registry/testing/CertificateSamples.java @@ -88,5 +88,40 @@ public final class CertificateSamples { */ public static final String SAMPLE_CERT2_HASH = "GNd6ZP8/n91t9UTnpxR8aH7aAW4+CpvufYx9ViGbcMY"; + /* + * openssl req -new -nodes -x509 -days 200 -newkey rsa:2048 -keyout client1.key -out client1.crt + * -subj "/C=US/ST=New York/L=New York/O=Google/OU=domain-registry-test/CN=client1" + */ + public static final String SAMPLE_CERT3 = + "-----BEGIN CERTIFICATE-----\n" + + "MIIDyzCCArOgAwIBAgIUJnhiVrxAxgwkLJzHPm1w/lBoNs4wDQYJKoZIhvcNAQEL\n" + + "BQAwdTELMAkGA1UEBhMCVVMxETAPBgNVBAgMCE5ldyBZb3JrMREwDwYDVQQHDAhO\n" + + "ZXcgWW9yazEPMA0GA1UECgwGR29vZ2xlMR0wGwYDVQQLDBRkb21haW4tcmVnaXN0\n" + + "cnktdGVzdDEQMA4GA1UEAwwHY2xpZW50MTAeFw0yMDEwMTIxNzU5NDFaFw0yMTA0\n" + + "MzAxNzU5NDFaMHUxCzAJBgNVBAYTAlVTMREwDwYDVQQIDAhOZXcgWW9yazERMA8G\n" + + "A1UEBwwITmV3IFlvcmsxDzANBgNVBAoMBkdvb2dsZTEdMBsGA1UECwwUZG9tYWlu\n" + + "LXJlZ2lzdHJ5LXRlc3QxEDAOBgNVBAMMB2NsaWVudDEwggEiMA0GCSqGSIb3DQEB\n" + + "AQUAA4IBDwAwggEKAoIBAQC0msirO7kXyGEC93stsNYGc02Z77Q2qfHFwaGYkUG8\n" + + "QvOF5SWN+jwTo5Td6Jj26A26a8MLCtK45TCBuMRNcUsHhajhT19ocphO20iY3zhi\n" + + "ycwV1id0iwME4kPd1m57BELRE9tUPOxF81/JQXdR1fwT5KRVHYRDWZhaZ5aBmlZY\n" + + "3t/H9Ly0RBYyApkMaGs3nlb94OOug6SouUfRt02S59ja3wsE2SVF/Eui647OXP7O\n" + + "QdYXofxuqLoNkE8EnAdl43/enGLiCIVd0G2lABibFF+gbxTtfgbg7YtfUZJdL+Mb\n" + + "RAcAtuLXEamNQ9H63JgVF16PlQVCDz2XyI3uCfPpDDiBAgMBAAGjUzBRMB0GA1Ud\n" + + "DgQWBBQ26bWk8qfEBjXs/xZ4m8JZyalnITAfBgNVHSMEGDAWgBQ26bWk8qfEBjXs\n" + + "/xZ4m8JZyalnITAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQAZ\n" + + "VcsgslBKanKOieJ5ik2d9qzOMXKfBuWPRFWbkC3t9i5awhHqnGAaj6nICnnMZIyt\n" + + "rdx5lZW5aaQyf0EP/90JAA8Xmty4A6MXmEjQAMiCOpP3A7eeS6Xglgi8IOZl4/bg\n" + + "LonW62TUkilo5IiFt/QklFTeHIjXB+OvA8+2Quqyd+zp7v6KnhXjvaomim78DhwE\n" + + "0PIUnjmiRpGpHfTVioTdfhPHZ2Y93Y8K7juL93sQog9aBu5m9XRJCY6wGyWPE83i\n" + + "kmLfGzjcnaJ6kqCd9xQRFZ0JwHmGlkAQvFoeengbNUqSyjyVgsOoNkEsrWwe/JFO\n" + + "iqBvjEhJlvRoefvkdR98\n" + + "-----END CERTIFICATE-----\n"; + + /* + * python -c "import sys;print sys.argv[1].decode('hex').encode('base64').strip('\n=')" $(openssl + * x509 -fingerprint -sha256 -in client1.crt | grep -Po '(?<=Fingerprint=).*' | sed s/://g) + */ + public static final String SAMPLE_CERT3_HASH = "GM2tYFuzdpDXN0lqpUXlsvrqk8OdMayryV+4/DOFZ0M"; + private CertificateSamples() {} } diff --git a/core/src/test/java/google/registry/tools/CommandTestCase.java b/core/src/test/java/google/registry/tools/CommandTestCase.java index 6a173d1b6..3ccb7ef70 100644 --- a/core/src/test/java/google/registry/tools/CommandTestCase.java +++ b/core/src/test/java/google/registry/tools/CommandTestCase.java @@ -175,7 +175,12 @@ public abstract class CommandTestCase { /** Returns a path to a known good certificate file. */ String getCertFilename() throws IOException { - return writeToNamedTmpFile("cert.pem", CertificateSamples.SAMPLE_CERT); + return getCertFilename(CertificateSamples.SAMPLE_CERT); + } + + /** Returns a path to a specified certificate file. */ + String getCertFilename(String certificateFile) throws IOException { + return writeToNamedTmpFile("cert.pem", certificateFile); } /** Reloads the given resource from Datastore. */ diff --git a/core/src/test/java/google/registry/tools/CreateRegistrarCommandTest.java b/core/src/test/java/google/registry/tools/CreateRegistrarCommandTest.java index 02d28da0b..a151953b1 100644 --- a/core/src/test/java/google/registry/tools/CreateRegistrarCommandTest.java +++ b/core/src/test/java/google/registry/tools/CreateRegistrarCommandTest.java @@ -19,9 +19,12 @@ import static com.google.common.truth.Truth8.assertThat; import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; 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.DatastoreHelper.createTlds; import static google.registry.testing.DatastoreHelper.persistNewRegistrar; +import static google.registry.util.DateTimeUtils.START_OF_TIME; import static org.joda.time.DateTimeZone.UTC; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.eq; @@ -31,11 +34,13 @@ import static org.mockito.Mockito.when; import com.beust.jcommander.ParameterException; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSortedMap; import com.google.common.collect.Range; import com.google.common.net.MediaType; import google.registry.model.registrar.Registrar; -import google.registry.testing.CertificateSamples; +import google.registry.util.CertificateChecker; import java.io.IOException; +import java.security.cert.CertificateException; import java.util.Optional; import org.joda.money.CurrencyUnit; import org.joda.time.DateTime; @@ -52,6 +57,12 @@ class CreateRegistrarCommandTest extends CommandTestCase @BeforeEach void beforeEach() { command.setConnection(connection); + command.certificateChecker = + new CertificateChecker( + ImmutableSortedMap.of(START_OF_TIME, 825, DateTime.parse("2020-09-01T00:00:00Z"), 398), + 30, + 2048, + fakeClock); } @Test @@ -354,12 +365,13 @@ class CreateRegistrarCommandTest extends CommandTestCase @Test void testSuccess_clientCertFileFlag() throws Exception { + fakeClock.setTo(DateTime.parse("2020-11-01T00:00:00Z")); runCommandForced( "--name=blobio", "--password=some_password", "--registrar_type=REAL", "--iana_id=8", - "--cert_file=" + getCertFilename(), + "--cert_file=" + getCertFilename(SAMPLE_CERT3), "--passcode=01234", "--icann_referral_email=foo@bar.test", "--street=\"123 Fake St\"", @@ -371,8 +383,67 @@ class CreateRegistrarCommandTest extends CommandTestCase Optional registrar = Registrar.loadByClientId("clientz"); assertThat(registrar).isPresent(); - assertThat(registrar.get().getClientCertificateHash()) - .isEqualTo(CertificateSamples.SAMPLE_CERT_HASH); + assertThat(registrar.get().getClientCertificateHash()).isEqualTo(SAMPLE_CERT3_HASH); + } + + @Test + void testFail_clientCertFileFlagWithViolation() throws Exception { + fakeClock.setTo(DateTime.parse("2020-10-01T00:00:00Z")); + CertificateException thrown = + assertThrows( + CertificateException.class, + () -> + runCommandForced( + "--name=blobio", + "--password=some_password", + "--registrar_type=REAL", + "--iana_id=8", + "--cert_file=" + getCertFilename(SAMPLE_CERT), + "--passcode=01234", + "--icann_referral_email=foo@bar.test", + "--street=\"123 Fake St\"", + "--city Fakington", + "--state MA", + "--zip 00351", + "--cc US", + "clientz")); + + assertThat(thrown.getMessage()) + .isEqualTo( + "Certificate validity period is too long; it must be less than or equal to 398" + + " days."); + Optional registrar = Registrar.loadByClientId("clientz"); + assertThat(registrar).isEmpty(); + } + + @Test + void testFail_clientCertFileFlagWithMultipleViolations() throws Exception { + fakeClock.setTo(DateTime.parse("2055-10-01T00:00:00Z")); + CertificateException thrown = + assertThrows( + CertificateException.class, + () -> + runCommandForced( + "--name=blobio", + "--password=some_password", + "--registrar_type=REAL", + "--iana_id=8", + "--cert_file=" + getCertFilename(SAMPLE_CERT), + "--passcode=01234", + "--icann_referral_email=foo@bar.test", + "--street=\"123 Fake St\"", + "--city Fakington", + "--state MA", + "--zip 00351", + "--cc US", + "clientz")); + + assertThat(thrown.getMessage()) + .isEqualTo( + "Certificate is expired.\nCertificate validity period is too long; it must be less" + + " than or equal to 398 days."); + Optional registrar = Registrar.loadByClientId("clientz"); + assertThat(registrar).isEmpty(); } @Test @@ -400,12 +471,13 @@ class CreateRegistrarCommandTest extends CommandTestCase @Test void testSuccess_failoverClientCertFileFlag() throws Exception { + fakeClock.setTo(DateTime.parse("2020-11-01T00:00:00Z")); runCommandForced( "--name=blobio", "--password=some_password", "--registrar_type=REAL", "--iana_id=8", - "--failover_cert_file=" + getCertFilename(), + "--failover_cert_file=" + getCertFilename(SAMPLE_CERT3), "--passcode=01234", "--icann_referral_email=foo@bar.test", "--street=\"123 Fake St\"", @@ -420,8 +492,68 @@ class CreateRegistrarCommandTest extends CommandTestCase Registrar registrar = registrarOptional.get(); assertThat(registrar.getClientCertificate()).isNull(); assertThat(registrar.getClientCertificateHash()).isNull(); - assertThat(registrar.getFailoverClientCertificate()).isEqualTo(SAMPLE_CERT); - assertThat(registrar.getFailoverClientCertificateHash()).isEqualTo(SAMPLE_CERT_HASH); + assertThat(registrar.getFailoverClientCertificate()).isEqualTo(SAMPLE_CERT3); + assertThat(registrar.getFailoverClientCertificateHash()).isEqualTo(SAMPLE_CERT3_HASH); + } + + @Test + void testFail_failoverClientCertFileFlagWithViolations() throws Exception { + fakeClock.setTo(DateTime.parse("2020-11-01T00:00:00Z")); + CertificateException thrown = + assertThrows( + CertificateException.class, + () -> + runCommandForced( + "--name=blobio", + "--password=some_password", + "--registrar_type=REAL", + "--iana_id=8", + "--failover_cert_file=" + getCertFilename(SAMPLE_CERT), + "--passcode=01234", + "--icann_referral_email=foo@bar.test", + "--street=\"123 Fake St\"", + "--city Fakington", + "--state MA", + "--zip 00351", + "--cc US", + "clientz")); + + assertThat(thrown.getMessage()) + .isEqualTo( + "Certificate validity period is too long; it must be less than or equal to 398" + + " days."); + Optional registrar = Registrar.loadByClientId("clientz"); + assertThat(registrar).isEmpty(); + } + + @Test + void testFail_failoverClientCertFileFlagWithMultipleViolations() throws Exception { + fakeClock.setTo(DateTime.parse("2055-11-01T00:00:00Z")); + CertificateException thrown = + assertThrows( + CertificateException.class, + () -> + runCommandForced( + "--name=blobio", + "--password=some_password", + "--registrar_type=REAL", + "--iana_id=8", + "--failover_cert_file=" + getCertFilename(SAMPLE_CERT), + "--passcode=01234", + "--icann_referral_email=foo@bar.test", + "--street=\"123 Fake St\"", + "--city Fakington", + "--state MA", + "--zip 00351", + "--cc US", + "clientz")); + + assertThat(thrown.getMessage()) + .isEqualTo( + "Certificate is expired.\nCertificate validity period is too long; it must be less" + + " than or equal to 398 days."); + Optional registrar = Registrar.loadByClientId("clientz"); + assertThat(registrar).isEmpty(); } @Test @@ -1052,7 +1184,7 @@ class CreateRegistrarCommandTest extends CommandTestCase @Test void testFailure_invalidCertFileContents() { assertThrows( - Exception.class, + CertificateException.class, () -> runCommandForced( "--name=blobio", @@ -1073,7 +1205,7 @@ class CreateRegistrarCommandTest extends CommandTestCase @Test void testFailure_invalidFailoverCertFileContents() { assertThrows( - IllegalArgumentException.class, + CertificateException.class, () -> runCommandForced( "--name=blobio", @@ -1094,7 +1226,7 @@ class CreateRegistrarCommandTest extends CommandTestCase @Test void testFailure_certHashAndCertFile() { assertThrows( - IllegalArgumentException.class, + CertificateException.class, () -> runCommandForced( "--name=blobio", diff --git a/core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java b/core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java index 9395404e5..992a33f73 100644 --- a/core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java +++ b/core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java @@ -19,10 +19,13 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth8.assertThat; import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; 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.DatastoreHelper.createTlds; import static google.registry.testing.DatastoreHelper.loadRegistrar; import static google.registry.testing.DatastoreHelper.persistResource; +import static google.registry.util.DateTimeUtils.START_OF_TIME; import static org.joda.time.DateTimeZone.UTC; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -30,20 +33,34 @@ import com.beust.jcommander.ParameterException; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.ImmutableSortedMap; import google.registry.model.registrar.Registrar; import google.registry.model.registrar.Registrar.State; import google.registry.model.registrar.Registrar.Type; import google.registry.persistence.VKey; import google.registry.testing.AppEngineExtension; +import google.registry.util.CertificateChecker; import google.registry.util.CidrAddressBlock; +import java.security.cert.CertificateException; import java.util.Optional; import org.joda.money.CurrencyUnit; import org.joda.time.DateTime; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; /** Unit tests for {@link UpdateRegistrarCommand}. */ class UpdateRegistrarCommandTest extends CommandTestCase { + @BeforeEach + void beforeEach() { + command.certificateChecker = + new CertificateChecker( + ImmutableSortedMap.of(START_OF_TIME, 825, DateTime.parse("2020-09-01T00:00:00Z"), 398), + 30, + 2048, + fakeClock); + } + @Test void testSuccess_alsoUpdateInCloudSql() throws Exception { assertThat(loadRegistrar("NewRegistrar").verifyPassword("some_password")).isFalse(); @@ -232,15 +249,94 @@ class UpdateRegistrarCommandTest extends CommandTestCase @Test void testSuccess_certFile() throws Exception { + fakeClock.setTo(DateTime.parse("2020-11-01T00:00:00Z")); Registrar registrar = loadRegistrar("NewRegistrar"); assertThat(registrar.getClientCertificate()).isNull(); assertThat(registrar.getClientCertificateHash()).isNull(); - runCommand("--cert_file=" + getCertFilename(), "--force", "NewRegistrar"); + runCommand("--cert_file=" + getCertFilename(SAMPLE_CERT3), "--force", "NewRegistrar"); registrar = loadRegistrar("NewRegistrar"); // NB: Hash was computed manually using 'openssl x509 -fingerprint -sha256 -in ...' and then // converting the result from a hex string to non-padded base64 encoded string. - assertThat(registrar.getClientCertificate()).isEqualTo(SAMPLE_CERT); - assertThat(registrar.getClientCertificateHash()).isEqualTo(SAMPLE_CERT_HASH); + assertThat(registrar.getClientCertificate()).isEqualTo(SAMPLE_CERT3); + assertThat(registrar.getClientCertificateHash()).isEqualTo(SAMPLE_CERT3_HASH); + } + + @Test + void testFail_certFileWithViolation() throws Exception { + fakeClock.setTo(DateTime.parse("2020-11-01T00:00:00Z")); + Registrar registrar = loadRegistrar("NewRegistrar"); + assertThat(registrar.getClientCertificate()).isNull(); + assertThat(registrar.getClientCertificateHash()).isNull(); + CertificateException thrown = + assertThrows( + CertificateException.class, + () -> runCommand("--cert_file=" + getCertFilename(), "--force", "NewRegistrar")); + assertThat(thrown.getMessage()) + .isEqualTo( + "Certificate validity period is too long; it must be less than or equal to 398" + + " days."); + assertThat(registrar.getClientCertificate()).isNull(); + } + + @Test + void testFail_certFileWithMultipleViolations() throws Exception { + fakeClock.setTo(DateTime.parse("2055-10-01T00:00:00Z")); + Registrar registrar = loadRegistrar("NewRegistrar"); + assertThat(registrar.getClientCertificate()).isNull(); + assertThat(registrar.getClientCertificateHash()).isNull(); + CertificateException thrown = + assertThrows( + CertificateException.class, + () -> runCommand("--cert_file=" + getCertFilename(), "--force", "NewRegistrar")); + assertThat(thrown.getMessage()) + .isEqualTo( + "Certificate is expired.\nCertificate validity period is too long; it must be less" + + " than or equal to 398 days."); + assertThat(registrar.getClientCertificate()).isNull(); + } + + @Test + void testFail_failoverCertFileWithViolation() throws Exception { + fakeClock.setTo(DateTime.parse("2020-11-01T00:00:00Z")); + Registrar registrar = loadRegistrar("NewRegistrar"); + assertThat(registrar.getFailoverClientCertificate()).isNull(); + CertificateException thrown = + assertThrows( + CertificateException.class, + () -> + runCommand("--failover_cert_file=" + getCertFilename(), "--force", "NewRegistrar")); + assertThat(thrown.getMessage()) + .isEqualTo( + "Certificate validity period is too long; it must be less than or equal to 398" + + " days."); + assertThat(registrar.getFailoverClientCertificate()).isNull(); + } + + @Test + void testFail_failoverCertFileWithMultipleViolations() throws Exception { + fakeClock.setTo(DateTime.parse("2055-10-01T00:00:00Z")); + Registrar registrar = loadRegistrar("NewRegistrar"); + assertThat(registrar.getFailoverClientCertificate()).isNull(); + CertificateException thrown = + assertThrows( + CertificateException.class, + () -> + runCommand("--failover_cert_file=" + getCertFilename(), "--force", "NewRegistrar")); + assertThat(thrown.getMessage()) + .isEqualTo( + "Certificate is expired.\nCertificate validity period is too long; it must be less" + + " than or equal to 398 days."); + assertThat(registrar.getFailoverClientCertificate()).isNull(); + } + + @Test + void testSuccess_failoverCertFile() throws Exception { + fakeClock.setTo(DateTime.parse("2020-11-01T00:00:00Z")); + Registrar registrar = loadRegistrar("NewRegistrar"); + assertThat(registrar.getFailoverClientCertificate()).isNull(); + runCommand("--failover_cert_file=" + getCertFilename(SAMPLE_CERT3), "--force", "NewRegistrar"); + registrar = loadRegistrar("NewRegistrar"); + assertThat(registrar.getFailoverClientCertificate()).isEqualTo(SAMPLE_CERT3); } @Test @@ -672,7 +768,7 @@ class UpdateRegistrarCommandTest extends CommandTestCase IllegalArgumentException.class, () -> runCommand( - "--cert_file=" + getCertFilename(), + "--cert_file=" + getCertFilename(SAMPLE_CERT3), "--cert_hash=ABCDEF", "--force", "NewRegistrar")); diff --git a/util/src/main/java/google/registry/util/CertificateChecker.java b/util/src/main/java/google/registry/util/CertificateChecker.java index e94911069..6f177f558 100644 --- a/util/src/main/java/google/registry/util/CertificateChecker.java +++ b/util/src/main/java/google/registry/util/CertificateChecker.java @@ -53,7 +53,6 @@ public class CertificateChecker { * ); * */ - // TODO(sarahbot): Inject this. public CertificateChecker( ImmutableSortedMap maxValidityLengthSchedule, int daysToExpiration,