From 8aa48af2ff5414bd804c4e07bb59134bec67c345 Mon Sep 17 00:00:00 2001 From: Lai Jiang Date: Thu, 14 Mar 2024 14:05:03 -0400 Subject: [PATCH] Remove java.util.Date There is one remaining instance in JpaTransactionManagerImpl that cannot be removed because DetachingTypedQuery is implementing TypedQuery, which has a method that expectred java.util.Date. --- config/presubmits.py | 11 +++- .../registry/config/DelegatedCredentials.java | 9 +-- .../flows/certs/CertificateChecker.java | 56 ++++++++----------- .../tmch/TmchCertificateAuthority.java | 4 +- ...eateOrUpdateBulkPricingPackageCommand.java | 10 ++-- .../registry/module/RequestComponentTest.java | 2 +- .../registry/testing/CloudTasksHelper.java | 3 +- .../handler/SslClientInitializerTest.java | 39 +++++-------- .../handler/SslInitializerTestUtils.java | 25 ++++----- .../handler/SslServerInitializerTest.java | 34 ++++------- .../util/SelfSignedCaCertificate.java | 31 ++++------ .../java/google/registry/util/X509Utils.java | 16 +++--- 12 files changed, 98 insertions(+), 142 deletions(-) diff --git a/config/presubmits.py b/config/presubmits.py index def50fda71..0d90145906 100644 --- a/config/presubmits.py +++ b/config/presubmits.py @@ -173,17 +173,24 @@ def fails(self, file): ): "JavaScript files should not include console logging.", PresubmitCheck( - r"org\.testcontainers\.shaded\.", + r".*org\.testcontainers\.shaded\..*", "java", {"/node_modules/"}, ): "Do not use shaded dependencies from testcontainers.", PresubmitCheck( - r"com\.google\.common\.truth\.Truth8", + r".*com\.google\.common\.truth\.Truth8.*", "java", {"/node_modules/"}, ): "Truth8 is deprecated. Use Truth instead.", + PresubmitCheck( + r".*java\.util\.Date.*", + "java", + {"/node_modules/", "JpaTransactionManagerImpl.java"}, + ): + "Do not use java.util.Date. Use classes in Java.date package instead.", + } # Note that this regex only works for one kind of Flyway file. If we want to diff --git a/core/src/main/java/google/registry/config/DelegatedCredentials.java b/core/src/main/java/google/registry/config/DelegatedCredentials.java index d1a05d9e0f..7853fdb59d 100644 --- a/core/src/main/java/google/registry/config/DelegatedCredentials.java +++ b/core/src/main/java/google/registry/config/DelegatedCredentials.java @@ -45,7 +45,6 @@ import java.math.BigDecimal; import java.time.Duration; import java.util.Collection; -import java.util.Date; import java.util.Map; import java.util.ServiceLoader; import org.apache.commons.codec.binary.Base64; @@ -195,10 +194,7 @@ public AccessToken refreshAccessToken() throws IOException { GenericData responseData = response.parseAs(GenericData.class); String accessToken = validateString(responseData, "access_token", PARSE_ERROR_PREFIX); int expiresInSeconds = validateInt32(responseData, "expires_in", PARSE_ERROR_PREFIX); - long expiresAtMilliseconds = clock.nowUtc().getMillis() + expiresInSeconds * 1000L; - @SuppressWarnings("JavaUtilDate") - AccessToken token = new AccessToken(accessToken, new Date(expiresAtMilliseconds)); - return token; + return new AccessToken(accessToken, clock.nowUtc().plusSeconds(expiresInSeconds).toDate()); } String createAssertion(JsonFactory jsonFactory, long currentTime) throws IOException { @@ -257,8 +253,7 @@ static int validateInt32(Map map, String key, String errorPrefix if (value == null) { throw new IOException(String.format(VALUE_NOT_FOUND_MESSAGE, errorPrefix, key)); } - if (value instanceof BigDecimal) { - BigDecimal bigDecimalValue = (BigDecimal) value; + if (value instanceof BigDecimal bigDecimalValue) { return bigDecimalValue.intValueExact(); } if (!(value instanceof Integer)) { 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 81dda2971f..6e548d132a 100644 --- a/core/src/main/java/google/registry/flows/certs/CertificateChecker.java +++ b/core/src/main/java/google/registry/flows/certs/CertificateChecker.java @@ -31,7 +31,6 @@ import java.security.cert.X509Certificate; import java.security.interfaces.ECPublicKey; import java.security.interfaces.RSAPublicKey; -import java.util.Date; import java.util.stream.Collectors; import javax.inject.Inject; import org.bouncycastle.jcajce.provider.asymmetric.util.EC5Util; @@ -106,12 +105,9 @@ private static boolean checkCurveName(PublicKey key, ImmutableSet allowe // 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; + if (key instanceof ECPublicKey ecKey) { params = EC5Util.convertSpec(ecKey.getParams()); - } else if (key instanceof org.bouncycastle.jce.interfaces.ECPublicKey) { - org.bouncycastle.jce.interfaces.ECPublicKey ecKey = - (org.bouncycastle.jce.interfaces.ECPublicKey) key; + } else if (key instanceof org.bouncycastle.jce.interfaces.ECPublicKey ecKey) { params = ecKey.getParameters(); } else { throw new IllegalArgumentException("Unrecognized instance of PublicKey."); @@ -148,7 +144,7 @@ private void handleCertViolations(ImmutableSet violations) if (!violations.isEmpty()) { String displayMessages = violations.stream() - .map(violation -> getViolationDisplayMessage(violation)) + .map(this::getViolationDisplayMessage) .collect(Collectors.joining("\n")); throw new InsecureCertificateException(violations, displayMessages); } @@ -162,7 +158,7 @@ public ImmutableSet checkCertificate(X509Certificate certi ImmutableSet.Builder violations = new ImmutableSet.Builder<>(); // Check if currently in validity period - Date now = clock.nowUtc().toDate(); + DateTime now = clock.nowUtc(); if (DateTimeComparator.getInstance().compare(certificate.getNotAfter(), now) < 0) { violations.add(CertificateViolation.EXPIRED); } else if (DateTimeComparator.getInstance().compare(certificate.getNotBefore(), now) > 0) { @@ -231,13 +227,13 @@ public boolean shouldReceiveExpiringNotification( DateTime lastExpiringNotificationSentDate, String certificateStr) { X509Certificate certificate = getCertificate(certificateStr); DateTime now = clock.nowUtc(); - // expiration date is one day after lastValidDate + // the expiration date is one day after lastValidDate DateTime lastValidDate = new DateTime(certificate.getNotAfter()); if (lastValidDate.isBefore(now)) { return false; } /* - * Client should receive a notification if : + * Client should receive a notification if: * 1) client has never received notification (lastExpiringNotificationSentDate is initially * set to START_OF_TIME) and the certificate has entered the expiring period, OR * 2) client has received notification but the interval between now and @@ -254,29 +250,21 @@ private String getViolationDisplayMessage(CertificateViolation certificateViolat // Yes, we'd rather do this as an instance method on the CertificateViolation enum itself, but // we can't because we need access to configuration (injected as instance variables) which you // can't get in a static enum context. - switch (certificateViolation) { - case EXPIRED: - return "Certificate is expired."; - case NOT_YET_VALID: - return "Certificate start date is in the future."; - case ALGORITHM_CONSTRAINED: - return "Certificate key algorithm must be RSA or ECDSA."; - case RSA_KEY_LENGTH_TOO_SHORT: - return String.format( - "RSA key length is too short; the minimum allowed length is %d bits.", - this.minimumRsaKeyLength); - case VALIDITY_LENGTH_TOO_LONG: - return String.format( - "Certificate validity period is too long; it must be less than or equal to %d days.", - this.maxValidityLengthSchedule.lastEntry().getValue()); - case INVALID_ECDSA_CURVE: - return String.format( - "The ECDSA key must use one of these algorithms: %s", allowedEcdsaCurves); - default: - throw new IllegalArgumentException( - String.format( - "Unknown CertificateViolation enum value: %s", certificateViolation.name())); - } + return switch (certificateViolation) { + case EXPIRED -> "Certificate is expired."; + case NOT_YET_VALID -> "Certificate start date is in the future."; + case ALGORITHM_CONSTRAINED -> "Certificate key algorithm must be RSA or ECDSA."; + case RSA_KEY_LENGTH_TOO_SHORT -> + String.format( + "RSA key length is too short; the minimum allowed length is %d bits.", + this.minimumRsaKeyLength); + case VALIDITY_LENGTH_TOO_LONG -> + String.format( + "Certificate validity period is too long; it must be less than or equal to %d days.", + this.maxValidityLengthSchedule.lastEntry().getValue()); + case INVALID_ECDSA_CURVE -> + String.format("The ECDSA key must use one of these algorithms: %s", allowedEcdsaCurves); + }; } /** @@ -295,7 +283,7 @@ public enum CertificateViolation { * Gets a suitable end-user-facing display message for this particular certificate violation. * *

