From 6d47ed2861feca30447c042cb2fb4d28c6ec2e6d Mon Sep 17 00:00:00 2001 From: Sarah Botwinick Date: Thu, 28 Jan 2021 18:23:29 -0500 Subject: [PATCH] Convert certificate strings to certificates --- .../google/registry/flows/TlsCredentials.java | 50 +++++++++++++++-- .../flows/certs/CertificateChecker.java | 15 +++++ .../ValidateLoginCredentialsCommand.java | 15 ++++- .../registry/flows/EppLoginTlsTest.java | 56 +++++++++++++++---- .../flows/session/LoginFlowViaTlsTest.java | 40 +++++++++++-- 5 files changed, 152 insertions(+), 24 deletions(-) diff --git a/core/src/main/java/google/registry/flows/TlsCredentials.java b/core/src/main/java/google/registry/flows/TlsCredentials.java index 48f64bdff..a5d493e20 100644 --- a/core/src/main/java/google/registry/flows/TlsCredentials.java +++ b/core/src/main/java/google/registry/flows/TlsCredentials.java @@ -16,6 +16,7 @@ package google.registry.flows; import static com.google.common.base.MoreObjects.toStringHelper; import static google.registry.request.RequestParameters.extractOptionalHeader; +import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; @@ -32,7 +33,12 @@ import google.registry.flows.certs.CertificateChecker.InsecureCertificateExcepti import google.registry.model.registrar.Registrar; import google.registry.request.Header; import google.registry.util.CidrAddressBlock; +import java.io.ByteArrayInputStream; import java.net.InetAddress; +import java.security.cert.CertificateException; +import java.security.cert.CertificateFactory; +import java.security.cert.X509Certificate; +import java.util.Base64; import java.util.Optional; import javax.inject.Inject; import javax.servlet.http.HttpServletRequest; @@ -149,14 +155,30 @@ public class TlsCredentials implements TransportCredentials { "Request from registrar %s did not include X-SSL-Full-Certificate.", registrar.getClientId()); } else { + X509Certificate passedCert; + X509Certificate storedCert; + X509Certificate storedFailoverCert; + + try { + storedCert = stringToCert(registrar.getClientCertificate()); + storedFailoverCert = stringToCert(registrar.getFailoverClientCertificate()); + passedCert = encodedCertStringToCert(clientCertificate.get()); + } catch (Exception e) { + //TODO(Sarahbot@): remove this catch once we know it's working + logger.atWarning().log( + "Error converting certificate string to certificate for %s: %s", + registrar.getClientId(), e); + validateCertificateHash(registrar); + return; + } + // Check if the certificate is equal to the one on file for the registrar. - if (clientCertificate.equals(registrar.getClientCertificate()) - || clientCertificate.equals(registrar.getFailoverClientCertificate())) { + if (passedCert.equals(storedCert) || passedCert.equals(storedFailoverCert)) { // Check certificate for any requirement violations // TODO(Sarahbot@): Throw exceptions instead of just logging once requirement enforcement // begins try { - certificateChecker.validateCertificate(clientCertificate.get()); + certificateChecker.validateCertificate(passedCert); } catch (InsecureCertificateException e) { // throw exception in unit tests and Sandbox if (RegistryEnvironment.get().equals(RegistryEnvironment.UNITTEST) @@ -176,7 +198,8 @@ public class TlsCredentials implements TransportCredentials { // Log an error and validate using certificate hash instead // TODO(sarahbot): throw a BadRegistrarCertificateException once hash is no longer used as // failover - logger.atWarning().log("Non-matching certificate for registrar %s.", registrar.getClientId()); + logger.atWarning().log( + "Non-matching certificate for registrar %s.", registrar.getClientId()); } validateCertificateHash(registrar); } @@ -220,6 +243,25 @@ public class TlsCredentials implements TransportCredentials { } } + private X509Certificate stringToCert(Optional certificateString) + throws CertificateException { + if (certificateString.isPresent()) { + ByteArrayInputStream inputStream = + new ByteArrayInputStream(certificateString.get().getBytes(UTF_8)); + CertificateFactory certificateFactory = CertificateFactory.getInstance("X.509"); + return (X509Certificate) certificateFactory.generateCertificate(inputStream); + } + return null; + } + + private X509Certificate encodedCertStringToCert(String encodedCertString) + throws CertificateException { + byte decodedCert[] = Base64.getDecoder().decode(encodedCertString); + ByteArrayInputStream inputStream = new ByteArrayInputStream(decodedCert); + CertificateFactory certificateFactory = CertificateFactory.getInstance("X.509"); + return (X509Certificate) certificateFactory.generateCertificate(inputStream); + } + @Override public String toString() { return toStringHelper(getClass()) diff --git a/core/src/main/java/google/registry/flows/certs/CertificateChecker.java b/core/src/main/java/google/registry/flows/certs/CertificateChecker.java index aeef1356b..85a7fc555 100644 --- a/core/src/main/java/google/registry/flows/certs/CertificateChecker.java +++ b/core/src/main/java/google/registry/flows/certs/CertificateChecker.java @@ -100,6 +100,21 @@ public class CertificateChecker { } } + /** + * Checks the given certificate string for violations and throws an exception if any violations + * exist. + */ + public void validateCertificate(X509Certificate certificate) throws InsecureCertificateException { + ImmutableSet violations = checkCertificate(certificate); + if (!violations.isEmpty()) { + String displayMessages = + violations.stream() + .map(violation -> getViolationDisplayMessage(violation)) + .collect(Collectors.joining("\n")); + throw new InsecureCertificateException(violations, displayMessages); + } + } + /** * Checks a given certificate for violations and returns a list of all the violations the * certificate has. diff --git a/core/src/main/java/google/registry/tools/ValidateLoginCredentialsCommand.java b/core/src/main/java/google/registry/tools/ValidateLoginCredentialsCommand.java index 07aa9868a..e5ff1bfdb 100644 --- a/core/src/main/java/google/registry/tools/ValidateLoginCredentialsCommand.java +++ b/core/src/main/java/google/registry/tools/ValidateLoginCredentialsCommand.java @@ -28,8 +28,11 @@ import google.registry.flows.TlsCredentials; import google.registry.flows.certs.CertificateChecker; import google.registry.model.registrar.Registrar; import google.registry.tools.params.PathParameter; +import java.io.ByteArrayInputStream; import java.nio.file.Files; import java.nio.file.Path; +import java.security.cert.CertificateFactory; +import java.util.Base64; import java.util.Optional; import javax.annotation.Nullable; import javax.inject.Inject; @@ -79,8 +82,16 @@ final class ValidateLoginCredentialsCommand implements CommandWithRemoteApi { "Can't specify both --cert_hash and --cert_file"); String clientCertificate = ""; if (clientCertificatePath != null) { - clientCertificate = new String(Files.readAllBytes(clientCertificatePath), US_ASCII); - clientCertificateHash = getCertificateHash(loadCertificate(clientCertificate)); + byte[] certificateBytes = Files.readAllBytes(clientCertificatePath); + // How proxy converts to a string + clientCertificate = + Base64.getEncoder() + .encodeToString( + CertificateFactory.getInstance("X.509") + .generateCertificate(new ByteArrayInputStream(certificateBytes)) + .getEncoded()); + clientCertificateHash = + getCertificateHash(loadCertificate(new String(certificateBytes, US_ASCII))); } Registrar registrar = checkArgumentPresent( diff --git a/core/src/test/java/google/registry/flows/EppLoginTlsTest.java b/core/src/test/java/google/registry/flows/EppLoginTlsTest.java index 40bb60764..5debd5e73 100644 --- a/core/src/test/java/google/registry/flows/EppLoginTlsTest.java +++ b/core/src/test/java/google/registry/flows/EppLoginTlsTest.java @@ -30,8 +30,12 @@ import google.registry.testing.AppEngineExtension; import google.registry.testing.CertificateSamples; import google.registry.testing.SystemPropertyExtension; import google.registry.util.SelfSignedCaCertificate; +import java.io.ByteArrayInputStream; import java.io.StringWriter; +import java.security.cert.CertificateException; +import java.security.cert.CertificateFactory; import java.security.cert.X509Certificate; +import java.util.Base64; import java.util.Optional; import java.util.logging.Level; import java.util.logging.Logger; @@ -43,6 +47,7 @@ import org.junit.jupiter.api.extension.RegisterExtension; import org.testcontainers.shaded.org.bouncycastle.openssl.jcajce.JcaMiscPEMGenerator; import org.testcontainers.shaded.org.bouncycastle.util.io.pem.PemObjectGenerator; import org.testcontainers.shaded.org.bouncycastle.util.io.pem.PemWriter; +import static java.nio.charset.StandardCharsets.UTF_8; /** Test logging in with TLS credentials. */ class EppLoginTlsTest extends EppTestCase { @@ -67,6 +72,8 @@ class EppLoginTlsTest extends EppTestCase { Logger.getLogger(TlsCredentials.class.getCanonicalName()); private final TestLogHandler handler = new TestLogHandler(); + private String encodedCertString; + void setCredentials(String clientCertificateHash, String clientCertificate) { setTransportCredentials( new TlsCredentials( @@ -78,7 +85,7 @@ class EppLoginTlsTest extends EppTestCase { } @BeforeEach - void beforeEach() { + void beforeEach() throws CertificateException { persistResource( loadRegistrar("NewRegistrar") .asBuilder() @@ -91,18 +98,26 @@ class EppLoginTlsTest extends EppTestCase { .setClientCertificate(CertificateSamples.SAMPLE_CERT2, DateTime.now(UTC)) .build()); loggerToIntercept.addHandler(handler); + // How proxy converts to a string + encodedCertString = + Base64.getEncoder() + .encodeToString( + CertificateFactory.getInstance("X.509") + .generateCertificate( + new ByteArrayInputStream(CertificateSamples.SAMPLE_CERT3.getBytes(UTF_8))) + .getEncoded()); } @Test void testLoginLogout() throws Exception { - setCredentials(CertificateSamples.SAMPLE_CERT3_HASH, CertificateSamples.SAMPLE_CERT3); + setCredentials(null, encodedCertString); assertThatLoginSucceeds("NewRegistrar", "foo-BAR2"); assertThatLogoutSucceeds(); } @Test void testLogin_wrongPasswordFails() throws Exception { - setCredentials(CertificateSamples.SAMPLE_CERT3_HASH, CertificateSamples.SAMPLE_CERT3); + setCredentials(CertificateSamples.SAMPLE_CERT3_HASH, encodedCertString); // For TLS login, we also check the epp xml password. assertThatLogin("NewRegistrar", "incorrect") .hasResponse( @@ -112,7 +127,7 @@ class EppLoginTlsTest extends EppTestCase { @Test void testMultiLogin() throws Exception { - setCredentials(CertificateSamples.SAMPLE_CERT3_HASH, CertificateSamples.SAMPLE_CERT3); + setCredentials(CertificateSamples.SAMPLE_CERT3_HASH, encodedCertString); assertThatLoginSucceeds("NewRegistrar", "foo-BAR2"); assertThatLogoutSucceeds(); assertThatLoginSucceeds("NewRegistrar", "foo-BAR2"); @@ -126,7 +141,7 @@ class EppLoginTlsTest extends EppTestCase { @Test void testNonAuthedLogin_fails() throws Exception { - setCredentials(CertificateSamples.SAMPLE_CERT_HASH, CertificateSamples.SAMPLE_CERT); + setCredentials(CertificateSamples.SAMPLE_CERT3_HASH, encodedCertString); assertThatLogin("TheRegistrar", "password2") .hasResponse( "response_error.xml", @@ -155,7 +170,7 @@ class EppLoginTlsTest extends EppTestCase { @Test void testGoodPrimaryCertificate() throws Exception { - setCredentials(CertificateSamples.SAMPLE_CERT3_HASH, CertificateSamples.SAMPLE_CERT3); + setCredentials(null, encodedCertString); DateTime now = DateTime.now(UTC); persistResource( loadRegistrar("NewRegistrar") @@ -168,7 +183,7 @@ class EppLoginTlsTest extends EppTestCase { @Test void testGoodFailoverCertificate() throws Exception { - setCredentials(CertificateSamples.SAMPLE_CERT3_HASH, CertificateSamples.SAMPLE_CERT3); + setCredentials(null, encodedCertString); DateTime now = DateTime.now(UTC); persistResource( loadRegistrar("NewRegistrar") @@ -181,7 +196,7 @@ class EppLoginTlsTest extends EppTestCase { @Test void testMissingPrimaryCertificateButHasFailover_usesFailover() throws Exception { - setCredentials(CertificateSamples.SAMPLE_CERT3_HASH, CertificateSamples.SAMPLE_CERT3); + setCredentials(null, encodedCertString); DateTime now = DateTime.now(UTC); persistResource( loadRegistrar("NewRegistrar") @@ -210,8 +225,16 @@ class EppLoginTlsTest extends EppTestCase { @Test void testCertificateDoesNotMeetRequirements_fails() throws Exception { + // How proxy converts to a string + String proxyEncoded = + Base64.getEncoder() + .encodeToString( + CertificateFactory.getInstance("X.509") + .generateCertificate( + new ByteArrayInputStream(CertificateSamples.SAMPLE_CERT.getBytes(UTF_8))) + .getEncoded()); // SAMPLE_CERT has a validity period that is too long - setCredentials(CertificateSamples.SAMPLE_CERT_HASH, CertificateSamples.SAMPLE_CERT); + setCredentials(CertificateSamples.SAMPLE_CERT_HASH, proxyEncoded); persistResource( loadRegistrar("NewRegistrar") .asBuilder() @@ -232,7 +255,6 @@ class EppLoginTlsTest extends EppTestCase { @Test void testCertificateDoesNotMeetMultipleRequirements_fails() throws Exception { - X509Certificate certificate = SelfSignedCaCertificate.create( "test", clock.nowUtc().plusDays(100), clock.nowUtc().plusDays(5000)) @@ -243,9 +265,11 @@ class EppLoginTlsTest extends EppTestCase { PemObjectGenerator generator = new JcaMiscPEMGenerator(certificate); pw.writeObject(generator); } + // How proxy converts to a string + String proxyEncoded = Base64.getEncoder().encodeToString(certificate.getEncoded()); // SAMPLE_CERT has a validity period that is too long - setCredentials(CertificateSamples.SAMPLE_CERT_HASH, sw.toString()); + setCredentials(null, proxyEncoded); persistResource( loadRegistrar("NewRegistrar") .asBuilder() @@ -270,7 +294,15 @@ class EppLoginTlsTest extends EppTestCase { void testCertificateDoesNotMeetRequirementsInProduction_succeeds() throws Exception { RegistryEnvironment.PRODUCTION.setup(systemPropertyExtension); // SAMPLE_CERT has a validity period that is too long - setCredentials(CertificateSamples.SAMPLE_CERT_HASH, CertificateSamples.SAMPLE_CERT); + // How proxy converts to a string + String proxyEncoded = + Base64.getEncoder() + .encodeToString( + CertificateFactory.getInstance("X.509") + .generateCertificate( + new ByteArrayInputStream(CertificateSamples.SAMPLE_CERT.getBytes(UTF_8))) + .getEncoded()); + setCredentials(null, proxyEncoded); persistResource( loadRegistrar("NewRegistrar") .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 2b0f6b4cd..9a86fa47f 100644 --- a/core/src/test/java/google/registry/flows/session/LoginFlowViaTlsTest.java +++ b/core/src/test/java/google/registry/flows/session/LoginFlowViaTlsTest.java @@ -17,6 +17,7 @@ package google.registry.flows.session; import static google.registry.testing.DatabaseHelper.persistResource; import static google.registry.util.DateTimeUtils.START_OF_TIME; import static org.joda.time.DateTimeZone.UTC; +import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; @@ -30,8 +31,13 @@ import google.registry.flows.certs.CertificateChecker; import google.registry.model.registrar.Registrar; import google.registry.testing.CertificateSamples; import google.registry.util.CidrAddressBlock; +import java.io.ByteArrayInputStream; +import java.security.cert.CertificateException; +import java.security.cert.CertificateFactory; +import java.util.Base64; import java.util.Optional; import org.joda.time.DateTime; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; /** Unit tests for {@link LoginFlow} when accessed via a TLS transport. */ @@ -54,6 +60,18 @@ public class LoginFlowViaTlsTest extends LoginFlowTestCase { 2048, ImmutableSet.of("secp256r1", "secp384r1"), clock); + private Optional encodedCertString; + + @BeforeEach + void beforeEach() throws CertificateException { + String proxyEncoded = + Base64.getEncoder() + .encodeToString( + CertificateFactory.getInstance("X.509") + .generateCertificate(new ByteArrayInputStream(GOOD_CERT.get().getBytes(UTF_8))) + .getEncoded()); + encodedCertString = Optional.of(proxyEncoded); + } @Override protected Registrar.Builder getRegistrarBuilder() { @@ -66,7 +84,8 @@ public class LoginFlowViaTlsTest extends LoginFlowTestCase { @Test void testSuccess_withGoodCredentials() throws Exception { persistResource(getRegistrarBuilder().build()); - credentials = new TlsCredentials(true, GOOD_CERT_HASH, GOOD_CERT, GOOD_IP, certificateChecker); + credentials = + new TlsCredentials(true, GOOD_CERT_HASH, encodedCertString, GOOD_IP, certificateChecker); doSuccessfulTest("login_valid.xml"); } @@ -78,7 +97,7 @@ 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, GOOD_CERT, GOOD_IPV6, certificateChecker); + new TlsCredentials(true, GOOD_CERT_HASH, encodedCertString, GOOD_IPV6, certificateChecker); doSuccessfulTest("login_valid.xml"); } @@ -90,7 +109,7 @@ 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, GOOD_CERT, GOOD_IPV6, certificateChecker); + new TlsCredentials(true, GOOD_CERT_HASH, encodedCertString, GOOD_IPV6, certificateChecker); doSuccessfulTest("login_valid.xml"); } @@ -100,14 +119,23 @@ public class LoginFlowViaTlsTest extends LoginFlowTestCase { getRegistrarBuilder() .setIpAddressAllowList(ImmutableList.of(CidrAddressBlock.create("192.168.1.255/24"))) .build()); - credentials = new TlsCredentials(true, GOOD_CERT_HASH, GOOD_CERT, GOOD_IP, certificateChecker); + credentials = + new TlsCredentials(true, GOOD_CERT_HASH, encodedCertString, GOOD_IP, certificateChecker); doSuccessfulTest("login_valid.xml"); } @Test - void testFailure_incorrectClientCertificateHash() { + void testFailure_incorrectClientCertificateHash() throws Exception { persistResource(getRegistrarBuilder().build()); - credentials = new TlsCredentials(true, BAD_CERT_HASH, BAD_CERT, GOOD_IP, certificateChecker); + String proxyEncoded = + Base64.getEncoder() + .encodeToString( + CertificateFactory.getInstance("X.509") + .generateCertificate(new ByteArrayInputStream(BAD_CERT.get().getBytes(UTF_8))) + .getEncoded()); + credentials = + new TlsCredentials( + true, BAD_CERT_HASH, Optional.of(proxyEncoded), GOOD_IP, certificateChecker); doFailingTest("login_valid.xml", BadRegistrarCertificateException.class); }