From 75acd574cc9867a96a966edb078245602618f704 Mon Sep 17 00:00:00 2001 From: Weimin Yu Date: Thu, 27 Apr 2023 14:42:11 -0400 Subject: [PATCH] Allow rotation when updating registrar cert (#2012) * Allow rotation when updating registrar cert When updating a registrar's primary cert, add a flag to activate rotation of previous primary cert to failover. This functionality is part of the prober ssl cert renewal automation. --- .../tools/CreateOrUpdateRegistrarCommand.java | 20 +++++++ .../tools/CreateRegistrarCommandTest.java | 28 +++++++++ .../tools/UpdateRegistrarCommandTest.java | 59 +++++++++++++++++++ 3 files changed, 107 insertions(+) diff --git a/core/src/main/java/google/registry/tools/CreateOrUpdateRegistrarCommand.java b/core/src/main/java/google/registry/tools/CreateOrUpdateRegistrarCommand.java index 6726f615e..878d57da7 100644 --- a/core/src/main/java/google/registry/tools/CreateOrUpdateRegistrarCommand.java +++ b/core/src/main/java/google/registry/tools/CreateOrUpdateRegistrarCommand.java @@ -17,6 +17,7 @@ package google.registry.tools; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Predicates.isNull; import static com.google.common.base.Strings.isNullOrEmpty; +import static com.google.common.base.Verify.verify; import static com.google.common.collect.ImmutableSet.toImmutableSet; import static google.registry.util.DomainNameUtils.canonicalizeHostname; import static google.registry.util.RegistrarUtils.normalizeRegistrarName; @@ -130,6 +131,13 @@ abstract class CreateOrUpdateRegistrarCommand extends MutatingCommand { validateWith = PathParameter.InputFile.class) Path clientCertificateFilename; + @Parameter( + names = "--rotate_primary_cert", + description = + "Used together with --cert_file when updating an registrar. " + + "If set, current cert is saved as failover.") + private Boolean rotatePrimaryCert = Boolean.FALSE; + @Nullable @Parameter( names = "--failover_cert_file", @@ -341,8 +349,20 @@ abstract class CreateOrUpdateRegistrarCommand extends MutatingCommand { if (!asciiCert.equals("")) { certificateChecker.validateCertificate(asciiCert); } + if (rotatePrimaryCert) { + verify( + oldRegistrar != null, + "--rotate_primary_cert cannot be set by create_registrar command."); + verify( + oldRegistrar.getClientCertificate().isPresent(), + "Primary cert is absent. Rotation may remove a failover certificate still in use."); + builder.setFailoverClientCertificate(oldRegistrar.getClientCertificate().get(), now); + } builder.setClientCertificate(asciiCert, now); } + if (rotatePrimaryCert && clientCertificateFilename == null) { + throw new IllegalArgumentException("--rotate_primary_cert must be used with --cert_file."); + } if (failoverClientCertificateFilename != null) { String asciiCert = diff --git a/core/src/test/java/google/registry/tools/CreateRegistrarCommandTest.java b/core/src/test/java/google/registry/tools/CreateRegistrarCommandTest.java index e17136ecd..3a3d6f85c 100644 --- a/core/src/test/java/google/registry/tools/CreateRegistrarCommandTest.java +++ b/core/src/test/java/google/registry/tools/CreateRegistrarCommandTest.java @@ -32,6 +32,7 @@ import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import com.beust.jcommander.ParameterException; +import com.google.common.base.VerifyException; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedMap; @@ -1785,4 +1786,31 @@ class CreateRegistrarCommandTest extends CommandTestCase .hasMessageThat() .isEqualTo("Provided email lolcat is not a valid email address"); } + + @Test + void testRotatePrimaryCertFlag_throwException() { + fakeClock.setTo(DateTime.parse("2020-11-01T00:00:00Z")); + VerifyException thrown = + assertThrows( + VerifyException.class, + () -> + runCommandForced( + "--name=blobio", + "--password=some_password", + "--registrar_type=REAL", + "--iana_id=8", + "--cert_file=" + getCertFilename(SAMPLE_CERT3), + "--rotate_primary_cert", + "--passcode=01234", + "--icann_referral_email=foo@bar.test", + "--street=\"123 Fake St\"", + "--city Fakington", + "--state MA", + "--zip 00351", + "--cc US", + "clientz")); + assertThat(thrown) + .hasMessageThat() + .isEqualTo("--rotate_primary_cert cannot be set by create_registrar command."); + } } diff --git a/core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java b/core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java index 77d3dc1b4..ca52b9fc7 100644 --- a/core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java +++ b/core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java @@ -30,6 +30,7 @@ import static org.joda.time.DateTimeZone.UTC; import static org.junit.jupiter.api.Assertions.assertThrows; import com.beust.jcommander.ParameterException; +import com.google.common.base.VerifyException; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -248,6 +249,64 @@ class UpdateRegistrarCommandTest extends CommandTestCase // converting the result from a hex string to non-padded base64 encoded string. assertThat(registrar.getClientCertificate()).hasValue(SAMPLE_CERT3); assertThat(registrar.getClientCertificateHash()).hasValue(SAMPLE_CERT3_HASH); + assertThat(registrar.getFailoverClientCertificate()).isEmpty(); + assertThat(registrar.getFailoverClientCertificateHash()).isEmpty(); + } + + @Test + void testSuccess_rotatePrimaryCert() throws Exception { + fakeClock.setTo(DateTime.parse("2020-11-01T00:00:00Z")); + persistResource( + loadRegistrar("NewRegistrar") + .asBuilder() + .setClientCertificate(SAMPLE_CERT3, fakeClock.nowUtc()) + .setFailoverClientCertificate(null, fakeClock.nowUtc()) + .build()); + + Registrar registrar = loadRegistrar("NewRegistrar"); + assertThat(registrar.getFailoverClientCertificate()).isEmpty(); + assertThat(registrar.getFailoverClientCertificateHash()).isEmpty(); + runCommand( + "--cert_file=" + getCertFilename(SAMPLE_CERT3), + "--rotate_primary_cert", + "--force", + "NewRegistrar"); + + registrar = loadRegistrar("NewRegistrar"); + assertThat(registrar.getFailoverClientCertificate()).hasValue(SAMPLE_CERT3); + assertThat(registrar.getFailoverClientCertificateHash()).hasValue(SAMPLE_CERT3_HASH); + } + + @Test + void test_rotatePrimaryCert_noPrimaryCert() throws Exception { + fakeClock.setTo(DateTime.parse("2020-11-01T00:00:00Z")); + Registrar registrar = loadRegistrar("NewRegistrar"); + assertThat(registrar.getClientCertificate()).isEmpty(); + assertThat(registrar.getClientCertificateHash()).isEmpty(); + VerifyException thrown = + assertThrows( + VerifyException.class, + () -> + runCommand( + "--cert_file=" + getCertFilename(SAMPLE_CERT3), + "--rotate_primary_cert", + "--force", + "NewRegistrar")); + assertThat(thrown) + .hasMessageThat() + .isEqualTo( + "Primary cert is absent. Rotation may remove a failover certificate still in use."); + } + + @Test + public void test_rotatePrimaryCert_withoutNewCertFile_throws() { + IllegalArgumentException thrown = + assertThrows( + IllegalArgumentException.class, + () -> runCommand("--rotate_primary_cert", "--force", "NewRegistrar")); + assertThat(thrown) + .hasMessageThat() + .isEqualTo("--rotate_primary_cert must be used with --cert_file."); } @Test