Refactor TmchCertificateAuthority

The main goal of this CL is to make the clock constructor injectable (so that tests do not need to use InjectRule to override the clock). The complication is that the clock is used by two static caches (ROOT_CACHE directly and CRL_CACHE indirectly). The clock is not actually used to construct the lock, but rather to verify that the root certificate is within its validity period.

For ROOT_CACHE we move the verification to its call sites. This adds a bit overhead because the validity check happens every time the cache is called, not just when the cache is built or refreshed. However this check is rather cheap.  Also the resources are included in the jar and the cache is valid for 1 year. Given that we deploy every week, there's not much point making it an expiring cache rather than a static map.

For CRL_CACHE we change the key to a tuple of TmchCaMode and X509Certificate. The certificate is no longer provided from the ROOT_CACHE directly and must be verified before it is provided as a cache key. We left the CRL verification inside the cache loader because it (signature verification) is more expensive compared to simple expiration check, and we do not want to do this every time the cache is called.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=218385684

javatests/google/registry/flows/EppTestCase.java

@@ -32,7 +32,6 @@ import google.registry.testing.FakeHttpSession;
 import google.registry.testing.FakeResponse;
 import google.registry.testing.InjectRule;
 import google.registry.testing.ShardableTestCase;
-import google.registry.tmch.TmchCertificateAuthority;
 import java.util.Map;
 import javax.annotation.Nullable;
 import org.joda.time.DateTime;
