From f431d1704668259800673f13c90474c4c99e9cb4 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Tue, 6 Sep 2022 14:22:05 -0400 Subject: [PATCH 1/8] Throw AuthenticationTagMismatchException for AEAD tag mismatches --- .../ref/System.Security.Cryptography.cs | 6 ++++++ .../src/System.Security.Cryptography.csproj | 1 + .../System/Security/Cryptography/AeadCommon.Windows.cs | 2 +- .../src/System/Security/Cryptography/AesCcm.Android.cs | 2 +- .../src/System/Security/Cryptography/AesCcm.OpenSsl.cs | 2 +- .../src/System/Security/Cryptography/AesGcm.Android.cs | 2 +- .../src/System/Security/Cryptography/AesGcm.OpenSsl.cs | 2 +- .../Security/Cryptography/ChaCha20Poly1305.Android.cs | 2 +- .../Security/Cryptography/ChaCha20Poly1305.OpenSsl.cs | 2 +- .../System.Security.Cryptography/tests/AesCcmTests.cs | 8 ++++---- .../System.Security.Cryptography/tests/AesGcmTests.cs | 6 +++--- .../tests/ChaCha20Poly1305Tests.cs | 6 +++--- 12 files changed, 24 insertions(+), 17 deletions(-) diff --git a/src/libraries/System.Security.Cryptography/ref/System.Security.Cryptography.cs b/src/libraries/System.Security.Cryptography/ref/System.Security.Cryptography.cs index 5be736fec669ef..53539093d0ec51 100644 --- a/src/libraries/System.Security.Cryptography/ref/System.Security.Cryptography.cs +++ b/src/libraries/System.Security.Cryptography/ref/System.Security.Cryptography.cs @@ -268,6 +268,12 @@ protected AsymmetricSignatureFormatter() { } public abstract void SetHashAlgorithm(string strName); public abstract void SetKey(System.Security.Cryptography.AsymmetricAlgorithm key); } + public sealed partial class AuthenticationTagMismatchException : System.Security.Cryptography.CryptographicException + { + public AuthenticationTagMismatchException() { } + public AuthenticationTagMismatchException(string? message) { } + public AuthenticationTagMismatchException(string? message, System.Exception? inner) { } + } [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")] [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] diff --git a/src/libraries/System.Security.Cryptography/src/System.Security.Cryptography.csproj b/src/libraries/System.Security.Cryptography/src/System.Security.Cryptography.csproj index cf25738f6e7bab..7deb4747a592c1 100644 --- a/src/libraries/System.Security.Cryptography/src/System.Security.Cryptography.csproj +++ b/src/libraries/System.Security.Cryptography/src/System.Security.Cryptography.csproj @@ -283,6 +283,7 @@ + diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AeadCommon.Windows.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AeadCommon.Windows.cs index fe10a0ed2ca10e..909f650a7aa3c2 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AeadCommon.Windows.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AeadCommon.Windows.cs @@ -104,7 +104,7 @@ public static unsafe void Decrypt( CryptographicOperations.ZeroMemory(plaintext); } - throw new CryptographicException(SR.Cryptography_AuthTagMismatch); + throw new AuthenticationTagMismatchException(); default: throw CreateCryptographicException(ntStatus); } diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesCcm.Android.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesCcm.Android.cs index 39247bed98e25d..ab736f287c67fb 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesCcm.Android.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesCcm.Android.cs @@ -147,7 +147,7 @@ private void DecryptCore( out bytesWritten)) { CryptographicOperations.ZeroMemory(plaintext); - throw new CryptographicException(SR.Cryptography_AuthTagMismatch); + throw new AuthenticationTagMismatchException(); } plaintextBytesWritten += bytesWritten; diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesCcm.OpenSsl.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesCcm.OpenSsl.cs index 6c78bee075e3a2..0aedd34fe2d1b3 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesCcm.OpenSsl.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesCcm.OpenSsl.cs @@ -104,7 +104,7 @@ private void DecryptCore( if (!Interop.Crypto.EvpCipherUpdate(ctx, plaintext, out int plaintextBytesWritten, ciphertext)) { plaintext.Clear(); - throw new CryptographicException(SR.Cryptography_AuthTagMismatch); + throw new AuthenticationTagMismatchException(); } if (plaintextBytesWritten != plaintext.Length) diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.Android.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.Android.cs index 3e36ef16d38f02..4db38f797331d7 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.Android.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.Android.cs @@ -147,7 +147,7 @@ private void DecryptCore( out bytesWritten)) { CryptographicOperations.ZeroMemory(plaintext); - throw new CryptographicException(SR.Cryptography_AuthTagMismatch); + throw new AuthenticationTagMismatchException(); } plaintextBytesWritten += bytesWritten; diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.OpenSsl.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.OpenSsl.cs index 1691f3357dbc2d..f11936951bc9d3 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.OpenSsl.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.OpenSsl.cs @@ -106,7 +106,7 @@ private void DecryptCore( out int bytesWritten)) { CryptographicOperations.ZeroMemory(plaintext); - throw new CryptographicException(SR.Cryptography_AuthTagMismatch); + throw new AuthenticationTagMismatchException(); } plaintextBytesWritten += bytesWritten; diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ChaCha20Poly1305.Android.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ChaCha20Poly1305.Android.cs index a36599202e3d1d..2ac7d255edbbc9 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ChaCha20Poly1305.Android.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ChaCha20Poly1305.Android.cs @@ -139,7 +139,7 @@ private void DecryptCore( out bytesWritten)) { CryptographicOperations.ZeroMemory(plaintext); - throw new CryptographicException(SR.Cryptography_AuthTagMismatch); + throw new AuthenticationTagMismatchException(); } plaintextBytesWritten += bytesWritten; diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ChaCha20Poly1305.OpenSsl.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ChaCha20Poly1305.OpenSsl.cs index 6fd211689ae82a..353f7429403228 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ChaCha20Poly1305.OpenSsl.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ChaCha20Poly1305.OpenSsl.cs @@ -106,7 +106,7 @@ private void DecryptCore( out int bytesWritten)) { CryptographicOperations.ZeroMemory(plaintext); - throw new CryptographicException(SR.Cryptography_AuthTagMismatch); + throw new AuthenticationTagMismatchException(); } plaintextBytesWritten += bytesWritten; diff --git a/src/libraries/System.Security.Cryptography/tests/AesCcmTests.cs b/src/libraries/System.Security.Cryptography/tests/AesCcmTests.cs index 1d662f9a085c85..8b68d7f1d61e3f 100644 --- a/src/libraries/System.Security.Cryptography/tests/AesCcmTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/AesCcmTests.cs @@ -34,7 +34,7 @@ public static void EncryptTamperAADDecrypt(int dataLength, int additionalDataLen additionalData[0] ^= 1; byte[] decrypted = new byte[dataLength]; - Assert.Throws( + Assert.Throws( () => aesCcm.Decrypt(nonce, ciphertext, tag, decrypted, additionalData)); } } @@ -305,7 +305,7 @@ public static void InplaceEncryptTamperTagDecrypt() tag[0] ^= 1; - Assert.Throws( + Assert.Throws( () => aesCcm.Decrypt(nonce, data, tag, data)); Assert.Equal(new byte[data.Length], data); } @@ -347,7 +347,7 @@ public static void AesCcmNistTestsTamperTag(AEADTest testCase) byte[] plaintext = new byte[testCase.Plaintext.Length]; RandomNumberGenerator.Fill(plaintext); - Assert.Throws( + Assert.Throws( () => aesCcm.Decrypt(testCase.Nonce, ciphertext, tag, plaintext, testCase.AssociatedData)); Assert.Equal(new byte[plaintext.Length], plaintext); } @@ -370,7 +370,7 @@ public static void AesCcmNistTestsTamperCiphertext(AEADTest testCase) byte[] plaintext = new byte[testCase.Plaintext.Length]; RandomNumberGenerator.Fill(plaintext); - Assert.Throws( + Assert.Throws( () => aesCcm.Decrypt(testCase.Nonce, ciphertext, tag, plaintext, testCase.AssociatedData)); Assert.Equal(new byte[plaintext.Length], plaintext); } diff --git a/src/libraries/System.Security.Cryptography/tests/AesGcmTests.cs b/src/libraries/System.Security.Cryptography/tests/AesGcmTests.cs index 1a41a2beb3f83d..1558f266c61e84 100644 --- a/src/libraries/System.Security.Cryptography/tests/AesGcmTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/AesGcmTests.cs @@ -34,7 +34,7 @@ public static void EncryptTamperAADDecrypt(int dataLength, int additionalDataLen additionalData[0] ^= 1; byte[] decrypted = new byte[dataLength]; - Assert.Throws( + Assert.Throws( () => aesGcm.Decrypt(nonce, ciphertext, tag, decrypted, additionalData)); } } @@ -354,7 +354,7 @@ public static void AesGcmNistTestsTamperTag(AEADTest testCase) byte[] plaintext = new byte[testCase.Plaintext.Length]; RandomNumberGenerator.Fill(plaintext); - Assert.Throws( + Assert.Throws( () => aesGcm.Decrypt(testCase.Nonce, ciphertext, tag, plaintext, testCase.AssociatedData)); Assert.Equal(new byte[plaintext.Length], plaintext); } @@ -377,7 +377,7 @@ public static void AesGcmNistTestsTamperCiphertext(AEADTest testCase) byte[] plaintext = new byte[testCase.Plaintext.Length]; RandomNumberGenerator.Fill(plaintext); - Assert.Throws( + Assert.Throws( () => aesGcm.Decrypt(testCase.Nonce, ciphertext, tag, plaintext, testCase.AssociatedData)); Assert.Equal(new byte[plaintext.Length], plaintext); } diff --git a/src/libraries/System.Security.Cryptography/tests/ChaCha20Poly1305Tests.cs b/src/libraries/System.Security.Cryptography/tests/ChaCha20Poly1305Tests.cs index 7d39b45922fb32..f381e17e73d190 100644 --- a/src/libraries/System.Security.Cryptography/tests/ChaCha20Poly1305Tests.cs +++ b/src/libraries/System.Security.Cryptography/tests/ChaCha20Poly1305Tests.cs @@ -35,7 +35,7 @@ public static void EncryptTamperAADDecrypt(int dataLength, int additionalDataLen additionalData[0] ^= 1; byte[] decrypted = new byte[dataLength]; - Assert.Throws( + Assert.Throws( () => chaChaPoly.Decrypt(nonce, ciphertext, tag, decrypted, additionalData)); } } @@ -271,7 +271,7 @@ public static void InplaceEncryptTamperTagDecrypt() tag[0] ^= 1; - Assert.Throws( + Assert.Throws( () => chaChaPoly.Decrypt(nonce, data, tag, data)); Assert.Equal(new byte[data.Length], data); } @@ -310,7 +310,7 @@ public static void Rfc8439TestsTamperTag(AEADTest testCase) tag[0] ^= 1; byte[] plaintext = RandomNumberGenerator.GetBytes(testCase.Plaintext.Length); - Assert.Throws( + Assert.Throws( () => chaChaPoly.Decrypt(testCase.Nonce, ciphertext, tag, plaintext, testCase.AssociatedData)); Assert.Equal(new byte[plaintext.Length], plaintext); } From cebac827ff2563b9305a17abe02556eef92eb2cf Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Tue, 6 Sep 2022 15:39:01 -0400 Subject: [PATCH 2/8] Properly handle Android --- .../Interop.Cipher.cs | 20 ++++++++ .../Security/Cryptography/AesCcm.Android.cs | 19 +++++-- .../Security/Cryptography/AesGcm.Android.cs | 19 +++++-- .../AuthenticationTagMismatchException.cs | 47 +++++++++++++++++ .../Cryptography/ChaCha20Poly1305.Android.cs | 19 +++++-- .../pal_cipher.c | 51 +++++++++++++++++++ .../pal_cipher.h | 1 + 7 files changed, 161 insertions(+), 15 deletions(-) create mode 100644 src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AuthenticationTagMismatchException.cs diff --git a/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.Cipher.cs b/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.Cipher.cs index 8b987ce31ea5c0..dc9fa576936491 100644 --- a/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.Cipher.cs +++ b/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.Cipher.cs @@ -136,6 +136,26 @@ internal static bool EvpCipherFinalEx( return EvpCipherFinalEx(ctx, ref MemoryMarshal.GetReference(output), out bytesWritten); } + [LibraryImport(Libraries.AndroidCryptoNative, EntryPoint = "AndroidCryptoNative_AeadCipherFinalEx")] + [return: MarshalAs(UnmanagedType.Bool)] + private static unsafe partial bool EvpAeadCipherFinalEx( + SafeEvpCipherCtxHandle ctx, + byte* outm, + out int outl, + [MarshalAs(UnmanagedType.Bool)] out bool authTagMismatch); + + internal static unsafe bool EvpAeadCipherFinalEx( + SafeEvpCipherCtxHandle ctx, + Span output, + out int bytesWritten, + out bool authTagMismatch) + { + fixed (byte* pOutput = output) + { + return EvpAeadCipherFinalEx(ctx, pOutput, out bytesWritten, out authTagMismatch); + } + } + [LibraryImport(Libraries.AndroidCryptoNative, EntryPoint = "AndroidCryptoNative_CipherSetTagLength")] [return: MarshalAs(UnmanagedType.Bool)] internal static partial bool CipherSetTagLength( diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesCcm.Android.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesCcm.Android.cs index ab736f287c67fb..b7089bd3d101d7 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesCcm.Android.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesCcm.Android.cs @@ -69,11 +69,13 @@ private void EncryptCore( throw new CryptographicException(); } - if (!Interop.Crypto.EvpCipherFinalEx( + if (!Interop.Crypto.EvpAeadCipherFinalEx( ctx, ciphertextAndTag.Slice(ciphertextBytesWritten), - out int bytesWritten)) + out int bytesWritten, + out bool authTagMismatch)) { + Debug.Assert(!authTagMismatch); throw new CryptographicException(); } @@ -141,13 +143,20 @@ private void DecryptCore( plaintextBytesWritten += bytesWritten; - if (!Interop.Crypto.EvpCipherFinalEx( + if (!Interop.Crypto.EvpAeadCipherFinalEx( ctx, plaintext.Slice(plaintextBytesWritten), - out bytesWritten)) + out bytesWritten, + out bool authTagMismatch)) { CryptographicOperations.ZeroMemory(plaintext); - throw new AuthenticationTagMismatchException(); + + if (authTagMismatch) + { + throw new AuthenticationTagMismatchException(); + } + + throw new CryptographicException(SR.Arg_CryptographyException); } plaintextBytesWritten += bytesWritten; diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.Android.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.Android.cs index 4db38f797331d7..471338a8e8a76c 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.Android.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.Android.cs @@ -74,11 +74,13 @@ private void EncryptCore( throw new CryptographicException(); } - if (!Interop.Crypto.EvpCipherFinalEx( + if (!Interop.Crypto.EvpAeadCipherFinalEx( _ctxHandle, ciphertextAndTag.Slice(ciphertextBytesWritten), - out int bytesWritten)) + out int bytesWritten, + out bool authTagMismatch)) { + Debug.Assert(!authTagMismatch); throw new CryptographicException(); } @@ -141,13 +143,20 @@ private void DecryptCore( plaintextBytesWritten += bytesWritten; - if (!Interop.Crypto.EvpCipherFinalEx( + if (!Interop.Crypto.EvpAeadCipherFinalEx( _ctxHandle, plaintext.Slice(plaintextBytesWritten), - out bytesWritten)) + out bytesWritten, + out bool authTagMismatch)) { CryptographicOperations.ZeroMemory(plaintext); - throw new AuthenticationTagMismatchException(); + + if (authTagMismatch) + { + throw new AuthenticationTagMismatchException(); + } + + throw new CryptographicException(SR.Arg_CryptographyException); } plaintextBytesWritten += bytesWritten; diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AuthenticationTagMismatchException.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AuthenticationTagMismatchException.cs new file mode 100644 index 00000000000000..6c63045da0f130 --- /dev/null +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AuthenticationTagMismatchException.cs @@ -0,0 +1,47 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Security.Cryptography +{ + /// + /// The exception that is thrown when a decryption operation with an authenticated cipher + /// has an authentication tag mismatch. + /// + public sealed class AuthenticationTagMismatchException : CryptographicException + { + /// + /// Initializes a new instance of the class with default + /// properties. + /// + public AuthenticationTagMismatchException() : base(SR.Cryptography_AuthTagMismatch) + { + } + + /// + /// Initializes a new instance of the class with a specified + /// error message. + /// + /// + /// The error message that explains the reason for the exception. + /// + public AuthenticationTagMismatchException(string? message) : base(message) + { + } + + /// + /// Initializes a new instance of the class with a specified + /// error message and a reference to the inner exception that is the cause of this exception. + /// + /// + /// The error message that explains the reason for the exception. + /// + /// + /// The exception that is the cause of the current exception. If the parameter is not + /// , the current exception is raised in a catch block that handles the inner exception. + /// + public AuthenticationTagMismatchException(string? message, Exception? inner) + : base(message, inner) + { + } + } +} diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ChaCha20Poly1305.Android.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ChaCha20Poly1305.Android.cs index 2ac7d255edbbc9..3c6f78d5425051 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ChaCha20Poly1305.Android.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ChaCha20Poly1305.Android.cs @@ -71,11 +71,13 @@ private void EncryptCore( throw new CryptographicException(); } - if (!Interop.Crypto.EvpCipherFinalEx( + if (!Interop.Crypto.EvpAeadCipherFinalEx( _ctxHandle, ciphertextAndTag.Slice(ciphertextBytesWritten), - out int bytesWritten)) + out int bytesWritten, + out bool authTagMismatch)) { + Debug.Assert(!authTagMismatch); throw new CryptographicException(); } @@ -133,13 +135,20 @@ private void DecryptCore( plaintextBytesWritten += bytesWritten; - if (!Interop.Crypto.EvpCipherFinalEx( + if (!Interop.Crypto.EvpAeadCipherFinalEx( _ctxHandle, plaintext.Slice(plaintextBytesWritten), - out bytesWritten)) + out bytesWritten, + out bool authTagMismatch)) { CryptographicOperations.ZeroMemory(plaintext); - throw new AuthenticationTagMismatchException(); + + if (authTagMismatch) + { + throw new AuthenticationTagMismatchException(); + } + + throw new CryptographicException(SR.Arg_CryptographyException); } plaintextBytesWritten += bytesWritten; 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 0b52b0a100a8c9..e8fb8e8c7d4c96 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 @@ -299,6 +299,57 @@ int32_t AndroidCryptoNative_CipherFinalEx(CipherCtx* ctx, uint8_t* outm, int32_t return CheckJNIExceptions(env) ? FAIL : SUCCESS; } + +int32_t AndroidCryptoNative_AeadCipherFinalEx(CipherCtx* ctx, uint8_t* outm, int32_t* outl, int32_t* authTagMismatch) +{ + if (!ctx) + return FAIL; + + abort_if_invalid_pointer_argument(outm); + abort_if_invalid_pointer_argument(outl); + abort_if_invalid_pointer_argument(authTagMismatch); + + JNIEnv* env = GetJNIEnv(); + + *outl = 0; + *authTagMismatch = 0; + + jbyteArray outBytes = (jbyteArray)(*env)->CallObjectMethod(env, ctx->cipher, g_cipherDoFinalMethod); + jthrowable ex = NULL; + + if (TryGetJNIException(env, &ex, false)) + { + if (ex == NULL) + { + return FAIL; + } + + jclass aeadException = (*env)->FindClass(env, "javax/crypto/spec/AEADBadTagException"); + + if (aeadException == NULL) + { + (*env)->DeleteLocalRef(env, ex); + return FAIL; + } + + if ((*env)->IsInstanceOf(env, ex, aeadException)) + { + *authTagMismatch = 1; + } + + (*env)->DeleteLocalRef(env, ex); + (*env)->DeleteLocalRef(env, aeadException); + return FAIL; + } + + jsize outBytesLen = (*env)->GetArrayLength(env, outBytes); + *outl = outBytesLen; + (*env)->GetByteArrayRegion(env, outBytes, 0, outBytesLen, (jbyte*) outm); + + (*env)->DeleteLocalRef(env, outBytes); + return CheckJNIExceptions(env) ? FAIL : SUCCESS; +} + int32_t AndroidCryptoNative_CipherCtxSetPadding(CipherCtx* ctx, int32_t padding) { if (!ctx) diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_cipher.h b/src/native/libs/System.Security.Cryptography.Native.Android/pal_cipher.h index 6b9555967a5a73..dc1bbe2211df53 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_cipher.h +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_cipher.h @@ -36,6 +36,7 @@ PALEXPORT int32_t AndroidCryptoNative_CipherCtxSetPadding(CipherCtx* ctx, int32_ PALEXPORT int32_t AndroidCryptoNative_CipherUpdateAAD(CipherCtx* ctx, uint8_t* in, int32_t inl); PALEXPORT int32_t AndroidCryptoNative_CipherUpdate(CipherCtx* ctx, uint8_t* out, int32_t* outl, uint8_t* in, int32_t inl); PALEXPORT int32_t AndroidCryptoNative_CipherFinalEx(CipherCtx* ctx, uint8_t* outm, int32_t* outl); +PALEXPORT int32_t AndroidCryptoNative_AeadCipherFinalEx(CipherCtx* ctx, uint8_t* outm, int32_t* outl, int32_t* authTagMismatch); PALEXPORT CipherInfo* AndroidCryptoNative_Aes128Ecb(void); PALEXPORT CipherInfo* AndroidCryptoNative_Aes128Cbc(void); PALEXPORT CipherInfo* AndroidCryptoNative_Aes128Cfb8(void); From 52359e91ba69b16e3f8c969701f92266e6db43e7 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Tue, 6 Sep 2022 21:38:01 +0000 Subject: [PATCH 3/8] Fix test --- src/libraries/System.Security.Cryptography/tests/AesGcmTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Security.Cryptography/tests/AesGcmTests.cs b/src/libraries/System.Security.Cryptography/tests/AesGcmTests.cs index 1558f266c61e84..6ef5391e40e535 100644 --- a/src/libraries/System.Security.Cryptography/tests/AesGcmTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/AesGcmTests.cs @@ -312,7 +312,7 @@ public static void InplaceEncryptTamperTagDecrypt() tag[0] ^= 1; - Assert.Throws( + Assert.Throws( () => aesGcm.Decrypt(nonce, data, tag, data)); Assert.Equal(new byte[data.Length], data); } From 3f88ad9fe94e82fb99bb6d2603ec5be0bc305a9d Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Tue, 6 Sep 2022 19:23:41 -0400 Subject: [PATCH 4/8] Restart CI From 700da4a19d817e15ba9d860a63713a92585551f9 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Wed, 28 Sep 2022 15:27:43 -0400 Subject: [PATCH 5/8] Keep AEADBadTagException as a gref --- .../pal_cipher.c | 11 +---------- .../pal_jni.c | 5 +++++ .../pal_jni.h | 3 +++ 3 files changed, 9 insertions(+), 10 deletions(-) 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 e8fb8e8c7d4c96..b09932392eea08 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 @@ -324,21 +324,12 @@ int32_t AndroidCryptoNative_AeadCipherFinalEx(CipherCtx* ctx, uint8_t* outm, int return FAIL; } - jclass aeadException = (*env)->FindClass(env, "javax/crypto/spec/AEADBadTagException"); - - if (aeadException == NULL) - { - (*env)->DeleteLocalRef(env, ex); - return FAIL; - } - - if ((*env)->IsInstanceOf(env, ex, aeadException)) + if ((*env)->IsInstanceOf(env, ex, g_AEADBadTagExceptionClass)) { *authTagMismatch = 1; } (*env)->DeleteLocalRef(env, ex); - (*env)->DeleteLocalRef(env, aeadException); return FAIL; } diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_jni.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_jni.c index 7aa8b45768be96..bf8e644766395c 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_jni.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_jni.c @@ -88,6 +88,9 @@ jclass g_sslCtxClass; jmethodID g_sslCtxGetDefaultMethod; jmethodID g_sslCtxGetDefaultSslParamsMethod; +// javax/crypto/spec/AEADBadTagException +jclass g_AEADBadTagExceptionClass; + // javax/crypto/spec/GCMParameterSpec jclass g_GCMParameterSpecClass; jmethodID g_GCMParameterSpecCtor; @@ -704,6 +707,8 @@ JNI_OnLoad(JavaVM *vm, void *reserved) g_ivPsClass = GetClassGRef(env, "javax/crypto/spec/IvParameterSpec"); g_ivPsCtor = GetMethod(env, false, g_ivPsClass, "", "([B)V"); + g_AEADBadTagExceptionClass = GetClassGRef(env, "javax/crypto/spec/AEADBadTagException"); + g_GCMParameterSpecClass = GetClassGRef(env, "javax/crypto/spec/GCMParameterSpec"); g_GCMParameterSpecCtor = GetMethod(env, false, g_GCMParameterSpecClass, "", "(I[B)V"); diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_jni.h b/src/native/libs/System.Security.Cryptography.Native.Android/pal_jni.h index f8d89e2513115c..9294c0e13cb348 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_jni.h +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_jni.h @@ -71,6 +71,9 @@ extern jmethodID g_cipherInitMethod; extern jmethodID g_cipherInit2Method; extern jmethodID g_getBlockSizeMethod; +// javax/crypto/spec/AEADBadTagException +extern jclass g_AEADBadTagExceptionClass; + // javax/crypto/spec/IvParameterSpec extern jclass g_ivPsClass; extern jmethodID g_ivPsCtor; From 8a04abb65b94209e2ae19e8f4f69df5eebf9366a Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Thu, 29 Sep 2022 09:55:28 -0400 Subject: [PATCH 6/8] Update src/native/libs/System.Security.Cryptography.Native.Android/pal_jni.c Co-authored-by: Filip Navara --- .../libs/System.Security.Cryptography.Native.Android/pal_jni.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_jni.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_jni.c index bf8e644766395c..0c4c732b44ef93 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_jni.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_jni.c @@ -707,7 +707,7 @@ JNI_OnLoad(JavaVM *vm, void *reserved) g_ivPsClass = GetClassGRef(env, "javax/crypto/spec/IvParameterSpec"); g_ivPsCtor = GetMethod(env, false, g_ivPsClass, "", "([B)V"); - g_AEADBadTagExceptionClass = GetClassGRef(env, "javax/crypto/spec/AEADBadTagException"); + g_AEADBadTagExceptionClass = GetClassGRef(env, "javax/crypto/AEADBadTagException"); g_GCMParameterSpecClass = GetClassGRef(env, "javax/crypto/spec/GCMParameterSpec"); g_GCMParameterSpecCtor = GetMethod(env, false, g_GCMParameterSpecClass, "", "(I[B)V"); From 4f1c3f8ef7f2a12d24603ecaa47770421cd8ec5a Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Thu, 29 Sep 2022 13:28:54 -0400 Subject: [PATCH 7/8] Do not pass null pointer for empty destinations --- .../Interop.Cipher.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.Cipher.cs b/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.Cipher.cs index dc9fa576936491..6b6f3ae229b4bc 100644 --- a/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.Cipher.cs +++ b/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.Cipher.cs @@ -150,7 +150,14 @@ internal static unsafe bool EvpAeadCipherFinalEx( out int bytesWritten, out bool authTagMismatch) { - fixed (byte* pOutput = output) + scoped Span notNullOutput = output; + + if (notNullOutput.IsEmpty) + { + notNullOutput = (stackalloc byte[1]).Slice(1); + } + + fixed (byte* pOutput = &MemoryMarshal.GetReference(notNullOutput)) { return EvpAeadCipherFinalEx(ctx, pOutput, out bytesWritten, out authTagMismatch); } From 379199708bbcbcb80476d5a58b484b24ad5334fc Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Thu, 29 Sep 2022 17:13:52 -0400 Subject: [PATCH 8/8] Add comment explaining null handling --- .../Interop.Cipher.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.Cipher.cs b/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.Cipher.cs index 6b6f3ae229b4bc..8cc861a948c2ec 100644 --- a/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.Cipher.cs +++ b/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.Cipher.cs @@ -152,6 +152,7 @@ internal static unsafe bool EvpAeadCipherFinalEx( { scoped Span notNullOutput = output; + // We can't pass null down to the native shim, so create a valid pointer if we have an empty span. if (notNullOutput.IsEmpty) { notNullOutput = (stackalloc byte[1]).Slice(1);