Don't create Date or Calendar objects for ASN.1 dates unless needed. (#1176)
* Don't create Date or Calendar objects for ASN.1 dates unless needed.
Continues to parse Cert/CRL dates on creation in order to detect
errors, but stores them as longs since the epoch rather than
Dates and lazily creates Dates directly from them rather than
requiring a Calendar.
diff --git a/common/src/jni/main/cpp/conscrypt/native_crypto.cc b/common/src/jni/main/cpp/conscrypt/native_crypto.cc
index 78951bda..e2dc9bd 100644
--- a/common/src/jni/main/cpp/conscrypt/native_crypto.cc
+++ b/common/src/jni/main/cpp/conscrypt/native_crypto.cc
@@ -4861,6 +4861,20 @@
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;
@@ -4875,7 +4889,7 @@
ASN1_TIME* notBefore = X509_get_notBefore(x509);
JNI_TRACE("X509_get_notBefore(%p) => %p", x509, notBefore);
- return reinterpret_cast<uintptr_t>(notBefore);
+ return ASN1_TIME_convert_to_posix(env, notBefore);
}
static jlong NativeCrypto_X509_get_notAfter(JNIEnv* env, jclass, jlong x509Ref,
@@ -4892,7 +4906,7 @@
ASN1_TIME* notAfter = X509_get_notAfter(x509);
JNI_TRACE("X509_get_notAfter(%p) => %p", x509, notAfter);
- return reinterpret_cast<uintptr_t>(notAfter);
+ return ASN1_TIME_convert_to_posix(env, notAfter);
}
// NOLINTNEXTLINE(runtime/int)
@@ -5528,7 +5542,7 @@
JNI_TRACE("get_X509_REVOKED_revocationDate(%p) => %p", revoked,
X509_REVOKED_get0_revocationDate(revoked));
- return reinterpret_cast<uintptr_t>(X509_REVOKED_get0_revocationDate(revoked));
+ return ASN1_TIME_convert_to_posix(env, X509_REVOKED_get0_revocationDate(revoked));
}
#ifdef __GNUC__
@@ -5622,7 +5636,7 @@
ASN1_TIME* lastUpdate = X509_CRL_get_lastUpdate(crl);
JNI_TRACE("X509_CRL_get_lastUpdate(%p) => %p", crl, lastUpdate);
- return reinterpret_cast<uintptr_t>(lastUpdate);
+ return ASN1_TIME_convert_to_posix(env, lastUpdate);
}
static jlong NativeCrypto_X509_CRL_get_nextUpdate(JNIEnv* env, jclass, jlong x509CrlRef,
@@ -5639,7 +5653,7 @@
ASN1_TIME* nextUpdate = X509_CRL_get_nextUpdate(crl);
JNI_TRACE("X509_CRL_get_nextUpdate(%p) => %p", crl, nextUpdate);
- return reinterpret_cast<uintptr_t>(nextUpdate);
+ return ASN1_TIME_convert_to_posix(env, nextUpdate);
}
static jbyteArray NativeCrypto_i2d_X509_REVOKED(JNIEnv* env, jclass, jlong x509RevokedRef) {
@@ -5663,63 +5677,6 @@
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.
@@ -11218,7 +11175,6 @@
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"),
diff --git a/common/src/main/java/org/conscrypt/NativeCrypto.java b/common/src/main/java/org/conscrypt/NativeCrypto.java
index 16b93f7..e42b166 100644
--- a/common/src/main/java/org/conscrypt/NativeCrypto.java
+++ b/common/src/main/java/org/conscrypt/NativeCrypto.java
@@ -611,10 +611,6 @@
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 -------------------------------------------------------
/**
diff --git a/common/src/main/java/org/conscrypt/OpenSSLX509CRL.java b/common/src/main/java/org/conscrypt/OpenSSLX509CRL.java
index d3983cd..9461faa 100644
--- a/common/src/main/java/org/conscrypt/OpenSSLX509CRL.java
+++ b/common/src/main/java/org/conscrypt/OpenSSLX509CRL.java
@@ -50,23 +50,15 @@
*/
final class OpenSSLX509CRL extends X509CRL {
private volatile long mContext;
- private final Date thisUpdate;
- private final Date nextUpdate;
+ private final long thisUpdate;
+ private final long 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 = 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();
+ thisUpdate = NativeCrypto.X509_CRL_get_lastUpdate(mContext, this);
+ nextUpdate = NativeCrypto.X509_CRL_get_nextUpdate(mContext, this);
}
static OpenSSLX509CRL fromX509DerInputStream(InputStream is) throws ParsingException {
@@ -278,12 +270,12 @@
@Override
public Date getThisUpdate() {
- return (Date) thisUpdate.clone();
+ return new Date(thisUpdate);
}
@Override
public Date getNextUpdate() {
- return (Date) nextUpdate.clone();
+ return new Date(nextUpdate);
}
@Override
diff --git a/common/src/main/java/org/conscrypt/OpenSSLX509CRLEntry.java b/common/src/main/java/org/conscrypt/OpenSSLX509CRLEntry.java
index 9a3db62..2d4846c 100644
--- a/common/src/main/java/org/conscrypt/OpenSSLX509CRLEntry.java
+++ b/common/src/main/java/org/conscrypt/OpenSSLX509CRLEntry.java
@@ -31,13 +31,13 @@
*/
final class OpenSSLX509CRLEntry extends X509CRLEntry {
private final long mContext;
- private final Date revocationDate;
+ private final long 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 = OpenSSLX509CRL.toDate(NativeCrypto.get_X509_REVOKED_revocationDate(mContext));
+ revocationDate = NativeCrypto.get_X509_REVOKED_revocationDate(mContext);
}
@Override
@@ -112,7 +112,7 @@
@Override
public Date getRevocationDate() {
- return (Date) revocationDate.clone();
+ return new Date(revocationDate);
}
@Override
diff --git a/common/src/main/java/org/conscrypt/OpenSSLX509Certificate.java b/common/src/main/java/org/conscrypt/OpenSSLX509Certificate.java
index f5e5c5f..925d60d 100644
--- a/common/src/main/java/org/conscrypt/OpenSSLX509Certificate.java
+++ b/common/src/main/java/org/conscrypt/OpenSSLX509Certificate.java
@@ -61,29 +61,15 @@
private transient volatile long mContext;
private transient Integer mHashCode;
- private final Date notBefore;
- private final Date notAfter;
+ private final long notBefore;
+ private final long 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 = 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();
+ notBefore = NativeCrypto.X509_get_notBefore(mContext, this);
+ notAfter = NativeCrypto.X509_get_notAfter(mContext, this);
}
public static OpenSSLX509Certificate fromX509DerInputStream(InputStream is)
@@ -260,12 +246,12 @@
CertificateNotYetValidException {
if (getNotBefore().compareTo(date) > 0) {
throw new CertificateNotYetValidException("Certificate not valid until "
- + getNotBefore().toString() + " (compared to " + date.toString() + ")");
+ + getNotBefore() + " (compared to " + date + ")");
}
if (getNotAfter().compareTo(date) < 0) {
throw new CertificateExpiredException("Certificate expired at "
- + getNotAfter().toString() + " (compared to " + date.toString() + ")");
+ + getNotAfter() + " (compared to " + date + ")");
}
}
@@ -291,12 +277,12 @@
@Override
public Date getNotBefore() {
- return (Date) notBefore.clone();
+ return new Date(notBefore);
}
@Override
public Date getNotAfter() {
- return (Date) notAfter.clone();
+ return new Date(notAfter);
}
@Override
diff --git a/common/src/test/java/org/conscrypt/HostnameVerifierTest.java b/common/src/test/java/org/conscrypt/HostnameVerifierTest.java
index a369ecd..487a42d 100644
--- a/common/src/test/java/org/conscrypt/HostnameVerifierTest.java
+++ b/common/src/test/java/org/conscrypt/HostnameVerifierTest.java
@@ -598,7 +598,7 @@
//
// Certificate generated using:-
// openssl req -x509 -nodes -days 36500 -subj "/CN=Google Inc" \
- // -addext "subjectAltName=DNS:*.com" -newkey rsa:512
+ // -addext "subjectAltName=DNS:*.com" -
SSLSession session = session(""
+ "-----BEGIN CERTIFICATE-----\n"
+ "MIIBlTCCAT+gAwIBAgIUe1RB6C61ZW/SEQpKiywSEJOEOUMwDQYJKoZIhvcNAQEL\n"
diff --git a/openjdk/build.gradle b/openjdk/build.gradle
index 2c0adb1..0e6357c 100644
--- a/openjdk/build.gradle
+++ b/openjdk/build.gradle
@@ -407,6 +407,7 @@
if (toolChain in Clang || toolChain in Gcc) {
cppCompiler.args "-Wall",
+ "-Werror",
"-fPIC",
"-O3",
"-std=c++17",
