diff --git a/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.Bignum.cs b/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.Bignum.cs index 9cf78a9d6b1fae..8c2fb821d897b9 100644 --- a/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.Bignum.cs +++ b/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.Bignum.cs @@ -35,12 +35,19 @@ internal static partial class Crypto // the array with zeroes. int offset = targetSize - compactSize; byte[] buf = new byte[targetSize]; + int written; fixed (byte* to = buf) { byte* start = to + offset; - BigNumToBinary(bignum, start); + written = BigNumToBinary(bignum, start); } + // A negative result means the native side cleared a pending JNI exception, so the + // output must be treated as invalid. Fail loudly rather than returning a buffer that + // may be zero-filled or only partially written. + if (written < 0) + throw new CryptographicException(); + return buf; } } diff --git a/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.X509Chain.cs b/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.X509Chain.cs index c096cb91350735..863da7370d847b 100644 --- a/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.X509Chain.cs +++ b/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.X509Chain.cs @@ -72,9 +72,9 @@ internal static X509Certificate2[] X509ChainGetCertificates(SafeX509ChainContext } finally { - // The native side can populate part of certPtrs and then fail (returning 0) if a JNI - // exception is thrown mid-loop, so release every non-null entry rather than only the - // first `res` entries. + // X509Certificate2 above duplicates each global ref, so release the native-returned + // refs here. On failure the native side already released and cleared its entries, so + // the surviving non-null entries are the ones we still own. for (int i = 0; i < certPtrs.Length; i++) { if (certPtrs[i] != IntPtr.Zero) @@ -112,6 +112,8 @@ private static unsafe partial int X509ChainGetErrors( internal static ValidationError[] X509ChainGetErrors(SafeX509ChainContextHandle ctx) { int count = Interop.AndroidCrypto.X509ChainGetErrorCount(ctx); + if (count < 0) + throw new CryptographicException(); if (count == 0) return Array.Empty(); diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_bignum.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_bignum.c index 67b80859d186b8..5d0af2e17d23cc 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_bignum.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_bignum.c @@ -13,22 +13,32 @@ int32_t AndroidCryptoNative_BigNumToBinary(jobject bignum, uint8_t* output) // bigNum.toByteArray() JNIEnv* env = GetJNIEnv(); - jbyteArray bytes = (jbyteArray)(*env)->CallObjectMethod(env, bignum, g_toByteArrayMethod); - jsize bytesLen = (*env)->GetArrayLength(env, bytes); + int32_t ret = -1; + jsize bytesLen = 0; + jsize startingIndex = 0; + jbyte leadingByte = 0; + INIT_LOCALS(loc, bytes); + + loc[bytes] = (jbyteArray)(*env)->CallObjectMethod(env, bignum, g_toByteArrayMethod); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + bytesLen = (*env)->GetArrayLength(env, loc[bytes]); // We strip the leading zero byte from the byte array. - jsize startingIndex = 0; - jbyte leadingByte; - (*env)->GetByteArrayRegion(env, bytes, 0, 1, &leadingByte); + (*env)->GetByteArrayRegion(env, loc[bytes], 0, 1, &leadingByte); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); if (leadingByte == 0) { startingIndex++; bytesLen--; } - (*env)->GetByteArrayRegion(env, bytes, startingIndex, bytesLen, (jbyte*)output); - (*env)->DeleteLocalRef(env, bytes); - return CheckJNIExceptions(env) ? FAIL : (int32_t)bytesLen; + (*env)->GetByteArrayRegion(env, loc[bytes], startingIndex, bytesLen, (jbyte*)output); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + ret = (int32_t)bytesLen; + +cleanup: + RELEASE_LOCALS(loc, env); + return ret; } int32_t AndroidCryptoNative_GetBigNumBytes(jobject bignum) @@ -65,8 +75,14 @@ int32_t AndroidCryptoNative_GetBigNumBytesIncludingPaddingByteForSign(jobject bi // Use the array here to get the leading zero byte if it exists. // bigNum.toByteArray().length(); JNIEnv* env = GetJNIEnv(); - jbyteArray bytes = (jbyteArray)(*env)->CallObjectMethod(env, bignum, g_toByteArrayMethod); - jsize bytesLen = (*env)->GetArrayLength(env, bytes); - (*env)->DeleteLocalRef(env, bytes); - return CheckJNIExceptions(env) ? FAIL : (int32_t)bytesLen; + int32_t ret = FAIL; + INIT_LOCALS(loc, bytes); + + loc[bytes] = (jbyteArray)(*env)->CallObjectMethod(env, bignum, g_toByteArrayMethod); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + ret = (int32_t)(*env)->GetArrayLength(env, loc[bytes]); + +cleanup: + RELEASE_LOCALS(loc, env); + return ret; } diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_cipher.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_cipher.c index b140e84b55e552..3c3b19e94e5d5b 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_cipher.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_cipher.c @@ -208,6 +208,8 @@ int32_t AndroidCryptoNative_CipherSetKeyAndIV(CipherCtx* ctx, uint8_t* key, uint // ivLength = cipher.getBlockSize(); JNIEnv *env = GetJNIEnv(); ctx->ivLength = (*env)->CallIntMethod(env, ctx->cipher, g_getBlockSizeMethod); + if (CheckJNIExceptions(env)) + return FAIL; } SaveTo(iv, &ctx->iv, (size_t)ctx->ivLength, /* overwrite */ true); @@ -241,11 +243,18 @@ int32_t AndroidCryptoNative_CipherUpdateAAD(CipherCtx* ctx, uint8_t* in, int32_t abort_if_invalid_pointer_argument(in); JNIEnv* env = GetJNIEnv(); - jbyteArray inDataBytes = make_java_byte_array(env, inl); - (*env)->SetByteArrayRegion(env, inDataBytes, 0, inl, (jbyte*)in); - (*env)->CallVoidMethod(env, ctx->cipher, g_cipherUpdateAADMethod, inDataBytes); - (*env)->DeleteLocalRef(env, inDataBytes); - return CheckJNIExceptions(env) ? FAIL : SUCCESS; + int32_t ret = FAIL; + INIT_LOCALS(loc, inDataBytes); + loc[inDataBytes] = make_java_byte_array(env, inl); + (*env)->SetByteArrayRegion(env, loc[inDataBytes], 0, inl, (jbyte*)in); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + (*env)->CallVoidMethod(env, ctx->cipher, g_cipherUpdateAADMethod, loc[inDataBytes]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + ret = SUCCESS; + +cleanup: + RELEASE_LOCALS(loc, env); + return ret; } int32_t AndroidCryptoNative_CipherUpdate(CipherCtx* ctx, uint8_t* outm, int32_t* outl, uint8_t* in, int32_t inl) @@ -261,22 +270,30 @@ int32_t AndroidCryptoNative_CipherUpdate(CipherCtx* ctx, uint8_t* outm, int32_t* abort_if_invalid_pointer_argument(in); JNIEnv* env = GetJNIEnv(); - jbyteArray inDataBytes = make_java_byte_array(env, inl); - (*env)->SetByteArrayRegion(env, inDataBytes, 0, inl, (jbyte*)in); + int32_t ret = FAIL; + INIT_LOCALS(loc, inDataBytes, outDataBytes); *outl = 0; - jbyteArray outDataBytes = (jbyteArray)(*env)->CallObjectMethod(env, ctx->cipher, g_cipherUpdateMethod, inDataBytes); - if (outDataBytes && outm) + loc[inDataBytes] = make_java_byte_array(env, inl); + (*env)->SetByteArrayRegion(env, loc[inDataBytes], 0, inl, (jbyte*)in); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + + loc[outDataBytes] = (jbyteArray)(*env)->CallObjectMethod(env, ctx->cipher, g_cipherUpdateMethod, loc[inDataBytes]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + + if (loc[outDataBytes] && outm) { - jsize outDataBytesLen = (*env)->GetArrayLength(env, outDataBytes); + jsize outDataBytesLen = (*env)->GetArrayLength(env, loc[outDataBytes]); + (*env)->GetByteArrayRegion(env, loc[outDataBytes], 0, outDataBytesLen, (jbyte*) outm); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); *outl = outDataBytesLen; - (*env)->GetByteArrayRegion(env, outDataBytes, 0, outDataBytesLen, (jbyte*) outm); - (*env)->DeleteLocalRef(env, outDataBytes); } + ret = SUCCESS; - (*env)->DeleteLocalRef(env, inDataBytes); - return CheckJNIExceptions(env) ? FAIL : SUCCESS; +cleanup: + RELEASE_LOCALS(loc, env); + return ret; } int32_t AndroidCryptoNative_CipherFinalEx(CipherCtx* ctx, uint8_t* outm, int32_t* outl) diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecdsa.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecdsa.c index 29de5869702ec7..ab2909ce8f887e 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecdsa.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecdsa.c @@ -25,24 +25,22 @@ int32_t AndroidCryptoNative_EcDsaSign(const uint8_t* dgst, int32_t dgstlen, uint abort_if_invalid_pointer_argument (siglen); JNIEnv* env = GetJNIEnv(); + int32_t ret = FAIL; + INIT_LOCALS(loc, signatureObject, privateKey); - jobject signatureObject = GetEcDsaSignatureObject(env); - if (!signatureObject) - { - return FAIL; - } + loc[signatureObject] = GetEcDsaSignatureObject(env); + if (!loc[signatureObject]) + goto cleanup; - jobject privateKey = (*env)->CallObjectMethod(env, key->keyPair, g_keyPairGetPrivateMethod); - if (!privateKey) - { - ReleaseLRef(env, signatureObject); - return FAIL; - } + loc[privateKey] = (*env)->CallObjectMethod(env, key->keyPair, g_keyPairGetPrivateMethod); + if (CheckJNIExceptions(env) || !loc[privateKey]) + goto cleanup; - int32_t returnValue = AndroidCryptoNative_SignWithSignatureObject(env, signatureObject, privateKey, dgst, dgstlen, sig, siglen); - ReleaseLRef(env, privateKey); - ReleaseLRef(env, signatureObject); - return returnValue; + ret = AndroidCryptoNative_SignWithSignatureObject(env, loc[signatureObject], loc[privateKey], dgst, dgstlen, sig, siglen); + +cleanup: + RELEASE_LOCALS(loc, env); + return ret; } int32_t AndroidCryptoNative_EcDsaVerify(const uint8_t* dgst, int32_t dgstlen, const uint8_t* sig, int32_t siglen, EC_KEY* key) diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_eckey.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_eckey.c index ccc79a4652a12d..d1b1271d00a9f8 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_eckey.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_eckey.c @@ -22,11 +22,21 @@ EC_KEY* AndroidCryptoNative_NewEcKeyFromKeys(JNIEnv *env, jobject /*ECPublicKey* { abort_if_invalid_pointer_argument (publicKey); + EC_KEY* ret = NULL; + INIT_LOCALS(loc, curveParameters); + if (!(*env)->IsInstanceOf(env, publicKey, g_ECPublicKeyClass)) - return NULL; + goto cleanup; + + loc[curveParameters] = (*env)->CallObjectMethod(env, publicKey, g_ECPublicKeyGetParams); + if (CheckJNIExceptions(env) || loc[curveParameters] == NULL) + goto cleanup; + + ret = AndroidCryptoNative_NewEcKey(AddGRef(env, loc[curveParameters]), AndroidCryptoNative_CreateKeyPair(env, publicKey, privateKey)); - jobject curveParameters = (*env)->CallObjectMethod(env, publicKey, g_ECPublicKeyGetParams); - return AndroidCryptoNative_NewEcKey(ToGRef(env, curveParameters), AndroidCryptoNative_CreateKeyPair(env, publicKey, privateKey)); +cleanup: + RELEASE_LOCALS(loc, env); + return ret; } #pragma clang diagnostic push @@ -73,73 +83,63 @@ EC_KEY* AndroidCryptoNative_EcKeyCreateByOid(const char* oid) abort_if_invalid_pointer_argument (oid); JNIEnv* env = GetJNIEnv(); + EC_KEY* ret = NULL; + INIT_LOCALS(loc, oidStr, ec, paramSpec, keyPairGenerator, keyPair, keyFactory, publicKey, keySpec, curveParameters); // Older versions of Android don't support mapping an OID to a curve name, // so do some of the common mappings here. - jstring oidStr; if (strcmp(oid, "1.3.132.0.33") == 0) { - oidStr = make_java_string(env, "secp224r1"); + loc[oidStr] = make_java_string(env, "secp224r1"); } else if (strcmp(oid, "1.3.132.0.34") == 0 || strcmp(oid, "nistP384") == 0) { - oidStr = make_java_string(env, "secp384r1"); + loc[oidStr] = make_java_string(env, "secp384r1"); } else if (strcmp(oid, "1.3.132.0.35") == 0 || strcmp(oid, "nistP521") == 0) { - oidStr = make_java_string(env, "secp521r1"); + loc[oidStr] = make_java_string(env, "secp521r1"); } else if (strcmp(oid, "1.2.840.10045.3.1.7") == 0 || strcmp(oid, "nistP256") == 0) { - oidStr = make_java_string(env, "secp256r1"); + loc[oidStr] = make_java_string(env, "secp256r1"); } else { - oidStr = make_java_string(env, oid); + loc[oidStr] = make_java_string(env, oid); } - jstring ec = make_java_string(env, "EC"); + loc[ec] = make_java_string(env, "EC"); // First, generate the key pair based on the curve defined by the oid. - jobject paramSpec = (*env)->NewObject(env, g_ECGenParameterSpecClass, g_ECGenParameterSpecCtor, oidStr); - ReleaseLRef(env, oidStr); - - jobject keyPairGenerator = - (*env)->CallStaticObjectMethod(env, g_keyPairGenClass, g_keyPairGenGetInstanceMethod, ec); - (*env)->CallVoidMethod(env, keyPairGenerator, g_keyPairGenInitializeWithParamsMethod, paramSpec); - - ReleaseLRef(env, paramSpec); - if (CheckJNIExceptions(env)) - { - LOG_DEBUG("Failed to create curve"); - ReleaseLRef(env, ec); - ReleaseLRef(env, keyPairGenerator); - return NULL; - } + loc[paramSpec] = (*env)->NewObject(env, g_ECGenParameterSpecClass, g_ECGenParameterSpecCtor, loc[oidStr]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - jobject keyPair = (*env)->CallObjectMethod(env, keyPairGenerator, g_keyPairGenGenKeyPairMethod); + loc[keyPairGenerator] = + (*env)->CallStaticObjectMethod(env, g_keyPairGenClass, g_keyPairGenGetInstanceMethod, loc[ec]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + (*env)->CallVoidMethod(env, loc[keyPairGenerator], g_keyPairGenInitializeWithParamsMethod, loc[paramSpec]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - ReleaseLRef(env, keyPairGenerator); + loc[keyPair] = (*env)->CallObjectMethod(env, loc[keyPairGenerator], g_keyPairGenGenKeyPairMethod); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); // Now that we have the key pair, we can get the curve parameters from the public key. - jobject keyFactory = (*env)->CallStaticObjectMethod(env, g_KeyFactoryClass, g_KeyFactoryGetInstanceMethod, ec); - jobject publicKey = (*env)->CallObjectMethod(env, keyPair, g_keyPairGetPublicMethod); - jobject keySpec = - (*env)->CallObjectMethod(env, keyFactory, g_KeyFactoryGetKeySpecMethod, publicKey, g_ECPublicKeySpecClass); + loc[keyFactory] = (*env)->CallStaticObjectMethod(env, g_KeyFactoryClass, g_KeyFactoryGetInstanceMethod, loc[ec]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + loc[publicKey] = (*env)->CallObjectMethod(env, loc[keyPair], g_keyPairGetPublicMethod); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + loc[keySpec] = + (*env)->CallObjectMethod(env, loc[keyFactory], g_KeyFactoryGetKeySpecMethod, loc[publicKey], g_ECPublicKeySpecClass); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - ReleaseLRef(env, ec); - ReleaseLRef(env, publicKey); - ReleaseLRef(env, keyFactory); + loc[curveParameters] = (*env)->CallObjectMethod(env, loc[keySpec], g_ECPublicKeySpecGetParams); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - if (CheckJNIExceptions(env)) - { - ReleaseLRef(env, keySpec); - ReleaseLRef(env, keyPair); - return NULL; - } + ret = AndroidCryptoNative_NewEcKey(AddGRef(env, loc[curveParameters]), AddGRef(env, loc[keyPair])); - jobject curveParameters = (*env)->CallObjectMethod(env, keySpec, g_ECPublicKeySpecGetParams); - ReleaseLRef(env, keySpec); - return AndroidCryptoNative_NewEcKey(ToGRef(env, curveParameters), ToGRef(env, keyPair)); +cleanup: + RELEASE_LOCALS(loc, env); + return ret; } int32_t AndroidCryptoNative_EcKeyGetSize(const EC_KEY* key, int32_t* keySize) @@ -175,40 +175,47 @@ int32_t AndroidCryptoNative_EcKeyGetSize(const EC_KEY* key, int32_t* keySize) int32_t AndroidCryptoNative_EcKeyGetCurveName(const EC_KEY* key, uint16_t** curveName) { abort_if_invalid_pointer_argument (curveName); + + *curveName = NULL; + if (!g_ECParameterSpecGetCurveName) { // We can't get the curve name. Treat all curves as unnamed. - *curveName = NULL; return SUCCESS; } abort_if_invalid_pointer_argument (key); JNIEnv* env = GetJNIEnv(); - jstring curveNameStr = (*env)->CallObjectMethod(env, key->curveParameters, g_ECParameterSpecGetCurveName); + int32_t ret = FAIL; + uint16_t* buffer = NULL; + INIT_LOCALS(loc, curveNameStr); - if (!curveNameStr) - { - *curveName = NULL; - return SUCCESS; - } + loc[curveNameStr] = (*env)->CallObjectMethod(env, key->curveParameters, g_ECParameterSpecGetCurveName); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - if (CheckJNIExceptions(env)) + if (!loc[curveNameStr]) { - *curveName = NULL; - return FAIL; + // No curve name available; treat as an unnamed curve. + ret = SUCCESS; + goto cleanup; } - jsize nameLength = (*env)->GetStringLength(env, curveNameStr); + jsize nameLength = (*env)->GetStringLength(env, loc[curveNameStr]); // add one for the null terminator. - uint16_t* buffer = xmalloc(sizeof(int16_t) * (size_t)(nameLength + 1)); + buffer = xmalloc(sizeof(int16_t) * (size_t)(nameLength + 1)); buffer[nameLength] = 0; - (*env)->GetStringRegion(env, curveNameStr, 0, nameLength, (jchar*)buffer); - (*env)->DeleteLocalRef(env, curveNameStr); + (*env)->GetStringRegion(env, loc[curveNameStr], 0, nameLength, (jchar*)buffer); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); *curveName = buffer; + buffer = NULL; + ret = SUCCESS; - return SUCCESS; +cleanup: + free(buffer); + RELEASE_LOCALS(loc, env); + return ret; } diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_hmac.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_hmac.c index ffced8680d1264..d5927cd8de33bf 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_hmac.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_hmac.c @@ -97,14 +97,25 @@ int32_t CryptoNative_HmacUpdate(jobject ctx, uint8_t* data, int32_t len) ARGS_NON_NULL_ALL static int32_t DoFinal(JNIEnv* env, jobject mac, uint8_t* data, int32_t* len) { + int32_t ret = FAIL; + jsize dataBytesLen = 0; + INIT_LOCALS(loc, dataBytes); + + *len = 0; + // mac.doFinal(); - jbyteArray dataBytes = (jbyteArray)(*env)->CallObjectMethod(env, mac, g_MacDoFinal); - jsize dataBytesLen = (*env)->GetArrayLength(env, dataBytes); + loc[dataBytes] = (jbyteArray)(*env)->CallObjectMethod(env, mac, g_MacDoFinal); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + dataBytesLen = (*env)->GetArrayLength(env, loc[dataBytes]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + (*env)->GetByteArrayRegion(env, loc[dataBytes], 0, dataBytesLen, (jbyte*) data); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); *len = (int32_t)dataBytesLen; - (*env)->GetByteArrayRegion(env, dataBytes, 0, dataBytesLen, (jbyte*) data); - (*env)->DeleteLocalRef(env, dataBytes); + ret = SUCCESS; - return CheckJNIExceptions(env) ? FAIL : SUCCESS; +cleanup: + RELEASE_LOCALS(loc, env); + return ret; } int32_t CryptoNative_HmacFinal(jobject ctx, uint8_t* data, int32_t* len) diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_misc.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_misc.c index 67d3737561e822..aefb20db7b5a2f 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_misc.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_misc.c @@ -15,18 +15,25 @@ int32_t CryptoNative_GetRandomBytes(uint8_t* buff, int32_t len) abort_unless(buff != NULL, "The 'buff' parameter must be a valid pointer"); JNIEnv* env = GetJNIEnv(); - jobject randObj = (*env)->NewObject(env, g_randClass, g_randCtor); - abort_unless(randObj != NULL,"Unable to create an instance of java/security/SecureRandom"); + int32_t ret = FAIL; + INIT_LOCALS(loc, randObj, buffArray); - jbyteArray buffArray = make_java_byte_array(env, len); - (*env)->SetByteArrayRegion(env, buffArray, 0, len, (jbyte*)buff); - (*env)->CallVoidMethod(env, randObj, g_randNextBytesMethod, buffArray); - (*env)->GetByteArrayRegion(env, buffArray, 0, len, (jbyte*)buff); + loc[randObj] = (*env)->NewObject(env, g_randClass, g_randCtor); + abort_unless(loc[randObj] != NULL,"Unable to create an instance of java/security/SecureRandom"); - (*env)->DeleteLocalRef(env, buffArray); - (*env)->DeleteLocalRef(env, randObj); + loc[buffArray] = make_java_byte_array(env, len); + (*env)->SetByteArrayRegion(env, loc[buffArray], 0, len, (jbyte*)buff); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + (*env)->CallVoidMethod(env, loc[randObj], g_randNextBytesMethod, loc[buffArray]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + (*env)->GetByteArrayRegion(env, loc[buffArray], 0, len, (jbyte*)buff); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + ret = SUCCESS; - return CheckJNIExceptions(env) ? FAIL : SUCCESS; +cleanup: + RELEASE_LOCALS(loc, env); + + return ret; } jobject AndroidCryptoNative_CreateKeyPair(JNIEnv* env, jobject publicKey, jobject privateKey) diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_pbkdf2.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_pbkdf2.c index 793b70a74d952c..9cede1619c3b88 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_pbkdf2.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_pbkdf2.c @@ -15,34 +15,37 @@ int32_t AndroidCryptoNative_Pbkdf2(const char* algorithmName, { JNIEnv* env = GetJNIEnv(); jint ret = FAIL; + INIT_LOCALS(loc, javaAlgorithmName, passwordBytes, saltByteBuffer, destinationBuffer); - jstring javaAlgorithmName = make_java_string(env, algorithmName); - jbyteArray passwordBytes = make_java_byte_array(env, passwordLength); - jobject destinationBuffer = (*env)->NewDirectByteBuffer(env, destination, destinationLength); - jobject saltByteBuffer = NULL; + loc[javaAlgorithmName] = make_java_string(env, algorithmName); + loc[passwordBytes] = make_java_byte_array(env, passwordLength); + loc[destinationBuffer] = (*env)->NewDirectByteBuffer(env, destination, destinationLength); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - if (javaAlgorithmName == NULL || passwordBytes == NULL || destinationBuffer == NULL) + if (loc[javaAlgorithmName] == NULL || loc[passwordBytes] == NULL || loc[destinationBuffer] == NULL) { goto cleanup; } if (password && passwordLength > 0) { - (*env)->SetByteArrayRegion(env, passwordBytes, 0, passwordLength, (const jbyte*)password); + (*env)->SetByteArrayRegion(env, loc[passwordBytes], 0, passwordLength, (const jbyte*)password); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); } if (salt && saltLength > 0) { - saltByteBuffer = (*env)->NewDirectByteBuffer(env, salt, saltLength); + loc[saltByteBuffer] = (*env)->NewDirectByteBuffer(env, salt, saltLength); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - if (saltByteBuffer == NULL) + if (loc[saltByteBuffer] == NULL) { goto cleanup; } } ret = (*env)->CallStaticIntMethod(env, g_PalPbkdf2, g_PalPbkdf2Pbkdf2OneShot, - javaAlgorithmName, passwordBytes, saltByteBuffer, iterations, destinationBuffer); + loc[javaAlgorithmName], loc[passwordBytes], loc[saltByteBuffer], iterations, loc[destinationBuffer]); if (CheckJNIExceptions(env)) { @@ -50,10 +53,7 @@ int32_t AndroidCryptoNative_Pbkdf2(const char* algorithmName, } cleanup: - (*env)->DeleteLocalRef(env, javaAlgorithmName); - (*env)->DeleteLocalRef(env, passwordBytes); - (*env)->DeleteLocalRef(env, saltByteBuffer); - (*env)->DeleteLocalRef(env, destinationBuffer); + RELEASE_LOCALS(loc, env); return ret; } diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_sslstream.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_sslstream.c index 9c3a1eb06d6b9e..ca928842af147a 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_sslstream.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_sslstream.c @@ -786,7 +786,9 @@ int32_t AndroidCryptoNative_SSLStreamSetTargetHost(SSLStream* sslStream, const c loc[params] = (*env)->CallObjectMethod(env, sslStream->sslEngine, g_SSLEngineGetSSLParameters); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); (*env)->CallVoidMethod(env, loc[params], g_SSLParametersSetServerNames, loc[nameList]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); (*env)->CallVoidMethod(env, sslStream->sslEngine, g_SSLEngineSetSSLParameters, loc[params]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); ret = SUCCESS; @@ -821,10 +823,10 @@ AndroidCryptoNative_SSLStreamRead(SSLStream* sslStream, uint8_t* buffer, int32_t abort_if_invalid_pointer_argument (sslStream); abort_if_invalid_pointer_argument (read); - jbyteArray data = NULL; JNIEnv* env = GetJNIEnv(); PAL_SSLStreamStatus ret = SSLStreamStatus_Error; *read = 0; + INIT_LOCALS(loc, dataArray); /* appInBuffer.flip(); @@ -844,7 +846,9 @@ AndroidCryptoNative_SSLStreamRead(SSLStream* sslStream, uint8_t* buffer, int32_t */ IGNORE_RETURN((*env)->CallObjectMethod(env, sslStream->appInBuffer, g_ByteBufferFlip)); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); int32_t rem = (*env)->CallIntMethod(env, sslStream->appInBuffer, g_ByteBufferRemaining); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); if (rem == 0) { IGNORE_RETURN((*env)->CallObjectMethod(env, sslStream->appInBuffer, g_ByteBufferCompact)); @@ -859,6 +863,7 @@ AndroidCryptoNative_SSLStreamRead(SSLStream* sslStream, uint8_t* buffer, int32_t } IGNORE_RETURN((*env)->CallObjectMethod(env, sslStream->appInBuffer, g_ByteBufferFlip)); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); if (IsHandshaking(handshakeStatus)) { @@ -867,17 +872,19 @@ AndroidCryptoNative_SSLStreamRead(SSLStream* sslStream, uint8_t* buffer, int32_t } rem = (*env)->CallIntMethod(env, sslStream->appInBuffer, g_ByteBufferRemaining); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); } if (rem > 0) { int32_t bytes_to_read = rem < length ? rem : length; - data = make_java_byte_array(env, bytes_to_read); - IGNORE_RETURN((*env)->CallObjectMethod(env, sslStream->appInBuffer, g_ByteBufferGet, data)); + loc[dataArray] = make_java_byte_array(env, bytes_to_read); + IGNORE_RETURN((*env)->CallObjectMethod(env, sslStream->appInBuffer, g_ByteBufferGet, loc[dataArray])); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); IGNORE_RETURN((*env)->CallObjectMethod(env, sslStream->appInBuffer, g_ByteBufferCompact)); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - (*env)->GetByteArrayRegion(env, data, 0, bytes_to_read, (jbyte*)buffer); + (*env)->GetByteArrayRegion(env, loc[dataArray], 0, bytes_to_read, (jbyte*)buffer); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); *read = bytes_to_read; ret = SSLStreamStatus_OK; } @@ -887,7 +894,7 @@ AndroidCryptoNative_SSLStreamRead(SSLStream* sslStream, uint8_t* buffer, int32_t } cleanup: - ReleaseLRef(env, data); + RELEASE_LOCALS(loc, env); return ret; } @@ -1122,6 +1129,8 @@ void AndroidCryptoNative_SSLStreamRequestClientAuthentication(SSLStream* sslStre // sslEngine.setWantClientAuth(true); (*env)->CallVoidMethod(env, sslStream->sslEngine, g_SSLEngineSetWantClientAuth, true); + // This function cannot report failure, so at least ensure a thrown exception doesn't leak to the next JNI call. + (void)CheckJNIExceptions(env); } int32_t AndroidCryptoNative_SSLStreamSetApplicationProtocols(SSLStream* sslStream, @@ -1165,6 +1174,7 @@ int32_t AndroidCryptoNative_SSLStreamSetApplicationProtocols(SSLStream* sslStrea (*env)->CallVoidMethod(env, loc[params], g_SSLParametersSetApplicationProtocols, loc[protocols]); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); (*env)->CallVoidMethod(env, sslStream->sslEngine, g_SSLEngineSetSSLParameters, loc[params]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); ret = SUCCESS; diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509.c index f9c0b50505179a..435f2c1b7ad33f 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509.c @@ -34,6 +34,7 @@ jobject /*X509Certificate*/ AndroidCryptoNative_X509Decode(const uint8_t* buf, i loc[bytes] = make_java_byte_array(env, len); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); (*env)->SetByteArrayRegion(env, loc[bytes], 0, len, (const jbyte*)buf); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); loc[stream] = (*env)->NewObject(env, g_ByteArrayInputStreamClass, g_ByteArrayInputStreamCtor, loc[bytes]); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); @@ -88,6 +89,7 @@ int32_t AndroidCryptoNative_X509DecodeCollection(const uint8_t* buf, // InputStream stream = new ByteArrayInputStream(bytes); loc[bytes] = make_java_byte_array(env, bufLen); (*env)->SetByteArrayRegion(env, loc[bytes], 0, bufLen, (const jbyte*)buf); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); loc[stream] = (*env)->NewObject(env, g_ByteArrayInputStreamClass, g_ByteArrayInputStreamCtor, loc[bytes]); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); @@ -215,6 +217,7 @@ PAL_X509ContentType AndroidCryptoNative_X509GetContentType(const uint8_t* buf, i // InputStream stream = new ByteArrayInputStream(bytes); loc[bytes] = make_java_byte_array(env, len); (*env)->SetByteArrayRegion(env, loc[bytes], 0, len, (const jbyte*)buf); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); loc[stream] = (*env)->NewObject(env, g_ByteArrayInputStreamClass, g_ByteArrayInputStreamCtor, loc[bytes]); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509chain.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509chain.c index 7fb44cf517ac9a..04e6a4c5614a45 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509chain.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509chain.c @@ -131,12 +131,15 @@ int32_t AndroidCryptoNative_X509ChainBuild(X509ChainContext* ctx, int64_t timeIn // Date date = new Date(timeInMsFromUnixEpoch); // params.setDate(date); loc[date] = (*env)->NewObject(env, g_DateClass, g_DateCtor, timeInMsFromUnixEpoch); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); (*env)->CallVoidMethod(env, params, g_PKIXBuilderParametersSetDate, loc[date]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); // Disable revocation checking when building the cert path. It will be handled in a validation pass if the path is // successfully built. // params.setRevocationEnabled(false); (*env)->CallVoidMethod(env, params, g_PKIXBuilderParametersSetRevocationEnabled, false); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); // String builderType = "PKIX"; // CertPathBuilder builder = CertPathBuilder.getInstance(builderType); @@ -144,15 +147,20 @@ int32_t AndroidCryptoNative_X509ChainBuild(X509ChainContext* ctx, int64_t timeIn loc[builderType] = make_java_string(env, "PKIX"); loc[builder] = (*env)->CallStaticObjectMethod(env, g_CertPathBuilderClass, g_CertPathBuilderGetInstance, loc[builderType]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); loc[result] = (*env)->CallObjectMethod(env, loc[builder], g_CertPathBuilderBuild, params); if (TryGetJNIException(env, &loc[ex], false /*printException*/)) { (*env)->CallBooleanMethod(env, ctx->errorList, g_ArrayListAdd, loc[ex]); + // Clear any exception raised while recording the build failure so it doesn't leak to managed code. + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); goto cleanup; } loc[certPath] = (*env)->CallObjectMethod(env, loc[result], g_PKIXCertPathBuilderResultGetCertPath); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); loc[trustAnchor] = (*env)->CallObjectMethod(env, loc[result], g_PKIXCertPathBuilderResultGetTrustAnchor); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); ctx->certPath = AddGRef(env, loc[certPath]); ctx->trustAnchor = AddGRef(env, loc[trustAnchor]); @@ -168,12 +176,21 @@ int32_t AndroidCryptoNative_X509ChainGetCertificateCount(X509ChainContext* ctx) abort_if_invalid_pointer_argument (ctx); JNIEnv* env = GetJNIEnv(); + int32_t ret = 0; + int certCount = 0; + INIT_LOCALS(loc, certPathList); + // List certPathList = certPath.getCertificates(); - jobject certPathList = (*env)->CallObjectMethod(env, ctx->certPath, g_CertPathGetCertificates); - int certCount = (int)(*env)->CallIntMethod(env, certPathList, g_CollectionSize); + loc[certPathList] = (*env)->CallObjectMethod(env, ctx->certPath, g_CertPathGetCertificates); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + certCount = (int)(*env)->CallIntMethod(env, loc[certPathList], g_CollectionSize); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + + ret = certCount + 1; // +1 for the trust anchor - (*env)->DeleteLocalRef(env, certPathList); - return certCount + 1; // +1 for the trust anchor +cleanup: + RELEASE_LOCALS(loc, env); + return ret; } int32_t AndroidCryptoNative_X509ChainGetCertificates(X509ChainContext* ctx, @@ -184,43 +201,57 @@ int32_t AndroidCryptoNative_X509ChainGetCertificates(X509ChainContext* ctx, JNIEnv* env = GetJNIEnv(); int32_t ret = FAIL; + int certCount = 0; + int32_t added = 0; + INIT_LOCALS(loc, certPathList, trustedCert, cert); // List certPathList = certPath.getCertificates(); - jobject certPathList = (*env)->CallObjectMethod(env, ctx->certPath, g_CertPathGetCertificates); - int certCount = (int)(*env)->CallIntMethod(env, certPathList, g_CollectionSize); + loc[certPathList] = (*env)->CallObjectMethod(env, ctx->certPath, g_CertPathGetCertificates); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + certCount = (int)(*env)->CallIntMethod(env, loc[certPathList], g_CollectionSize); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); if (certsLen < certCount + 1) goto cleanup; abort_if_invalid_pointer_argument (certs); + // Populate `certs`, tracking how many entries we've stored (`added`) so that any failure + // can roll them back -- the caller must never observe a partially populated array. // for (int i = 0; i < certPathList.size(); ++i) { // Certificate cert = certPathList.get(i); // certs[i] = cert; // } - int32_t i; - for (i = 0; i < certCount; ++i) + for (int32_t i = 0; i < certCount; ++i) { - jobject cert = (*env)->CallObjectMethod(env, certPathList, g_ListGet, i); + loc[cert] = (*env)->CallObjectMethod(env, loc[certPathList], g_ListGet, i); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - certs[i] = ToGRef(env, cert); + certs[added++] = ToGRef(env, loc[cert]); + loc[cert] = NULL; } // Certificate trustedCert = trustAnchor.getTrustedCert(); // certs[i] = trustedCert; - jobject trustedCert = (*env)->CallObjectMethod(env, ctx->trustAnchor, g_TrustAnchorGetTrustedCert); - if (i == 0 || !(*env)->IsSameObject(env, certs[i-1], trustedCert)) - { - certs[i] = ToGRef(env, trustedCert); - ret = i + 1; - } - else + loc[trustedCert] = (*env)->CallObjectMethod(env, ctx->trustAnchor, g_TrustAnchorGetTrustedCert); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + if (added == 0 || !(*env)->IsSameObject(env, certs[added - 1], loc[trustedCert])) { - ret = i; - certs[i] = NULL; + certs[added++] = AddGRef(env, loc[trustedCert]); } + ret = added; + cleanup: - (*env)->DeleteLocalRef(env, certPathList); + if (ret == FAIL) + { + // Release any global refs we already stored and clear the slots so the caller never + // observes a partially populated array when we fail partway through. + for (int32_t i = 0; i < added; ++i) + { + ReleaseGRef(env, certs[i]); + certs[i] = NULL; + } + } + RELEASE_LOCALS(loc, env); return ret; } @@ -230,12 +261,21 @@ int32_t AndroidCryptoNative_X509ChainGetErrorCount(X509ChainContext* ctx) abort_unless(ctx->errorList != NULL, "errorList is NULL in X509ChainContext"); JNIEnv* env = GetJNIEnv(); - int32_t count = (*env)->CallIntMethod(env, ctx->errorList, g_CollectionSize); + int32_t count = -1; + int32_t errorCount = 0; + int32_t revocationErrorCount = 0; + + errorCount = (*env)->CallIntMethod(env, ctx->errorList, g_CollectionSize); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); if (ctx->revocationErrorList != NULL) { - count += (*env)->CallIntMethod(env, ctx->revocationErrorList, g_CollectionSize); + revocationErrorCount = (*env)->CallIntMethod(env, ctx->revocationErrorList, g_CollectionSize); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); } + count = errorCount + revocationErrorCount; + +cleanup: return count; } @@ -310,34 +350,40 @@ static PAL_X509ChainStatusFlags ChainStatusFromValidatorExceptionReason(JNIEnv* return PAL_X509ChainPartialChain; } -static void PopulateValidationError(JNIEnv* env, jobject error, bool isRevocationError, ValidationError* out) +static int32_t PopulateValidationError(JNIEnv* env, jobject error, bool isRevocationError, ValidationError* out) { + int32_t ret = FAIL; int index = -1; PAL_X509ChainStatusFlags chainStatus = PAL_X509ChainNoError; + uint16_t* messagePtr = NULL; + INIT_LOCALS(loc, reason, message); + if ((*env)->IsInstanceOf(env, error, g_CertPathValidatorExceptionClass)) { index = (*env)->CallIntMethod(env, error, g_CertPathValidatorExceptionGetIndex); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); // Get the reason and convert it to a chain status flag. - jobject reason = (*env)->CallObjectMethod(env, error, g_CertPathValidatorExceptionGetReason); - chainStatus = ChainStatusFromValidatorExceptionReason(env, reason); - (*env)->DeleteLocalRef(env, reason); + loc[reason] = (*env)->CallObjectMethod(env, error, g_CertPathValidatorExceptionGetReason); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + chainStatus = ChainStatusFromValidatorExceptionReason(env, loc[reason]); } else { chainStatus = isRevocationError ? PAL_X509ChainRevocationStatusUnknown : PAL_X509ChainPartialChain; } - jobject message = (*env)->CallObjectMethod(env, error, g_ThrowableGetMessage); - uint16_t* messagePtr = NULL; - if (message != NULL) + loc[message] = (*env)->CallObjectMethod(env, error, g_ThrowableGetMessage); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + if (loc[message] != NULL) { - jsize messageLen = message == NULL ? 0 : (*env)->GetStringLength(env, message); + jsize messageLen = (*env)->GetStringLength(env, loc[message]); // +1 for null terminator messagePtr = xmalloc(sizeof(uint16_t) * (size_t)(messageLen + 1)); messagePtr[messageLen] = '\0'; - (*env)->GetStringRegion(env, message, 0, messageLen, (jchar*)messagePtr); + (*env)->GetStringRegion(env, loc[message], 0, messageLen, (jchar*)messagePtr); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); } // If the error is known to be from revocation checking, but couldn't be mapped to a revocation status, @@ -350,8 +396,13 @@ static void PopulateValidationError(JNIEnv* env, jobject error, bool isRevocatio out->message = messagePtr; out->index = index; out->chainStatus = chainStatus; + messagePtr = NULL; + ret = SUCCESS; - (*env)->DeleteLocalRef(env, message); +cleanup: + free(messagePtr); + RELEASE_LOCALS(loc, env); + return ret; } int32_t AndroidCryptoNative_X509ChainGetErrors(X509ChainContext* ctx, ValidationError* errors, int32_t errorsLen) @@ -362,10 +413,15 @@ int32_t AndroidCryptoNative_X509ChainGetErrors(X509ChainContext* ctx, Validation JNIEnv* env = GetJNIEnv(); int32_t ret = FAIL; + int32_t populated = 0; + int32_t errorCount = 0; + int32_t revocationErrorCount = 0; - int32_t errorCount = (*env)->CallIntMethod(env, ctx->errorList, g_CollectionSize); - int32_t revocationErrorCount = + errorCount = (*env)->CallIntMethod(env, ctx->errorList, g_CollectionSize); + ON_EXCEPTION_PRINT_AND_GOTO(exit); + revocationErrorCount = ctx->revocationErrorList == NULL ? 0 : (*env)->CallIntMethod(env, ctx->revocationErrorList, g_CollectionSize); + ON_EXCEPTION_PRINT_AND_GOTO(exit); if (errorsLen < errorCount + revocationErrorCount) goto exit; @@ -380,8 +436,11 @@ int32_t AndroidCryptoNative_X509ChainGetErrors(X509ChainContext* ctx, Validation { jobject error = (*env)->CallObjectMethod(env, ctx->errorList, g_ListGet, i); ON_EXCEPTION_PRINT_AND_GOTO(exit); - PopulateValidationError(env, error, false /*isRevocationError*/, &errors[i]); + int32_t status = PopulateValidationError(env, error, false /*isRevocationError*/, &errors[populated]); (*env)->DeleteLocalRef(env, error); + if (status != SUCCESS) + goto exit; + populated++; } // for (int i = 0; i < revocationErrorList.size(); ++i) { @@ -393,14 +452,27 @@ int32_t AndroidCryptoNative_X509ChainGetErrors(X509ChainContext* ctx, Validation { jobject error = (*env)->CallObjectMethod(env, ctx->revocationErrorList, g_ListGet, i); ON_EXCEPTION_PRINT_AND_GOTO(exit); - PopulateValidationError(env, error, true /*isRevocationError*/, &errors[errorCount + i]); + int32_t status = PopulateValidationError(env, error, true /*isRevocationError*/, &errors[populated]); (*env)->DeleteLocalRef(env, error); + if (status != SUCCESS) + goto exit; + populated++; } } ret = SUCCESS; exit: + if (ret != SUCCESS) + { + // The managed caller only frees ValidationError.message on the success path, so release any + // buffers already populated here to avoid leaking them when we fail partway through. + for (int32_t i = 0; i < populated; ++i) + { + free(errors[i].message); + errors[i].message = NULL; + } + } return ret; } @@ -414,24 +486,34 @@ int32_t AndroidCryptoNative_X509ChainSetCustomTrustStore(X509ChainContext* ctx, } JNIEnv* env = GetJNIEnv(); + int32_t ret = FAIL; + INIT_LOCALS(loc, anchors, anchor); + // HashSet anchors = new HashSet(customTrustStoreLen); // for (Certificate cert : customTrustStore) { // TrustAnchor anchor = new TrustAnchor(cert, null); // anchors.Add(anchor); // } - jobject anchors = (*env)->NewObject(env, g_HashSetClass, g_HashSetCtorWithCapacity, customTrustStoreLen); + loc[anchors] = (*env)->NewObject(env, g_HashSetClass, g_HashSetCtorWithCapacity, customTrustStoreLen); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); for (int i = 0; i < customTrustStoreLen; ++i) { - jobject anchor = (*env)->NewObject(env, g_TrustAnchorClass, g_TrustAnchorCtor, customTrustStore[i], NULL); - (*env)->CallBooleanMethod(env, anchors, g_HashSetAdd, anchor); - (*env)->DeleteLocalRef(env, anchor); + loc[anchor] = (*env)->NewObject(env, g_TrustAnchorClass, g_TrustAnchorCtor, customTrustStore[i], NULL); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + (*env)->CallBooleanMethod(env, loc[anchors], g_HashSetAdd, loc[anchor]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + ReleaseLRef(env, loc[anchor]); + loc[anchor] = NULL; } // params.setTrustAnchors(anchors); - (*env)->CallVoidMethod(env, ctx->params, g_PKIXBuilderParametersSetTrustAnchors, anchors); + (*env)->CallVoidMethod(env, ctx->params, g_PKIXBuilderParametersSetTrustAnchors, loc[anchors]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + ret = SUCCESS; - (*env)->DeleteLocalRef(env, anchors); - return CheckJNIExceptions(env) ? FAIL : SUCCESS; +cleanup: + RELEASE_LOCALS(loc, env); + return ret; } static jobject /*CertPath*/ CreateCertPathFromAnchor(JNIEnv* env, jobject /*TrustAnchor*/ trustAnchor) @@ -441,11 +523,14 @@ static jobject /*CertPath*/ CreateCertPathFromAnchor(JNIEnv* env, jobject /*Trus // ArrayList certList = new ArrayList(1); loc[certList] = (*env)->NewObject(env, g_ArrayListClass, g_ArrayListCtorWithCapacity, 1); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); // Certificate trustedCert = trustAnchor.getTrustedCert(); // certList.add(trustedCert); loc[trustedCert] = (*env)->CallObjectMethod(env, trustAnchor, g_TrustAnchorGetTrustedCert); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); (*env)->CallBooleanMethod(env, loc[certList], g_ArrayListAdd, loc[trustedCert]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); // CertificateFactory certFactory = CertificateFactory.getInstance("X.509"); loc[certFactoryType] = make_java_string(env, "X.509"); @@ -475,7 +560,7 @@ static int32_t ValidateWithRevocation(JNIEnv* env, abort_if_invalid_pointer_argument (validator); int32_t ret = FAIL; - INIT_LOCALS(loc, certPathFromAnchor, options, checker, result, ex); + INIT_LOCALS(loc, certPathList, certPathFromAnchor, options, checker, result, ex, revocationErrorList, endOnly); if (revocationMode == X509RevocationMode_Offline) { @@ -495,14 +580,18 @@ static int32_t ValidateWithRevocation(JNIEnv* env, { // List certPathList = certPath.getCertificates(); // int certCount = certPathList.size(); - jobject certPathList = (*env)->CallObjectMethod(env, ctx->certPath, g_CertPathGetCertificates); - int certCount = (int)(*env)->CallIntMethod(env, certPathList, g_CollectionSize); + loc[certPathList] = (*env)->CallObjectMethod(env, ctx->certPath, g_CertPathGetCertificates); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + int certCount = (int)(*env)->CallIntMethod(env, loc[certPathList], g_CollectionSize); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); if (certCount == 0) { // If the chain only consists only of the trust anchor, create a path with just // the trust anchor for revocation checking for end certificate only. This should // still pass normal (non-revocation) validation when it is the only certificate. loc[certPathFromAnchor] = CreateCertPathFromAnchor(env, ctx->trustAnchor); + if (loc[certPathFromAnchor] == NULL) + goto cleanup; certPathToUse = loc[certPathFromAnchor]; } else @@ -515,13 +604,13 @@ static int32_t ValidateWithRevocation(JNIEnv* env, // HashSet options = new HashSet(3); // options.add(PKIXRevocationChecker.Option.ONLY_END_ENTITY); loc[options] = (*env)->NewObject(env, g_HashSetClass, g_HashSetCtorWithCapacity, 3); - jobject endOnly = (*env)->GetStaticObjectField( + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + loc[endOnly] = (*env)->GetStaticObjectField( env, g_PKIXRevocationCheckerOptionClass, g_PKIXRevocationCheckerOptionOnlyEndEntity); - (*env)->CallBooleanMethod(env, loc[options], g_HashSetAdd, endOnly); - (*env)->DeleteLocalRef(env, endOnly); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + (*env)->CallBooleanMethod(env, loc[options], g_HashSetAdd, loc[endOnly]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); } - - (*env)->DeleteLocalRef(env, certPathList); } else { @@ -540,23 +629,30 @@ static int32_t ValidateWithRevocation(JNIEnv* env, { // checker.setOptions(options); (*env)->CallVoidMethod(env, loc[checker], g_PKIXRevocationCheckerSetOptions, loc[options]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); } // params.addCertPathChecker(checker); (*env)->CallVoidMethod(env, params, g_PKIXBuilderParametersAddCertPathChecker, loc[checker]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); // params.setRevocationEnabled(true); // PKIXCertPathValidatorResult result = (PKIXCertPathValidatorResult)validator.validate(certPathToUse, params); (*env)->CallVoidMethod(env, params, g_PKIXBuilderParametersSetRevocationEnabled, true); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); loc[result] = (*env)->CallObjectMethod(env, validator, g_CertPathValidatorValidate, certPathToUse, params); if (TryGetJNIException(env, &loc[ex], false /*printException*/)) { if (ctx->revocationErrorList == NULL) { - ctx->revocationErrorList = ToGRef(env, (*env)->NewObject(env, g_ArrayListClass, g_ArrayListCtor)); + loc[revocationErrorList] = (*env)->NewObject(env, g_ArrayListClass, g_ArrayListCtor); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + ctx->revocationErrorList = AddGRef(env, loc[revocationErrorList]); } (*env)->CallBooleanMethod(env, ctx->revocationErrorList, g_ArrayListAdd, loc[ex]); + // Clear any exception raised while recording the revocation failure so it doesn't leak to managed code. + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); } ret = SUCCESS; @@ -592,6 +688,8 @@ int32_t AndroidCryptoNative_X509ChainValidate(X509ChainContext* ctx, if (TryGetJNIException(env, &loc[ex], false /*printException*/)) { (*env)->CallBooleanMethod(env, ctx->errorList, g_ArrayListAdd, loc[ex]); + // Clear any exception raised while recording the validation failure so it doesn't leak to managed code. + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); ret = SUCCESS; } else if (revocationMode != X509RevocationMode_NoCheck)