Add certificate checks for create and update registrar commands (#837)

* Add certificatechecks for create and update registrar commands

* Add CertificateCheckerModule

* Remove commented out code

* Still tring to get dependency injection to work

* Get this actually working

* Add tests for multiple violations

* Small formatting fixes

* Rename configs and fix collectors

* Add checks for failover client certificate

* Fix formatting
This commit is contained in:
sarahcaseybot 2020-10-22 11:43:22 -04:00 committed by GitHub
parent 0b73e9032c
commit 93d922af6f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 463 additions and 19 deletions

View file

@ -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() {}
}

View file

@ -175,7 +175,12 @@ public abstract class CommandTestCase<C extends Command> {
/** 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. */

View file

@ -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<CreateRegistrarCommand>
@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<CreateRegistrarCommand>
@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<CreateRegistrarCommand>
Optional<Registrar> 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 = 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 = Registrar.loadByClientId("clientz");
assertThat(registrar).isEmpty();
}
@Test
@ -400,12 +471,13 @@ class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarCommand>
@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<CreateRegistrarCommand>
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 = 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 = Registrar.loadByClientId("clientz");
assertThat(registrar).isEmpty();
}
@Test
@ -1052,7 +1184,7 @@ class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarCommand>
@Test
void testFailure_invalidCertFileContents() {
assertThrows(
Exception.class,
CertificateException.class,
() ->
runCommandForced(
"--name=blobio",
@ -1073,7 +1205,7 @@ class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarCommand>
@Test
void testFailure_invalidFailoverCertFileContents() {
assertThrows(
IllegalArgumentException.class,
CertificateException.class,
() ->
runCommandForced(
"--name=blobio",
@ -1094,7 +1226,7 @@ class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarCommand>
@Test
void testFailure_certHashAndCertFile() {
assertThrows(
IllegalArgumentException.class,
CertificateException.class,
() ->
runCommandForced(
"--name=blobio",

View file

@ -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<UpdateRegistrarCommand> {
@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<UpdateRegistrarCommand>
@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<UpdateRegistrarCommand>
IllegalArgumentException.class,
() ->
runCommand(
"--cert_file=" + getCertFilename(),
"--cert_file=" + getCertFilename(SAMPLE_CERT3),
"--cert_hash=ABCDEF",
"--force",
"NewRegistrar"));