From 33f9bc30b78c6cdef2884a1bdeffd9b166d36987 Mon Sep 17 00:00:00 2001 From: Lai Jiang Date: Tue, 28 Feb 2023 10:00:18 -0500 Subject: [PATCH] Ignore invalid old CRL when performing update. (#1946) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There is no point comparing the old CRL to the new ones when the old one is invalid. This could happen when the CA cert rotates, after which the old CRL stop being valid as it fails signature verification against the new cert. This change will allow us to keep updating the CRL after a CA rotation without having to manually delete the old CRL from the database. See b/270983553. - - - This change is [Reviewable](https://reviewable.io/reviews/google/nomulus/1946) --- .../registry/tmch/TmchCertificateAuthority.java | 12 ++++++++++-- .../main/java/google/registry/util/X509Utils.java | 7 ++++--- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/google/registry/tmch/TmchCertificateAuthority.java b/core/src/main/java/google/registry/tmch/TmchCertificateAuthority.java index e1c21b63b..0fbcff250 100644 --- a/core/src/main/java/google/registry/tmch/TmchCertificateAuthority.java +++ b/core/src/main/java/google/registry/tmch/TmchCertificateAuthority.java @@ -22,6 +22,7 @@ import static google.registry.util.ResourceUtils.readResourceUtf8; import com.github.benmanes.caffeine.cache.CacheLoader; import com.github.benmanes.caffeine.cache.LoadingCache; import com.google.common.collect.ImmutableMap; +import com.google.common.flogger.FluentLogger; import google.registry.config.RegistryConfig.Config; import google.registry.config.RegistryConfig.ConfigModule.TmchCaMode; import google.registry.model.CacheUtils; @@ -55,6 +56,7 @@ public final class TmchCertificateAuthority { private static final String ROOT_CRT_PILOT_FILE = "icann-tmch-pilot.crt"; private static final String CRL_FILE = "icann-tmch.crl"; private static final String CRL_PILOT_FILE = "icann-tmch-pilot.crl"; + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private final TmchCaMode tmchCaMode; private final Clock clock; @@ -142,8 +144,14 @@ public final class TmchCertificateAuthority { * @see X509Utils#verifyCrl */ public void updateCrl(String asciiCrl, String url) throws GeneralSecurityException { - X509CRL crl = X509Utils.loadCrl(asciiCrl); - X509Utils.verifyCrl(getAndValidateRoot(), getCrl(), crl, clock.nowUtc().toDate()); + X509CRL newCrl = X509Utils.loadCrl(asciiCrl); + X509CRL oldCrl = null; + try { + oldCrl = getCrl(); + } catch (Exception e) { + logger.atWarning().withCause(e).log("Old CRL is invalid, ignored during CRL update."); + } + X509Utils.verifyCrl(getAndValidateRoot(), oldCrl, newCrl, clock.nowUtc().toDate()); TmchCrl.set(asciiCrl, url); } diff --git a/util/src/main/java/google/registry/util/X509Utils.java b/util/src/main/java/google/registry/util/X509Utils.java index ecd4250f3..7304b4f9c 100644 --- a/util/src/main/java/google/registry/util/X509Utils.java +++ b/util/src/main/java/google/registry/util/X509Utils.java @@ -44,6 +44,7 @@ import java.util.Base64; import java.util.Date; import java.util.NoSuchElementException; import java.util.Optional; +import javax.annotation.Nullable; import javax.annotation.Tainted; /** X.509 Public Key Infrastructure (PKI) helper functions. */ @@ -163,12 +164,12 @@ public final class X509Utils { * are correct with respect to {@code now}. * * @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. */ public static void verifyCrl( - X509Certificate rootCert, X509CRL oldCrl, @Tainted X509CRL newCrl, Date now) + X509Certificate rootCert, @Nullable X509CRL oldCrl, @Tainted X509CRL newCrl, Date now) throws GeneralSecurityException { - if (newCrl.getThisUpdate().before(oldCrl.getThisUpdate())) { + if (oldCrl != null && newCrl.getThisUpdate().before(oldCrl.getThisUpdate())) { throw new CRLException(String.format( "New CRL is more out of date than our current CRL. %s < %s\n%s", newCrl.getThisUpdate(), oldCrl.getThisUpdate(), newCrl));