Some parsing and serializing fixes. (#219)
This fixes a memory leak in NativeCrypto_i2d_PKCS7. It never frees
derBytes. Also removing a dependency on the legacy ASN.1 stack.
diff --git a/common/src/jni/main/cpp/NativeCrypto.cpp b/common/src/jni/main/cpp/NativeCrypto.cpp
index e4256c2..7220acc 100644
--- a/common/src/jni/main/cpp/NativeCrypto.cpp
+++ b/common/src/jni/main/cpp/NativeCrypto.cpp
@@ -308,6 +308,35 @@
}
/**
+ * Finishes a pending CBB and returns a jbyteArray with the contents.
+ */
+jbyteArray CBBToByteArray(JNIEnv* env, CBB* cbb) {
+ uint8_t *data;
+ size_t len;
+ if (!CBB_finish(cbb, &data, &len)) {
+ Errors::jniThrowRuntimeException(env, "CBB_finish failed");
+ JNI_TRACE("creating byte array failed");
+ return nullptr;
+ }
+ bssl::UniquePtr<uint8_t> free_data(data);
+
+ ScopedLocalRef<jbyteArray> byteArray(env, env->NewByteArray(static_cast<jsize>(len)));
+ if (byteArray.get() == nullptr) {
+ JNI_TRACE("creating byte array failed");
+ return nullptr;
+ }
+
+ ScopedByteArrayRW bytes(env, byteArray.get());
+ if (bytes.get() == nullptr) {
+ JNI_TRACE("using byte array failed");
+ return nullptr;
+ }
+
+ memcpy(bytes.get(), data, len);
+ return byteArray.release();
+}
+
+/**
* Converts ASN.1 BIT STRING to a jbooleanArray.
*/
jbooleanArray ASN1BitStringToBooleanArray(JNIEnv* env, ASN1_BIT_STRING* bitStr) {
@@ -1028,90 +1057,106 @@
}
/*
- * static native byte[] i2d_PKCS8_PRIV_KEY_INFO(int, byte[])
+ * static native byte[] EVP_marshal_private_key(long)
*/
-static jbyteArray NativeCrypto_i2d_PKCS8_PRIV_KEY_INFO(JNIEnv* env, jclass, jobject pkeyRef) {
+static jbyteArray NativeCrypto_EVP_marshal_private_key(JNIEnv* env, jclass, jobject pkeyRef) {
EVP_PKEY* pkey = fromContextObject<EVP_PKEY>(env, pkeyRef);
- JNI_TRACE("i2d_PKCS8_PRIV_KEY_INFO(%p)", pkey);
+ JNI_TRACE("EVP_marshal_private_key(%p)", pkey);
if (pkey == nullptr) {
return nullptr;
}
- bssl::UniquePtr<PKCS8_PRIV_KEY_INFO> pkcs8(EVP_PKEY2PKCS8(pkey));
- if (pkcs8.get() == nullptr) {
- Errors::throwExceptionIfNecessary(env, "NativeCrypto_i2d_PKCS8_PRIV_KEY_INFO");
- JNI_TRACE("key=%p i2d_PKCS8_PRIV_KEY_INFO => error from key to PKCS8", pkey);
+ bssl::ScopedCBB cbb;
+ if (!CBB_init(cbb.get(), 64)) {
+ Errors::jniThrowOutOfMemory(env, "CBB_init failed");
+ JNI_TRACE("CBB_init failed");
return nullptr;
}
- return ASN1ToByteArray<PKCS8_PRIV_KEY_INFO>(env, pkcs8.get(), i2d_PKCS8_PRIV_KEY_INFO);
+ if (!EVP_marshal_private_key(cbb.get(), pkey)) {
+ Errors::throwExceptionIfNecessary(env, "EVP_marshal_private_key");
+ JNI_TRACE("key=%p EVP_marshal_private_key => error", pkey);
+ return nullptr;
+ }
+
+ return CBBToByteArray(env, cbb.get());
}
/*
- * static native int d2i_PKCS8_PRIV_KEY_INFO(byte[])
+ * static native long EVP_parse_private_key(byte[])
*/
-static jlong NativeCrypto_d2i_PKCS8_PRIV_KEY_INFO(JNIEnv* env, jclass, jbyteArray keyJavaBytes) {
- JNI_TRACE("d2i_PKCS8_PRIV_KEY_INFO(%p)", keyJavaBytes);
+static jlong NativeCrypto_EVP_parse_private_key(JNIEnv* env, jclass, jbyteArray keyJavaBytes) {
+ JNI_TRACE("EVP_parse_private_key(%p)", keyJavaBytes);
ScopedByteArrayRO bytes(env, keyJavaBytes);
if (bytes.get() == nullptr) {
- JNI_TRACE("bytes=%p d2i_PKCS8_PRIV_KEY_INFO => threw exception", keyJavaBytes);
+ JNI_TRACE("bytes=%p EVP_parse_private_key => threw exception", keyJavaBytes);
return 0;
}
- const unsigned char* tmp = reinterpret_cast<const unsigned char*>(bytes.get());
- bssl::UniquePtr<PKCS8_PRIV_KEY_INFO> pkcs8(
- d2i_PKCS8_PRIV_KEY_INFO(nullptr, &tmp, static_cast<long>(bytes.size())));
- if (pkcs8.get() == nullptr) {
- Errors::throwExceptionIfNecessary(env, "d2i_PKCS8_PRIV_KEY_INFO");
- JNI_TRACE("ssl=%p d2i_PKCS8_PRIV_KEY_INFO => error from DER to PKCS8", keyJavaBytes);
+ CBS cbs;
+ CBS_init(&cbs, reinterpret_cast<const uint8_t*>(bytes.get()), bytes.size());
+ bssl::UniquePtr<EVP_PKEY> pkey(EVP_parse_private_key(&cbs));
+ if (!pkey || CBS_len(&cbs) != 0) {
+ Errors::throwParsingException(env, "Error parsing private key");
+ JNI_TRACE("bytes=%p EVP_parse_private_key => threw exception", keyJavaBytes);
return 0;
}
- bssl::UniquePtr<EVP_PKEY> pkey(EVP_PKCS82PKEY(pkcs8.get()));
- if (pkey.get() == nullptr) {
- Errors::throwExceptionIfNecessary(env, "d2i_PKCS8_PRIV_KEY_INFO");
- JNI_TRACE("ssl=%p d2i_PKCS8_PRIV_KEY_INFO => error from PKCS8 to key", keyJavaBytes);
- return 0;
- }
-
- JNI_TRACE("bytes=%p d2i_PKCS8_PRIV_KEY_INFO => %p", keyJavaBytes, pkey.get());
+ JNI_TRACE("bytes=%p EVP_parse_private_key => %p", keyJavaBytes, pkey.get());
return reinterpret_cast<uintptr_t>(pkey.release());
}
/*
- * static native byte[] i2d_PUBKEY(int)
+ * static native byte[] EVP_marshal_public_key(long)
*/
-static jbyteArray NativeCrypto_i2d_PUBKEY(JNIEnv* env, jclass, jobject pkeyRef) {
+static jbyteArray NativeCrypto_EVP_marshal_public_key(JNIEnv* env, jclass, jobject pkeyRef) {
EVP_PKEY* pkey = fromContextObject<EVP_PKEY>(env, pkeyRef);
- JNI_TRACE("i2d_PUBKEY(%p)", pkey);
+ JNI_TRACE("EVP_marshal_public_key(%p)", pkey);
+
if (pkey == nullptr) {
return nullptr;
}
- return ASN1ToByteArray<EVP_PKEY>(env, pkey, reinterpret_cast<int (*) (EVP_PKEY*, uint8_t **)>(i2d_PUBKEY));
+
+ bssl::ScopedCBB cbb;
+ if (!CBB_init(cbb.get(), 64)) {
+ Errors::jniThrowOutOfMemory(env, "CBB_init failed");
+ JNI_TRACE("CBB_init failed");
+ return nullptr;
+ }
+
+ if (!EVP_marshal_public_key(cbb.get(), pkey)) {
+ Errors::throwExceptionIfNecessary(env, "EVP_marshal_public_key");
+ JNI_TRACE("key=%p EVP_marshal_public_key => error", pkey);
+ return nullptr;
+ }
+
+ return CBBToByteArray(env, cbb.get());
}
/*
- * static native int d2i_PUBKEY(byte[])
+ * static native long EVP_parse_public_key(byte[])
*/
-static jlong NativeCrypto_d2i_PUBKEY(JNIEnv* env, jclass, jbyteArray javaBytes) {
- JNI_TRACE("d2i_PUBKEY(%p)", javaBytes);
+static jlong NativeCrypto_EVP_parse_public_key(JNIEnv* env, jclass, jbyteArray keyJavaBytes) {
+ JNI_TRACE("EVP_parse_public_key(%p)", keyJavaBytes);
- ScopedByteArrayRO bytes(env, javaBytes);
+ ScopedByteArrayRO bytes(env, keyJavaBytes);
if (bytes.get() == nullptr) {
- JNI_TRACE("d2i_PUBKEY(%p) => threw error", javaBytes);
+ JNI_TRACE("bytes=%p EVP_parse_public_key => threw exception", keyJavaBytes);
return 0;
}
- const unsigned char* tmp = reinterpret_cast<const unsigned char*>(bytes.get());
- bssl::UniquePtr<EVP_PKEY> pkey(d2i_PUBKEY(nullptr, &tmp, static_cast<long>(bytes.size())));
- if (pkey.get() == nullptr) {
- JNI_TRACE("bytes=%p d2i_PUBKEY => threw exception", javaBytes);
- Errors::throwExceptionIfNecessary(env, "d2i_PUBKEY");
+ CBS cbs;
+ CBS_init(&cbs, reinterpret_cast<const uint8_t*>(bytes.get()), bytes.size());
+ bssl::UniquePtr<EVP_PKEY> pkey(EVP_parse_public_key(&cbs));
+ if (!pkey || CBS_len(&cbs) != 0) {
+ Errors::throwParsingException(env, "Error parsing public key");
+ JNI_TRACE("bytes=%p EVP_parse_public_key => threw exception", keyJavaBytes);
return 0;
}
+ JNI_TRACE("bytes=%p EVP_parse_public_key => %p", keyJavaBytes, pkey.get());
return reinterpret_cast<uintptr_t>(pkey.release());
}
@@ -4437,29 +4482,7 @@
sk_X509_free(stack);
- uint8_t *derBytes;
- size_t derLen;
- if (!CBB_finish(out.get(), &derBytes, &derLen)) {
- Errors::throwExceptionIfNecessary(env, "CBB_finish");
- return nullptr;
- }
-
- ScopedLocalRef<jbyteArray> byteArray(env, env->NewByteArray(static_cast<jsize>(derLen)));
- if (byteArray.get() == nullptr) {
- JNI_TRACE("creating byte array failed");
- return nullptr;
- }
-
- ScopedByteArrayRW bytes(env, byteArray.get());
- if (bytes.get() == nullptr) {
- JNI_TRACE("using byte array failed");
- return nullptr;
- }
-
- uint8_t* p = reinterpret_cast<unsigned char*>(bytes.get());
- memcpy(p, derBytes, derLen);
-
- return byteArray.release();
+ return CBBToByteArray(env, out.get());
}
static jlongArray NativeCrypto_PEM_read_bio_PKCS7(JNIEnv* env, jclass, jlong bioRef, jint which) {
@@ -4638,29 +4661,7 @@
}
}
- uint8_t *out;
- size_t out_len;
- if (!CBB_finish(result.get(), &out, &out_len)) {
- return nullptr;
- }
- std::unique_ptr<uint8_t> out_storage(out);
-
- ScopedLocalRef<jbyteArray> byteArray(env, env->NewByteArray(static_cast<jsize>(out_len)));
- if (byteArray.get() == nullptr) {
- JNI_TRACE("ASN1_seq_pack_X509(%p) => creating byte array failed", certs);
- return nullptr;
- }
-
- ScopedByteArrayRW bytes(env, byteArray.get());
- if (bytes.get() == nullptr) {
- JNI_TRACE("ASN1_seq_pack_X509(%p) => using byte array failed", certs);
- return nullptr;
- }
-
- uint8_t *p = reinterpret_cast<uint8_t*>(bytes.get());
- memcpy(p, out, out_len);
-
- return byteArray.release();
+ return CBBToByteArray(env, result.get());
}
static void NativeCrypto_X509_free(JNIEnv* env, jclass, jlong x509Ref) {
@@ -9162,10 +9163,10 @@
CONSCRYPT_NATIVE_METHOD(NativeCrypto, EVP_PKEY_print_params, "(" REF_EVP_PKEY ")Ljava/lang/String;"),
CONSCRYPT_NATIVE_METHOD(NativeCrypto, EVP_PKEY_free, "(J)V"),
CONSCRYPT_NATIVE_METHOD(NativeCrypto, EVP_PKEY_cmp, "(" REF_EVP_PKEY REF_EVP_PKEY ")I"),
- CONSCRYPT_NATIVE_METHOD(NativeCrypto, i2d_PKCS8_PRIV_KEY_INFO, "(" REF_EVP_PKEY ")[B"),
- CONSCRYPT_NATIVE_METHOD(NativeCrypto, d2i_PKCS8_PRIV_KEY_INFO, "([B)J"),
- CONSCRYPT_NATIVE_METHOD(NativeCrypto, i2d_PUBKEY, "(" REF_EVP_PKEY ")[B"),
- CONSCRYPT_NATIVE_METHOD(NativeCrypto, d2i_PUBKEY, "([B)J"),
+ CONSCRYPT_NATIVE_METHOD(NativeCrypto, EVP_marshal_private_key, "(" REF_EVP_PKEY ")[B"),
+ CONSCRYPT_NATIVE_METHOD(NativeCrypto, EVP_parse_private_key, "([B)J"),
+ CONSCRYPT_NATIVE_METHOD(NativeCrypto, EVP_marshal_public_key, "(" REF_EVP_PKEY ")[B"),
+ CONSCRYPT_NATIVE_METHOD(NativeCrypto, EVP_parse_public_key, "([B)J"),
CONSCRYPT_NATIVE_METHOD(NativeCrypto, PEM_read_bio_PUBKEY, "(J)J"),
CONSCRYPT_NATIVE_METHOD(NativeCrypto, PEM_read_bio_PrivateKey, "(J)J"),
CONSCRYPT_NATIVE_METHOD(NativeCrypto, getRSAPrivateKeyWrapper, "(Ljava/security/PrivateKey;[B)J"),
diff --git a/common/src/main/java/org/conscrypt/NativeCrypto.java b/common/src/main/java/org/conscrypt/NativeCrypto.java
index b77805e..773c38c 100644
--- a/common/src/main/java/org/conscrypt/NativeCrypto.java
+++ b/common/src/main/java/org/conscrypt/NativeCrypto.java
@@ -80,13 +80,13 @@
static native int EVP_PKEY_cmp(NativeRef.EVP_PKEY pkey1, NativeRef.EVP_PKEY pkey2);
- static native byte[] i2d_PKCS8_PRIV_KEY_INFO(NativeRef.EVP_PKEY pkey);
+ static native byte[] EVP_marshal_private_key(NativeRef.EVP_PKEY pkey);
- static native long d2i_PKCS8_PRIV_KEY_INFO(byte[] data);
+ static native long EVP_parse_private_key(byte[] data);
- static native byte[] i2d_PUBKEY(NativeRef.EVP_PKEY pkey);
+ static native byte[] EVP_marshal_public_key(NativeRef.EVP_PKEY pkey);
- static native long d2i_PUBKEY(byte[] data);
+ static native long EVP_parse_public_key(byte[] data);
static native long PEM_read_bio_PUBKEY(long bioCtx);
diff --git a/common/src/main/java/org/conscrypt/OpenSSLECPrivateKey.java b/common/src/main/java/org/conscrypt/OpenSSLECPrivateKey.java
index c0e1654..a466812 100644
--- a/common/src/main/java/org/conscrypt/OpenSSLECPrivateKey.java
+++ b/common/src/main/java/org/conscrypt/OpenSSLECPrivateKey.java
@@ -161,7 +161,7 @@
@Override
public byte[] getEncoded() {
- return NativeCrypto.i2d_PKCS8_PRIV_KEY_INFO(key.getNativeRef());
+ return NativeCrypto.EVP_marshal_private_key(key.getNativeRef());
}
@Override
@@ -214,7 +214,7 @@
@Override
public int hashCode() {
- return Arrays.hashCode(NativeCrypto.i2d_PKCS8_PRIV_KEY_INFO(key.getNativeRef()));
+ return Arrays.hashCode(NativeCrypto.EVP_marshal_private_key(key.getNativeRef()));
}
@Override
@@ -231,7 +231,7 @@
byte[] encoded = (byte[]) stream.readObject();
- key = new OpenSSLKey(NativeCrypto.d2i_PKCS8_PRIV_KEY_INFO(encoded));
+ key = new OpenSSLKey(NativeCrypto.EVP_parse_private_key(encoded));
group = new OpenSSLECGroupContext(new NativeRef.EC_GROUP(
NativeCrypto.EC_KEY_get1_group(key.getNativeRef())));
}
diff --git a/common/src/main/java/org/conscrypt/OpenSSLECPublicKey.java b/common/src/main/java/org/conscrypt/OpenSSLECPublicKey.java
index 6e32810..792a282 100644
--- a/common/src/main/java/org/conscrypt/OpenSSLECPublicKey.java
+++ b/common/src/main/java/org/conscrypt/OpenSSLECPublicKey.java
@@ -87,7 +87,7 @@
@Override
public byte[] getEncoded() {
- return NativeCrypto.i2d_PUBKEY(key.getNativeRef());
+ return NativeCrypto.EVP_marshal_public_key(key.getNativeRef());
}
@Override
@@ -143,7 +143,7 @@
@Override
public int hashCode() {
- return Arrays.hashCode(NativeCrypto.i2d_PUBKEY(key.getNativeRef()));
+ return Arrays.hashCode(NativeCrypto.EVP_marshal_public_key(key.getNativeRef()));
}
@Override
@@ -156,7 +156,7 @@
byte[] encoded = (byte[]) stream.readObject();
- key = new OpenSSLKey(NativeCrypto.d2i_PUBKEY(encoded));
+ key = new OpenSSLKey(NativeCrypto.EVP_parse_public_key(encoded));
group = new OpenSSLECGroupContext(new NativeRef.EC_GROUP(
NativeCrypto.EC_KEY_get1_group(key.getNativeRef())));
}
diff --git a/common/src/main/java/org/conscrypt/OpenSSLKey.java b/common/src/main/java/org/conscrypt/OpenSSLKey.java
index 1ca0b2c..54af772 100644
--- a/common/src/main/java/org/conscrypt/OpenSSLKey.java
+++ b/common/src/main/java/org/conscrypt/OpenSSLKey.java
@@ -73,7 +73,7 @@
throw new InvalidKeyException("Key encoding is null");
}
- return new OpenSSLKey(NativeCrypto.d2i_PKCS8_PRIV_KEY_INFO(key.getEncoded()));
+ return new OpenSSLKey(NativeCrypto.EVP_parse_private_key(key.getEncoded()));
}
/**
@@ -177,7 +177,7 @@
if (encoded == null) {
return null;
}
- return new OpenSSLKey(NativeCrypto.d2i_PKCS8_PRIV_KEY_INFO(encoded));
+ return new OpenSSLKey(NativeCrypto.EVP_parse_private_key(encoded));
}
/**
@@ -222,7 +222,7 @@
}
try {
- return new OpenSSLKey(NativeCrypto.d2i_PUBKEY(key.getEncoded()));
+ return new OpenSSLKey(NativeCrypto.EVP_parse_public_key(key.getEncoded()));
} catch (Exception e) {
throw new InvalidKeyException(e);
}
@@ -267,7 +267,7 @@
final OpenSSLKey key;
try {
- key = new OpenSSLKey(NativeCrypto.d2i_PUBKEY(x509KeySpec.getEncoded()));
+ key = new OpenSSLKey(NativeCrypto.EVP_parse_public_key(x509KeySpec.getEncoded()));
} catch (Exception e) {
throw new InvalidKeySpecException(e);
}
@@ -300,7 +300,7 @@
final OpenSSLKey key;
try {
- key = new OpenSSLKey(NativeCrypto.d2i_PKCS8_PRIV_KEY_INFO(pkcs8KeySpec.getEncoded()));
+ key = new OpenSSLKey(NativeCrypto.EVP_parse_private_key(pkcs8KeySpec.getEncoded()));
} catch (Exception e) {
throw new InvalidKeySpecException(e);
}
diff --git a/common/src/main/java/org/conscrypt/OpenSSLRSAPrivateKey.java b/common/src/main/java/org/conscrypt/OpenSSLRSAPrivateKey.java
index 259e38e..8c2e4df 100644
--- a/common/src/main/java/org/conscrypt/OpenSSLRSAPrivateKey.java
+++ b/common/src/main/java/org/conscrypt/OpenSSLRSAPrivateKey.java
@@ -198,7 +198,7 @@
@Override
public final byte[] getEncoded() {
- return NativeCrypto.i2d_PKCS8_PRIV_KEY_INFO(key.getNativeRef());
+ return NativeCrypto.EVP_marshal_private_key(key.getNativeRef());
}
@Override
diff --git a/common/src/main/java/org/conscrypt/OpenSSLRSAPublicKey.java b/common/src/main/java/org/conscrypt/OpenSSLRSAPublicKey.java
index 40d878d..ffc672f 100644
--- a/common/src/main/java/org/conscrypt/OpenSSLRSAPublicKey.java
+++ b/common/src/main/java/org/conscrypt/OpenSSLRSAPublicKey.java
@@ -96,7 +96,7 @@
@Override
public byte[] getEncoded() {
- return NativeCrypto.i2d_PUBKEY(key.getNativeRef());
+ return NativeCrypto.EVP_marshal_public_key(key.getNativeRef());
}
private void ensureReadParams() {
diff --git a/platform/src/main/java/org/conscrypt/InternalUtil.java b/platform/src/main/java/org/conscrypt/InternalUtil.java
index 40937c8..dbc6137 100644
--- a/platform/src/main/java/org/conscrypt/InternalUtil.java
+++ b/platform/src/main/java/org/conscrypt/InternalUtil.java
@@ -30,7 +30,7 @@
@Internal
public final class InternalUtil {
public static PublicKey logKeyToPublicKey(byte[] logKey) throws NoSuchAlgorithmException {
- return new OpenSSLKey(NativeCrypto.d2i_PUBKEY(logKey)).getPublicKey();
+ return new OpenSSLKey(NativeCrypto.EVP_parse_public_key(logKey)).getPublicKey();
}
public static PublicKey readPublicKeyPem(InputStream pem) throws InvalidKeyException, NoSuchAlgorithmException {
