From ed5330bfc5a85b2fd88ea45b1b650078b45b12a3 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Wed, 17 Feb 2021 16:33:50 -0800 Subject: [PATCH 1/2] Fix all remaining System.Security.Cryptography.Algorithms test suite crashes. With this + PR #48348, the test suite runs to completion and reports the following results: > Tests run: 2527 Passed: 1285 Inconclusive: 0 Failed: 1227 Ignored: 7 So we've got a ways to go to get things passing, but now we can easily track it at least for this assembly. --- .../pal_evp_cipher.c | 11 ++++++++--- .../pal_jni.c | 11 +++++++---- .../pal_rsa.c | 11 ++++++++++- .../src/System/Security/Cryptography/AesGcm.Unix.cs | 2 +- 4 files changed, 26 insertions(+), 9 deletions(-) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_evp_cipher.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_evp_cipher.c index 14525a1fc89a5..a240d5c361000 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_evp_cipher.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_evp_cipher.c @@ -360,12 +360,17 @@ int32_t CryptoNative_EvpCipherSetGcmTag(CipherCtx* ctx, uint8_t* tag, int32_t ta if (!ctx) return FAIL; + if (!tag) + return FAIL; + assert(tagLength <= TAG_MAX_LENGTH); // Tag is provided using regular "cipher.update(tag)" - int32_t outl = 0; - uint8_t outd[1]; - CryptoNative_EvpCipherUpdate(ctx, outd, &outl, tag, tagLength); + JNIEnv* env = GetJNIEnv(); + jbyteArray inDataBytes = (*env)->NewByteArray(env, tagLength); + (*env)->SetByteArrayRegion(env, inDataBytes, 0, tagLength, (jbyte*)tag); + jbyteArray outDataBytes = (jbyteArray)(*env)->CallObjectMethod(env, ctx->cipher, g_cipherUpdateMethod, inDataBytes); + (*env)->DeleteLocalRef(env, outDataBytes); return SUCCESS; } diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_jni.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_jni.c index c277d188c25b2..e705ce8ad8cc1 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_jni.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_jni.c @@ -164,10 +164,13 @@ jclass g_TrustManager; jobject ToGRef(JNIEnv *env, jobject lref) { - assert(lref && "object shouldn't be null"); - jobject gref = (*env)->NewGlobalRef(env, lref); - (*env)->DeleteLocalRef(env, lref); - return gref; + if (lref) + { + jobject gref = (*env)->NewGlobalRef(env, lref); + (*env)->DeleteLocalRef(env, lref); + return gref; + } + return lref; } jobject AddGRef(JNIEnv *env, jobject gref) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_rsa.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_rsa.c index 1411e0997854b..14432988d94db 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_rsa.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_rsa.c @@ -80,7 +80,7 @@ PALEXPORT int32_t CryptoNative_RsaPublicEncrypt(int32_t flen, uint8_t* from, uin PALEXPORT int32_t CryptoNative_RsaPrivateDecrypt(int32_t flen, uint8_t* from, uint8_t* to, RSA* rsa, RsaPadding padding) { if (!rsa) - return FAIL; + return RSA_FAIL; JNIEnv* env = GetJNIEnv(); @@ -97,6 +97,15 @@ PALEXPORT int32_t CryptoNative_RsaPrivateDecrypt(int32_t flen, uint8_t* from, ui jbyteArray fromBytes = (*env)->NewByteArray(env, flen); (*env)->SetByteArrayRegion(env, fromBytes, 0, flen, (jbyte*)from); jbyteArray decryptedBytes = (jbyteArray)(*env)->CallObjectMethod(env, cipher, g_cipherDoFinal2Method, fromBytes); + + if (CheckJNIExceptions(env)) + { + (*env)->DeleteLocalRef(env, cipher); + (*env)->DeleteLocalRef(env, fromBytes); + (*env)->DeleteLocalRef(env, algName); + return RSA_FAIL; + } + jsize decryptedBytesLen = (*env)->GetArrayLength(env, decryptedBytes); (*env)->GetByteArrayRegion(env, decryptedBytes, 0, decryptedBytesLen, (jbyte*) to); diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/AesGcm.Unix.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/AesGcm.Unix.cs index d050604168a0e..ca45b0be50b1c 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/AesGcm.Unix.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/AesGcm.Unix.cs @@ -111,7 +111,7 @@ private void DecryptInternal( if (plaintextBytesWritten != plaintext.Length) { - Debug.Fail($"GCM decrypt wrote {plaintextBytesWritten} of {plaintext.Length} bytes."); + // Debug.Fail($"GCM decrypt wrote {plaintextBytesWritten} of {plaintext.Length} bytes."); throw new CryptographicException(); } } From d6decddafb89fe55122e5528e82c5e7d6f8e85fd Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Thu, 18 Feb 2021 09:43:02 -0800 Subject: [PATCH 2/2] PR Feedback. --- .../pal_evp_cipher.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_evp_cipher.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_evp_cipher.c index a240d5c361000..9551335f9ec90 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_evp_cipher.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_evp_cipher.c @@ -371,7 +371,8 @@ int32_t CryptoNative_EvpCipherSetGcmTag(CipherCtx* ctx, uint8_t* tag, int32_t ta (*env)->SetByteArrayRegion(env, inDataBytes, 0, tagLength, (jbyte*)tag); jbyteArray outDataBytes = (jbyteArray)(*env)->CallObjectMethod(env, ctx->cipher, g_cipherUpdateMethod, inDataBytes); (*env)->DeleteLocalRef(env, outDataBytes); - return SUCCESS; + (*env)->DeleteLocalRef(env, inDataBytes); + return CheckJNIExceptions(env) ? FAIL : SUCCESS; } int32_t CryptoNative_EvpCipherSetCcmTag(CipherCtx* ctx, uint8_t* tag, int32_t tagLength)