Skip to content

Commit 75fa855

Browse files
miguelaranda0miguelaranda0
authored
Address upstream comments for OpenSSLX509CRLEntry (#1349)
Bug: 383304212 Change-Id: I00829f965cce494f5221661f51be91c9fc8e1a9f
1 parent 91bdcc2 commit 75fa855

3 files changed

Lines changed: 79 additions & 58 deletions

File tree

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

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -5462,7 +5462,8 @@ static jbyteArray NativeCrypto_X509_get_serialNumber(JNIEnv* env, jclass, jlong
54625462
}
54635463

54645464
static jbyteArray NativeCrypto_X509_REVOKED_get_serialNumber(JNIEnv* env, jclass,
5465-
jlong x509RevokedRef) {
5465+
jlong x509RevokedRef,
5466+
CONSCRYPT_UNUSED jobject holder) {
54665467
CHECK_ERROR_QUEUE_ON_RETURN;
54675468
X509_REVOKED* revoked = reinterpret_cast<X509_REVOKED*>(static_cast<uintptr_t>(x509RevokedRef));
54685469
JNI_TRACE("X509_REVOKED_get_serialNumber(%p)", revoked);
@@ -5992,7 +5993,7 @@ static jlong NativeCrypto_X509_CRL_get_ext(JNIEnv* env, jclass, jlong x509CrlRef
59925993
}
59935994

59945995
static jlong NativeCrypto_X509_REVOKED_get_ext(JNIEnv* env, jclass, jlong x509RevokedRef,
5995-
jstring oid) {
5996+
jstring oid, CONSCRYPT_UNUSED jobject holder) {
59965997
CHECK_ERROR_QUEUE_ON_RETURN;
59975998
X509_REVOKED* revoked = reinterpret_cast<X509_REVOKED*>(static_cast<uintptr_t>(x509RevokedRef));
59985999
JNI_TRACE("X509_REVOKED_get_ext(%p, %p)", revoked, oid);
@@ -6019,7 +6020,8 @@ static jlong NativeCrypto_X509_REVOKED_dup(JNIEnv* env, jclass, jlong x509Revoke
60196020
return reinterpret_cast<uintptr_t>(dup);
60206021
}
60216022

6022-
static void NativeCrypto_X509_REVOKED_free(JNIEnv* env, jclass, jlong x509RevokedRef) {
6023+
static void NativeCrypto_X509_REVOKED_free(JNIEnv* env, jclass, jlong x509RevokedRef,
6024+
CONSCRYPT_UNUSED jobject holder) {
60236025
CHECK_ERROR_QUEUE_ON_RETURN;
60246026
X509_REVOKED* revoked = reinterpret_cast<X509_REVOKED*>(static_cast<uintptr_t>(x509RevokedRef));
60256027
JNI_TRACE("X509_REVOKED_free(%p)", revoked);
@@ -6031,11 +6033,10 @@ static void NativeCrypto_X509_REVOKED_free(JNIEnv* env, jclass, jlong x509Revoke
60316033
}
60326034

60336035
X509_REVOKED_free(revoked);
6034-
revoked = nullptr;
60356036
}
60366037

6037-
static jlong NativeCrypto_get_X509_REVOKED_revocationDate(JNIEnv* env, jclass,
6038-
jlong x509RevokedRef) {
6038+
static jlong NativeCrypto_get_X509_REVOKED_revocationDate(JNIEnv* env, jclass, jlong x509RevokedRef,
6039+
CONSCRYPT_UNUSED jobject holder) {
60396040
CHECK_ERROR_QUEUE_ON_RETURN;
60406041
X509_REVOKED* revoked = reinterpret_cast<X509_REVOKED*>(static_cast<uintptr_t>(x509RevokedRef));
60416042
JNI_TRACE("get_X509_REVOKED_revocationDate(%p)", revoked);
@@ -6055,8 +6056,8 @@ static jlong NativeCrypto_get_X509_REVOKED_revocationDate(JNIEnv* env, jclass,
60556056
#pragma GCC diagnostic push
60566057
#pragma GCC diagnostic ignored "-Wwrite-strings"
60576058
#endif
6058-
static void NativeCrypto_X509_REVOKED_print(JNIEnv* env, jclass, jlong bioRef,
6059-
jlong x509RevokedRef) {
6059+
static void NativeCrypto_X509_REVOKED_print(JNIEnv* env, jclass, jlong bioRef, jlong x509RevokedRef,
6060+
CONSCRYPT_UNUSED jobject holder) {
60606061
CHECK_ERROR_QUEUE_ON_RETURN;
60616062
BIO* bio = reinterpret_cast<BIO*>(static_cast<uintptr_t>(bioRef));
60626063
X509_REVOKED* revoked = reinterpret_cast<X509_REVOKED*>(static_cast<uintptr_t>(x509RevokedRef));
@@ -7342,7 +7343,8 @@ static jbyteArray NativeCrypto_X509_CRL_get_ext_oid(JNIEnv* env, jclass, jlong x
73427343
}
73437344

73447345
static jbyteArray NativeCrypto_X509_REVOKED_get_ext_oid(JNIEnv* env, jclass, jlong x509RevokedRef,
7345-
jstring oidString) {
7346+
jstring oidString,
7347+
CONSCRYPT_UNUSED jobject holder) {
73467348
CHECK_ERROR_QUEUE_ON_RETURN;
73477349
X509_REVOKED* revoked = reinterpret_cast<X509_REVOKED*>(static_cast<uintptr_t>(x509RevokedRef));
73487350
JNI_TRACE("X509_REVOKED_get_ext_oid(%p, %p)", revoked, oidString);
@@ -7422,7 +7424,8 @@ static jobjectArray NativeCrypto_get_X509_CRL_ext_oids(JNIEnv* env, jclass, jlon
74227424
}
74237425

74247426
static jobjectArray NativeCrypto_get_X509_REVOKED_ext_oids(JNIEnv* env, jclass,
7425-
jlong x509RevokedRef, jint critical) {
7427+
jlong x509RevokedRef, jint critical,
7428+
CONSCRYPT_UNUSED jobject holder) {
74267429
CHECK_ERROR_QUEUE_ON_RETURN;
74277430
// NOLINTNEXTLINE(runtime/int)
74287431
JNI_TRACE("get_X509_CRL_ext_oids(0x%llx, %d)", (long long)x509RevokedRef, critical);
@@ -11651,6 +11654,7 @@ static jlong NativeCrypto_SSL_get1_session(JNIEnv* env, jclass, jlong ssl_addres
1165111654
#define REF_BIO_IN_STREAM "L" TO_STRING(JNI_JARJAR_PREFIX) "org/conscrypt/OpenSSLBIOInputStream;"
1165211655
#define REF_X509 "L" TO_STRING(JNI_JARJAR_PREFIX) "org/conscrypt/OpenSSLX509Certificate;"
1165311656
#define REF_X509_CRL "L" TO_STRING(JNI_JARJAR_PREFIX) "org/conscrypt/OpenSSLX509CRL;"
11657+
#define REF_X509_REVOKED "L" TO_STRING(JNI_JARJAR_PREFIX) "org/conscrypt/OpenSSLX509CRLEntry;"
1165411658
#define REF_SSL "L" TO_STRING(JNI_JARJAR_PREFIX) "org/conscrypt/NativeSsl;"
1165511659
#define REF_SSL_CTX "L" TO_STRING(JNI_JARJAR_PREFIX) "org/conscrypt/AbstractSessionContext;"
1165611660
static JNINativeMethod sNativeCryptoMethods[] = {
@@ -11828,13 +11832,15 @@ static JNINativeMethod sNativeCryptoMethods[] = {
1182811832
CONSCRYPT_NATIVE_METHOD(X509_CRL_verify, "(J" REF_X509_CRL REF_EVP_PKEY ")V"),
1182911833
CONSCRYPT_NATIVE_METHOD(X509_CRL_get_lastUpdate, "(J" REF_X509_CRL ")J"),
1183011834
CONSCRYPT_NATIVE_METHOD(X509_CRL_get_nextUpdate, "(J" REF_X509_CRL ")J"),
11831-
CONSCRYPT_NATIVE_METHOD(X509_REVOKED_get_ext_oid, "(JLjava/lang/String;)[B"),
11832-
CONSCRYPT_NATIVE_METHOD(X509_REVOKED_get_serialNumber, "(J)[B"),
11833-
CONSCRYPT_NATIVE_METHOD(X509_REVOKED_print, "(JJ)V"),
11834-
CONSCRYPT_NATIVE_METHOD(get_X509_REVOKED_revocationDate, "(J)J"),
11835+
CONSCRYPT_NATIVE_METHOD(X509_REVOKED_get_ext_oid,
11836+
"(JLjava/lang/String;" REF_X509_REVOKED ")[B"),
11837+
CONSCRYPT_NATIVE_METHOD(X509_REVOKED_get_serialNumber, "(J" REF_X509_REVOKED ")[B"),
11838+
CONSCRYPT_NATIVE_METHOD(X509_REVOKED_print, "(JJ" REF_X509_REVOKED ")V"),
11839+
CONSCRYPT_NATIVE_METHOD(get_X509_REVOKED_revocationDate, "(J" REF_X509_REVOKED ")J"),
1183511840
CONSCRYPT_NATIVE_METHOD(get_X509_ext_oids, "(J" REF_X509 "I)[Ljava/lang/String;"),
1183611841
CONSCRYPT_NATIVE_METHOD(get_X509_CRL_ext_oids, "(J" REF_X509_CRL "I)[Ljava/lang/String;"),
11837-
CONSCRYPT_NATIVE_METHOD(get_X509_REVOKED_ext_oids, "(JI)[Ljava/lang/String;"),
11842+
CONSCRYPT_NATIVE_METHOD(get_X509_REVOKED_ext_oids,
11843+
"(JI" REF_X509_REVOKED ")[Ljava/lang/String;"),
1183811844
CONSCRYPT_NATIVE_METHOD(get_X509_GENERAL_NAME_stack,
1183911845
"(J" REF_X509 "I)[[Ljava/lang/Object;"),
1184011846
CONSCRYPT_NATIVE_METHOD(X509_get_notBefore, "(J" REF_X509 ")J"),
@@ -11862,10 +11868,10 @@ static JNINativeMethod sNativeCryptoMethods[] = {
1186211868
CONSCRYPT_NATIVE_METHOD(X509_CRL_get_issuer_name, "(J" REF_X509_CRL ")[B"),
1186311869
CONSCRYPT_NATIVE_METHOD(X509_CRL_get_version, "(J" REF_X509_CRL ")J"),
1186411870
CONSCRYPT_NATIVE_METHOD(X509_CRL_get_ext, "(J" REF_X509_CRL "Ljava/lang/String;)J"),
11865-
CONSCRYPT_NATIVE_METHOD(X509_REVOKED_get_ext, "(JLjava/lang/String;)J"),
11871+
CONSCRYPT_NATIVE_METHOD(X509_REVOKED_get_ext, "(JLjava/lang/String;" REF_X509_REVOKED ")J"),
1186611872
CONSCRYPT_NATIVE_METHOD(X509_REVOKED_dup, "(J)J"),
11867-
CONSCRYPT_NATIVE_METHOD(X509_REVOKED_free, "(J)V"),
11868-
CONSCRYPT_NATIVE_METHOD(i2d_X509_REVOKED, "(J)[B"),
11873+
CONSCRYPT_NATIVE_METHOD(X509_REVOKED_free, "(J" REF_X509_REVOKED ")V"),
11874+
CONSCRYPT_NATIVE_METHOD(i2d_X509_REVOKED, "(J" REF_X509_REVOKED ")[B"),
1186911875
CONSCRYPT_NATIVE_METHOD(X509_supported_extension, "(J)I"),
1187011876
CONSCRYPT_NATIVE_METHOD(ASN1_TIME_to_Calendar, "(JLjava/util/Calendar;)V"),
1187111877
CONSCRYPT_NATIVE_METHOD(asn1_read_init, "([B)J"),

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

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -745,22 +745,36 @@ static native long X509_CRL_get_nextUpdate(long x509CrlCtx, OpenSSLX509CRL holde
745745

746746
@FastNative static native long X509_REVOKED_dup(long x509RevokedCtx);
747747

748-
@FastNative static native byte[] i2d_X509_REVOKED(long x509RevokedCtx);
748+
@FastNative
749+
static native byte[] i2d_X509_REVOKED(long x509RevokedCtx, OpenSSLX509CRLEntry holder);
749750

750-
@FastNative static native String[] get_X509_REVOKED_ext_oids(long x509ctx, int critical);
751+
@FastNative
752+
static native String[] get_X509_REVOKED_ext_oids(
753+
long x509ctx, int critical, OpenSSLX509CRLEntry holder);
751754

752-
@FastNative static native byte[] X509_REVOKED_get_ext_oid(long x509RevokedCtx, String oid);
755+
@FastNative
756+
static native byte[] X509_REVOKED_get_ext_oid(
757+
long x509RevokedCtx, String oid, OpenSSLX509CRLEntry holder);
753758

754-
@FastNative static native byte[] X509_REVOKED_get_serialNumber(long x509RevokedCtx);
759+
@FastNative
760+
static native byte[] X509_REVOKED_get_serialNumber(
761+
long x509RevokedCtx, OpenSSLX509CRLEntry holder);
755762

756-
@FastNative static native long X509_REVOKED_get_ext(long x509RevokedCtx, String oid);
763+
@FastNative
764+
static native long X509_REVOKED_get_ext(
765+
long x509RevokedCtx, String oid, OpenSSLX509CRLEntry holder);
757766

758767
/** Returns ASN1_TIME reference. */
759-
@FastNative static native long get_X509_REVOKED_revocationDate(long x509RevokedCtx);
768+
@FastNative
769+
static native long get_X509_REVOKED_revocationDate(
770+
long x509RevokedCtx, OpenSSLX509CRLEntry holder);
760771

761-
@FastNative static native void X509_REVOKED_print(long bioRef, long x509RevokedCtx);
772+
@FastNative
773+
static native void X509_REVOKED_print(
774+
long bioRef, long x509RevokedCtx, OpenSSLX509CRLEntry holder);
762775

763-
@FastNative static native void X509_REVOKED_free(long x509RevokedCtx);
776+
@FastNative
777+
static native void X509_REVOKED_free(long x509RevokedCtx, OpenSSLX509CRLEntry holder);
764778

765779
// --- X509_EXTENSION ------------------------------------------------------
766780

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

Lines changed: 33 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -30,31 +30,33 @@
3030
/**
3131
* An implementation of {@link X509CRLEntry} based on BoringSSL.
3232
*/
33-
final class OpenSSLX509CRLEntry extends X509CRLEntry implements AutoCloseable {
33+
final class OpenSSLX509CRLEntry extends X509CRLEntry {
3434
private final long mContext;
3535
private final Date revocationDate;
3636

3737
OpenSSLX509CRLEntry(long ctx) throws ParsingException {
3838
mContext = ctx;
3939
// The legacy X509 OpenSSL APIs don't validate ASN1_TIME structures until access, so
4040
// parse them here because this is the only time we're allowed to throw ParsingException
41-
revocationDate = OpenSSLX509CRL.toDate(NativeCrypto.get_X509_REVOKED_revocationDate(mContext));
41+
revocationDate =
42+
OpenSSLX509CRL.toDate(NativeCrypto.get_X509_REVOKED_revocationDate(mContext, this));
4243
}
4344

4445
@Override
4546
public Set<String> getCriticalExtensionOIDs() {
46-
String[] critOids =
47-
NativeCrypto.get_X509_REVOKED_ext_oids(mContext,
48-
NativeCrypto.EXTENSION_TYPE_CRITICAL);
47+
String[] critOids = NativeCrypto.get_X509_REVOKED_ext_oids(
48+
mContext, NativeCrypto.EXTENSION_TYPE_CRITICAL, this);
4949

5050
/*
5151
* This API has a special case that if there are no extensions, we
5252
* should return null. So if we have no critical extensions, we'll check
5353
* non-critical extensions.
5454
*/
5555
if ((critOids.length == 0)
56-
&& (NativeCrypto.get_X509_REVOKED_ext_oids(mContext,
57-
NativeCrypto.EXTENSION_TYPE_NON_CRITICAL).length == 0)) {
56+
&& (NativeCrypto.get_X509_REVOKED_ext_oids(
57+
mContext, NativeCrypto.EXTENSION_TYPE_NON_CRITICAL, this)
58+
.length
59+
== 0)) {
5860
return null;
5961
}
6062

@@ -63,23 +65,24 @@ public Set<String> getCriticalExtensionOIDs() {
6365

6466
@Override
6567
public byte[] getExtensionValue(String oid) {
66-
return NativeCrypto.X509_REVOKED_get_ext_oid(mContext, oid);
68+
return NativeCrypto.X509_REVOKED_get_ext_oid(mContext, oid, this);
6769
}
6870

6971
@Override
7072
public Set<String> getNonCriticalExtensionOIDs() {
71-
String[] critOids =
72-
NativeCrypto.get_X509_REVOKED_ext_oids(mContext,
73-
NativeCrypto.EXTENSION_TYPE_NON_CRITICAL);
73+
String[] critOids = NativeCrypto.get_X509_REVOKED_ext_oids(
74+
mContext, NativeCrypto.EXTENSION_TYPE_NON_CRITICAL, this);
7475

7576
/*
7677
* This API has a special case that if there are no extensions, we
7778
* should return null. So if we have no non-critical extensions, we'll
7879
* check critical extensions.
7980
*/
8081
if ((critOids.length == 0)
81-
&& (NativeCrypto.get_X509_REVOKED_ext_oids(mContext,
82-
NativeCrypto.EXTENSION_TYPE_CRITICAL).length == 0)) {
82+
&& (NativeCrypto.get_X509_REVOKED_ext_oids(
83+
mContext, NativeCrypto.EXTENSION_TYPE_CRITICAL, this)
84+
.length
85+
== 0)) {
8386
return null;
8487
}
8588

@@ -88,11 +91,10 @@ public Set<String> getNonCriticalExtensionOIDs() {
8891

8992
@Override
9093
public boolean hasUnsupportedCriticalExtension() {
91-
final String[] criticalOids =
92-
NativeCrypto.get_X509_REVOKED_ext_oids(mContext,
93-
NativeCrypto.EXTENSION_TYPE_CRITICAL);
94+
final String[] criticalOids = NativeCrypto.get_X509_REVOKED_ext_oids(
95+
mContext, NativeCrypto.EXTENSION_TYPE_CRITICAL, this);
9496
for (String oid : criticalOids) {
95-
final long extensionRef = NativeCrypto.X509_REVOKED_get_ext(mContext, oid);
97+
final long extensionRef = NativeCrypto.X509_REVOKED_get_ext(mContext, oid, this);
9698
if (NativeCrypto.X509_supported_extension(extensionRef) != 1) {
9799
return true;
98100
}
@@ -103,12 +105,12 @@ public boolean hasUnsupportedCriticalExtension() {
103105

104106
@Override
105107
public byte[] getEncoded() throws CRLException {
106-
return NativeCrypto.i2d_X509_REVOKED(mContext);
108+
return NativeCrypto.i2d_X509_REVOKED(mContext, this);
107109
}
108110

109111
@Override
110112
public BigInteger getSerialNumber() {
111-
return new BigInteger(NativeCrypto.X509_REVOKED_get_serialNumber(mContext));
113+
return new BigInteger(NativeCrypto.X509_REVOKED_get_serialNumber(mContext, this));
112114
}
113115

114116
@Override
@@ -119,36 +121,35 @@ public Date getRevocationDate() {
119121

120122
@Override
121123
public boolean hasExtensions() {
122-
return (NativeCrypto.get_X509_REVOKED_ext_oids(mContext,
123-
NativeCrypto.EXTENSION_TYPE_NON_CRITICAL).length != 0)
124-
|| (NativeCrypto.get_X509_REVOKED_ext_oids(mContext,
125-
NativeCrypto.EXTENSION_TYPE_CRITICAL).length != 0);
124+
return (NativeCrypto.get_X509_REVOKED_ext_oids(
125+
mContext, NativeCrypto.EXTENSION_TYPE_NON_CRITICAL, this)
126+
.length
127+
!= 0)
128+
|| (NativeCrypto.get_X509_REVOKED_ext_oids(
129+
mContext, NativeCrypto.EXTENSION_TYPE_CRITICAL, this)
130+
.length
131+
!= 0);
126132
}
127133

128134
@Override
129135
public String toString() {
130136
ByteArrayOutputStream os = new ByteArrayOutputStream();
131137
long bioCtx = NativeCrypto.create_BIO_OutputStream(os);
132138
try {
133-
NativeCrypto.X509_REVOKED_print(bioCtx, mContext);
139+
NativeCrypto.X509_REVOKED_print(bioCtx, mContext, this);
134140
return os.toString();
135141
} finally {
136142
NativeCrypto.BIO_free_all(bioCtx);
137143
}
138144
}
139145

140-
@Override
141-
public void close() {
142-
if (mContext != 0) {
143-
NativeCrypto.X509_REVOKED_free(mContext);
144-
}
145-
}
146-
147146
@Override
148147
@SuppressWarnings("Finalize")
149148
protected void finalize() throws Throwable {
150149
try {
151-
close();
150+
if (mContext != 0) {
151+
NativeCrypto.X509_REVOKED_free(mContext, this);
152+
}
152153
} finally {
153154
super.finalize();
154155
}

0 commit comments

Comments
 (0)