diff --git a/core/src/main/java/google/registry/flows/TlsCredentials.java b/core/src/main/java/google/registry/flows/TlsCredentials.java index 4c23c799c..180534761 100644 --- a/core/src/main/java/google/registry/flows/TlsCredentials.java +++ b/core/src/main/java/google/registry/flows/TlsCredentials.java @@ -33,6 +33,7 @@ import google.registry.flows.certs.CertificateChecker.InsecureCertificateExcepti import google.registry.model.registrar.Registrar; import google.registry.request.Header; import google.registry.util.CidrAddressBlock; +import google.registry.util.Clock; import java.io.ByteArrayInputStream; import java.net.InetAddress; import java.security.cert.CertificateException; @@ -41,6 +42,7 @@ import java.util.Base64; import java.util.Optional; import javax.inject.Inject; import javax.servlet.http.HttpServletRequest; +import org.joda.time.DateTime; /** * Container and validation for TLS certificate and IP-allow-listing. @@ -63,12 +65,15 @@ import javax.servlet.http.HttpServletRequest; public class TlsCredentials implements TransportCredentials { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + private static final DateTime CERT_ENFORCEMENT_START_TIME = + DateTime.parse("2021-03-01T16:00:00Z"); private final boolean requireSslCertificates; private final Optional clientCertificateHash; private final Optional clientCertificate; private final Optional clientInetAddr; private final CertificateChecker certificateChecker; + private final Clock clock; @Inject public TlsCredentials( @@ -76,12 +81,14 @@ public class TlsCredentials implements TransportCredentials { @Header("X-SSL-Certificate") Optional clientCertificateHash, @Header("X-SSL-Full-Certificate") Optional clientCertificate, @Header("X-Forwarded-For") Optional clientAddress, - CertificateChecker certificateChecker) { + CertificateChecker certificateChecker, + Clock clock) { this.requireSslCertificates = requireSslCertificates; this.clientCertificateHash = clientCertificateHash; this.clientCertificate = clientCertificate; this.clientInetAddr = clientAddress.map(TlsCredentials::parseInetAddress); this.certificateChecker = certificateChecker; + this.clock = clock; } static InetAddress parseInetAddress(String asciiAddr) { @@ -180,9 +187,12 @@ public class TlsCredentials implements TransportCredentials { try { certificateChecker.validateCertificate(passedCert); } catch (InsecureCertificateException e) { + // TODO(Sarahbot@): Remove this if statement after March 1. After March 1, exception + // should be thrown in all environments. // throw exception in unit tests and Sandbox if (RegistryEnvironment.get().equals(RegistryEnvironment.UNITTEST) - || RegistryEnvironment.get().equals(RegistryEnvironment.SANDBOX)) { + || RegistryEnvironment.get().equals(RegistryEnvironment.SANDBOX) + || clock.nowUtc().isAfter(CERT_ENFORCEMENT_START_TIME)) { throw new CertificateContainsSecurityViolationsException(e); } logger.atWarning().log( @@ -204,6 +214,9 @@ public class TlsCredentials implements TransportCredentials { } private void validateCertificateHash(Registrar registrar) throws AuthenticationErrorException { + logger.atWarning().log( + "Error validating certificate for %s, attempting to validate using certificate hash.", + registrar.getClientId()); // Check the certificate hash as a failover // TODO(sarahbot): Remove hash checks once certificate checks are working. if (!registrar.getClientCertificateHash().isPresent() diff --git a/core/src/main/java/google/registry/tools/ValidateLoginCredentialsCommand.java b/core/src/main/java/google/registry/tools/ValidateLoginCredentialsCommand.java index fcd5ec040..f00ff3d8c 100644 --- a/core/src/main/java/google/registry/tools/ValidateLoginCredentialsCommand.java +++ b/core/src/main/java/google/registry/tools/ValidateLoginCredentialsCommand.java @@ -29,6 +29,7 @@ import google.registry.flows.TlsCredentials; import google.registry.flows.certs.CertificateChecker; import google.registry.model.registrar.Registrar; import google.registry.tools.params.PathParameter; +import google.registry.util.Clock; import java.nio.file.Files; import java.nio.file.Path; import java.util.Optional; @@ -72,6 +73,7 @@ final class ValidateLoginCredentialsCommand implements CommandWithRemoteApi { private String clientIpAddress = "10.0.0.1"; @Inject CertificateChecker certificateChecker; + @Inject Clock clock; @Override public void run() throws Exception { @@ -92,7 +94,8 @@ final class ValidateLoginCredentialsCommand implements CommandWithRemoteApi { Optional.ofNullable(clientCertificateHash), Optional.ofNullable(encodedCertificate), Optional.ofNullable(clientIpAddress), - certificateChecker) + certificateChecker, + clock) .validate(registrar, password); checkState( registrar.isLive(), "Registrar %s has non-live state: %s", clientId, registrar.getState()); diff --git a/core/src/test/java/google/registry/flows/EppLoginTlsTest.java b/core/src/test/java/google/registry/flows/EppLoginTlsTest.java index 2f2aa97ed..a60ed3075 100644 --- a/core/src/test/java/google/registry/flows/EppLoginTlsTest.java +++ b/core/src/test/java/google/registry/flows/EppLoginTlsTest.java @@ -79,7 +79,8 @@ class EppLoginTlsTest extends EppTestCase { Optional.ofNullable(clientCertificateHash), Optional.ofNullable(clientCertificate), Optional.of("192.168.1.100:54321"), - certificateChecker)); + certificateChecker, + clock)); } @BeforeEach @@ -275,7 +276,8 @@ class EppLoginTlsTest extends EppTestCase { @Test // TODO(sarahbot@): Remove this test once requirements are enforced in production - void testCertificateDoesNotMeetRequirementsInProduction_succeeds() throws Exception { + void testCertificateDoesNotMeetRequirementsInProduction_beforeStartDate_succeeds() + throws Exception { RegistryEnvironment.PRODUCTION.setup(systemPropertyExtension); // SAMPLE_CERT has a validity period that is too long String proxyEncoded = encodeX509CertificateFromPemString(CertificateSamples.SAMPLE_CERT); @@ -288,7 +290,9 @@ class EppLoginTlsTest extends EppTestCase { .build()); // Even though the certificate contains security violations, the login will succeed in // production - assertThatLoginSucceeds("NewRegistrar", "foo-BAR2"); + assertThatLogin("NewRegistrar", "foo-BAR2") + .atTime(DateTime.parse("2020-01-01T16:00:00Z")) + .hasSuccessfulLogin(); assertAboutLogs() .that(handler) .hasLogAtLevelWithMessage( @@ -298,6 +302,31 @@ class EppLoginTlsTest extends EppTestCase { + " days."); } + @Test + void testCertificateDoesNotMeetRequirementsInProduction_afterStartDate_fails() throws Exception { + RegistryEnvironment.PRODUCTION.setup(systemPropertyExtension); + String proxyEncoded = encodeX509CertificateFromPemString(CertificateSamples.SAMPLE_CERT); + // SAMPLE_CERT has a validity period that is too long + setCredentials(CertificateSamples.SAMPLE_CERT_HASH, proxyEncoded); + persistResource( + loadRegistrar("NewRegistrar") + .asBuilder() + .setClientCertificate(CertificateSamples.SAMPLE_CERT, clock.nowUtc()) + .setFailoverClientCertificate(CertificateSamples.SAMPLE_CERT2, clock.nowUtc()) + .build()); + assertThatLogin("NewRegistrar", "foo-BAR2") + .atTime(DateTime.parse("2021-04-01T16:00:00Z")) + .hasResponse( + "response_error.xml", + ImmutableMap.of( + "CODE", + "2200", + "MSG", + "Registrar certificate contains the following security violations:\n" + + "Certificate validity period is too long; it must be less than or equal to" + + " 398 days.")); + } + @Test void testRegistrarCertificateContainsExtraMetadata_succeeds() throws Exception { String certPem = diff --git a/core/src/test/java/google/registry/flows/FlowRunnerTest.java b/core/src/test/java/google/registry/flows/FlowRunnerTest.java index 694cbcc0f..dedec3507 100644 --- a/core/src/test/java/google/registry/flows/FlowRunnerTest.java +++ b/core/src/test/java/google/registry/flows/FlowRunnerTest.java @@ -157,7 +157,8 @@ class FlowRunnerTest { Optional.of("abc123def"), Optional.of("cert046F5A3"), Optional.of("127.0.0.1"), - certificateChecker); + certificateChecker, + clock); flowRunner.run(eppMetricBuilder); assertThat(Splitter.on("\n\t").split(findFirstLogMessageByPrefix(handler, "EPP Command\n\t"))) .contains( diff --git a/core/src/test/java/google/registry/flows/TlsCredentialsTest.java b/core/src/test/java/google/registry/flows/TlsCredentialsTest.java index e0d480f20..720fba483 100644 --- a/core/src/test/java/google/registry/flows/TlsCredentialsTest.java +++ b/core/src/test/java/google/registry/flows/TlsCredentialsTest.java @@ -71,7 +71,8 @@ final class TlsCredentialsTest { Optional.empty(), Optional.empty(), Optional.of("192.168.1.1"), - certificateChecker); + certificateChecker, + clock); persistResource( loadRegistrar("TheRegistrar") .asBuilder() @@ -86,7 +87,12 @@ final class TlsCredentialsTest { void test_missingIpAddress_doesntAllowAccess() { TlsCredentials tls = new TlsCredentials( - false, Optional.of("certHash"), Optional.empty(), Optional.empty(), certificateChecker); + false, + Optional.of("certHash"), + Optional.empty(), + Optional.empty(), + certificateChecker, + clock); persistResource( loadRegistrar("TheRegistrar") .asBuilder() @@ -106,7 +112,8 @@ final class TlsCredentialsTest { Optional.of("certHash"), Optional.of("cert"), Optional.of("192.168.1.1"), - certificateChecker); + certificateChecker, + clock); persistResource( loadRegistrar("TheRegistrar") .asBuilder() @@ -133,7 +140,8 @@ final class TlsCredentialsTest { Optional.of("hash"), Optional.of(SAMPLE_CERT), Optional.of("192.168.1.1"), - certificateChecker); + certificateChecker, + clock); persistResource(loadRegistrar("TheRegistrar").asBuilder().build()); assertThrows( RegistrarCertificateNotConfiguredException.class, @@ -148,7 +156,8 @@ final class TlsCredentialsTest { Optional.of("certHash"), Optional.of("cert"), Optional.of("192.168.1.1"), - certificateChecker); + certificateChecker, + clock); persistResource( loadRegistrar("TheRegistrar") .asBuilder() 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 e4245efde..8c5c15fce 100644 --- a/core/src/test/java/google/registry/flows/session/LoginFlowViaTlsTest.java +++ b/core/src/test/java/google/registry/flows/session/LoginFlowViaTlsTest.java @@ -83,7 +83,8 @@ public class LoginFlowViaTlsTest extends LoginFlowTestCase { void testSuccess_withGoodCredentials() throws Exception { persistResource(getRegistrarBuilder().build()); credentials = - new TlsCredentials(true, GOOD_CERT_HASH, encodedCertString, GOOD_IP, certificateChecker); + new TlsCredentials( + true, GOOD_CERT_HASH, encodedCertString, GOOD_IP, certificateChecker, clock); doSuccessfulTest("login_valid.xml"); } @@ -111,7 +112,12 @@ public class LoginFlowViaTlsTest extends LoginFlowTestCase { String encodedCertificate = Base64.getEncoder().encodeToString(certificate.getEncoded()); credentials = new TlsCredentials( - true, Optional.empty(), Optional.of(encodedCertificate), GOOD_IP, certificateChecker); + true, + Optional.empty(), + Optional.of(encodedCertificate), + GOOD_IP, + certificateChecker, + clock); doSuccessfulTest("login_valid.xml"); } @@ -123,7 +129,8 @@ public class LoginFlowViaTlsTest extends LoginFlowTestCase { ImmutableList.of(CidrAddressBlock.create("2001:db8:0:0:0:0:1:1/32"))) .build()); credentials = - new TlsCredentials(true, GOOD_CERT_HASH, encodedCertString, GOOD_IPV6, certificateChecker); + new TlsCredentials( + true, GOOD_CERT_HASH, encodedCertString, GOOD_IPV6, certificateChecker, clock); doSuccessfulTest("login_valid.xml"); } @@ -135,7 +142,8 @@ public class LoginFlowViaTlsTest extends LoginFlowTestCase { ImmutableList.of(CidrAddressBlock.create("2001:db8:0:0:0:0:1:1/32"))) .build()); credentials = - new TlsCredentials(true, GOOD_CERT_HASH, encodedCertString, GOOD_IPV6, certificateChecker); + new TlsCredentials( + true, GOOD_CERT_HASH, encodedCertString, GOOD_IPV6, certificateChecker, clock); doSuccessfulTest("login_valid.xml"); } @@ -146,7 +154,8 @@ public class LoginFlowViaTlsTest extends LoginFlowTestCase { .setIpAddressAllowList(ImmutableList.of(CidrAddressBlock.create("192.168.1.255/24"))) .build()); credentials = - new TlsCredentials(true, GOOD_CERT_HASH, encodedCertString, GOOD_IP, certificateChecker); + new TlsCredentials( + true, GOOD_CERT_HASH, encodedCertString, GOOD_IP, certificateChecker, clock); doSuccessfulTest("login_valid.xml"); } @@ -156,7 +165,7 @@ public class LoginFlowViaTlsTest extends LoginFlowTestCase { String proxyEncoded = encodeX509CertificateFromPemString(BAD_CERT.get()); credentials = new TlsCredentials( - true, BAD_CERT_HASH, Optional.of(proxyEncoded), GOOD_IP, certificateChecker); + true, BAD_CERT_HASH, Optional.of(proxyEncoded), GOOD_IP, certificateChecker, clock); doFailingTest("login_valid.xml", BadRegistrarCertificateException.class); } @@ -165,7 +174,8 @@ public class LoginFlowViaTlsTest extends LoginFlowTestCase { void testSuccess_missingClientCertificate() throws Exception { persistResource(getRegistrarBuilder().build()); credentials = - new TlsCredentials(true, GOOD_CERT_HASH, Optional.empty(), GOOD_IP, certificateChecker); + new TlsCredentials( + true, GOOD_CERT_HASH, Optional.empty(), GOOD_IP, certificateChecker, clock); doSuccessfulTest("login_valid.xml"); } @@ -173,7 +183,8 @@ public class LoginFlowViaTlsTest extends LoginFlowTestCase { void testFailure_missingClientCertificateAndHash() { persistResource(getRegistrarBuilder().build()); credentials = - new TlsCredentials(true, Optional.empty(), Optional.empty(), GOOD_IP, certificateChecker); + new TlsCredentials( + true, Optional.empty(), Optional.empty(), GOOD_IP, certificateChecker, clock); doFailingTest("login_valid.xml", MissingRegistrarCertificateException.class); } @@ -187,7 +198,8 @@ public class LoginFlowViaTlsTest extends LoginFlowTestCase { CidrAddressBlock.create(InetAddresses.forString("2001:db8::1"), 128))) .build()); credentials = - new TlsCredentials(true, GOOD_CERT_HASH, GOOD_CERT, Optional.empty(), certificateChecker); + new TlsCredentials( + true, GOOD_CERT_HASH, GOOD_CERT, Optional.empty(), certificateChecker, clock); doFailingTest("login_valid.xml", BadRegistrarIpAddressException.class); } @@ -200,7 +212,8 @@ 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(true, GOOD_CERT_HASH, GOOD_CERT, BAD_IP, certificateChecker); + credentials = + new TlsCredentials(true, GOOD_CERT_HASH, GOOD_CERT, BAD_IP, certificateChecker, clock); doFailingTest("login_valid.xml", BadRegistrarIpAddressException.class); } @@ -213,7 +226,8 @@ 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(true, GOOD_CERT_HASH, GOOD_CERT, BAD_IPV6, certificateChecker); + credentials = + new TlsCredentials(true, GOOD_CERT_HASH, GOOD_CERT, BAD_IPV6, certificateChecker, clock); doFailingTest("login_valid.xml", BadRegistrarIpAddressException.class); } }