Note that the {@link CertificateChecker} instance must be passed in because it contains - * configuration values (e.g. minimum RSA key length) that go into the error message text. + * configuration values (e.g., minimum RSA key length) that go into the error message text. */ public String getDisplayMessage(CertificateChecker certificateChecker) { return certificateChecker.getViolationDisplayMessage(this); diff --git a/core/src/main/java/google/registry/tmch/TmchCertificateAuthority.java b/core/src/main/java/google/registry/tmch/TmchCertificateAuthority.java index 0fbcff2508..8c0c52812d 100644 --- a/core/src/main/java/google/registry/tmch/TmchCertificateAuthority.java +++ b/core/src/main/java/google/registry/tmch/TmchCertificateAuthority.java @@ -127,7 +127,7 @@ private static ImmutableMap loadRootCertificates() */ public void verify(X509Certificate cert) throws GeneralSecurityException { synchronized (TmchCertificateAuthority.class) { - X509Utils.verifyCertificate(getAndValidateRoot(), getCrl(), cert, clock.nowUtc().toDate()); + X509Utils.verifyCertificate(getAndValidateRoot(), getCrl(), cert, clock.nowUtc()); } } @@ -151,7 +151,7 @@ public void updateCrl(String asciiCrl, String url) throws GeneralSecurityExcepti } catch (Exception e) { logger.atWarning().withCause(e).log("Old CRL is invalid, ignored during CRL update."); } - X509Utils.verifyCrl(getAndValidateRoot(), oldCrl, newCrl, clock.nowUtc().toDate()); + X509Utils.verifyCrl(getAndValidateRoot(), oldCrl, newCrl, clock.nowUtc()); TmchCrl.set(asciiCrl, url); } diff --git a/core/src/main/java/google/registry/tools/CreateOrUpdateBulkPricingPackageCommand.java b/core/src/main/java/google/registry/tools/CreateOrUpdateBulkPricingPackageCommand.java index 3fdd65e123..d397f7cfb9 100644 --- a/core/src/main/java/google/registry/tools/CreateOrUpdateBulkPricingPackageCommand.java +++ b/core/src/main/java/google/registry/tools/CreateOrUpdateBulkPricingPackageCommand.java @@ -22,7 +22,7 @@ import google.registry.model.domain.token.AllocationToken.TokenType; import google.registry.model.domain.token.BulkPricingPackage; import google.registry.persistence.VKey; -import java.util.Date; +import google.registry.tools.params.DateTimeParameter; import java.util.List; import java.util.Optional; import javax.annotation.Nullable; @@ -56,9 +56,11 @@ abstract class CreateOrUpdateBulkPricingPackageCommand extends MutatingCommand { @Nullable @Parameter( names = "--next_billing_date", + converter = DateTimeParameter.class, + validateWith = DateTimeParameter.class, description = "The next date that the bulk pricing package should be billed for its annual fee") - Date nextBillingDate; + DateTime nextBillingDate; /** Returns the existing BulkPricingPackage or null if it does not exist. */ @Nullable @@ -105,9 +107,7 @@ protected final void init() throws Exception { Optional.ofNullable(maxCreates).ifPresent(builder::setMaxCreates); Optional.ofNullable(price).ifPresent(builder::setBulkPrice); Optional.ofNullable(nextBillingDate) - .ifPresent( - nextBillingDate -> - builder.setNextBillingDate(new DateTime(nextBillingDate))); + .ifPresent(nextBillingDate -> builder.setNextBillingDate(nextBillingDate)); if (clearLastNotificationSent()) { builder.setLastNotificationSent(null); } diff --git a/core/src/test/java/google/registry/module/RequestComponentTest.java b/core/src/test/java/google/registry/module/RequestComponentTest.java index 89d313dec9..99ab6e7359 100644 --- a/core/src/test/java/google/registry/module/RequestComponentTest.java +++ b/core/src/test/java/google/registry/module/RequestComponentTest.java @@ -18,6 +18,7 @@ import com.google.common.base.Splitter; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import google.registry.module.backend.BackendRequestComponent; import google.registry.module.bsa.BsaRequestComponent; import google.registry.module.frontend.FrontendRequestComponent; @@ -28,7 +29,6 @@ import java.util.ArrayList; import java.util.List; import org.junit.jupiter.api.Test; -import org.testcontainers.shaded.com.google.common.collect.ImmutableMap; /** Unit tests for {@link RequestComponent}. */ public class RequestComponentTest { diff --git a/core/src/test/java/google/registry/testing/CloudTasksHelper.java b/core/src/test/java/google/registry/testing/CloudTasksHelper.java index 51c05dd0ac..839acd1b72 100644 --- a/core/src/test/java/google/registry/testing/CloudTasksHelper.java +++ b/core/src/test/java/google/registry/testing/CloudTasksHelper.java @@ -40,7 +40,6 @@ import com.google.common.collect.Multimaps; import com.google.common.net.HttpHeaders; import com.google.common.net.MediaType; -import com.google.common.truth.Truth8; import com.google.protobuf.Timestamp; import com.google.protobuf.util.Timestamps; import dagger.Module; @@ -133,7 +132,7 @@ public List getTestTasksFor(String queue) { public void assertTasksEnqueuedWithProperty( String queueName, Function propertyGetter, String... expectedTaskProperties) { // Ordering is irrelevant but duplicates should be considered independently. - Truth8.assertThat(getTestTasksFor(queueName).stream().map(propertyGetter)) + assertThat(getTestTasksFor(queueName).stream().map(propertyGetter)) .containsExactly((Object[]) expectedTaskProperties); } diff --git a/networking/src/test/java/google/registry/networking/handler/SslClientInitializerTest.java b/networking/src/test/java/google/registry/networking/handler/SslClientInitializerTest.java index 983b18349e..90962e1c39 100644 --- a/networking/src/test/java/google/registry/networking/handler/SslClientInitializerTest.java +++ b/networking/src/test/java/google/registry/networking/handler/SslClientInitializerTest.java @@ -43,12 +43,10 @@ import java.security.cert.CertificateExpiredException; import java.security.cert.CertificateNotYetValidException; import java.security.cert.X509Certificate; -import java.time.Duration; -import java.time.Instant; -import java.util.Date; import java.util.stream.Stream; import javax.net.ssl.SSLException; import javax.net.ssl.SSLSession; +import org.joda.time.DateTime; import org.junit.jupiter.api.extension.RegisterExtension; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; @@ -62,7 +60,7 @@ * the overhead of routing traffic through the network layer, even if it were to go through * loopback. It also alleviates the need to pick a free port to use. * - *

The local addresses used in each test method must to be different, otherwise tests run in + *

The local addresses used in each test method must be different, otherwise tests run in * parallel may interfere with each other. */ class SslClientInitializerTest { @@ -204,7 +202,7 @@ void testSuccess_customTrustManager_acceptCertSignedByTrustedCa(SslProvider sslP // Generate a new key pair. KeyPair keyPair = getKeyPair(); - // Generate a self signed certificate, and use it to sign the key pair. + // Generate a self-signed certificate, and use it to sign the key pair. SelfSignedCaCertificate ssc = SelfSignedCaCertificate.create(); X509Certificate cert = signKeyPair(ssc, keyPair, SSL_HOST); @@ -212,7 +210,7 @@ void testSuccess_customTrustManager_acceptCertSignedByTrustedCa(SslProvider sslP PrivateKey privateKey = keyPair.getPrivate(); nettyExtension.setUpServer(localAddress, getServerHandler(false, privateKey, cert)); - // Set up the client to trust the self signed cert used to sign the cert that server provides. + // Set up the client to trust the self-signed cert used to sign the cert that server provides. SslClientInitializer sslClientInitializer = new SslClientInitializer<>( sslProvider, @@ -239,21 +237,17 @@ void testFailure_customTrustManager_serverCertExpired(SslProvider sslProvider) t // Generate a new key pair. KeyPair keyPair = getKeyPair(); - // Generate a self signed certificate, and use it to sign the key pair. + // Generate a self-signed certificate, and use it to sign the key pair. SelfSignedCaCertificate ssc = SelfSignedCaCertificate.create(); X509Certificate cert = signKeyPair( - ssc, - keyPair, - SSL_HOST, - Date.from(Instant.now().minus(Duration.ofDays(2))), - Date.from(Instant.now().minus(Duration.ofDays(1)))); + ssc, keyPair, SSL_HOST, DateTime.now().minusDays(2), DateTime.now().minusDays(1)); // Set up the server to use the signed cert and private key to perform handshake; PrivateKey privateKey = keyPair.getPrivate(); nettyExtension.setUpServer(localAddress, getServerHandler(false, privateKey, cert)); - // Set up the client to trust the self signed cert used to sign the cert that server provides. + // Set up the client to trust the self-signed cert used to sign the cert that server provides. SslClientInitializer sslClientInitializer = new SslClientInitializer<>( sslProvider, @@ -280,21 +274,16 @@ void testFailure_customTrustManager_serverCertNotYetValid(SslProvider sslProvide // Generate a new key pair. KeyPair keyPair = getKeyPair(); - // Generate a self signed certificate, and use it to sign the key pair. + // Generate a self-signed certificate, and use it to sign the key pair. SelfSignedCaCertificate ssc = SelfSignedCaCertificate.create(); X509Certificate cert = - signKeyPair( - ssc, - keyPair, - SSL_HOST, - Date.from(Instant.now().plus(Duration.ofDays(1))), - Date.from(Instant.now().plus(Duration.ofDays(2)))); + signKeyPair(ssc, keyPair, SSL_HOST, DateTime.now().plusDays(1), DateTime.now().plusDays(2)); // Set up the server to use the signed cert and private key to perform handshake; PrivateKey privateKey = keyPair.getPrivate(); nettyExtension.setUpServer(localAddress, getServerHandler(false, privateKey, cert)); - // Set up the client to trust the self signed cert used to sign the cert that server provides. + // Set up the client to trust the self-signed cert used to sign the cert that server provides. SslClientInitializer sslClientInitializer = new SslClientInitializer<>( sslProvider, @@ -333,7 +322,7 @@ void testSuccess_customTrustManager_acceptSelfSignedCert_clientCertRequired( SslClientInitializerTest::hostProvider, SslClientInitializerTest::portProvider, ImmutableList.of(serverSsc.cert()), - () -> clientSsc.key(), + clientSsc::key, () -> ImmutableList.of(clientSsc.cert())); nettyExtension.setUpClient(localAddress, sslClientInitializer); @@ -360,7 +349,7 @@ void testFailure_customTrustManager_wrongHostnameInCertificate(SslProvider sslPr // Generate a new key pair. KeyPair keyPair = getKeyPair(); - // Generate a self signed certificate, and use it to sign the key pair. + // Generate a self-signed certificate, and use it to sign the key pair. SelfSignedCaCertificate ssc = SelfSignedCaCertificate.create(); X509Certificate cert = signKeyPair(ssc, keyPair, "wrong.com"); @@ -368,7 +357,7 @@ void testFailure_customTrustManager_wrongHostnameInCertificate(SslProvider sslPr PrivateKey privateKey = keyPair.getPrivate(); nettyExtension.setUpServer(localAddress, getServerHandler(false, privateKey, cert)); - // Set up the client to trust the self signed cert used to sign the cert that server provides. + // Set up the client to trust the self-signed cert used to sign the cert that server provides. SslClientInitializer sslClientInitializer = new SslClientInitializer<>( sslProvider, @@ -379,7 +368,7 @@ void testFailure_customTrustManager_wrongHostnameInCertificate(SslProvider sslPr null); nettyExtension.setUpClient(localAddress, sslClientInitializer); - // When the client rejects the server cert due to wrong hostname, both the client and server + // When the client rejects the server cert due to the wrong hostname, both the client and server // should throw exceptions. nettyExtension.assertThatClientRootCause().isInstanceOf(CertificateException.class); nettyExtension.assertThatClientRootCause().hasMessageThat().contains(SSL_HOST); diff --git a/networking/src/test/java/google/registry/networking/handler/SslInitializerTestUtils.java b/networking/src/test/java/google/registry/networking/handler/SslInitializerTestUtils.java index c98df8cd4f..3f726f2916 100644 --- a/networking/src/test/java/google/registry/networking/handler/SslInitializerTestUtils.java +++ b/networking/src/test/java/google/registry/networking/handler/SslInitializerTestUtils.java @@ -27,9 +27,6 @@ import java.security.KeyPairGenerator; import java.security.SecureRandom; import java.security.cert.X509Certificate; -import java.time.Duration; -import java.time.Instant; -import java.util.Date; import java.util.concurrent.ExecutionException; import javax.net.ssl.SSLSession; import org.bouncycastle.asn1.x500.X500Name; @@ -40,6 +37,7 @@ import org.bouncycastle.jce.provider.BouncyCastleProvider; import org.bouncycastle.operator.ContentSigner; import org.bouncycastle.operator.jcajce.JcaContentSignerBuilder; +import org.joda.time.DateTime; /** * Utility class that provides methods used by {@link SslClientInitializerTest} and {@link @@ -67,13 +65,13 @@ public static KeyPair getKeyPair() throws Exception { } /** - * Signs the given key pair with the given self signed certificate to generate a certificate with + * Signs the given key pair with the given self-signed certificate to generate a certificate with * the given validity range. * * @return signed public key (of the key pair) certificate */ public static X509Certificate signKeyPair( - SelfSignedCaCertificate ssc, KeyPair keyPair, String hostname, Date from, Date to) + SelfSignedCaCertificate ssc, KeyPair keyPair, String hostname, DateTime from, DateTime to) throws Exception { X500Name subjectDnName = new X500Name("CN=" + hostname); BigInteger serialNumber = BigInteger.valueOf(System.currentTimeMillis()); @@ -81,7 +79,12 @@ public static X509Certificate signKeyPair( ContentSigner sigGen = new JcaContentSignerBuilder("SHA256WithRSAEncryption").build(ssc.key()); X509v3CertificateBuilder v3CertGen = new JcaX509v3CertificateBuilder( - issuerDnName, serialNumber, from, to, subjectDnName, keyPair.getPublic()); + issuerDnName, + serialNumber, + from.toDate(), + to.toDate(), + subjectDnName, + keyPair.getPublic()); X509CertificateHolder certificateHolder = v3CertGen.build(sigGen); return new JcaX509CertificateConverter() @@ -90,7 +93,7 @@ public static X509Certificate signKeyPair( } /** - * Signs the given key pair with the given self signed certificate to generate a certificate that + * Signs the given key pair with the given self-signed certificate to generate a certificate that * is valid from yesterday to tomorrow. * * @return signed public key (of the key pair) certificate @@ -98,11 +101,7 @@ public static X509Certificate signKeyPair( public static X509Certificate signKeyPair( SelfSignedCaCertificate ssc, KeyPair keyPair, String hostname) throws Exception { return signKeyPair( - ssc, - keyPair, - hostname, - Date.from(Instant.now().minus(Duration.ofDays(1))), - Date.from(Instant.now().plus(Duration.ofDays(1)))); + ssc, keyPair, hostname, DateTime.now().minusDays(1), DateTime.now().plusDays(1)); } /** @@ -110,7 +109,7 @@ public static X509Certificate signKeyPair( * and verifies if it is echoed back correctly. * * @param certs The certificate that the server should provide. - * @return The SSL session in current channel, can be used for further validation. + * @return The SSL session in the current channel, can be used for further validation. */ static SSLSession setUpSslChannel(Channel channel, X509Certificate... certs) throws Exception { SslHandler sslHandler = channel.pipeline().get(SslHandler.class); diff --git a/networking/src/test/java/google/registry/networking/handler/SslServerInitializerTest.java b/networking/src/test/java/google/registry/networking/handler/SslServerInitializerTest.java index c73070876b..024d9b100e 100644 --- a/networking/src/test/java/google/registry/networking/handler/SslServerInitializerTest.java +++ b/networking/src/test/java/google/registry/networking/handler/SslServerInitializerTest.java @@ -41,11 +41,8 @@ import java.security.cert.CertificateExpiredException; import java.security.cert.CertificateNotYetValidException; import java.security.cert.X509Certificate; -import java.time.Duration; -import java.time.Instant; import java.util.Arrays; import java.util.Collections; -import java.util.Date; import java.util.List; import java.util.stream.Stream; import javax.net.ssl.SSLEngine; @@ -53,6 +50,7 @@ import javax.net.ssl.SSLHandshakeException; import javax.net.ssl.SSLParameters; import javax.net.ssl.SSLSession; +import org.joda.time.DateTime; import org.junit.jupiter.api.extension.RegisterExtension; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; @@ -66,7 +64,7 @@ * the overhead of routing traffic through the network layer, even if it were to go through * loopback. It also alleviates the need to pick a free port to use. * - *

The local addresses used in each test method must to be different, otherwise tests run in + *

The local addresses used in each test method must be different, otherwise tests run in * parallel may interfere with each other. */ class SslServerInitializerTest { @@ -202,9 +200,7 @@ void testFailure_cipherNotAccepted(SslProvider sslProvider) throws Exception { localAddress, getServerHandler(true, true, sslProvider, serverSsc.key(), serverSsc.cert())); SelfSignedCaCertificate clientSsc = SelfSignedCaCertificate.create( - "CLIENT", - Date.from(Instant.now().minus(Duration.ofDays(2))), - Date.from(Instant.now().plus(Duration.ofDays(1)))); + "CLIENT", DateTime.now().minusDays(2), DateTime.now().plusDays(1)); nettyExtension.setUpClient( localAddress, getClientHandler( @@ -237,9 +233,7 @@ void testSuccess_someCiphersNotAccepted(SslProvider sslProvider) throws Exceptio Suppliers.ofInstance(ImmutableList.of(serverSsc.cert())))); SelfSignedCaCertificate clientSsc = SelfSignedCaCertificate.create( - "CLIENT", - Date.from(Instant.now().minus(Duration.ofDays(2))), - Date.from(Instant.now().plus(Duration.ofDays(1)))); + "CLIENT", DateTime.now().minusDays(2), DateTime.now().plusDays(1)); nettyExtension.setUpClient( localAddress, getClientHandler( @@ -271,20 +265,18 @@ void testFailure_protocolNotAccepted(SslProvider sslProvider) throws Exception { localAddress, getServerHandler(true, true, sslProvider, serverSsc.key(), serverSsc.cert())); SelfSignedCaCertificate clientSsc = SelfSignedCaCertificate.create( - "CLIENT", - Date.from(Instant.now().minus(Duration.ofDays(2))), - Date.from(Instant.now().plus(Duration.ofDays(1)))); + "CLIENT", DateTime.now().minusDays(2), DateTime.now().plusDays(1)); nettyExtension.setUpClient( localAddress, getClientHandler( sslProvider, serverSsc.cert(), clientSsc.key(), clientSsc.cert(), "TLSv1.1", null)); ImmutableList jdkVersion = - Arrays.asList(System.getProperty("java.version").split("\\.")).stream() + Arrays.stream(System.getProperty("java.version").split("\\.")) .map(Integer::parseInt) .collect(ImmutableList.toImmutableList()); - // In JDK v11.0.11 and above TLS 1.1 is not supported any more, in which case attempting to + // In JDK v11.0.11 and above, TLS 1.1 is not supported anymore, in which case attempting to // connect with TLS 1.1 results in a ClosedChannelException instead of a SSLHandShakeException. // See https://www.oracle.com/java/technologies/javase/11-0-11-relnotes.html#JDK-8202343 Class rootCause = @@ -309,9 +301,7 @@ void testFailure_clientCertExpired(SslProvider sslProvider) throws Exception { localAddress, getServerHandler(true, true, sslProvider, serverSsc.key(), serverSsc.cert())); SelfSignedCaCertificate clientSsc = SelfSignedCaCertificate.create( - "CLIENT", - Date.from(Instant.now().minus(Duration.ofDays(2))), - Date.from(Instant.now().minus(Duration.ofDays(1)))); + "CLIENT", DateTime.now().minusDays(2), DateTime.now().minusDays(1)); nettyExtension.setUpClient( localAddress, getClientHandler(sslProvider, serverSsc.cert(), clientSsc.key(), clientSsc.cert())); @@ -332,9 +322,7 @@ void testFailure_clientCertNotYetValid(SslProvider sslProvider) throws Exception localAddress, getServerHandler(true, true, sslProvider, serverSsc.key(), serverSsc.cert())); SelfSignedCaCertificate clientSsc = SelfSignedCaCertificate.create( - "CLIENT", - Date.from(Instant.now().plus(Duration.ofDays(1))), - Date.from(Instant.now().plus(Duration.ofDays(2)))); + "CLIENT", DateTime.now().plusDays(1), DateTime.now().plusDays(2)); nettyExtension.setUpClient( localAddress, getClientHandler(sslProvider, serverSsc.cert(), clientSsc.key(), clientSsc.cert())); @@ -446,8 +434,8 @@ void testFailure_wrongHostnameInCertificate(SslProvider sslProvider) throws Exce localAddress, getClientHandler(sslProvider, serverSsc.cert(), clientSsc.key(), clientSsc.cert())); - // When the client rejects the server cert due to wrong hostname, both the server and the client - // throw exceptions. + // When the client rejects the server cert due to the wrong hostname, both the server and the + // client throw exceptions. nettyExtension.assertThatClientRootCause().isInstanceOf(CertificateException.class); nettyExtension.assertThatClientRootCause().hasMessageThat().contains(SSL_HOST); nettyExtension.assertThatServerRootCause().isInstanceOf(SSLException.class); diff --git a/util/src/main/java/google/registry/util/SelfSignedCaCertificate.java b/util/src/main/java/google/registry/util/SelfSignedCaCertificate.java index cc0d630ab8..7be8dac919 100644 --- a/util/src/main/java/google/registry/util/SelfSignedCaCertificate.java +++ b/util/src/main/java/google/registry/util/SelfSignedCaCertificate.java @@ -23,9 +23,6 @@ import java.security.PrivateKey; import java.security.SecureRandom; import java.security.cert.X509Certificate; -import java.time.Duration; -import java.time.Instant; -import java.util.Date; import java.util.Random; import org.bouncycastle.asn1.ASN1ObjectIdentifier; import org.bouncycastle.asn1.x500.X500Name; @@ -44,9 +41,8 @@ public class SelfSignedCaCertificate { private static final String DEFAULT_ISSUER_FQDN = "registry-test"; - private static final Date DEFAULT_NOT_BEFORE = - Date.from(Instant.now().minus(Duration.ofHours(1))); - private static final Date DEFAULT_NOT_AFTER = Date.from(Instant.now().plus(Duration.ofDays(1))); + private static final DateTime DEFAULT_NOT_BEFORE = DateTime.now().minusHours(1); + private static final DateTime DEFAULT_NOT_AFTER = DateTime.now().plusDays(1); private static final Random RANDOM = new Random(); private static final BouncyCastleProvider PROVIDER = new BouncyCastleProvider(); @@ -80,24 +76,14 @@ public static SelfSignedCaCertificate create(String fqdn) throws Exception { return create(fqdn, DEFAULT_NOT_BEFORE, DEFAULT_NOT_AFTER); } - public static SelfSignedCaCertificate create(String fqdn, Date from, Date to) throws Exception { - return create(keyGen.generateKeyPair(), fqdn, from, to); - } - public static SelfSignedCaCertificate create(String fqdn, DateTime from, DateTime to) throws Exception { - return create(keyGen.generateKeyPair(), fqdn, from.toDate(), to.toDate()); - } - - public static SelfSignedCaCertificate create(KeyPair keyPair, String fqdn, Date from, Date to) - throws Exception { - return new SelfSignedCaCertificate(keyPair.getPrivate(), createCaCert(keyPair, fqdn, from, to)); + return create(keyGen.generateKeyPair(), fqdn, from, to); } public static SelfSignedCaCertificate create( KeyPair keyPair, String fqdn, DateTime from, DateTime to) throws Exception { - return new SelfSignedCaCertificate( - keyPair.getPrivate(), createCaCert(keyPair, fqdn, from.toDate(), to.toDate())); + return new SelfSignedCaCertificate(keyPair.getPrivate(), createCaCert(keyPair, fqdn, from, to)); } static KeyPairGenerator createKeyPairGenerator() { @@ -111,7 +97,7 @@ static KeyPairGenerator createKeyPairGenerator() { } /** Returns a self-signed Certificate Authority (CA) certificate. */ - static X509Certificate createCaCert(KeyPair keyPair, String fqdn, Date from, Date to) + static X509Certificate createCaCert(KeyPair keyPair, String fqdn, DateTime from, DateTime to) throws Exception { X500Name owner = new X500Name("CN=" + fqdn); String publicKeyAlg = keyPair.getPublic().getAlgorithm(); @@ -121,7 +107,12 @@ static X509Certificate createCaCert(KeyPair keyPair, String fqdn, Date from, Dat new JcaContentSignerBuilder(signatureAlgorithm).build(keyPair.getPrivate()); X509v3CertificateBuilder builder = new JcaX509v3CertificateBuilder( - owner, new BigInteger(64, RANDOM), from, to, owner, keyPair.getPublic()); + owner, + new BigInteger(64, RANDOM), + from.toDate(), + to.toDate(), + owner, + keyPair.getPublic()); // Mark cert as CA by adding basicConstraint with cA=true to the builder BasicConstraints basicConstraints = new BasicConstraints(true); diff --git a/util/src/main/java/google/registry/util/X509Utils.java b/util/src/main/java/google/registry/util/X509Utils.java index da9cc4f3e7..ce0afd80ad 100644 --- a/util/src/main/java/google/registry/util/X509Utils.java +++ b/util/src/main/java/google/registry/util/X509Utils.java @@ -41,11 +41,11 @@ 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; import javax.annotation.Nullable; import javax.annotation.Tainted; +import org.joda.time.DateTime; import org.joda.time.DateTimeComparator; /** X.509 Public Key Infrastructure (PKI) helper functions. */ @@ -140,13 +140,13 @@ public static X509CRL loadCrl(String asciiCrl) throws GeneralSecurityException { *

Support for certificate chains has not been implemented. * * @throws GeneralSecurityException for unsupported protocols, certs not signed by the TMCH, - * parsing errors, encoding errors, if the CRL is expired, or if the CRL is older than the - * one currently in memory. + * parsing errors, encoding errors, if the CRL is expired, or if the CRL is older than the one + * currently in memory. */ public static void verifyCertificate( - X509Certificate rootCert, X509CRL crl, @Tainted X509Certificate cert, Date now) - throws GeneralSecurityException { - cert.checkValidity(checkNotNull(now, "now")); + X509Certificate rootCert, X509CRL crl, @Tainted X509Certificate cert, DateTime now) + throws GeneralSecurityException { + cert.checkValidity(checkNotNull(now, "now").toDate()); cert.verify(rootCert.getPublicKey()); if (crl.isRevoked(cert)) { X509CRLEntry entry = crl.getRevokedCertificate(cert); @@ -168,7 +168,7 @@ public static void verifyCertificate( * incorrect keys, and for invalid, old, not-yet-valid or revoked certificates. */ public static void verifyCrl( - X509Certificate rootCert, @Nullable X509CRL oldCrl, @Tainted X509CRL newCrl, Date now) + X509Certificate rootCert, @Nullable X509CRL oldCrl, @Tainted X509CRL newCrl, DateTime now) throws GeneralSecurityException { if (oldCrl != null && DateTimeComparator.getInstance().compare(newCrl.getThisUpdate(), oldCrl.getThisUpdate()) @@ -178,7 +178,7 @@ public static void verifyCrl( "New CRL is more out of date than our current CRL. %s < %s\n%s", newCrl.getThisUpdate(), oldCrl.getThisUpdate(), newCrl)); } - if (DateTimeComparator.getInstance().compare(newCrl.getNextUpdate(), now) < 0) { + if (DateTimeComparator.getInstance().compare(new DateTime(newCrl.getNextUpdate()), now) < 0) { throw new CRLException("CRL has expired.\n" + newCrl); } newCrl.verify(rootCert.getPublicKey());