@@ -58,8 +57,6 @@ public class EppTestCase extends ShardableTestCase {
   public void initTestCase() {
     // For transactional flows
     inject.setStaticField(Ofy.class, "clock", clock);
-    // For SignedMark signature validity
-    inject.setStaticField(TmchCertificateAuthority.class, "clock", clock);
   }
 
   /**

javatests/google/registry/flows/EppTestComponent.java

@@ -80,7 +80,7 @@ interface EppTestComponent {
       return create(
           clock,
           metricBuilder,
-          new TmchXmlSignature(new TmchCertificateAuthority(TmchCaMode.PILOT)));
+          new TmchXmlSignature(new TmchCertificateAuthority(TmchCaMode.PILOT, clock)));
     }
 
     public static FakesAndMocksModule create(

javatests/google/registry/flows/FlowTestCase.java

@@ -110,8 +110,6 @@ public abstract class FlowTestCase<F extends Flow> extends ShardableTestCase {
     ofy().saveWithoutBackup().entity(new ClaimsListSingleton()).now();
     // For transactional flows
     inject.setStaticField(Ofy.class, "clock", clock);
-    // For SignedMark signature validity
-    inject.setStaticField(TmchCertificateAuthority.class, "clock", clock);
  }
 
   protected void removeServiceExtensionUri(String uri) {
@@ -286,7 +284,7 @@ public abstract class FlowTestCase<F extends Flow> extends ShardableTestCase {
     TmchXmlSignature tmchXmlSignature =
         testTmchXmlSignature != null
             ? testTmchXmlSignature
-            : new TmchXmlSignature(new TmchCertificateAuthority(tmchCaMode));
+            : new TmchXmlSignature(new TmchCertificateAuthority(tmchCaMode, clock));
     return DaggerEppTestComponent.builder()
         .fakesAndMocksModule(FakesAndMocksModule.create(clock, eppMetricBuilder, tmchXmlSignature))
         .build()

javatests/google/registry/tmch/TmchActionTestCase.java

@@ -24,7 +24,6 @@ import com.google.appengine.api.urlfetch.URLFetchService;
 import google.registry.testing.AppEngineRule;
 import google.registry.testing.BouncyCastleProviderRule;
 import google.registry.testing.FakeClock;
-import google.registry.testing.InjectRule;
 import google.registry.testing.MockitoJUnitRule;
 import org.junit.Before;
 import org.junit.Rule;
@@ -44,7 +43,6 @@ public class TmchActionTestCase {
   @Rule public final AppEngineRule appEngine = AppEngineRule.builder().withDatastore().build();
   @Rule public final MockitoJUnitRule mocks = MockitoJUnitRule.create();
   @Rule public final BouncyCastleProviderRule bouncy = new BouncyCastleProviderRule();
-  @Rule public final InjectRule inject = new InjectRule();
 
   @Mock URLFetchService fetchService;
   @Mock HTTPResponse httpResponse;
@@ -55,7 +53,6 @@ public class TmchActionTestCase {
 
   @Before
   public void commonBefore() throws Exception {
-    inject.setStaticField(TmchCertificateAuthority.class, "clock", clock);
     marksdb.fetchService = fetchService;
     marksdb.tmchMarksdbUrl = MARKSDB_URL;
     marksdb.marksdbPublicKey = TmchData.loadPublicKey(TmchTestData.loadBytes("pubkey"));

javatests/google/registry/tmch/TmchCertificateAuthorityTest.java

@@ -25,13 +25,11 @@ import static google.registry.util.X509Utils.loadCertificate;
 import google.registry.model.tmch.TmchCrl;
 import google.registry.testing.AppEngineRule;
 import google.registry.testing.FakeClock;
-import google.registry.testing.InjectRule;
 import java.security.SignatureException;
 import java.security.cert.CertificateExpiredException;
 import java.security.cert.CertificateNotYetValidException;
 import java.security.cert.CertificateRevokedException;
 import org.joda.time.DateTime;
-import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -48,38 +46,35 @@ public class TmchCertificateAuthorityTest {
   public final AppEngineRule appEngine = AppEngineRule.builder()
       .withDatastore()
       .build();
-  @Rule
-  public final InjectRule inject = new InjectRule();
 
   private FakeClock clock = new FakeClock(DateTime.parse("2014-01-01T00:00:00Z"));
 
-  @Before
-  public void before() {
-    inject.setStaticField(TmchCertificateAuthority.class, "clock", clock);
-  }
-
   @Test
   public void testFailure_prodRootExpired() {
-    TmchCertificateAuthority tmchCertificateAuthority = new TmchCertificateAuthority(PRODUCTION);
+    TmchCertificateAuthority tmchCertificateAuthority =
+        new TmchCertificateAuthority(PRODUCTION, clock);
     clock.setTo(DateTime.parse("2024-01-01T00:00:00Z"));
     CertificateExpiredException e =
-        assertThrows(CertificateExpiredException.class, tmchCertificateAuthority::getRoot);
+        assertThrows(
+            CertificateExpiredException.class, tmchCertificateAuthority::getAndValidateRoot);
     assertThat(e).hasMessageThat().containsMatch("NotAfter: Sun Jul 23 23:59:59 UTC 2023");
   }
 
   @Test
   public void testFailure_prodRootNotYetValid() {
-    TmchCertificateAuthority tmchCertificateAuthority = new TmchCertificateAuthority(PRODUCTION);
+    TmchCertificateAuthority tmchCertificateAuthority =
+        new TmchCertificateAuthority(PRODUCTION, clock);
     clock.setTo(DateTime.parse("2000-01-01T00:00:00Z"));
     CertificateNotYetValidException e =
-        assertThrows(CertificateNotYetValidException.class, tmchCertificateAuthority::getRoot);
+        assertThrows(
+            CertificateNotYetValidException.class, tmchCertificateAuthority::getAndValidateRoot);
     assertThat(e).hasMessageThat().containsMatch("NotBefore: Wed Jul 24 00:00:00 UTC 2013");
   }
 
   @Test
   public void testFailure_crlDoesntMatchCerts() {
     // Use the prod cl, which won't match our test certificate.
-    TmchCertificateAuthority tmchCertificateAuthority = new TmchCertificateAuthority(PILOT);
+    TmchCertificateAuthority tmchCertificateAuthority = new TmchCertificateAuthority(PILOT, clock);
     TmchCrl.set(
         readResourceUtf8(TmchCertificateAuthority.class, "icann-tmch.crl"), "http://cert.crl");
     SignatureException e =
@@ -91,13 +86,14 @@ public class TmchCertificateAuthorityTest {
 
   @Test
   public void testSuccess_verify() throws Exception {
-    TmchCertificateAuthority tmchCertificateAuthority = new TmchCertificateAuthority(PILOT);
+    TmchCertificateAuthority tmchCertificateAuthority = new TmchCertificateAuthority(PILOT, clock);
     tmchCertificateAuthority.verify(loadCertificate(GOOD_TEST_CERTIFICATE));
   }
 
   @Test
   public void testFailure_verifySignatureDoesntMatch() {
-    TmchCertificateAuthority tmchCertificateAuthority = new TmchCertificateAuthority(PRODUCTION);
+    TmchCertificateAuthority tmchCertificateAuthority =
+        new TmchCertificateAuthority(PRODUCTION, clock);
     SignatureException e =
         assertThrows(
             SignatureException.class,
@@ -107,7 +103,7 @@ public class TmchCertificateAuthorityTest {
 
   @Test
   public void testFailure_verifyRevoked() {
-    TmchCertificateAuthority tmchCertificateAuthority = new TmchCertificateAuthority(PILOT);
+    TmchCertificateAuthority tmchCertificateAuthority = new TmchCertificateAuthority(PILOT, clock);
     CertificateRevokedException thrown =
         assertThrows(
             CertificateRevokedException.class,

javatests/google/registry/tmch/TmchCrlActionTest.java

@@ -36,7 +36,7 @@ public class TmchCrlActionTest extends TmchActionTestCase {
   private TmchCrlAction newTmchCrlAction(TmchCaMode tmchCaMode) throws MalformedURLException {
     TmchCrlAction action = new TmchCrlAction();
     action.marksdb = marksdb;
-    action.tmchCertificateAuthority = new TmchCertificateAuthority(tmchCaMode);
+    action.tmchCertificateAuthority = new TmchCertificateAuthority(tmchCaMode, clock);
     action.tmchCrlUrl = new URL("http://sloth.lol/tmch.crl");
     return action;
   }

javatests/google/registry/tmch/TmchTestDataExpirationTest.java

@@ -24,6 +24,7 @@ import google.registry.flows.domain.DomainFlowTmchUtils;
 import google.registry.model.smd.EncodedSignedMark;
 import google.registry.testing.AppEngineRule;
 import google.registry.util.ResourceUtils;
+import google.registry.util.SystemClock;
 import java.nio.file.Path;
 import org.joda.time.DateTime;
 import org.junit.Rule;
@@ -43,7 +44,8 @@ public class TmchTestDataExpirationTest {
   public void testActiveSignedMarkFiles_areValidAndNotExpired() throws Exception {
     DomainFlowTmchUtils tmchUtils =
         new DomainFlowTmchUtils(
-            new TmchXmlSignature(new TmchCertificateAuthority(TmchCaMode.PILOT)));
+            new TmchXmlSignature(
+                new TmchCertificateAuthority(TmchCaMode.PILOT, new SystemClock())));
 
     for (Path path : listFiles(TmchTestDataExpirationTest.class, "testdata/active/")) {
       if (path.toString().endsWith(".smd")) {

javatests/google/registry/tmch/TmchXmlSignatureTest.java

@@ -72,17 +72,17 @@ public class TmchXmlSignatureTest {
   private final FakeClock clock = new FakeClock(DateTime.parse("2018-05-15T23:15:37.4Z"));
 
   private byte[] smdData;
-  private TmchXmlSignature tmchXmlSignature;
+  private TmchXmlSignature tmchXmlSignature =
+      new TmchXmlSignature(new TmchCertificateAuthority(TmchCaMode.PILOT, clock));
 
   @Before
   public void before() {
-    inject.setStaticField(TmchCertificateAuthority.class, "clock", clock);
-    tmchXmlSignature = new TmchXmlSignature(new TmchCertificateAuthority(TmchCaMode.PILOT));
   }
 
   @Test
   public void testWrongCertificateAuthority() {
-    tmchXmlSignature = new TmchXmlSignature(new TmchCertificateAuthority(TmchCaMode.PRODUCTION));
+    tmchXmlSignature =
+        new TmchXmlSignature(new TmchCertificateAuthority(TmchCaMode.PRODUCTION, clock));
     smdData = loadSmd("active/Court-Agent-Arab-Active.smd");
     CertificateSignatureException e =
         assertThrows(CertificateSignatureException.class, () -> tmchXmlSignature.verify(smdData));

javatests/google/registry/tools/UpdateSmdCommandTest.java

@@ -75,8 +75,6 @@ public class UpdateSmdCommandTest extends CommandTestCase<UpdateSmdCommand> {
 
   @Before
   public void init() {
-    // For SignedMark signature validity
-    inject.setStaticField(TmchCertificateAuthority.class, "clock", clock);
     inject.setStaticField(Ofy.class, "clock", clock);
     createTld("xn--q9jyb4c");
     clock.advanceOneMilli();
@@ -87,7 +85,7 @@ public class UpdateSmdCommandTest extends CommandTestCase<UpdateSmdCommand> {
         .build());
     clock.advanceOneMilli();
     command.tmchUtils =
-        new DomainFlowTmchUtils(new TmchXmlSignature(new TmchCertificateAuthority(PILOT)));
+        new DomainFlowTmchUtils(new TmchXmlSignature(new TmchCertificateAuthority(PILOT, clock)));
   }
 
   private DomainApplication reloadDomainApplication() {