diff --git a/java/google/registry/tmch/TmchCertificateAuthority.java b/java/google/registry/tmch/TmchCertificateAuthority.java index a3ec73f60..9db65809f 100644 --- a/java/google/registry/tmch/TmchCertificateAuthority.java +++ b/java/google/registry/tmch/TmchCertificateAuthority.java @@ -15,7 +15,7 @@ package google.registry.tmch; import static google.registry.config.RegistryConfig.ConfigModule.TmchCaMode.PILOT; -import static google.registry.config.RegistryConfig.getSingletonCachePersistDuration; +import static google.registry.config.RegistryConfig.ConfigModule.TmchCaMode.PRODUCTION; import static google.registry.config.RegistryConfig.getSingletonCacheRefreshDuration; import static google.registry.util.ResourceUtils.readResourceUtf8; import static java.util.concurrent.TimeUnit.MILLISECONDS; @@ -23,17 +23,16 @@ import static java.util.concurrent.TimeUnit.MILLISECONDS; import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; +import com.google.common.collect.ImmutableMap; import google.registry.config.RegistryConfig.Config; import google.registry.config.RegistryConfig.ConfigModule.TmchCaMode; import google.registry.model.tmch.TmchCrl; import google.registry.util.Clock; -import google.registry.util.NonFinalForTesting; -import google.registry.util.SystemClock; import google.registry.util.X509Utils; import java.security.GeneralSecurityException; +import java.security.cert.CertificateParsingException; import java.security.cert.X509CRL; import java.security.cert.X509Certificate; -import java.util.concurrent.ExecutionException; import javax.annotation.concurrent.Immutable; import javax.annotation.concurrent.ThreadSafe; import javax.inject.Inject; @@ -58,9 +57,12 @@ public final class TmchCertificateAuthority { private static final String CRL_PILOT_FILE = "icann-tmch-pilot.crl"; private final TmchCaMode tmchCaMode; + private final Clock clock; - public @Inject TmchCertificateAuthority(@Config("tmchCaMode") TmchCaMode tmchCaMode) { + @Inject + public TmchCertificateAuthority(@Config("tmchCaMode") TmchCaMode tmchCaMode, Clock clock) { this.tmchCaMode = tmchCaMode; + this.clock = clock; } /** @@ -70,8 +72,8 @@ public final class TmchCertificateAuthority { * string into an X509CRL instance is expensive and should itself be cached. * *

Note that the stored CRL won't exist for tests, and on deployed environments will always - * correspond to the correct CRL for the given TMCH CA mode because {@link TmchCrlAction} can - * only persist the correct one for this given environment. + * correspond to the correct CRL for the given TMCH CA mode because {@link TmchCrlAction} can only + * persist the correct one for this given environment. */ private static final LoadingCache CRL_CACHE = CacheBuilder.newBuilder() @@ -89,37 +91,28 @@ public final class TmchCertificateAuthority { crlContents = storedCrl.getCrl(); } X509CRL crl = X509Utils.loadCrl(crlContents); - try { - crl.verify(ROOT_CACHE.get(tmchCaMode).getPublicKey()); - return crl; - } catch (ExecutionException e) { - if (e.getCause() instanceof GeneralSecurityException) { - throw (GeneralSecurityException) e.getCause(); - } else { - throw new RuntimeException("Unexpected exception while loading CRL", e); - } - } - }}); + crl.verify(ROOT_CERTS.get(tmchCaMode).getPublicKey()); + return crl; + } + }); - /** A cached function that loads the CRT from a jar resource. */ - private static final LoadingCache ROOT_CACHE = - CacheBuilder.newBuilder() - .expireAfterWrite(getSingletonCachePersistDuration().getMillis(), MILLISECONDS) - .build( - new CacheLoader() { - @Override - public X509Certificate load(final TmchCaMode tmchCaMode) - throws GeneralSecurityException { - String file = (tmchCaMode == PILOT) ? ROOT_CRT_PILOT_FILE : ROOT_CRT_FILE; - X509Certificate root = - X509Utils.loadCertificate( - readResourceUtf8(TmchCertificateAuthority.class, file)); - root.checkValidity(clock.nowUtc().toDate()); - return root; - }}); + /** CRTs from a jar resource. */ + private static final ImmutableMap ROOT_CERTS = + loadRootCertificates(); - @NonFinalForTesting - private static Clock clock = new SystemClock(); + private static ImmutableMap loadRootCertificates() { + try { + return ImmutableMap.of( + PILOT, + X509Utils.loadCertificate( + readResourceUtf8(TmchCertificateAuthority.class, ROOT_CRT_PILOT_FILE)), + PRODUCTION, + X509Utils.loadCertificate( + readResourceUtf8(TmchCertificateAuthority.class, ROOT_CRT_FILE))); + } catch (CertificateParsingException e) { + throw new RuntimeException(e); + } + } /** * Check that {@code cert} is signed by the ICANN TMCH CA root and not revoked. @@ -132,7 +125,7 @@ public final class TmchCertificateAuthority { */ public void verify(X509Certificate cert) throws GeneralSecurityException { synchronized (TmchCertificateAuthority.class) { - X509Utils.verifyCertificate(getRoot(), getCrl(), cert, clock.nowUtc().toDate()); + X509Utils.verifyCertificate(getAndValidateRoot(), getCrl(), cert, clock.nowUtc().toDate()); } } @@ -145,21 +138,26 @@ public final class TmchCertificateAuthority { * refreshes itself. * * @throws GeneralSecurityException for unsupported protocols, certs not signed by the TMCH, - * incorrect keys, and for invalid, old, not-yet-valid or revoked certificates. + * incorrect keys, and for invalid, old, not-yet-valid or revoked certificates. * @see X509Utils#verifyCrl */ public void updateCrl(String asciiCrl, String url) throws GeneralSecurityException { X509CRL crl = X509Utils.loadCrl(asciiCrl); - X509Utils.verifyCrl(getRoot(), getCrl(), crl, clock.nowUtc().toDate()); + X509Utils.verifyCrl(getAndValidateRoot(), getCrl(), crl, clock.nowUtc().toDate()); TmchCrl.set(asciiCrl, url); } - public X509Certificate getRoot() throws GeneralSecurityException { + public X509Certificate getAndValidateRoot() throws GeneralSecurityException { try { - return ROOT_CACHE.get(tmchCaMode); + X509Certificate root = ROOT_CERTS.get(tmchCaMode); + // The current production certificate expires on 2023-07-23. Future code monkey be reminded, + // if you are looking at this code because the next line throws an exception, ask ICANN for a + // new root certificate! (preferably before the current one expires...) + root.checkValidity(clock.nowUtc().toDate()); + return root; } catch (Exception e) { - if (e.getCause() instanceof GeneralSecurityException) { - throw (GeneralSecurityException) e.getCause(); + if (e instanceof GeneralSecurityException) { + throw (GeneralSecurityException) e; } else if (e instanceof RuntimeException) { throw (RuntimeException) e; } diff --git a/javatests/google/registry/flows/EppTestCase.java b/javatests/google/registry/flows/EppTestCase.java index 9385849f4..86b0001d1 100644 --- a/javatests/google/registry/flows/EppTestCase.java +++ b/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); } /** diff --git a/javatests/google/registry/flows/EppTestComponent.java b/javatests/google/registry/flows/EppTestComponent.java index cfe140422..72206f538 100644 --- a/javatests/google/registry/flows/EppTestComponent.java +++ b/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( diff --git a/javatests/google/registry/flows/FlowTestCase.java b/javatests/google/registry/flows/FlowTestCase.java index 145730833..80e25ec28 100644 --- a/javatests/google/registry/flows/FlowTestCase.java +++ b/javatests/google/registry/flows/FlowTestCase.java @@ -110,8 +110,6 @@ public abstract class FlowTestCase 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 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() diff --git a/javatests/google/registry/tmch/TmchActionTestCase.java b/javatests/google/registry/tmch/TmchActionTestCase.java index 4d75a29e1..cefa7902b 100644 --- a/javatests/google/registry/tmch/TmchActionTestCase.java +++ b/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")); diff --git a/javatests/google/registry/tmch/TmchCertificateAuthorityTest.java b/javatests/google/registry/tmch/TmchCertificateAuthorityTest.java index 128243a53..1d84565ba 100644 --- a/javatests/google/registry/tmch/TmchCertificateAuthorityTest.java +++ b/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, diff --git a/javatests/google/registry/tmch/TmchCrlActionTest.java b/javatests/google/registry/tmch/TmchCrlActionTest.java index 5d11b455b..6f3b447c3 100644 --- a/javatests/google/registry/tmch/TmchCrlActionTest.java +++ b/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; } diff --git a/javatests/google/registry/tmch/TmchTestDataExpirationTest.java b/javatests/google/registry/tmch/TmchTestDataExpirationTest.java index 57e9d9247..202ce8de1 100644 --- a/javatests/google/registry/tmch/TmchTestDataExpirationTest.java +++ b/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")) { diff --git a/javatests/google/registry/tmch/TmchXmlSignatureTest.java b/javatests/google/registry/tmch/TmchXmlSignatureTest.java index 30460b034..9bf94c5c1 100644 --- a/javatests/google/registry/tmch/TmchXmlSignatureTest.java +++ b/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)); diff --git a/javatests/google/registry/tools/UpdateSmdCommandTest.java b/javatests/google/registry/tools/UpdateSmdCommandTest.java index c1334b2ee..d112bf311 100644 --- a/javatests/google/registry/tools/UpdateSmdCommandTest.java +++ b/javatests/google/registry/tools/UpdateSmdCommandTest.java @@ -75,8 +75,6 @@ public class UpdateSmdCommandTest extends CommandTestCase { @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 { .build()); clock.advanceOneMilli(); command.tmchUtils = - new DomainFlowTmchUtils(new TmchXmlSignature(new TmchCertificateAuthority(PILOT))); + new DomainFlowTmchUtils(new TmchXmlSignature(new TmchCertificateAuthority(PILOT, clock))); } private DomainApplication reloadDomainApplication() {