Make cross database comparison recursive (#942)

* Make cross database comparison recursive

Cross-database comparison was previously just a shallow check: fields marked
with DoNotCompare on nested objects were still compared.  This causes problems
in some cases where there are nested immutable objects.

This change introduces recursive comparison.  It also provides a
hasCorrectHashCode() method that verifies that an object has not been mutated
since the hash code was calculated, which has been a problem in certain cases.

Finally, this also fixes the problem of objects that are mutated in multiple
transactions: we were previously comparing against the value in datastore, but
this doesn't work in these cases because the object in datastore may have
changed since the transaction that we are verifying.  Instead, check against
the value that we would have persisted in the original transaction.

* Changes requested in review

* Converted check method interfaces

Per review discussion, converted check method interface so that they
consistently return a ComparisonResult object which encapsulates a success
indicator and an optional error message.

* Another round of changes on ImmutableObjectSubject

* Final changes for review

Removed unnecessary null check, minor reformatting.

(this also removes an obsolete nullness assertion from an earlier commit that
should have been fixed in the rebase)

* Try removing that nullness check import again....

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 google.registry.util.X509Utils.loadCertificate;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
@@ -32,7 +33,11 @@ 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.X509Certificate;
+import java.util.Base64;
 import java.util.Optional;
 import javax.inject.Inject;
 import javax.servlet.http.HttpServletRequest;
@@ -149,14 +154,31 @@ public class TlsCredentials implements TransportCredentials {
           "Request from registrar %s did not include X-SSL-Full-Certificate.",
           registrar.getClientId());
     } else {
+      X509Certificate passedCert;
+      Optional<X509Certificate> storedCert;
+      Optional<X509Certificate> storedFailoverCert;
+
+      try {
+        storedCert = deserializePemCert(registrar.getClientCertificate());
+        storedFailoverCert = deserializePemCert(registrar.getFailoverClientCertificate());
+        passedCert = decodeCertString(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.orElse(null))
+          || passedCert.equals(storedFailoverCert.orElse(null))) {
         // 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)
@@ -220,9 +242,26 @@ public class TlsCredentials implements TransportCredentials {
     }
   }
 
+  // Converts a PEM formatted certificate string into an X509Certificate
+  private Optional<X509Certificate> deserializePemCert(Optional<String> certificateString)
+      throws CertificateException {
+    if (certificateString.isPresent()) {
+      return Optional.of(loadCertificate(certificateString.get()));
+    }
+    return Optional.empty();
+  }
+
+  // Decodes the string representation of an encoded certificate back into an X509Certificate
+  private X509Certificate decodeCertString(String encodedCertString) throws CertificateException {
+    byte decodedCert[] = Base64.getDecoder().decode(encodedCertString);
+    ByteArrayInputStream inputStream = new ByteArrayInputStream(decodedCert);
+    return loadCertificate(inputStream);
+  }
+
   @Override
   public String toString() {
     return toStringHelper(getClass())
+        .add("clientCertificate", clientCertificate.orElse(null))
         .add("clientCertificateHash", clientCertificateHash.orElse(null))
         .add("clientAddress", clientInetAddr.orElse(null))
         .toString();
@@ -288,7 +327,7 @@ public class TlsCredentials implements TransportCredentials {
     static Optional<String> provideClientCertificate(HttpServletRequest req) {
       // Note: This header is actually required, we just want to handle its absence explicitly
       // by throwing an EPP exception rather than a generic Bad Request exception.
-      return extractOptionalHeader(req, "X-SSL-Full_Certificate");
+      return extractOptionalHeader(req, "X-SSL-Full-Certificate");
     }
 
     @Provides

core/src/main/java/google/registry/flows/certs/CertificateChecker.java

@@ -90,7 +90,19 @@ public class CertificateChecker {
    * exist.
    */
   public void validateCertificate(String certificateString) throws InsecureCertificateException {
-    ImmutableSet<CertificateViolation> violations = checkCertificate(certificateString);
+    handleCertViolations(checkCertificate(certificateString));
+  }
+
+  /**
+   * Checks the given certificate string for violations and throws an exception if any violations
+   * exist.
+   */
+  public void validateCertificate(X509Certificate certificate) throws InsecureCertificateException {
+    handleCertViolations(checkCertificate(certificate));
+  }
+
+  private void handleCertViolations(ImmutableSet<CertificateViolation> violations)
+      throws InsecureCertificateException {
     if (!violations.isEmpty()) {
       String displayMessages =
           violations.stream()

core/src/main/java/google/registry/tools/ValidateLoginCredentialsCommand.java

@@ -18,6 +18,7 @@ import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkState;
 import static com.google.common.base.Strings.isNullOrEmpty;
 import static google.registry.util.PreconditionsUtils.checkArgumentPresent;
+import static google.registry.util.X509Utils.encodeX509CertificateFromPemString;
 import static google.registry.util.X509Utils.getCertificateHash;
 import static google.registry.util.X509Utils.loadCertificate;
 import static java.nio.charset.StandardCharsets.US_ASCII;
@@ -77,10 +78,11 @@ final class ValidateLoginCredentialsCommand implements CommandWithRemoteApi {
     checkArgument(
         clientCertificatePath == null || isNullOrEmpty(clientCertificateHash),
         "Can't specify both --cert_hash and --cert_file");
-    String clientCertificate = "";
+    String encodedCertificate = "";
     if (clientCertificatePath != null) {
-      clientCertificate = new String(Files.readAllBytes(clientCertificatePath), US_ASCII);
-      clientCertificateHash = getCertificateHash(loadCertificate(clientCertificate));
+      String certificateString = new String(Files.readAllBytes(clientCertificatePath), US_ASCII);
+      encodedCertificate = encodeX509CertificateFromPemString(certificateString);
+      clientCertificateHash = getCertificateHash(loadCertificate(clientCertificatePath));
     }
     Registrar registrar =
         checkArgumentPresent(
@@ -88,7 +90,7 @@ final class ValidateLoginCredentialsCommand implements CommandWithRemoteApi {
     new TlsCredentials(
             true,
             Optional.ofNullable(clientCertificateHash),
-            Optional.ofNullable(clientCertificate),
+            Optional.ofNullable(encodedCertificate),
             Optional.ofNullable(clientIpAddress),
             certificateChecker)
         .validate(registrar, password);

core/src/test/java/google/registry/flows/EppLoginTlsTest.java

@@ -18,6 +18,8 @@ import static google.registry.testing.DatabaseHelper.loadRegistrar;
 import static google.registry.testing.DatabaseHelper.persistResource;
 import static google.registry.testing.LogsSubject.assertAboutLogs;
 import static google.registry.util.DateTimeUtils.START_OF_TIME;
+import static google.registry.util.X509Utils.encodeX509Certificate;
+import static google.registry.util.X509Utils.encodeX509CertificateFromPemString;
 import static org.joda.time.DateTimeZone.UTC;
 
 import com.google.common.collect.ImmutableMap;
@@ -31,6 +33,7 @@ import google.registry.testing.CertificateSamples;
 import google.registry.testing.SystemPropertyExtension;
 import google.registry.util.SelfSignedCaCertificate;
 import java.io.StringWriter;
+import java.security.cert.CertificateException;
 import java.security.cert.X509Certificate;
 import java.util.Optional;
 import java.util.logging.Level;
@@ -67,6 +70,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 +83,7 @@ class EppLoginTlsTest extends EppTestCase {
   }
 
   @BeforeEach
-  void beforeEach() {
+  void beforeEach() throws CertificateException {
     persistResource(
         loadRegistrar("NewRegistrar")
             .asBuilder()
@@ -91,18 +96,19 @@ class EppLoginTlsTest extends EppTestCase {
             .setClientCertificate(CertificateSamples.SAMPLE_CERT2, DateTime.now(UTC))
             .build());
     loggerToIntercept.addHandler(handler);
+    encodedCertString = encodeX509CertificateFromPemString(CertificateSamples.SAMPLE_CERT3);
   }
 
   @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 +118,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 +132,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 +161,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 +174,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 +187,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 +216,9 @@ class EppLoginTlsTest extends EppTestCase {
 
   @Test
   void testCertificateDoesNotMeetRequirements_fails() throws Exception {
+    String proxyEncoded = encodeX509CertificateFromPemString(CertificateSamples.SAMPLE_CERT);
     // 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 +239,6 @@ class EppLoginTlsTest extends EppTestCase {
 
   @Test
   void testCertificateDoesNotMeetMultipleRequirements_fails() throws Exception {
-
     X509Certificate certificate =
         SelfSignedCaCertificate.create(
                 "test", clock.nowUtc().plusDays(100), clock.nowUtc().plusDays(5000))
@@ -244,8 +250,10 @@ class EppLoginTlsTest extends EppTestCase {
       pw.writeObject(generator);
     }
 
+    String proxyEncoded = encodeX509Certificate(certificate);
+
     // 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 +278,8 @@ 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);
+    String proxyEncoded = encodeX509CertificateFromPemString(CertificateSamples.SAMPLE_CERT);
+    setCredentials(null, proxyEncoded);
     persistResource(
         loadRegistrar("NewRegistrar")
             .asBuilder()
@@ -288,4 +297,61 @@ class EppLoginTlsTest extends EppTestCase {
                 + " Certificate validity period is too long; it must be less than or equal to 398"
                 + " days.");
   }
+
+  @Test
+  void testRegistrarCertificateContainsExtraMetadata_succeeds() throws Exception {
+    String certPem =
+        String.format(
+            "Bag Attributes\n"
+                + "    localKeyID: 1F 1C 3A 3A 4C 03 EC C4 BC 7A C3 21 A9 F2 13 66 21 B8 7B 26 \n"
+                + "subject=/C=US/ST=New York/L=New"
+                + " York/O=Test/OU=ABC/CN=tester.test/emailAddress=test-certificate@test.test\n"
+                + "issuer=/C=US/ST=NY/L=NYC/O=ABC/OU=TEST CA/CN=TEST"
+                + " CA/emailAddress=testing@test.test\n"
+                + "%s",
+            CertificateSamples.SAMPLE_CERT3);
+
+    setCredentials(null, encodeX509CertificateFromPemString(certPem));
+    DateTime now = DateTime.now(UTC);
+    persistResource(
+        loadRegistrar("NewRegistrar")
+            .asBuilder()
+            .setClientCertificate(certPem, now)
+            .setFailoverClientCertificate(CertificateSamples.SAMPLE_CERT2, now)
+            .build());
+    assertThatLoginSucceeds("NewRegistrar", "foo-BAR2");
+  }
+
+  @Test
+  void testRegistrarCertificateContainsExtraMetadataAndViolations_fails() throws Exception {
+    String certPem =
+        String.format(
+            "Bag Attributes\n"
+                + "    localKeyID: 1F 1C 3A 3A 4C 03 EC C4 BC 7A C3 21 A9 F2 13 66 21 B8 7B 26 \n"
+                + "subject=/C=US/ST=New York/L=New"
+                + " York/O=Test/OU=ABC/CN=tester.test/emailAddress=test-certificate@test.test\n"
+                + "issuer=/C=US/ST=NY/L=NYC/O=ABC/OU=TEST CA/CN=TEST"
+                + " CA/emailAddress=testing@test.test\n"
+                + "%s",
+            CertificateSamples.SAMPLE_CERT);
+
+    setCredentials(null, encodeX509CertificateFromPemString(certPem));
+    DateTime now = DateTime.now(UTC);
+    persistResource(
+        loadRegistrar("NewRegistrar")
+            .asBuilder()
+            .setClientCertificate(certPem, now)
+            .setFailoverClientCertificate(CertificateSamples.SAMPLE_CERT2, now)
+            .build());
+    assertThatLogin("NewRegistrar", "foo-BAR2")
+        .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."));
+  }
 }

core/src/test/java/google/registry/flows/FlowRunnerTest.java

@@ -155,12 +155,14 @@ class FlowRunnerTest {
         new TlsCredentials(
             true,
             Optional.of("abc123def"),
-            Optional.of("cert"),
+            Optional.of("cert046F5A3"),
             Optional.of("127.0.0.1"),
             certificateChecker);
     flowRunner.run(eppMetricBuilder);
     assertThat(Splitter.on("\n\t").split(findFirstLogMessageByPrefix(handler, "EPP Command\n\t")))
-        .contains("TlsCredentials{clientCertificateHash=abc123def, clientAddress=/127.0.0.1}");
+        .contains(
+            "TlsCredentials{clientCertificate=cert046F5A3, clientCertificateHash=abc123def,"
+                + " clientAddress=/127.0.0.1}");
   }
 
   @Test

core/src/test/java/google/registry/flows/TlsCredentialsTest.java

@@ -118,10 +118,11 @@ final class TlsCredentialsTest {
     tls.validateCertificate(Registrar.loadByClientId("TheRegistrar").get());
   }
 
+  @Test
   void testProvideClientCertificate() {
     HttpServletRequest req = mock(HttpServletRequest.class);
     when(req.getHeader("X-SSL-Full-Certificate")).thenReturn("data");
-    assertThat(TlsCredentials.EppTlsModule.provideClientCertificate(req)).isEqualTo("data");
+    assertThat(TlsCredentials.EppTlsModule.provideClientCertificate(req)).hasValue("data");
   }
 
   @Test

core/src/test/java/google/registry/flows/session/LoginFlowViaTlsTest.java

@@ -16,6 +16,7 @@ package google.registry.flows.session;
 
 import static google.registry.testing.DatabaseHelper.persistResource;
 import static google.registry.util.DateTimeUtils.START_OF_TIME;
+import static google.registry.util.X509Utils.encodeX509CertificateFromPemString;
 import static org.joda.time.DateTimeZone.UTC;
 
 import com.google.common.collect.ImmutableList;
@@ -30,9 +31,18 @@ import google.registry.flows.certs.CertificateChecker;
 import google.registry.model.registrar.Registrar;
 import google.registry.testing.CertificateSamples;
 import google.registry.util.CidrAddressBlock;
+import google.registry.util.SelfSignedCaCertificate;
+import java.io.StringWriter;
+import java.security.cert.CertificateException;
+import java.security.cert.X509Certificate;
+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;
+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;
 
 /** Unit tests for {@link LoginFlow} when accessed via a TLS transport. */
 public class LoginFlowViaTlsTest extends LoginFlowTestCase {
@@ -54,6 +64,12 @@ public class LoginFlowViaTlsTest extends LoginFlowTestCase {
           2048,
           ImmutableSet.of("secp256r1", "secp384r1"),
           clock);
+  private Optional<String> encodedCertString;
+
+  @BeforeEach
+  void beforeEach() throws CertificateException {
+    encodedCertString = Optional.of(encodeX509CertificateFromPemString(GOOD_CERT.get()));
+  }
 
   @Override
   protected Registrar.Builder getRegistrarBuilder() {
@@ -66,7 +82,36 @@ 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");
+  }
+
+  @Test
+  void testSuccess_withNewlyConstructedCertificate() throws Exception {
+    X509Certificate certificate =
+        SelfSignedCaCertificate.create(
+                "test", clock.nowUtc().minusDays(100), clock.nowUtc().plusDays(150))
+            .cert();
+
+    StringWriter sw = new StringWriter();
+    try (PemWriter pw = new PemWriter(sw)) {
+      PemObjectGenerator generator = new JcaMiscPEMGenerator(certificate);
+      pw.writeObject(generator);
+    }
+
+    persistResource(
+        super.getRegistrarBuilder()
+            .setClientCertificate(sw.toString(), DateTime.now(UTC))
+            .setIpAddressAllowList(
+                ImmutableList.of(
+                    CidrAddressBlock.create(InetAddresses.forString(GOOD_IP.get()), 32)))
+            .build());
+
+    String encodedCertificate = Base64.getEncoder().encodeToString(certificate.getEncoded());
+    credentials =
+        new TlsCredentials(
+            true, Optional.empty(), Optional.of(encodedCertificate), GOOD_IP, certificateChecker);
     doSuccessfulTest("login_valid.xml");
   }
 
@@ -78,7 +123,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 +135,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 +145,18 @@ 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 = encodeX509CertificateFromPemString(BAD_CERT.get());
+    credentials =
+        new TlsCredentials(
+            true, BAD_CERT_HASH, Optional.of(proxyEncoded), GOOD_IP, certificateChecker);
     doFailingTest("login_valid.xml", BadRegistrarCertificateException.class);
   }
 

core/src/test/java/google/registry/model/ImmutableObjectSubject.java

@@ -14,10 +14,14 @@
 
 package google.registry.model;
 
+import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.truth.Truth.assertAbout;
 import static com.google.common.truth.Truth.assertThat;
 
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
 import com.google.common.truth.Correspondence;
 import com.google.common.truth.Correspondence.BinaryPredicate;
 import com.google.common.truth.FailureMetadata;
@@ -25,10 +29,15 @@ import com.google.common.truth.SimpleSubjectBuilder;
 import com.google.common.truth.Subject;
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Field;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
+import java.util.function.Predicate;
+import java.util.stream.Collector;
 import javax.annotation.Nullable;
 
 /** Truth subject for asserting things about ImmutableObjects that are not built in. */
@@ -62,17 +71,295 @@ public final class ImmutableObjectSubject extends Subject {
    * <p>This is used to verify that entities stored in both cloud SQL and Datastore are identical.
    */
   public void isEqualAcrossDatabases(@Nullable ImmutableObject expected) {
+    ComparisonResult result =
+        checkObjectAcrossDatabases(
+            actual, expected, actual != null ? actual.getClass().getName() : "null");
+    if (result.isFailure()) {
+      throw new AssertionError(result.getMessage());
+    }
+  }
+
+  // The following "check" methods implement a recursive check of immutable object equality across
+  // databases.  All of them function in both assertive and predicate modes: if "path" is
+  // provided (not null) then they throw AssertionError's with detailed error messages.  If
+  // it is null, they return true for equal objects and false for inequal ones.
+  //
+  // The reason for this dual-mode behavior is that all of these methods can either be used in the
+  // context of a test assertion (in which case we want a detailed error message describing exactly
+  // the location in a complex object where a difference was discovered) or in the context of a
+  // membership check in a set (in which case we don't care about the specific location of the first
+  // difference, we just want to be able to determine if the object "is equal to" another object as
+  // efficiently as possible -- see checkSetAcrossDatabase()).
+
+  @VisibleForTesting
+  static ComparisonResult checkObjectAcrossDatabases(
+      @Nullable Object actual, @Nullable Object expected, @Nullable String path) {
+    if (Objects.equals(actual, expected)) {
+      return ComparisonResult.createSuccess();
+    }
+
+    // They're different, do a more detailed comparison.
+
+    // Check for null first (we can assume both variables are not null at this point).
     if (actual == null) {
-      assertThat(expected).isNull();
+      return ComparisonResult.createFailure(path, "expected ", expected, "got null.");
+    } else if (expected == null) {
+      return ComparisonResult.createFailure(path, "expected null, got ", actual);
+
+      // For immutable objects, we have to recurse since the contained
+      // object could have DoNotCompare fields, too.
+    } else if (expected instanceof ImmutableObject) {
+      // We only verify that actual is an ImmutableObject so we get a good error message instead
+      // of a context-less ClassCastException.
+      if (!(actual instanceof ImmutableObject)) {
+        return ComparisonResult.createFailure(path, actual, " is not an immutable object.");
+      }
+
+      return checkImmutableAcrossDatabases(
+          (ImmutableObject) actual, (ImmutableObject) expected, path);
+    } else if (expected instanceof Map) {
+      if (!(actual instanceof Map)) {
+        return ComparisonResult.createFailure(path, actual, " is not a Map.");
+      }
+
+      // This would likely be more efficient if we could assume that keys can be compared across
+      // databases using .equals(), however we cannot guarantee key equality so the simplest and
+      // most correct way to accomplish this is by reusing the set comparison.
+      return checkSetAcrossDatabases(
+          ((Map<?, ?>) actual).entrySet(), ((Map<?, ?>) expected).entrySet(), path, "Map");
+    } else if (expected instanceof Set) {
+      if (!(actual instanceof Set)) {
+        return ComparisonResult.createFailure(path, actual, " is not a Set.");
+      }
+
+      return checkSetAcrossDatabases((Set<?>) actual, (Set<?>) expected, path, "Set");
+    } else if (expected instanceof Collection) {
+      if (!(actual instanceof Collection)) {
+        return ComparisonResult.createFailure(path, actual, " is not a Collection.");
+      }
+
+      return checkListAcrossDatabases((Collection<?>) actual, (Collection<?>) expected, path);
+      // Give Map.Entry special treatment to facilitate the use of Set comparison for verification
+      // of Map.
+    } else if (expected instanceof Map.Entry) {
+      if (!(actual instanceof Map.Entry)) {
+        return ComparisonResult.createFailure(path, actual, " is not a Map.Entry.");
+      }
+
+      // Check both the key and value.  We can always ignore the path here, this should only be
+      // called from within a set comparison.
+      ComparisonResult result;
+      if ((result =
+              checkObjectAcrossDatabases(
+                  ((Map.Entry<?, ?>) actual).getKey(), ((Map.Entry<?, ?>) expected).getKey(), null))
+          .isFailure()) {
+        return result;
+      }
+      if ((result =
+              checkObjectAcrossDatabases(
+                  ((Map.Entry<?, ?>) actual).getValue(),
+                  ((Map.Entry<?, ?>) expected).getValue(),
+                  null))
+          .isFailure()) {
+        return result;
+      }
     } else {
-      assertThat(expected).isNotNull();
+      // Since we know that the objects are not equal and since any other types can not be expected
+      // to contain DoNotCompare elements, this condition is always a failure.
+      return ComparisonResult.createFailure(path, actual, " is not equal to ", expected);
     }
-    if (actual != null) {
-      Map<Field, Object> actualFields = filterFields(actual, ImmutableObject.DoNotCompare.class);
-      Map<Field, Object> expectedFields =
-          filterFields(expected, ImmutableObject.DoNotCompare.class);
-      assertThat(actualFields).containsExactlyEntriesIn(expectedFields);
+
+    return ComparisonResult.createSuccess();
+  }
+
+  private static ComparisonResult checkSetAcrossDatabases(
+      Set<?> actual, Set<?> expected, String path, String type) {
+    // Unfortunately, we can't just check to see whether one set "contains" all of the elements of
+    // the other, as the cross database checks don't require strict equality.  Instead we have to do
+    // an N^2 comparison to search for an equivalent element.
+
+    // Objects in expected that aren't in actual.  We use "identity sets" here and below because we
+    // want to keep track of the _objects themselves_ rather than rely upon any overridable notion
+    // of equality.
+    Set<Object> missing = path != null ? Sets.newIdentityHashSet() : null;
+
+    // Objects from actual that have matching elements in expected.
+    Set<Object> found = Sets.newIdentityHashSet();
+
+    // Build missing and found.
+    for (Object expectedElem : expected) {
+      boolean gotMatch = false;
+      for (Object actualElem : actual) {
+        if (!checkObjectAcrossDatabases(actualElem, expectedElem, null).isFailure()) {
+          gotMatch = true;
+
+          // Add the element to the set of expected elements that were "found" in actual.  If the
+          // element matches multiple elements in "expected," we have a basic problem with this
+          // kind of set that we'll want to know about.
+          if (!found.add(actualElem)) {
+            return ComparisonResult.createFailure(
+                path, "element ", actualElem, " matches multiple elements in ", expected);
+          }
+
+          break;
+        }
+      }
+
+      if (!gotMatch) {
+        if (path == null) {
+          return ComparisonResult.createFailure();
+        }
+        missing.add(expectedElem);
+      }
     }
+
+    if (path != null) {
+      // Provide a detailed message consisting of any missing or unexpected items.
+
+      // Build a set of all objects in actual that don't have counterparts in expected.
+      Set<Object> unexpected =
+          actual.stream()
+              .filter(actualElem -> !found.contains(actualElem))
+              .collect(
+                  Collector.of(
+                      Sets::newIdentityHashSet,
+                      Set::add,
+                      (result, values) -> {
+                        result.addAll(values);
+                        return result;
+                      }));
+
+      if (!missing.isEmpty() || !unexpected.isEmpty()) {
+        String message = type + " does not contain the expected contents.";
+        if (!missing.isEmpty()) {
+          message += "  It is missing: " + formatItems(missing.iterator());
+        }
+
+        if (!unexpected.isEmpty()) {
+          message += "  It contains additional elements: " + formatItems(unexpected.iterator());
+        }
+
+        return ComparisonResult.createFailure(path, message);
+      }
+
+      // We just need to check if there were any objects in "actual" that were not in "expected"
+      // (where "found" is a proxy for "expected").
+    } else if (actual.stream().anyMatch(Predicate.not(found::contains))) {
+      return ComparisonResult.createFailure();
+    }
+
+    return ComparisonResult.createSuccess();
+  }
+
+  private static ComparisonResult checkListAcrossDatabases(
+      Collection<?> actual, Collection<?> expected, @Nullable String path) {
+    Iterator<?> actualIter = actual.iterator();
+    Iterator<?> expectedIter = expected.iterator();
+    int index = 0;
+    while (actualIter.hasNext() && expectedIter.hasNext()) {
+      Object actualItem = actualIter.next();
+      Object expectedItem = expectedIter.next();
+      ComparisonResult result =
+          checkObjectAcrossDatabases(
+              actualItem, expectedItem, path != null ? path + "[" + index + "]" : null);
+      if (result.isFailure()) {
+        return result;
+      }
+      ++index;
+    }
+
+    if (actualIter.hasNext()) {
+      return ComparisonResult.createFailure(
+          path, "has additional items: ", formatItems(actualIter));
+    }
+
+    if (expectedIter.hasNext()) {
+      return ComparisonResult.createFailure(path, "missing items: ", formatItems(expectedIter));
+    }
+
+    return ComparisonResult.createSuccess();
+  }
+
+  /** Recursive helper for isEqualAcrossDatabases. */
+  private static ComparisonResult checkImmutableAcrossDatabases(
+      ImmutableObject actual, ImmutableObject expected, String path) {
+    Map<Field, Object> actualFields = filterFields(actual, ImmutableObject.DoNotCompare.class);
+    Map<Field, Object> expectedFields = filterFields(expected, ImmutableObject.DoNotCompare.class);
+
+    for (Map.Entry<Field, Object> entry : expectedFields.entrySet()) {
+      if (!actualFields.containsKey(entry.getKey())) {
+        return ComparisonResult.createFailure(path, "is missing field ", entry.getKey().getName());
+      }
+
+      // Verify that the field values are the same.
+      Object expectedFieldValue = entry.getValue();
+      Object actualFieldValue = actualFields.get(entry.getKey());
+      ComparisonResult result =
+          checkObjectAcrossDatabases(
+              actualFieldValue,
+              expectedFieldValue,
+              path != null ? path + "." + entry.getKey().getName() : null);
+      if (result.isFailure()) {
+        return result;
+      }
+    }
+
+    // Check for fields in actual that are not in expected.
+    for (Map.Entry<Field, Object> entry : actualFields.entrySet()) {
+      if (!expectedFields.containsKey(entry.getKey())) {
+        return ComparisonResult.createFailure(
+            path, "has additional field ", entry.getKey().getName());
+      }
+    }
+
+    return ComparisonResult.createSuccess();
+  }
+
+  private static String formatItems(Iterator<?> iter) {
+    return Joiner.on(", ").join(iter);
+  }
+
+  /** Encapsulates success/failure result in recursive comparison with optional error string. */
+  static class ComparisonResult {
+    boolean succeeded;
+    String message;
+
+    private ComparisonResult(boolean succeeded, @Nullable String message) {
+      this.succeeded = succeeded;
+      this.message = message;
+    }
+
+    static ComparisonResult createFailure() {
+      return new ComparisonResult(false, null);
+    }
+
+    static ComparisonResult createFailure(@Nullable String path, Object... message) {
+      return new ComparisonResult(false, "At " + path + ": " + Joiner.on("").join(message));
+    }
+
+    static ComparisonResult createSuccess() {
+      return new ComparisonResult(true, null);
+    }
+
+    String getMessage() {
+      checkNotNull(message);
+      return message;
+    }
+
+    boolean isFailure() {
+      return !succeeded;
+    }
+  }
+
+  /**
+   * Checks that the hash value reported by {@code actual} is correct.
+   *
+   * <p>This is used in the replay tests to ensure that hibernate hasn't modified any fields that
+   * are not marked as @Insignificant while loading.
+   */
+  public void hasCorrectHashValue() {
+    assertThat(Arrays.hashCode(actual.getSignificantFields().values().toArray()))
+        .isEqualTo(actual.hashCode());
   }
 
   public static Correspondence<ImmutableObject, ImmutableObject> immutableObjectCorrespondence(
@@ -109,7 +396,8 @@ public final class ImmutableObjectSubject extends Subject {
     }
   }
 
-  public static Map<Field, Object> filterFields(ImmutableObject original, String... ignoredFields) {
+  private static Map<Field, Object> filterFields(
+      ImmutableObject original, String... ignoredFields) {
     ImmutableSet<String> ignoredFieldSet = ImmutableSet.copyOf(ignoredFields);
     Map<Field, Object> originalFields = ModelUtils.getFieldValues(original);
     // don't use ImmutableMap or a stream->collect model since we can have nulls
@@ -123,7 +411,7 @@ public final class ImmutableObjectSubject extends Subject {
   }
 
   /** Filter out fields with the given annotation. */
-  public static Map<Field, Object> filterFields(
+  private static Map<Field, Object> filterFields(
       ImmutableObject original, Class<? extends Annotation> annotation) {
     Map<Field, Object> originalFields = ModelUtils.getFieldValues(original);
     // don't use ImmutableMap or a stream->collect model since we can have nulls

core/src/test/java/google/registry/model/ImmutableObjectSubjectTest.java

@@ -0,0 +1,481 @@
+// Copyright 2021 The Nomulus Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package google.registry.model;
+
+import static com.google.common.truth.Truth.assertThat;
+import static google.registry.model.ImmutableObjectSubject.ComparisonResult;
+import static google.registry.model.ImmutableObjectSubject.assertAboutImmutableObjects;
+import static google.registry.model.ImmutableObjectSubject.checkObjectAcrossDatabases;
+import static org.junit.Assert.assertThrows;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import java.util.regex.Pattern;
+import javax.annotation.Nullable;
+import org.junit.jupiter.api.Test;
+
+public class ImmutableObjectSubjectTest {
+
+  // Unique id to assign to the "ignored" field so that it always gets a unique value.
+  private static int uniqueId = 0;
+
+  @Test
+  void testCrossDatabase_nulls() {
+    assertAboutImmutableObjects().that(null).isEqualAcrossDatabases(null);
+    assertAboutImmutableObjects()
+        .that(makeTestAtom(null))
+        .isEqualAcrossDatabases(makeTestAtom(null));
+
+    assertThat(checkObjectAcrossDatabases(null, makeTestAtom("foo"), null).isFailure()).isTrue();
+    assertThat(checkObjectAcrossDatabases(null, makeTestAtom("foo"), null).isFailure()).isTrue();
+  }
+
+  @Test
+  void testCrossDatabase_equalObjects() {
+    TestImmutableObject actual = makeTestObj();
+    assertAboutImmutableObjects().that(actual).isEqualAcrossDatabases(actual);
+    assertAboutImmutableObjects().that(actual).isEqualAcrossDatabases(makeTestObj());
+    assertThat(checkObjectAcrossDatabases(makeTestObj(), makeTestObj(), null).isFailure())
+        .isFalse();
+  }
+
+  @Test
+  void testCrossDatabase_simpleFieldFailure() {
+    AssertionError e =
+        assertThrows(
+            AssertionError.class,
+            () ->
+                assertAboutImmutableObjects()
+                    .that(makeTestObj())
+                    .isEqualAcrossDatabases(makeTestObj().withStringField("bar")));
+    assertThat(e)
+        .hasMessageThat()
+        .contains(
+            "At google.registry.model.ImmutableObjectSubjectTest$TestImmutableObject.stringField:");
+    assertThat(
+            checkObjectAcrossDatabases(makeTestObj(), makeTestObj().withStringField(null), null)
+                .isFailure())
+        .isTrue();
+  }
+
+  @Test
+  void testCrossDatabase_nestedImmutableFailure() {
+    // Repeat the null checks to verify that the attribute path is preserved.
+    AssertionError e =
+        assertThrows(
+            AssertionError.class,
+            () ->
+                assertAboutImmutableObjects()
+                    .that(makeTestObj())
+                    .isEqualAcrossDatabases(makeTestObj().withNested(null)));
+    assertThat(e)
+        .hasMessageThat()
+        .contains(
+            "At google.registry.model.ImmutableObjectSubjectTest$TestImmutableObject.nested:"
+                + " expected null, got TestImmutableObject");
+    e =
+        assertThrows(
+            AssertionError.class,
+            () ->
+                assertAboutImmutableObjects()
+                    .that(makeTestObj().withNested(null))
+                    .isEqualAcrossDatabases(makeTestObj()));
+    assertThat(e)
+        .hasMessageThat()
+        .contains(
+            "At google.registry.model.ImmutableObjectSubjectTest$TestImmutableObject.nested:"
+                + " expected TestImmutableObject");
+    assertThat(e).hasMessageThat().contains("got null.");
+
+    // Test with a field difference.
+    e =
+        assertThrows(
+            AssertionError.class,
+            () ->
+                assertAboutImmutableObjects()
+                    .that(makeTestObj())
+                    .isEqualAcrossDatabases(
+                        makeTestObj().withNested(makeTestObj().withNested(null))));
+    assertThat(e)
+        .hasMessageThat()
+        .contains(
+            "At google.registry.model.ImmutableObjectSubjectTest$"
+                + "TestImmutableObject.nested.stringField:");
+    assertThat(
+            checkObjectAcrossDatabases(makeTestObj(), makeTestObj().withNested(null), null)
+                .isFailure())
+        .isTrue();
+  }
+
+  @Test
+  void testCrossDatabase_listFailure() {
+    AssertionError e =
+        assertThrows(
+            AssertionError.class,
+            () ->
+                assertAboutImmutableObjects()
+                    .that(makeTestObj())
+                    .isEqualAcrossDatabases(makeTestObj().withList(null)));
+    assertThat(e)
+        .hasMessageThat()
+        .contains(
+            "At google.registry.model.ImmutableObjectSubjectTest$" + "TestImmutableObject.list:");
+    e =
+        assertThrows(
+            AssertionError.class,
+            () ->
+                assertAboutImmutableObjects()
+                    .that(makeTestObj())
+                    .isEqualAcrossDatabases(
+                        makeTestObj().withList(ImmutableList.of(makeTestAtom("wack")))));
+    assertThat(e)
+        .hasMessageThat()
+        .contains(
+            "At google.registry.model.ImmutableObjectSubjectTest$"
+                + "TestImmutableObject.list[0].stringField:");
+    e =
+        assertThrows(
+            AssertionError.class,
+            () ->
+                assertAboutImmutableObjects()
+                    .that(makeTestObj())
+                    .isEqualAcrossDatabases(
+                        makeTestObj()
+                            .withList(
+                                ImmutableList.of(
+                                    makeTestAtom("baz"),
+                                    makeTestAtom("bot"),
+                                    makeTestAtom("boq")))));
+    assertThat(e)
+        .hasMessageThat()
+        .contains(
+            "At google.registry.model.ImmutableObjectSubjectTest$"
+                + "TestImmutableObject.list: missing items");
+    // Make sure multiple additional items get formatted nicely.
+    assertThat(e).hasMessageThat().contains("}, TestImmutableObject");
+    e =
+        assertThrows(
+            AssertionError.class,
+            () ->
+                assertAboutImmutableObjects()
+                    .that(makeTestObj())
+                    .isEqualAcrossDatabases(makeTestObj().withList(ImmutableList.of())));
+    assertThat(e)
+        .hasMessageThat()
+        .contains(
+            "At google.registry.model.ImmutableObjectSubjectTest$"
+                + "TestImmutableObject.list: has additional items");
+    assertThat(
+            checkObjectAcrossDatabases(
+                    makeTestObj(),
+                    makeTestObj()
+                        .withList(ImmutableList.of(makeTestAtom("baz"), makeTestAtom("gauze"))),
+                    null)
+                .isFailure())
+        .isTrue();
+    assertThat(
+            checkObjectAcrossDatabases(
+                    makeTestObj(), makeTestObj().withList(ImmutableList.of()), null)
+                .isFailure())
+        .isTrue();
+    assertThat(
+            checkObjectAcrossDatabases(
+                    makeTestObj(),
+                    makeTestObj().withList(ImmutableList.of(makeTestAtom("gauze"))),
+                    null)
+                .isFailure())
+        .isTrue();
+  }
+
+  @Test
+  void testCrossDatabase_setFailure() {
+    AssertionError e =
+        assertThrows(
+            AssertionError.class,
+            () ->
+                assertAboutImmutableObjects()
+                    .that(makeTestObj())
+                    .isEqualAcrossDatabases(makeTestObj().withSet(null)));
+    assertThat(e)
+        .hasMessageThat()
+        .contains(
+            "At google.registry.model.ImmutableObjectSubjectTest$"
+                + "TestImmutableObject.set: expected null, got ");
+
+    e =
+        assertThrows(
+            AssertionError.class,
+            () ->
+                assertAboutImmutableObjects()
+                    .that(makeTestObj())
+                    .isEqualAcrossDatabases(
+                        makeTestObj().withSet(ImmutableSet.of(makeTestAtom("jim")))));
+    assertThat(e)
+        .hasMessageThat()
+        .containsMatch(
+            Pattern.compile(
+                "Set does not contain the expected contents.  "
+                    + "It is missing: .*jim.*  It contains additional elements: .*bob",
+                Pattern.DOTALL));
+
+    // Trickery here to verify that multiple items that both match existing items in the set trigger
+    // an error: we can add two of the same items because equality for purposes of the set includes
+    // the DoNotCompare field.
+    e =
+        assertThrows(
+            AssertionError.class,
+            () ->
+                assertAboutImmutableObjects()
+                    .that(makeTestObj())
+                    .isEqualAcrossDatabases(
+                        makeTestObj()
+                            .withSet(ImmutableSet.of(makeTestAtom("bob"), makeTestAtom("bob")))));
+    assertThat(e)
+        .hasMessageThat()
+        .containsMatch(
+            Pattern.compile(
+                "At google.registry.model.ImmutableObjectSubjectTest\\$TestImmutableObject.set: "
+                    + "element .*bob.* matches multiple elements in .*bob.*bob",
+                Pattern.DOTALL));
+    e =
+        assertThrows(
+            AssertionError.class,
+            () ->
+                assertAboutImmutableObjects()
+                    .that(
+                        makeTestObj()
+                            .withSet(ImmutableSet.of(makeTestAtom("bob"), makeTestAtom("bob"))))
+                    .isEqualAcrossDatabases(makeTestObj()));
+    assertThat(e)
+        .hasMessageThat()
+        .containsMatch(
+            Pattern.compile(
+                "At google.registry.model.ImmutableObjectSubjectTest\\$TestImmutableObject.set: "
+                    + "Set does not contain the expected contents.  It contains additional "
+                    + "elements: .*bob",
+                Pattern.DOTALL));
+
+    assertThat(
+            checkObjectAcrossDatabases(
+                    makeTestObj(),
+                    makeTestObj()
+                        .withSet(ImmutableSet.of(makeTestAtom("bob"), makeTestAtom("robert"))),
+                    null)
+                .isFailure())
+        .isTrue();
+    assertThat(
+            checkObjectAcrossDatabases(
+                    makeTestObj(), makeTestObj().withSet(ImmutableSet.of()), null)
+                .isFailure())
+        .isTrue();
+    assertThat(
+            checkObjectAcrossDatabases(
+                    makeTestObj(),
+                    makeTestObj()
+                        .withSet(ImmutableSet.of(makeTestAtom("bob"), makeTestAtom("bob"))),
+                    null)
+                .isFailure())
+        .isTrue();
+    // We don't test the case where actual's set contains multiple items matching the single item in
+    // the expected set: that path is the same as the "additional contents" path.
+  }
+
+  @Test
+  void testCrossDatabase_mapFailure() {
+    AssertionError e =
+        assertThrows(
+            AssertionError.class,
+            () ->
+                assertAboutImmutableObjects()
+                    .that(makeTestObj())
+                    .isEqualAcrossDatabases(makeTestObj().withMap(null)));
+    assertThat(e)
+        .hasMessageThat()
+        .contains(
+            "At google.registry.model.ImmutableObjectSubjectTest$"
+                + "TestImmutableObject.map: expected null, got ");
+
+    e =
+        assertThrows(
+            AssertionError.class,
+            () ->
+                assertAboutImmutableObjects()
+                    .that(makeTestObj())
+                    .isEqualAcrossDatabases(
+                        makeTestObj()
+                            .withMap(ImmutableMap.of(makeTestAtom("difk"), makeTestAtom("difv")))));
+    assertThat(e)
+        .hasMessageThat()
+        .containsMatch(
+            Pattern.compile(
+                "Map does not contain the expected contents.  "
+                    + "It is missing: .*difk.*difv.*  It contains additional elements: .*key.*val",
+                Pattern.DOTALL));
+    assertThat(
+            checkObjectAcrossDatabases(
+                    makeTestObj(),
+                    makeTestObj()
+                        .withMap(
+                            ImmutableMap.of(
+                                makeTestAtom("key"), makeTestAtom("val"),
+                                makeTestAtom("otherk"), makeTestAtom("otherv"))),
+                    null)
+                .isFailure())
+        .isTrue();
+    assertThat(
+            checkObjectAcrossDatabases(
+                    makeTestObj(), makeTestObj().withMap(ImmutableMap.of()), null)
+                .isFailure())
+        .isTrue();
+  }
+
+  @Test
+  void testCrossDatabase_typeChecks() {
+    ComparisonResult result = checkObjectAcrossDatabases("blech", makeTestObj(), "xxx");
+    assertThat(result.getMessage()).isEqualTo("At xxx: blech is not an immutable object.");
+    assertThat(result.isFailure()).isTrue();
+    assertThat(checkObjectAcrossDatabases("blech", makeTestObj(), null).isFailure()).isTrue();
+
+    result = checkObjectAcrossDatabases("blech", ImmutableMap.of(), "xxx");
+    assertThat(result.getMessage()).isEqualTo("At xxx: blech is not a Map.");
+    assertThat(result.isFailure()).isTrue();
+    assertThat(checkObjectAcrossDatabases("blech", ImmutableMap.of(), null).isFailure()).isTrue();
+
+    result = checkObjectAcrossDatabases("blech", ImmutableList.of(), "xxx");
+    assertThat(result.getMessage()).isEqualTo("At xxx: blech is not a Collection.");
+    assertThat(result.isFailure()).isTrue();
+    assertThat(checkObjectAcrossDatabases("blech", ImmutableList.of(), null).isFailure()).isTrue();
+
+    for (ImmutableMap.Entry<String, String> entry : ImmutableMap.of("foo", "bar").entrySet()) {
+      result = checkObjectAcrossDatabases("blech", entry, "xxx");
+      assertThat(result.getMessage()).isEqualTo("At xxx: blech is not a Map.Entry.");
+      assertThat(result.isFailure()).isTrue();
+      assertThat(checkObjectAcrossDatabases("blech", entry, "xxx").isFailure()).isTrue();
+    }
+  }
+
+  @Test
+  void testCrossDatabase_checkAdditionalFields() {
+    AssertionError e =
+        assertThrows(
+            AssertionError.class,
+            () ->
+                assertAboutImmutableObjects()
+                    .that(DerivedImmutableObject.create())
+                    .isEqualAcrossDatabases(makeTestAtom(null)));
+    assertThat(e)
+        .hasMessageThat()
+        .contains(
+            "At google.registry.model.ImmutableObjectSubjectTest$DerivedImmutableObject: "
+                + "has additional field extraField");
+
+    assertThat(
+            checkObjectAcrossDatabases(DerivedImmutableObject.create(), makeTestAtom(null), null)
+                .isFailure())
+        .isTrue();
+  }
+
+  @Test
+  void testHasCorrectHashValue() {
+    TestImmutableObject object = makeTestObj();
+    assertAboutImmutableObjects().that(object).hasCorrectHashValue();
+
+    object.stringField = "changed value!";
+    assertThrows(
+        AssertionError.class,
+        () -> assertAboutImmutableObjects().that(object).hasCorrectHashValue());
+  }
+
+  /** Make a test object with all fields set up. */
+  TestImmutableObject makeTestObj() {
+    return TestImmutableObject.create(
+        "foo",
+        makeTestAtom("bar"),
+        ImmutableList.of(makeTestAtom("baz")),
+        ImmutableSet.of(makeTestAtom("bob")),
+        ImmutableMap.of(makeTestAtom("key"), makeTestAtom("val")));
+  }
+
+  /** Make a test object without the collection fields. */
+  TestImmutableObject makeTestAtom(String stringField) {
+    return TestImmutableObject.create(stringField, null, null, null, null);
+  }
+
+  static class TestImmutableObject extends ImmutableObject {
+    String stringField;
+    TestImmutableObject nested;
+    ImmutableList<TestImmutableObject> list;
+    ImmutableSet<TestImmutableObject> set;
+    ImmutableMap<TestImmutableObject, TestImmutableObject> map;
+
+    @ImmutableObject.DoNotCompare int ignored;
+
+    static TestImmutableObject create(
+        String stringField,
+        TestImmutableObject nested,
+        ImmutableList<TestImmutableObject> list,
+        ImmutableSet<TestImmutableObject> set,
+        ImmutableMap<TestImmutableObject, TestImmutableObject> map) {
+      TestImmutableObject instance = new TestImmutableObject();
+      instance.stringField = stringField;
+      instance.nested = nested;
+      instance.list = list;
+      instance.set = set;
+      instance.map = map;
+      instance.ignored = ++uniqueId;
+      return instance;
+    }
+
+    TestImmutableObject withStringField(@Nullable String stringField) {
+      TestImmutableObject result = ImmutableObject.clone(this);
+      result.stringField = stringField;
+      return result;
+    }
+
+    TestImmutableObject withNested(@Nullable TestImmutableObject nested) {
+      TestImmutableObject result = ImmutableObject.clone(this);
+      result.nested = nested;
+      return result;
+    }
+
+    TestImmutableObject withList(@Nullable ImmutableList<TestImmutableObject> list) {
+      TestImmutableObject result = ImmutableObject.clone(this);
+      result.list = list;
+      return result;
+    }
+
+    TestImmutableObject withSet(@Nullable ImmutableSet<TestImmutableObject> set) {
+      TestImmutableObject result = ImmutableObject.clone(this);
+      result.set = set;
+      return result;
+    }
+
+    TestImmutableObject withMap(
+        @Nullable ImmutableMap<TestImmutableObject, TestImmutableObject> map) {
+      TestImmutableObject result = ImmutableObject.clone(this);
+      result.map = map;
+      return result;
+    }
+  }
+
+  static class DerivedImmutableObject extends TestImmutableObject {
+    String extraField;
+
+    static DerivedImmutableObject create() {
+      return new DerivedImmutableObject();
+    }
+  }
+}

core/src/test/java/google/registry/testing/ReplayExtension.java

@@ -17,7 +17,6 @@ package google.registry.testing;
 import static com.google.common.truth.Truth.assertThat;
 import static google.registry.model.ImmutableObjectSubject.assertAboutImmutableObjects;
 import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
-import static google.registry.persistence.transaction.TransactionManagerFactory.ofyTm;
 
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
@@ -26,6 +25,7 @@ import google.registry.model.ImmutableObject;
 import google.registry.model.ofy.ReplayQueue;
 import google.registry.model.ofy.TransactionInfo;
 import google.registry.persistence.VKey;
+import google.registry.schema.replay.DatastoreEntity;
 import java.util.Optional;
 import org.junit.jupiter.api.extension.AfterEachCallback;
 import org.junit.jupiter.api.extension.BeforeEachCallback;
@@ -106,16 +106,20 @@ public class ReplayExtension implements BeforeEachCallback, AfterEachCallback {
           continue;
         }
 
+        // Since the object may have changed in datastore by the time we're doing the replay, we
+        // have to compare the current value in SQL (which we just mutated) against the value that
+        // we originally would have persisted (that being the object in the entry).
         VKey<?> vkey = VKey.from(entry.getKey());
-        Optional<?> ofyValue = ofyTm().transact(() -> ofyTm().loadByKeyIfPresent(vkey));
         Optional<?> jpaValue = jpaTm().transact(() -> jpaTm().loadByKeyIfPresent(vkey));
         if (entry.getValue().equals(TransactionInfo.Delete.SENTINEL)) {
           assertThat(jpaValue.isPresent()).isFalse();
-          assertThat(ofyValue.isPresent()).isFalse();
         } else {
+          ImmutableObject immutJpaObject = (ImmutableObject) jpaValue.get();
+          assertAboutImmutableObjects().that(immutJpaObject).hasCorrectHashValue();
           assertAboutImmutableObjects()
-              .that((ImmutableObject) jpaValue.get())
-              .isEqualAcrossDatabases((ImmutableObject) ofyValue.get());
+              .that(immutJpaObject)
+              .isEqualAcrossDatabases(
+                  (ImmutableObject) ((DatastoreEntity) entry.getValue()).toSqlEntity().get());
         }
       }
     }

proxy/src/test/java/google/registry/proxy/handler/EppServiceHandlerTest.java

@@ -21,6 +21,7 @@ import static google.registry.proxy.TestUtils.assertHttpRequestEquivalent;
 import static google.registry.proxy.TestUtils.makeEppHttpResponse;
 import static google.registry.proxy.handler.ProxyProtocolHandler.REMOTE_ADDRESS_KEY;
 import static google.registry.util.X509Utils.getCertificateHash;
+import static google.registry.util.X509Utils.loadCertificate;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.Mockito.mock;
@@ -47,6 +48,7 @@ import io.netty.util.concurrent.Promise;
 import java.io.ByteArrayInputStream;
 import java.security.cert.CertificateFactory;
 import java.security.cert.X509Certificate;
+import java.util.Base64;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -239,6 +241,23 @@ class EppServiceHandlerTest {
     assertThat(channel.isActive()).isTrue();
   }
 
+  @Test
+  void testSuccess_requestContainsEncodedCertificate() throws Exception {
+    setHandshakeSuccess();
+    // First inbound message is hello.
+    channel.readInbound();
+    String content = "<epp>stuff</epp>";
+    channel.writeInbound(Unpooled.wrappedBuffer(content.getBytes(UTF_8)));
+    FullHttpRequest request = channel.readInbound();
+    assertThat(request).isEqualTo(makeEppHttpRequestWithCertificate(content));
+    String encodedCert = request.headers().get("X-SSL-Full-Certificate");
+    assertThat(encodedCert).isNotEqualTo(SAMPLE_CERT);
+    X509Certificate decodedCert =
+        loadCertificate(new ByteArrayInputStream(Base64.getDecoder().decode(encodedCert)));
+    X509Certificate pemCert = loadCertificate(SAMPLE_CERT);
+    assertThat(decodedCert).isEqualTo(pemCert);
+  }
+
   @Test
   void testSuccess_sendCertificateOnlyBeforeLogin() throws Exception {
     setHandshakeSuccess();

util/src/main/java/google/registry/util/X509Utils.java

@@ -32,6 +32,7 @@ import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 import java.security.cert.CRLException;
 import java.security.cert.CRLReason;
+import java.security.cert.CertificateEncodingException;
 import java.security.cert.CertificateException;
 import java.security.cert.CertificateFactory;
 import java.security.cert.CertificateParsingException;
@@ -39,6 +40,7 @@ import java.security.cert.CertificateRevokedException;
 import java.security.cert.X509CRL;
 import java.security.cert.X509CRLEntry;
 import java.security.cert.X509Certificate;
+import java.util.Base64;
 import java.util.Date;
 import java.util.NoSuchElementException;
 import java.util.Optional;
@@ -177,5 +179,20 @@ public final class X509Utils {
     newCrl.verify(rootCert.getPublicKey());
   }
 
+  /** Constructs an X.509 certificate from a PEM string and encodes it. */
+  public static String encodeX509CertificateFromPemString(String certificateString)
+      throws CertificateException {
+    return encodeX509Certificate(loadCertificate(certificateString));
+  }
+
+  /**
+   * Encodes an X.509 certificate in the same form that the proxy encodes a certificate before
+   * passing it via an HTTP header.
+   */
+  public static String encodeX509Certificate(X509Certificate certificate)
+      throws CertificateEncodingException {
+    return Base64.getEncoder().encodeToString(certificate.getEncoded());
+  }
+
   private X509Utils() {}
 }