Revert "Don't create Date or Calendar objects for ASN.1 dates unless needed."

common/src/jni/main/cpp/conscrypt/native_crypto.cc

@@ -4904,20 +4904,6 @@ static jobjectArray NativeCrypto_get_X509_GENERAL_NAME_stack(JNIEnv* env, jclass
     return joa.release();
 }
 
-/*
- * Converts an ASN1_TIME to epoch time in milliseconds.
- */
-static jlong ASN1_TIME_convert_to_posix(JNIEnv* env, const ASN1_TIME* time) {
-    int64_t retval;
-    if (!ASN1_TIME_to_posix(time, &retval)) {
-        JNI_TRACE("ASN1_TIME_convert_to_posix(%p) => Invalid date value", time);
-        conscrypt::jniutil::throwParsingException(env, "Invalid date value");
-        return 0;
-    }
-    // ASN1_TIME_to_posix can only return years from 0000 to 9999, so this won't overflow.
-    return static_cast<jlong>(retval * 1000);
-}
-
 static jlong NativeCrypto_X509_get_notBefore(JNIEnv* env, jclass, jlong x509Ref,
                                              CONSCRYPT_UNUSED jobject holder) {
     CHECK_ERROR_QUEUE_ON_RETURN;
@@ -4932,7 +4918,7 @@ static jlong NativeCrypto_X509_get_notBefore(JNIEnv* env, jclass, jlong x509Ref,
 
     ASN1_TIME* notBefore = X509_get_notBefore(x509);
     JNI_TRACE("X509_get_notBefore(%p) => %p", x509, notBefore);
-    return ASN1_TIME_convert_to_posix(env, notBefore);
+    return reinterpret_cast<uintptr_t>(notBefore);
 }
 
 static jlong NativeCrypto_X509_get_notAfter(JNIEnv* env, jclass, jlong x509Ref,
@@ -4949,7 +4935,7 @@ static jlong NativeCrypto_X509_get_notAfter(JNIEnv* env, jclass, jlong x509Ref,
 
     ASN1_TIME* notAfter = X509_get_notAfter(x509);
     JNI_TRACE("X509_get_notAfter(%p) => %p", x509, notAfter);
-    return ASN1_TIME_convert_to_posix(env, notAfter);
+    return reinterpret_cast<uintptr_t>(notAfter);
 }
 
 // NOLINTNEXTLINE(runtime/int)
@@ -5585,7 +5571,7 @@ static jlong NativeCrypto_get_X509_REVOKED_revocationDate(JNIEnv* env, jclass,
 
     JNI_TRACE("get_X509_REVOKED_revocationDate(%p) => %p", revoked,
               X509_REVOKED_get0_revocationDate(revoked));
-    return ASN1_TIME_convert_to_posix(env, X509_REVOKED_get0_revocationDate(revoked));
+    return reinterpret_cast<uintptr_t>(X509_REVOKED_get0_revocationDate(revoked));
 }
 
 #ifdef __GNUC__
@@ -5679,7 +5665,7 @@ static jlong NativeCrypto_X509_CRL_get_lastUpdate(JNIEnv* env, jclass, jlong x50
 
     ASN1_TIME* lastUpdate = X509_CRL_get_lastUpdate(crl);
     JNI_TRACE("X509_CRL_get_lastUpdate(%p) => %p", crl, lastUpdate);
-    return ASN1_TIME_convert_to_posix(env, lastUpdate);
+    return reinterpret_cast<uintptr_t>(lastUpdate);
 }
 
 static jlong NativeCrypto_X509_CRL_get_nextUpdate(JNIEnv* env, jclass, jlong x509CrlRef,
@@ -5696,7 +5682,7 @@ static jlong NativeCrypto_X509_CRL_get_nextUpdate(JNIEnv* env, jclass, jlong x50
 
     ASN1_TIME* nextUpdate = X509_CRL_get_nextUpdate(crl);
     JNI_TRACE("X509_CRL_get_nextUpdate(%p) => %p", crl, nextUpdate);
-    return ASN1_TIME_convert_to_posix(env, nextUpdate);
+    return reinterpret_cast<uintptr_t>(nextUpdate);
 }
 
 static jbyteArray NativeCrypto_i2d_X509_REVOKED(JNIEnv* env, jclass, jlong x509RevokedRef) {
@@ -5720,6 +5706,63 @@ static jint NativeCrypto_X509_supported_extension(JNIEnv* env, jclass, jlong x50
     return X509_supported_extension(ext);
 }
 
+static inline bool decimal_to_integer(const char* data, size_t len, int* out) {
+    int ret = 0;
+    for (size_t i = 0; i < len; i++) {
+        ret *= 10;
+        if (data[i] < '0' || data[i] > '9') {
+            return false;
+        }
+        ret += data[i] - '0';
+    }
+    *out = ret;
+    return true;
+}
+
+static void NativeCrypto_ASN1_TIME_to_Calendar(JNIEnv* env, jclass, jlong asn1TimeRef,
+                                               jobject calendar) {
+    CHECK_ERROR_QUEUE_ON_RETURN;
+    ASN1_TIME* asn1Time = reinterpret_cast<ASN1_TIME*>(static_cast<uintptr_t>(asn1TimeRef));
+    JNI_TRACE("ASN1_TIME_to_Calendar(%p, %p)", asn1Time, calendar);
+
+    if (asn1Time == nullptr) {
+        conscrypt::jniutil::throwNullPointerException(env, "asn1Time == null");
+        return;
+    }
+
+    if (!ASN1_TIME_check(asn1Time)) {
+        conscrypt::jniutil::throwParsingException(env, "Invalid date format");
+        return;
+    }
+
+    bssl::UniquePtr<ASN1_GENERALIZEDTIME> gen(ASN1_TIME_to_generalizedtime(asn1Time, nullptr));
+    if (gen.get() == nullptr) {
+        conscrypt::jniutil::throwParsingException(env,
+                                                  "ASN1_TIME_to_generalizedtime returned null");
+        return;
+    }
+
+    if (ASN1_STRING_length(gen.get()) < 14 || ASN1_STRING_get0_data(gen.get()) == nullptr) {
+        conscrypt::jniutil::throwNullPointerException(env, "gen->length < 14 || gen->data == null");
+        return;
+    }
+
+    int year, mon, mday, hour, min, sec;
+    const char* data = reinterpret_cast<const char*>(ASN1_STRING_get0_data(gen.get()));
+    if (!decimal_to_integer(data, 4, &year) ||
+        !decimal_to_integer(data + 4, 2, &mon) ||
+        !decimal_to_integer(data + 6, 2, &mday) ||
+        !decimal_to_integer(data + 8, 2, &hour) ||
+        !decimal_to_integer(data + 10, 2, &min) ||
+        !decimal_to_integer(data + 12, 2, &sec)) {
+        conscrypt::jniutil::throwParsingException(env, "Invalid date format");
+        return;
+    }
+
+    env->CallVoidMethod(calendar, conscrypt::jniutil::calendar_setMethod, year, mon - 1, mday, hour,
+                        min, sec);
+}
+
 // A CbsHandle is a structure used to manage resources allocated by asn1_read-*
 // functions so that they can be freed properly when finished.  This struct owns
 // all objects pointed to by its members.
@@ -11219,6 +11262,7 @@ static JNINativeMethod sNativeCryptoMethods[] = {
         CONSCRYPT_NATIVE_METHOD(X509_REVOKED_dup, "(J)J"),
         CONSCRYPT_NATIVE_METHOD(i2d_X509_REVOKED, "(J)[B"),
         CONSCRYPT_NATIVE_METHOD(X509_supported_extension, "(J)I"),
+        CONSCRYPT_NATIVE_METHOD(ASN1_TIME_to_Calendar, "(JLjava/util/Calendar;)V"),
         CONSCRYPT_NATIVE_METHOD(asn1_read_init, "([B)J"),
         CONSCRYPT_NATIVE_METHOD(asn1_read_sequence, "(J)J"),
         CONSCRYPT_NATIVE_METHOD(asn1_read_next_tag_is, "(JI)Z"),

common/src/main/java/org/conscrypt/NativeCrypto.java

@@ -637,6 +637,10 @@
 
     static native int X509_supported_extension(long x509ExtensionRef);
 
+    // --- ASN1_TIME -----------------------------------------------------------
+
+    static native void ASN1_TIME_to_Calendar(long asn1TimeCtx, Calendar cal) throws ParsingException;
+
     // --- ASN1 Encoding -------------------------------------------------------
 
     /**

common/src/main/java/org/conscrypt/OpenSSLX509CRL.java

@@ -48,15 +48,23 @@
  */
 final class OpenSSLX509CRL extends X509CRL {
     private volatile long mContext;
-    private final long thisUpdate;
-    private final long nextUpdate;
+    private final Date thisUpdate;
+    private final Date nextUpdate;
 
     private OpenSSLX509CRL(long ctx) throws ParsingException {
         mContext = ctx;
         // The legacy X509 OpenSSL APIs don't validate ASN1_TIME structures until access, so
         // parse them here because this is the only time we're allowed to throw ParsingException
-        thisUpdate = NativeCrypto.X509_CRL_get_lastUpdate(mContext, this);
-        nextUpdate = NativeCrypto.X509_CRL_get_nextUpdate(mContext, this);
+        thisUpdate = toDate(NativeCrypto.X509_CRL_get_lastUpdate(mContext, this));
+        nextUpdate = toDate(NativeCrypto.X509_CRL_get_nextUpdate(mContext, this));
+    }
+
+    // Package-visible because it's also used by OpenSSLX509CRLEntry
+    static Date toDate(long asn1time) throws ParsingException {
+        Calendar calendar = Calendar.getInstance(TimeZone.getTimeZone("UTC"));
+        calendar.set(Calendar.MILLISECOND, 0);
+        NativeCrypto.ASN1_TIME_to_Calendar(asn1time, calendar);
+        return calendar.getTime();
     }
 
     static OpenSSLX509CRL fromX509DerInputStream(InputStream is) throws ParsingException {
@@ -268,12 +276,12 @@
 
     @Override
     public Date getThisUpdate() {
-        return new Date(thisUpdate);
+        return (Date) thisUpdate.clone();
     }
 
     @Override
     public Date getNextUpdate() {
-        return new Date(nextUpdate);
+        return (Date) nextUpdate.clone();
     }
 
     @Override

common/src/main/java/org/conscrypt/OpenSSLX509CRLEntry.java

@@ -31,13 +31,13 @@
  */
 final class OpenSSLX509CRLEntry extends X509CRLEntry {
     private final long mContext;
-    private final long revocationDate;
+    private final Date revocationDate;
 
     OpenSSLX509CRLEntry(long ctx) throws ParsingException {
         mContext = ctx;
         // The legacy X509 OpenSSL APIs don't validate ASN1_TIME structures until access, so
         // parse them here because this is the only time we're allowed to throw ParsingException
-        revocationDate = NativeCrypto.get_X509_REVOKED_revocationDate(mContext);
+        revocationDate = OpenSSLX509CRL.toDate(NativeCrypto.get_X509_REVOKED_revocationDate(mContext));
     }
 
     @Override
@@ -112,7 +112,7 @@ public BigInteger getSerialNumber() {
 
     @Override
     public Date getRevocationDate() {
-        return new Date(revocationDate);
+        return (Date) revocationDate.clone();
     }
 
     @Override

common/src/main/java/org/conscrypt/OpenSSLX509Certificate.java

@@ -59,15 +59,29 @@
     private transient volatile long mContext;
     private transient Integer mHashCode;
 
-    private final long notBefore;
-    private final long notAfter;
+    private final Date notBefore;
+    private final Date notAfter;
 
     OpenSSLX509Certificate(long ctx) throws ParsingException {
         mContext = ctx;
         // The legacy X509 OpenSSL APIs don't validate ASN1_TIME structures until access, so
         // parse them here because this is the only time we're allowed to throw ParsingException
-        notBefore = NativeCrypto.X509_get_notBefore(mContext, this);
-        notAfter = NativeCrypto.X509_get_notAfter(mContext, this);
+        notBefore = toDate(NativeCrypto.X509_get_notBefore(mContext, this));
+        notAfter = toDate(NativeCrypto.X509_get_notAfter(mContext, this));
+    }
+
+    // A non-throwing constructor used when we have already parsed the dates
+    private OpenSSLX509Certificate(long ctx, Date notBefore, Date notAfter) {
+        mContext = ctx;
+        this.notBefore = notBefore;
+        this.notAfter = notAfter;
+    }
+
+    private static Date toDate(long asn1time) throws ParsingException {
+        Calendar calendar = Calendar.getInstance(TimeZone.getTimeZone("UTC"));
+        calendar.set(Calendar.MILLISECOND, 0);
+        NativeCrypto.ASN1_TIME_to_Calendar(asn1time, calendar);
+        return calendar.getTime();
     }
 
     public static OpenSSLX509Certificate fromX509DerInputStream(InputStream is)
@@ -244,12 +258,12 @@
             CertificateNotYetValidException {
         if (getNotBefore().compareTo(date) > 0) {
             throw new CertificateNotYetValidException("Certificate not valid until "
-                    + getNotBefore() + " (compared to " + date + ")");
+                    + getNotBefore().toString() + " (compared to " + date.toString() + ")");
         }
 
         if (getNotAfter().compareTo(date) < 0) {
             throw new CertificateExpiredException("Certificate expired at "
-                    + getNotAfter() + " (compared to " + date + ")");
+                    + getNotAfter().toString() + " (compared to " + date.toString() + ")");
         }
     }
 
@@ -275,12 +289,12 @@
 
     @Override
     public Date getNotBefore() {
-        return new Date(notBefore);
+        return (Date) notBefore.clone();
     }
 
     @Override
     public Date getNotAfter() {
-        return new Date(notAfter);
+        return (Date) notAfter.clone();
     }
 
     @Override

common/src/test/java/org/conscrypt/HostnameVerifierTest.java

@@ -598,7 +598,7 @@ public void subjectAltNameWithToplevelWildcard() throws Exception {
         //
         // Certificate generated using:-
         //     openssl req -x509 -nodes -days 36500 -subj "/CN=Google Inc" \
-        //         -addext "subjectAltName=DNS:*.com" -
+        //         -addext "subjectAltName=DNS:*.com" -newkey rsa:512
         SSLSession session = session(""
                 + "-----BEGIN CERTIFICATE-----\n"
                 + "MIIBlTCCAT+gAwIBAgIUe1RB6C61ZW/SEQpKiywSEJOEOUMwDQYJKoZIhvcNAQEL\n"

openjdk/build.gradle

@@ -391,7 +391,6 @@ model {
 
                     if (toolChain in Clang || toolChain in Gcc) {
                         cppCompiler.args "-Wall",
-                                "-Werror",
                                 "-fPIC",
                                 "-O3",
                                 "-std=c++17",