diff --git a/core/src/main/java/google/registry/config/RegistryConfig.java b/core/src/main/java/google/registry/config/RegistryConfig.java index d8a551e8e..c98319435 100644 --- a/core/src/main/java/google/registry/config/RegistryConfig.java +++ b/core/src/main/java/google/registry/config/RegistryConfig.java @@ -1267,10 +1267,16 @@ public final class RegistryConfig { @Provides @Config("expirationWarningDays") - public static int provideDaysToExpiration(RegistryConfigSettings config) { + public static int provideExpirationWarningDays(RegistryConfigSettings config) { return config.sslCertificateValidation.expirationWarningDays; } + @Provides + @Config("expirationWarningIntervalDays") + public static int provideExpirationWarningIntervalDays(RegistryConfigSettings config) { + return config.sslCertificateValidation.expirationWarningIntervalDays; + } + @Provides @Config("minimumRsaKeyLength") public static int provideMinimumRsaKeyLength(RegistryConfigSettings config) { diff --git a/core/src/main/java/google/registry/config/RegistryConfigSettings.java b/core/src/main/java/google/registry/config/RegistryConfigSettings.java index 9b0431f77..cf7c5ce22 100644 --- a/core/src/main/java/google/registry/config/RegistryConfigSettings.java +++ b/core/src/main/java/google/registry/config/RegistryConfigSettings.java @@ -228,6 +228,7 @@ public class RegistryConfigSettings { public static class SslCertificateValidation { public Map maxValidityDaysSchedule; public int expirationWarningDays; + public int expirationWarningIntervalDays; public int minimumRsaKeyLength; public Set allowedEcdsaCurves; public String expirationWarningEmailBodyText; diff --git a/core/src/main/java/google/registry/config/files/default-config.yaml b/core/src/main/java/google/registry/config/files/default-config.yaml index 9f63f3f2d..729dd22b1 100644 --- a/core/src/main/java/google/registry/config/files/default-config.yaml +++ b/core/src/main/java/google/registry/config/files/default-config.yaml @@ -452,6 +452,8 @@ sslCertificateValidation: # The number of days before a certificate expires that indicates the # certificate is nearing expiration and warnings should be sent. expirationWarningDays: 30 + # The minimum number of days between two successive expiring notification emails. + expirationWarningIntervalDays: 15 # Text for expiring certificate notification email subject. expirationWarningEmailSubjectText: Certificate Expring Within 30 Days. # Text for expiring certificate notification email body that accepts 3 parameters: diff --git a/core/src/main/java/google/registry/flows/certs/CertificateChecker.java b/core/src/main/java/google/registry/flows/certs/CertificateChecker.java index 404ec1f3c..ab97f6565 100644 --- a/core/src/main/java/google/registry/flows/certs/CertificateChecker.java +++ b/core/src/main/java/google/registry/flows/certs/CertificateChecker.java @@ -24,6 +24,7 @@ import google.registry.config.RegistryConfig.Config; import google.registry.util.Clock; import google.registry.util.DateTimeUtils; import java.io.ByteArrayInputStream; +import java.io.StringWriter; import java.security.PublicKey; import java.security.cert.CertificateException; import java.security.cert.CertificateFactory; @@ -32,10 +33,15 @@ import java.security.interfaces.ECPublicKey; import java.security.interfaces.RSAPublicKey; import java.util.Date; import java.util.stream.Collectors; +import javax.annotation.Nullable; import javax.inject.Inject; import org.bouncycastle.jcajce.provider.asymmetric.util.EC5Util; import org.bouncycastle.jce.ECNamedCurveTable; import org.bouncycastle.jce.spec.ECNamedCurveParameterSpec; +import org.bouncycastle.jce.spec.ECParameterSpec; +import org.bouncycastle.openssl.jcajce.JcaMiscPEMGenerator; +import org.bouncycastle.util.io.pem.PemObjectGenerator; +import org.bouncycastle.util.io.pem.PemWriter; import org.joda.time.DateTime; import org.joda.time.Days; @@ -43,10 +49,11 @@ import org.joda.time.Days; public class CertificateChecker { private final ImmutableSortedMap maxValidityLengthSchedule; - private final int daysToExpiration; + private final int expirationWarningDays; private final int minimumRsaKeyLength; private final Clock clock; private final ImmutableSet allowedEcdsaCurves; + private final int expirationWarningIntervalDays; /** * Constructs a CertificateChecker instance with the specified configuration parameters. @@ -72,6 +79,7 @@ public class CertificateChecker { @Config("maxValidityDaysSchedule") ImmutableSortedMap maxValidityDaysSchedule, @Config("expirationWarningDays") int expirationWarningDays, + @Config("expirationWarningIntervalDays") int expirationWarningIntervalDays, @Config("minimumRsaKeyLength") int minimumRsaKeyLength, @Config("allowedEcdsaCurves") ImmutableSet allowedEcdsaCurves, Clock clock) { @@ -79,12 +87,46 @@ public class CertificateChecker { maxValidityDaysSchedule.containsKey(START_OF_TIME), "Max validity length schedule must contain an entry for START_OF_TIME"); this.maxValidityLengthSchedule = maxValidityDaysSchedule; - this.daysToExpiration = expirationWarningDays; + this.expirationWarningDays = expirationWarningDays; this.minimumRsaKeyLength = minimumRsaKeyLength; this.allowedEcdsaCurves = allowedEcdsaCurves; + this.expirationWarningIntervalDays = expirationWarningIntervalDays; this.clock = clock; } + private static int getValidityLengthInDays(X509Certificate certificate) { + DateTime start = DateTime.parse(certificate.getNotBefore().toInstant().toString()); + DateTime end = DateTime.parse(certificate.getNotAfter().toInstant().toString()); + return Days.daysBetween(start.withTimeAtStartOfDay(), end.withTimeAtStartOfDay()).getDays(); + } + + /** Checks if the curve used for a public key is in the list of acceptable curves. */ + private static boolean checkCurveName(PublicKey key, ImmutableSet allowedEcdsaCurves) { + ECParameterSpec params; + // These 2 different instances of PublicKey need to be handled separately since their OIDs are + // encoded differently. More details on this can be found at + // https://stackoverflow.com/questions/49895713/how-to-find-the-matching-curve-name-from-an-ecpublickey. + if (key instanceof ECPublicKey) { + ECPublicKey ecKey = (ECPublicKey) key; + params = EC5Util.convertSpec(ecKey.getParams(), false); + } else if (key instanceof org.bouncycastle.jce.interfaces.ECPublicKey) { + org.bouncycastle.jce.interfaces.ECPublicKey ecKey = + (org.bouncycastle.jce.interfaces.ECPublicKey) key; + params = ecKey.getParameters(); + } else { + throw new IllegalArgumentException("Unrecognized instance of PublicKey."); + } + return allowedEcdsaCurves.stream() + .anyMatch( + curve -> { + ECNamedCurveParameterSpec cParams = ECNamedCurveTable.getParameterSpec(curve); + return cParams.getN().equals(params.getN()) + && cParams.getH().equals(params.getH()) + && cParams.getCurve().equals(params.getCurve()) + && cParams.getG().equals(params.getG()); + }); + } + /** * Checks the given certificate string for violations and throws an exception if any violations * exist. @@ -156,18 +198,7 @@ public class CertificateChecker { * the violations the certificate has. */ public ImmutableSet checkCertificate(String certificateString) { - X509Certificate certificate; - - try { - certificate = - (X509Certificate) - CertificateFactory.getInstance("X509") - .generateCertificate(new ByteArrayInputStream(certificateString.getBytes(UTF_8))); - } catch (CertificateException e) { - throw new IllegalArgumentException("Unable to read given certificate."); - } - - return checkCertificate(certificate); + return checkCertificate(getCertificate(certificateString)); } /** @@ -181,42 +212,59 @@ public class CertificateChecker { public boolean isNearingExpiration(X509Certificate certificate) { Date nearingExpirationDate = DateTime.parse(certificate.getNotAfter().toInstant().toString()) - .minusDays(daysToExpiration) + .minusDays(expirationWarningDays) .toDate(); return clock.nowUtc().toDate().after(nearingExpirationDate); } - private static int getValidityLengthInDays(X509Certificate certificate) { - DateTime start = DateTime.parse(certificate.getNotBefore().toInstant().toString()); - DateTime end = DateTime.parse(certificate.getNotAfter().toInstant().toString()); - return Days.daysBetween(start.withTimeAtStartOfDay(), end.withTimeAtStartOfDay()).getDays(); + /** Converts the given string to a certificate object. */ + public X509Certificate getCertificate(String certificateStr) { + X509Certificate certificate; + try { + certificate = + (X509Certificate) + CertificateFactory.getInstance("X509") + .generateCertificate(new ByteArrayInputStream(certificateStr.getBytes(UTF_8))); + } catch (CertificateException e) { + throw new IllegalArgumentException( + String.format("Unable to read given certificate %s", certificateStr), e); + } + return certificate; } - /** Checks if the curve used for a public key is in the list of acceptable curves. */ - private static boolean checkCurveName(PublicKey key, ImmutableSet allowedEcdsaCurves) { - org.bouncycastle.jce.spec.ECParameterSpec params; - // These 2 different instances of PublicKey need to be handled separately since their OIDs are - // encoded differently. More details on this can be found at - // https://stackoverflow.com/questions/49895713/how-to-find-the-matching-curve-name-from-an-ecpublickey. - if (key instanceof ECPublicKey) { - ECPublicKey ecKey = (ECPublicKey) key; - params = EC5Util.convertSpec(ecKey.getParams(), false); - } else if (key instanceof org.bouncycastle.jce.interfaces.ECPublicKey) { - org.bouncycastle.jce.interfaces.ECPublicKey ecKey = - (org.bouncycastle.jce.interfaces.ECPublicKey) key; - params = ecKey.getParameters(); - } else { - throw new IllegalArgumentException("Unrecognized instance of PublicKey."); + /** Serializes the certificate object to a certificate string. */ + public String serializeCertificate(X509Certificate certificate) throws Exception { + StringWriter sw = new StringWriter(); + try (PemWriter pw = new PemWriter(sw)) { + PemObjectGenerator generator = new JcaMiscPEMGenerator(certificate); + pw.writeObject(generator); } - return allowedEcdsaCurves.stream() - .anyMatch( - curve -> { - ECNamedCurveParameterSpec cParams = ECNamedCurveTable.getParameterSpec(curve); - return cParams.getN().equals(params.getN()) - && cParams.getH().equals(params.getH()) - && cParams.getCurve().equals(params.getCurve()) - && cParams.getG().equals(params.getG()); - }); + return sw.toString(); + } + + /** Returns whether the client should receive a notification email. */ + public boolean shouldReceiveExpiringNotification( + @Nullable DateTime lastExpiringNotificationSentDate, String certificateStr) { + X509Certificate certificate = getCertificate(certificateStr); + DateTime now = clock.nowUtc(); + // expiration date is one day after lastValidDate + Date lastValidDate = certificate.getNotAfter(); + if (lastValidDate.before(now.toDate())) { + return false; + } + /* + * Client should receive a notification if : + * 1) client has never received notification and the certificate has entered + * the expiring period, OR + * 2) client has received notification but the interval between now and + * lastExpiringNotificationSentDate is greater than expirationWarningIntervalDays. + */ + return !lastValidDate.after(now.plusDays(expirationWarningDays).toDate()) + && (lastExpiringNotificationSentDate == null + || !lastExpiringNotificationSentDate + .plusDays(expirationWarningIntervalDays) + .toDate() + .after(now.toDate())); } private String getViolationDisplayMessage(CertificateViolation certificateViolation) { diff --git a/core/src/test/java/google/registry/flows/EppLoginTlsTest.java b/core/src/test/java/google/registry/flows/EppLoginTlsTest.java index 68686a682..bedb6bb92 100644 --- a/core/src/test/java/google/registry/flows/EppLoginTlsTest.java +++ b/core/src/test/java/google/registry/flows/EppLoginTlsTest.java @@ -55,6 +55,7 @@ class EppLoginTlsTest extends EppTestCase { new CertificateChecker( ImmutableSortedMap.of(START_OF_TIME, 825, DateTime.parse("2020-09-01T00:00:00Z"), 398), 30, + 15, 2048, ImmutableSet.of("secp256r1", "secp384r1"), clock); diff --git a/core/src/test/java/google/registry/flows/FlowRunnerTest.java b/core/src/test/java/google/registry/flows/FlowRunnerTest.java index 1d6bd9e1b..39fc503b9 100644 --- a/core/src/test/java/google/registry/flows/FlowRunnerTest.java +++ b/core/src/test/java/google/registry/flows/FlowRunnerTest.java @@ -64,6 +64,7 @@ class FlowRunnerTest { new CertificateChecker( ImmutableSortedMap.of(START_OF_TIME, 825, DateTime.parse("2020-09-01T00:00:00Z"), 398), 30, + 15, 2048, ImmutableSet.of("secp256r1", "secp384r1"), clock); diff --git a/core/src/test/java/google/registry/flows/TlsCredentialsTest.java b/core/src/test/java/google/registry/flows/TlsCredentialsTest.java index 80689f500..b09e0fb51 100644 --- a/core/src/test/java/google/registry/flows/TlsCredentialsTest.java +++ b/core/src/test/java/google/registry/flows/TlsCredentialsTest.java @@ -53,6 +53,7 @@ final class TlsCredentialsTest { new CertificateChecker( ImmutableSortedMap.of(START_OF_TIME, 825, DateTime.parse("2020-09-01T00:00:00Z"), 398), 30, + 15, 2048, ImmutableSet.of("secp256r1", "secp384r1"), clock); diff --git a/core/src/test/java/google/registry/flows/certs/CertificateCheckerTest.java b/core/src/test/java/google/registry/flows/certs/CertificateCheckerTest.java index 029c0cbb5..299778355 100644 --- a/core/src/test/java/google/registry/flows/certs/CertificateCheckerTest.java +++ b/core/src/test/java/google/registry/flows/certs/CertificateCheckerTest.java @@ -50,6 +50,7 @@ class CertificateCheckerTest { new CertificateChecker( ImmutableSortedMap.of(START_OF_TIME, 825, DateTime.parse("2020-09-01T00:00:00Z"), 398), 30, + 15, 2048, ImmutableSet.of("secp256r1", "secp384r1"), fakeClock); @@ -214,7 +215,7 @@ class CertificateCheckerTest { assertThrows( IllegalArgumentException.class, () -> certificateChecker.checkCertificate("bad certificate string")); - assertThat(thrown).hasMessageThat().isEqualTo("Unable to read given certificate."); + assertThat(thrown).hasMessageThat().contains("Unable to read given certificate"); } @Test @@ -241,6 +242,169 @@ class CertificateCheckerTest { assertThat(certificateChecker.isNearingExpiration(certificate)).isFalse(); } + @Test + void test_getCertificate_throwsException_invalidCertificateString() { + IllegalArgumentException thrown = + assertThrows( + IllegalArgumentException.class, + () -> certificateChecker.getCertificate("invalidCertificateString")); + assertThat(thrown).hasMessageThat().contains("Unable to read given certificate"); + } + + @Test + void test_getCertificate_returnsCertificateObject() throws Exception { + X509Certificate certificate = + SelfSignedCaCertificate.create( + SSL_HOST, + DateTime.parse("2020-09-02T00:00:00Z"), + DateTime.parse("2021-10-01T00:00:00Z")) + .cert(); + String certificateStr = certificateChecker.serializeCertificate(certificate); + assertThat(certificateChecker.getCertificate(certificateStr).equals(certificate)).isTrue(); + } + + @Test + void test_serializeCertificate_returnsCertificateString() throws Exception { + X509Certificate certificate = + SelfSignedCaCertificate.create( + SSL_HOST, + DateTime.parse("2020-09-02T00:00:00Z"), + DateTime.parse("2021-10-01T00:00:00Z")) + .cert(); + assertThat( + certificateChecker.getCertificate(certificateChecker.serializeCertificate(certificate))) + .isEqualTo(certificate); + } + + @Test + void test_shouldReceiveExpiringNotification_returnsFalse_greaterThan30() throws Exception { + fakeClock.setTo(DateTime.parse("2021-07-20T00:00:00Z")); + X509Certificate certificate = + SelfSignedCaCertificate.create( + SSL_HOST, + DateTime.parse("2020-09-02T00:00:00Z"), + DateTime.parse("2021-10-01T00:00:00Z")) + .cert(); + String certificateStr = certificateChecker.serializeCertificate(certificate); + assertThat(certificateChecker.shouldReceiveExpiringNotification(null, certificateStr)) + .isFalse(); + } + + @Test + void test_shouldReceiveExpiringNotification_returnsFalse_hasExpired() throws Exception { + fakeClock.setTo(DateTime.parse("2021-10-20T00:00:00Z")); + X509Certificate certificate = + SelfSignedCaCertificate.create( + SSL_HOST, + DateTime.parse("2020-09-02T00:00:00Z"), + DateTime.parse("2021-10-01T00:00:00Z")) + .cert(); + String certificateStr = certificateChecker.serializeCertificate(certificate); + assertThat(certificateChecker.shouldReceiveExpiringNotification(null, certificateStr)) + .isFalse(); + } + + @Test + void test_shouldReceiveExpiringNotification_returnsTrue_on15days() throws Exception { + fakeClock.setTo(DateTime.parse("2021-08-16T00:00:00Z")); + X509Certificate certificate = + SelfSignedCaCertificate.create( + SSL_HOST, + DateTime.parse("2020-09-02T00:00:00Z"), + DateTime.parse("2021-08-30T00:00:00Z")) + .cert(); + String certificateStr = certificateChecker.serializeCertificate(certificate); + DateTime lastExpiringNotificationSentDate = DateTime.parse("2021-08-01T00:00:00Z"); + assertThat( + certificateChecker.shouldReceiveExpiringNotification( + lastExpiringNotificationSentDate, certificateStr)) + .isTrue(); + } + + @Test + void test_shouldReceiveExpiringNotification_returnsTrue_on30days() throws Exception { + fakeClock.setTo(DateTime.parse("2021-01-01T00:00:00Z")); + X509Certificate certificate = + SelfSignedCaCertificate.create( + SSL_HOST, + DateTime.parse("2020-01-02T00:00:00Z"), + DateTime.parse("2021-01-30T00:00:00Z")) + .cert(); + String certificateStr = certificateChecker.serializeCertificate(certificate); + assertThat(certificateChecker.shouldReceiveExpiringNotification(null, certificateStr)).isTrue(); + } + + @Test + void test_shouldReceiveExpiringNotification_returnsFalse_between30and15_lastSentDateIsNotNull() + throws Exception { + fakeClock.setTo(DateTime.parse("2021-07-05T00:00:00Z")); + X509Certificate certificate = + SelfSignedCaCertificate.create( + SSL_HOST, + DateTime.parse("2020-06-02T00:00:00Z"), + DateTime.parse("2021-07-25T00:00:00Z")) + .cert(); + String certificateStr = certificateChecker.serializeCertificate(certificate); + DateTime lastExpiringNotificationSentDate = DateTime.parse("2021-07-04T00:00:00Z"); + assertThat( + certificateChecker.shouldReceiveExpiringNotification( + lastExpiringNotificationSentDate, certificateStr)) + .isFalse(); + } + + @Test + void test_shouldReceiveExpiringNotification_returnsTrue_between30and15_lastSentDateIsNull() + throws Exception { + fakeClock.setTo(DateTime.parse("2021-07-05T00:00:00Z")); + X509Certificate certificate = + SelfSignedCaCertificate.create( + SSL_HOST, + DateTime.parse("2020-06-02T00:00:00Z"), + DateTime.parse("2021-07-25T00:00:00Z")) + .cert(); + String certificateStr = certificateChecker.serializeCertificate(certificate); + assertThat(certificateChecker.shouldReceiveExpiringNotification(null, certificateStr)).isTrue(); + } + + @Test + void + test_shouldReceiveExpiringNotification_returnsFalse_between15and0_lastSentDateBetween30and15_butLate() + throws Exception { + fakeClock.setTo(DateTime.parse("2021-07-05T00:00:00Z")); + X509Certificate certificate = + SelfSignedCaCertificate.create( + SSL_HOST, + DateTime.parse("2020-06-02T00:00:00Z"), + DateTime.parse("2021-07-18T00:00:00Z")) + .cert(); + String certificateStr = certificateChecker.serializeCertificate(certificate); + DateTime lastExpiringNotificationSentDate = DateTime.parse("2021-07-01T00:00:00Z"); + // The first notification date is late and difference between now and + // lastExpiringNotificationSentDate is within expirationWarningIntervalDays. + assertThat( + certificateChecker.shouldReceiveExpiringNotification( + lastExpiringNotificationSentDate, certificateStr)) + .isFalse(); + } + + @Test + void test_shouldReceiveExpiringNotification_returnsTrue_between15and0_lastSentDateBetween30and15() + throws Exception { + fakeClock.setTo(DateTime.parse("2021-07-05T00:00:00Z")); + X509Certificate certificate = + SelfSignedCaCertificate.create( + SSL_HOST, + DateTime.parse("2020-06-02T00:00:00Z"), + DateTime.parse("2021-07-18T00:00:00Z")) + .cert(); + String certificateStr = certificateChecker.serializeCertificate(certificate); + DateTime lastExpiringNotificationSentDate = DateTime.parse("2021-06-20T00:00:00Z"); + assertThat( + certificateChecker.shouldReceiveExpiringNotification( + lastExpiringNotificationSentDate, certificateStr)) + .isTrue(); + } + @Test void test_CertificateViolation_RsaKeyLengthDisplayMessageFormatsCorrectly() { assertThat(RSA_KEY_LENGTH_TOO_SHORT.getDisplayMessage(certificateChecker)) diff --git a/core/src/test/java/google/registry/flows/session/LoginFlowViaTlsTest.java b/core/src/test/java/google/registry/flows/session/LoginFlowViaTlsTest.java index 26fa84b28..532f2e5ca 100644 --- a/core/src/test/java/google/registry/flows/session/LoginFlowViaTlsTest.java +++ b/core/src/test/java/google/registry/flows/session/LoginFlowViaTlsTest.java @@ -50,6 +50,7 @@ public class LoginFlowViaTlsTest extends LoginFlowTestCase { new CertificateChecker( ImmutableSortedMap.of(START_OF_TIME, 825, DateTime.parse("2020-09-01T00:00:00Z"), 398), 30, + 15, 2048, ImmutableSet.of("secp256r1", "secp384r1"), clock); diff --git a/core/src/test/java/google/registry/tools/CreateRegistrarCommandTest.java b/core/src/test/java/google/registry/tools/CreateRegistrarCommandTest.java index ad2c08ded..9ff678d45 100644 --- a/core/src/test/java/google/registry/tools/CreateRegistrarCommandTest.java +++ b/core/src/test/java/google/registry/tools/CreateRegistrarCommandTest.java @@ -67,6 +67,7 @@ class CreateRegistrarCommandTest extends CommandTestCase new CertificateChecker( ImmutableSortedMap.of(START_OF_TIME, 825, DateTime.parse("2020-09-01T00:00:00Z"), 398), 30, + 15, 2048, ImmutableSet.of("secp256r1", "secp384r1"), fakeClock); diff --git a/core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java b/core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java index d09da3866..971703843 100644 --- a/core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java +++ b/core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java @@ -53,6 +53,7 @@ class UpdateRegistrarCommandTest extends CommandTestCase new CertificateChecker( ImmutableSortedMap.of(START_OF_TIME, 825, DateTime.parse("2020-09-01T00:00:00Z"), 398), 30, + 15, 2048, ImmutableSet.of("secp256r1", "secp384r1"), fakeClock); diff --git a/core/src/test/java/google/registry/tools/ValidateLoginCredentialsCommandTest.java b/core/src/test/java/google/registry/tools/ValidateLoginCredentialsCommandTest.java index 921aeb5dc..2f4308a50 100644 --- a/core/src/test/java/google/registry/tools/ValidateLoginCredentialsCommandTest.java +++ b/core/src/test/java/google/registry/tools/ValidateLoginCredentialsCommandTest.java @@ -65,6 +65,7 @@ class ValidateLoginCredentialsCommandTest extends CommandTestCase