Add cert enforcement in production start date (#953)

* Add start date for cert enforcement in production

* Add TODO to remove start date check after start date

* revert changes to package-lock.json

* Make start time a constant
This commit is contained in:
sarahcaseybot 2021-02-04 16:30:23 -05:00 committed by GitHub
parent 58f37a8bf6
commit dd2787fd4d
6 changed files with 92 additions and 23 deletions

View file

@ -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<String> clientCertificateHash;
private final Optional<String> clientCertificate;
private final Optional<InetAddress> 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<String> clientCertificateHash,
@Header("X-SSL-Full-Certificate") Optional<String> clientCertificate,
@Header("X-Forwarded-For") Optional<String> 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()

View file

@ -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());

View file

@ -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 =

View file

@ -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(

View file

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

View file

@ -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);
}
}