Skip to content

Commit

Permalink
Don't create Date or Calendar objects for ASN.1 dates unless needed.
Browse files Browse the repository at this point in the history 
Continutes 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.
  • Loading branch information
@prbprbprb
prbprbprb committed Oct 20, 2023
1 parent 23ca210 commit f97bf04
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 93 deletions.
75 changes: 25 additions & 50 deletions common/src/jni/main/cpp/conscrypt/native_crypto.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4861,6 +4861,26 @@ static jobjectArray NativeCrypto_get_X509_GENERAL_NAME_stack(JNIEnv* env, jclass
return joa.release();
}

/*
* Checks an ASN1_TIME for validity, converts to epoch time in milliseconds.
*/
static jlong ASN1_TIME_check_and_convert_to_posix(JNIEnv* env, const ASN1_TIME* time) {
if (!ASN1_TIME_check(time)) {
JNI_TRACE("ASN1_time_check_and_convert(%p) => Invalid date format", time);
conscrypt::jniutil::throwParsingException(env, "Invalid date format");
return 0;
}

int64_t retval;
if (!ASN1_TIME_to_posix(time, &retval)) {
JNI_TRACE("ASN1_time_check_and_convert(%p) => Invalid date value", time);
conscrypt::jniutil::throwParsingException(env, "Invalid date value");
return 0;
}

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;
Expand All @@ -4875,7 +4895,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 reinterpret_cast<uintptr_t>(notBefore);
return ASN1_TIME_check_and_convert_to_posix(env, notBefore);
}

static jlong NativeCrypto_X509_get_notAfter(JNIEnv* env, jclass, jlong x509Ref,
Expand All @@ -4892,7 +4912,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 reinterpret_cast<uintptr_t>(notAfter);
return ASN1_TIME_check_and_convert_to_posix(env, notAfter);
}

// NOLINTNEXTLINE(runtime/int)
Expand Down Expand Up @@ -5528,7 +5548,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 reinterpret_cast<uintptr_t>(X509_REVOKED_get0_revocationDate(revoked));
return ASN1_TIME_check_and_convert_to_posix(env, X509_REVOKED_get0_revocationDate(revoked));
}

#ifdef __GNUC__
Expand Down Expand Up @@ -5622,7 +5642,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 reinterpret_cast<uintptr_t>(lastUpdate);
return ASN1_TIME_check_and_convert_to_posix(env, lastUpdate);
}

static jlong NativeCrypto_X509_CRL_get_nextUpdate(JNIEnv* env, jclass, jlong x509CrlRef,
Expand All @@ -5639,7 +5659,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 reinterpret_cast<uintptr_t>(nextUpdate);
return ASN1_TIME_check_and_convert_to_posix(env, nextUpdate);
}

static jbyteArray NativeCrypto_i2d_X509_REVOKED(JNIEnv* env, jclass, jlong x509RevokedRef) {
Expand Down Expand Up @@ -5676,50 +5696,6 @@ static inline bool decimal_to_integer(const char* data, size_t len, int* out) {
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.
Expand Down Expand Up @@ -11218,7 +11194,6 @@ 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"),
Expand Down
4 changes: 0 additions & 4 deletions common/src/main/java/org/conscrypt/NativeCrypto.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -611,10 +611,6 @@ static native void X509_CRL_verify(long x509CrlCtx, OpenSSLX509CRL holder,

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 -------------------------------------------------------

/**
Expand Down
20 changes: 6 additions & 14 deletions common/src/main/java/org/conscrypt/OpenSSLX509CRL.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -278,12 +270,12 @@ public X500Principal getIssuerX500Principal() {

@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
Expand Down
6 changes: 3 additions & 3 deletions common/src/main/java/org/conscrypt/OpenSSLX509CRLEntry.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -112,7 +112,7 @@ public BigInteger getSerialNumber() {

@Override
public Date getRevocationDate() {
return (Date) revocationDate.clone();
return new Date(revocationDate);
}

@Override
Expand Down
30 changes: 8 additions & 22 deletions common/src/main/java/org/conscrypt/OpenSSLX509Certificate.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,29 +61,15 @@ public final class OpenSSLX509Certificate extends X509Certificate {
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)
Expand Down Expand Up @@ -260,12 +246,12 @@ public void checkValidity(Date date) throws CertificateExpiredException,
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 + ")");
}
}

Expand All @@ -291,12 +277,12 @@ public Principal getSubjectDN() {

@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
Expand Down

0 comments on commit f97bf04

Please sign in to comment.