From 7b4a0befbe4c458f4f0178bce9feb8c221054b88 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Sat, 1 Oct 2022 14:28:54 -0400 Subject: [PATCH 1/8] Implement AES-GCM with CryptoKit on macOS --- .../Interop.Aead.cs | 105 ++++++++++++++ .../src/Resources/Strings.resx | 3 + .../src/System.Security.Cryptography.csproj | 5 +- .../Security/Cryptography/AesGcm.OpenSsl.cs | 92 +------------ .../Security/Cryptography/AesGcm.macOS.cs | 128 ++++++++++++++++++ .../Cryptography/AesGcmOpenSslCommon.cs | 118 ++++++++++++++++ .../tests/AesGcmTests.cs | 63 +++++++-- .../entrypoints.c | 2 + .../pal_swiftbindings.h | 28 ++++ .../pal_swiftbindings.swift | 85 +++++++++++- 10 files changed, 525 insertions(+), 104 deletions(-) create mode 100644 src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.macOS.cs create mode 100644 src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcmOpenSslCommon.cs diff --git a/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Aead.cs b/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Aead.cs index 62102ac0a5939f..d2a23ba54b48c6 100644 --- a/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Aead.cs +++ b/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Aead.cs @@ -86,6 +86,81 @@ internal static unsafe void ChaCha20Poly1305Decrypt( } } } + internal static unsafe void AesGcmEncrypt( + ReadOnlySpan key, + ReadOnlySpan nonce, + ReadOnlySpan plaintext, + Span ciphertext, + Span tag, + ReadOnlySpan aad) + { + fixed (byte* keyPtr = key) + fixed (byte* noncePtr = nonce) + fixed (byte* plaintextPtr = plaintext) + fixed (byte* ciphertextPtr = ciphertext) + fixed (byte* tagPtr = tag) + fixed (byte* aadPtr = aad) + { + const int Success = 1; + int result = AppleCryptoNative_AesGcmEncrypt( + keyPtr, key.Length, + noncePtr, nonce.Length, + plaintextPtr, plaintext.Length, + ciphertextPtr, ciphertext.Length, + tagPtr, tag.Length, + aadPtr, aad.Length); + + if (result != Success) + { + Debug.Assert(result == 0); + CryptographicOperations.ZeroMemory(ciphertext); + CryptographicOperations.ZeroMemory(tag); + throw new CryptographicException(); + } + } + } + + internal static unsafe void AesGcmDecrypt( + ReadOnlySpan key, + ReadOnlySpan nonce, + ReadOnlySpan ciphertext, + ReadOnlySpan tag, + Span plaintext, + ReadOnlySpan aad) + { + fixed (byte* keyPtr = key) + fixed (byte* noncePtr = nonce) + fixed (byte* ciphertextPtr = ciphertext) + fixed (byte* tagPtr = tag) + fixed (byte* plaintextPtr = plaintext) + fixed (byte* aadPtr = aad) + { + const int Success = 1; + const int AuthTagMismatch = -1; + int result = AppleCryptoNative_AesGcmDecrypt( + keyPtr, key.Length, + noncePtr, nonce.Length, + ciphertextPtr, ciphertext.Length, + tagPtr, tag.Length, + plaintextPtr, plaintext.Length, + aadPtr, aad.Length); + + if (result != Success) + { + CryptographicOperations.ZeroMemory(plaintext); + + if (result == AuthTagMismatch) + { + throw new AuthenticationTagMismatchException(); + } + else + { + Debug.Assert(result == 0); + throw new CryptographicException(); + } + } + } + } [LibraryImport(Libraries.AppleCryptoNative)] private static unsafe partial int AppleCryptoNative_ChaCha20Poly1305Encrypt( @@ -116,5 +191,35 @@ private static unsafe partial int AppleCryptoNative_ChaCha20Poly1305Decrypt( int plaintextLength, byte* aadPtr, int aadLength); + + [LibraryImport(Libraries.AppleCryptoNative)] + private static unsafe partial int AppleCryptoNative_AesGcmEncrypt( + byte* keyPtr, + int keyLength, + byte* noncePtr, + int nonceLength, + byte* plaintextPtr, + int plaintextLength, + byte* ciphertextPtr, + int ciphertextLength, + byte* tagPtr, + int tagLength, + byte* aadPtr, + int aadLength); + + [LibraryImport(Libraries.AppleCryptoNative)] + private static unsafe partial int AppleCryptoNative_AesGcmDecrypt( + byte* keyPtr, + int keyLength, + byte* noncePtr, + int nonceLength, + byte* ciphertextPtr, + int ciphertextLength, + byte* tagPtr, + int tagLength, + byte* plaintextPtr, + int plaintextLength, + byte* aadPtr, + int aadLength); } } diff --git a/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx b/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx index 4dc40bba0b1b7a..24df1872d61a4e 100644 --- a/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx +++ b/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx @@ -801,6 +801,9 @@ The home directory of the current user could not be determined. + + The current platform only supports 128-bit AES-GCM tags. + Windows Cryptography Next Generation (CNG) is not supported on this platform. 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 b8d2342e227977..e39f05c4bb7933 100644 --- a/src/libraries/System.Security.Cryptography/src/System.Security.Cryptography.csproj +++ b/src/libraries/System.Security.Cryptography/src/System.Security.Cryptography.csproj @@ -783,6 +783,8 @@ Link="Common\System\Text\UrlBase64Encoding.cs" /> + + @@ -870,7 +872,6 @@ - + + 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 f11936951bc9d3..b007d0f55fa2d2 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 @@ -16,7 +16,7 @@ public sealed partial class AesGcm [MemberNotNull(nameof(_ctxHandle))] private void ImportKey(ReadOnlySpan key) { - _ctxHandle = Interop.Crypto.EvpCipherCreatePartial(GetCipher(key.Length * 8)); + _ctxHandle = Interop.Crypto.EvpCipherCreatePartial(AesGcmOpenSslCommon.GetCipher(key.Length * 8)); Interop.Crypto.CheckValidOpenSslHandle(_ctxHandle); Interop.Crypto.EvpCipherSetKeyAndIV( @@ -32,44 +32,9 @@ private void EncryptCore( ReadOnlySpan plaintext, Span ciphertext, Span tag, - ReadOnlySpan associatedData = default) + ReadOnlySpan associatedData) { - Interop.Crypto.EvpCipherSetKeyAndIV( - _ctxHandle, - Span.Empty, - nonce, - Interop.Crypto.EvpCipherDirection.Encrypt); - - if (associatedData.Length != 0) - { - if (!Interop.Crypto.EvpCipherUpdate(_ctxHandle, Span.Empty, out _, associatedData)) - { - throw Interop.Crypto.CreateOpenSslCryptographicException(); - } - } - - if (!Interop.Crypto.EvpCipherUpdate(_ctxHandle, ciphertext, out int ciphertextBytesWritten, plaintext)) - { - throw Interop.Crypto.CreateOpenSslCryptographicException(); - } - - if (!Interop.Crypto.EvpCipherFinalEx( - _ctxHandle, - ciphertext.Slice(ciphertextBytesWritten), - out int bytesWritten)) - { - throw Interop.Crypto.CreateOpenSslCryptographicException(); - } - - ciphertextBytesWritten += bytesWritten; - - if (ciphertextBytesWritten != ciphertext.Length) - { - Debug.Fail($"GCM encrypt wrote {ciphertextBytesWritten} of {ciphertext.Length} bytes."); - throw new CryptographicException(); - } - - Interop.Crypto.EvpCipherGetGcmTag(_ctxHandle, tag); + AesGcmOpenSslCommon.Encrypt(_ctxHandle, nonce, plaintext, ciphertext, tag, associatedData); } private void DecryptCore( @@ -79,56 +44,7 @@ private void DecryptCore( Span plaintext, ReadOnlySpan associatedData) { - Interop.Crypto.EvpCipherSetKeyAndIV( - _ctxHandle, - ReadOnlySpan.Empty, - nonce, - Interop.Crypto.EvpCipherDirection.Decrypt); - - if (associatedData.Length != 0) - { - if (!Interop.Crypto.EvpCipherUpdate(_ctxHandle, Span.Empty, out _, associatedData)) - { - throw Interop.Crypto.CreateOpenSslCryptographicException(); - } - } - - if (!Interop.Crypto.EvpCipherUpdate(_ctxHandle, plaintext, out int plaintextBytesWritten, ciphertext)) - { - throw Interop.Crypto.CreateOpenSslCryptographicException(); - } - - Interop.Crypto.EvpCipherSetGcmTag(_ctxHandle, tag); - - if (!Interop.Crypto.EvpCipherFinalEx( - _ctxHandle, - plaintext.Slice(plaintextBytesWritten), - out int bytesWritten)) - { - CryptographicOperations.ZeroMemory(plaintext); - throw new AuthenticationTagMismatchException(); - } - - plaintextBytesWritten += bytesWritten; - - if (plaintextBytesWritten != plaintext.Length) - { - Debug.Fail($"GCM decrypt wrote {plaintextBytesWritten} of {plaintext.Length} bytes."); - throw new CryptographicException(); - } - } - - private static IntPtr GetCipher(int keySizeInBits) - { - switch (keySizeInBits) - { - case 128: return Interop.Crypto.EvpAes128Gcm(); - case 192: return Interop.Crypto.EvpAes192Gcm(); - case 256: return Interop.Crypto.EvpAes256Gcm(); - default: - Debug.Fail("Key size should already be validated"); - return IntPtr.Zero; - } + AesGcmOpenSslCommon.Decrypt(_ctxHandle, nonce, ciphertext, tag, plaintext, associatedData); } public void Dispose() diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.macOS.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.macOS.cs new file mode 100644 index 00000000000000..5542f74210130c --- /dev/null +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.macOS.cs @@ -0,0 +1,128 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; +using Microsoft.Win32.SafeHandles; + +namespace System.Security.Cryptography +{ + public sealed partial class AesGcm + { + // Apple CryptoKit does not support short authentication tags. Since .NET originally supported AES-GCM via + // OpenSSL, which does support short tags, we need to continue to support them. If a caller supplies a short + // tag we will continue to use OpenSSL if it is available. Otherwise, use CryptoKit. + private static readonly bool s_openSslAvailable = Interop.OpenSslNoInit.OpenSslIsAvailable; + private const int CryptoKitSupportedTagSizeInBytes = 16; + + private byte[]? _key; + + // CryptoKit added ChaCha20Poly1305 in macOS 10.15, which is our minimum target for macOS. We still may end + // up throwing a platform not supported if a caller uses a short authentication tag and OpenSSL is not + // available. But recommended use of AES-GCM with a 16-byte tag is supported. + public static bool IsSupported => true; + + [MemberNotNull(nameof(_key))] + private void ImportKey(ReadOnlySpan key) + { + // We should only be calling this in the constructor, so there shouldn't be a previous key. + Debug.Assert(_key is null); + + // Pin the array on the POH so that the GC doesn't move it around to allow zeroing to be more effective. + _key = GC.AllocateArray(key.Length, pinned: true); + key.CopyTo(_key); + } + + private void EncryptCore( + ReadOnlySpan nonce, + ReadOnlySpan plaintext, + Span ciphertext, + Span tag, + ReadOnlySpan associatedData) + { + CheckDisposed(); + + if (tag.Length != CryptoKitSupportedTagSizeInBytes) + { + using (SafeEvpCipherCtxHandle ctxHandle = CreateOpenSslHandle()) + { + AesGcmOpenSslCommon.Encrypt(ctxHandle, nonce, plaintext, ciphertext, tag, associatedData); + } + } + else + { + Interop.AppleCrypto.AesGcmEncrypt( + _key, + nonce, + plaintext, + ciphertext, + tag, + associatedData); + } + } + + private void DecryptCore( + ReadOnlySpan nonce, + ReadOnlySpan ciphertext, + ReadOnlySpan tag, + Span plaintext, + ReadOnlySpan associatedData) + { + CheckDisposed(); + + if (tag.Length != CryptoKitSupportedTagSizeInBytes) + { + using (SafeEvpCipherCtxHandle ctxHandle = CreateOpenSslHandle()) + { + AesGcmOpenSslCommon.Decrypt(ctxHandle, nonce, ciphertext, tag, plaintext, associatedData); + } + } + else + { + Interop.AppleCrypto.AesGcmDecrypt( + _key, + nonce, + ciphertext, + tag, + plaintext, + associatedData); + } + } + + public void Dispose() + { + CryptographicOperations.ZeroMemory(_key); + _key = null; + } + + [MemberNotNull(nameof(_key))] + private void CheckDisposed() + { + ObjectDisposedException.ThrowIf(_key is null, this); + } + + private SafeEvpCipherCtxHandle CreateOpenSslHandle() + { + Debug.Assert(_key is not null); + + // We should only get here if the tag size is not 128-bit. If that happens, and OpenSSL is not available, + // then we can't proceed. + if (!s_openSslAvailable) + { + throw new PlatformNotSupportedException(SR.PlatformNotSupported_AesGcmTagSize); + } + + IntPtr cipherHandle = AesGcmOpenSslCommon.GetCipher(_key.Length * 8); + SafeEvpCipherCtxHandle ctxHandle = Interop.Crypto.EvpCipherCreatePartial(cipherHandle); + + Interop.Crypto.CheckValidOpenSslHandle(ctxHandle); + Interop.Crypto.EvpCipherSetKeyAndIV( + ctxHandle, + _key, + Span.Empty, + Interop.Crypto.EvpCipherDirection.NoChange); + Interop.Crypto.EvpCipherSetGcmNonceLength(ctxHandle, NonceSize); + return ctxHandle; + } + } +} diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcmOpenSslCommon.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcmOpenSslCommon.cs new file mode 100644 index 00000000000000..58644c640eb5fe --- /dev/null +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcmOpenSslCommon.cs @@ -0,0 +1,118 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; +using Microsoft.Win32.SafeHandles; + +namespace System.Security.Cryptography +{ + internal static class AesGcmOpenSslCommon + { + internal static void Encrypt( + SafeEvpCipherCtxHandle ctxHandle, + ReadOnlySpan nonce, + ReadOnlySpan plaintext, + Span ciphertext, + Span tag, + ReadOnlySpan associatedData = default) + { + Interop.Crypto.EvpCipherSetKeyAndIV( + ctxHandle, + Span.Empty, + nonce, + Interop.Crypto.EvpCipherDirection.Encrypt); + + if (associatedData.Length != 0) + { + if (!Interop.Crypto.EvpCipherUpdate(ctxHandle, Span.Empty, out _, associatedData)) + { + throw Interop.Crypto.CreateOpenSslCryptographicException(); + } + } + + if (!Interop.Crypto.EvpCipherUpdate(ctxHandle, ciphertext, out int ciphertextBytesWritten, plaintext)) + { + throw Interop.Crypto.CreateOpenSslCryptographicException(); + } + + if (!Interop.Crypto.EvpCipherFinalEx( + ctxHandle, + ciphertext.Slice(ciphertextBytesWritten), + out int bytesWritten)) + { + throw Interop.Crypto.CreateOpenSslCryptographicException(); + } + + ciphertextBytesWritten += bytesWritten; + + if (ciphertextBytesWritten != ciphertext.Length) + { + Debug.Fail($"GCM encrypt wrote {ciphertextBytesWritten} of {ciphertext.Length} bytes."); + throw new CryptographicException(); + } + + Interop.Crypto.EvpCipherGetGcmTag(ctxHandle, tag); + } + + internal static void Decrypt( + SafeEvpCipherCtxHandle ctxHandle, + ReadOnlySpan nonce, + ReadOnlySpan ciphertext, + ReadOnlySpan tag, + Span plaintext, + ReadOnlySpan associatedData) + { + Interop.Crypto.EvpCipherSetKeyAndIV( + ctxHandle, + ReadOnlySpan.Empty, + nonce, + Interop.Crypto.EvpCipherDirection.Decrypt); + + if (associatedData.Length != 0) + { + if (!Interop.Crypto.EvpCipherUpdate(ctxHandle, Span.Empty, out _, associatedData)) + { + throw Interop.Crypto.CreateOpenSslCryptographicException(); + } + } + + if (!Interop.Crypto.EvpCipherUpdate(ctxHandle, plaintext, out int plaintextBytesWritten, ciphertext)) + { + throw Interop.Crypto.CreateOpenSslCryptographicException(); + } + + Interop.Crypto.EvpCipherSetGcmTag(ctxHandle, tag); + + if (!Interop.Crypto.EvpCipherFinalEx( + ctxHandle, + plaintext.Slice(plaintextBytesWritten), + out int bytesWritten)) + { + CryptographicOperations.ZeroMemory(plaintext); + throw new AuthenticationTagMismatchException(); + } + + plaintextBytesWritten += bytesWritten; + + if (plaintextBytesWritten != plaintext.Length) + { + Debug.Fail($"GCM decrypt wrote {plaintextBytesWritten} of {plaintext.Length} bytes."); + throw new CryptographicException(); + } + } + + internal static IntPtr GetCipher(int keySizeInBits) + { + switch (keySizeInBits) + { + case 128: return Interop.Crypto.EvpAes128Gcm(); + case 192: return Interop.Crypto.EvpAes192Gcm(); + case 256: return Interop.Crypto.EvpAes256Gcm(); + default: + Debug.Fail("Key size should already be validated"); + return IntPtr.Zero; + } + } + } +} diff --git a/src/libraries/System.Security.Cryptography/tests/AesGcmTests.cs b/src/libraries/System.Security.Cryptography/tests/AesGcmTests.cs index 6ef5391e40e535..82b8d249d3f9eb 100644 --- a/src/libraries/System.Security.Cryptography/tests/AesGcmTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/AesGcmTests.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Linq; +using Microsoft.DotNet.XUnitExtensions; using Test.Cryptography; using Xunit; @@ -11,7 +12,9 @@ namespace System.Security.Cryptography.Tests [ConditionalClass(typeof(AesGcm), nameof(AesGcm.IsSupported))] public class AesGcmTests : CommonAEADTests { - [Theory] + private const int CryptoKitSupportedTagSizeInBytes = 16; + + [ConditionalTheory] [ActiveIssue("https://github.com/dotnet/runtime/issues/51332", TestPlatforms.iOS | TestPlatforms.tvOS | TestPlatforms.MacCatalyst)] [MemberData(nameof(EncryptTamperAADDecryptTestInputs))] public static void EncryptTamperAADDecrypt(int dataLength, int additionalDataLength) @@ -23,7 +26,7 @@ public static void EncryptTamperAADDecrypt(int dataLength, int additionalDataLen byte[] ciphertext = new byte[dataLength]; byte[] key = new byte[16]; byte[] nonce = new byte[AesGcm.NonceByteSizes.MinSize]; - byte[] tag = new byte[AesGcm.TagByteSizes.MinSize]; + byte[] tag = new byte[AesGcm.TagByteSizes.MaxSize]; RandomNumberGenerator.Fill(key); RandomNumberGenerator.Fill(nonce); @@ -82,7 +85,7 @@ public static void ValidNonceSize(int nonceSize) byte[] ciphertext = new byte[dataLength]; byte[] key = new byte[16]; byte[] nonce = new byte[nonceSize]; - byte[] tag = new byte[AesGcm.TagByteSizes.MinSize]; + byte[] tag = new byte[AesGcm.TagByteSizes.MaxSize]; RandomNumberGenerator.Fill(key); RandomNumberGenerator.Fill(nonce); @@ -132,11 +135,22 @@ public static void ValidTagSize(int tagSize) using (var aesGcm = new AesGcm(key)) { - aesGcm.Encrypt(nonce, plaintext, ciphertext, tag); - - byte[] decrypted = new byte[dataLength]; - aesGcm.Decrypt(nonce, ciphertext, tag, decrypted); - Assert.Equal(plaintext, decrypted); + if (PlatformDetection.IsOSX && + tagSize != CryptoKitSupportedTagSizeInBytes && + !PlatformDetection.OpenSslPresentOnSystem) + { + byte[] decrypted = new byte[dataLength]; + Assert.Throws(() => aesGcm.Encrypt(nonce, plaintext, ciphertext, tag)); + Assert.Throws(() => aesGcm.Decrypt(nonce, ciphertext, tag, decrypted)); + } + else + { + aesGcm.Encrypt(nonce, plaintext, ciphertext, tag); + + byte[] decrypted = new byte[dataLength]; + aesGcm.Decrypt(nonce, ciphertext, tag, decrypted); + Assert.Equal(plaintext, decrypted); + } } } @@ -152,7 +166,9 @@ public static void TwoEncryptionsAndDecryptionsUsingOneInstance() byte[] nonce2 = "8ba10892e8b87d031196bf99".HexToByteArray(); byte[] expectedCiphertext1 = "f1af1fb2d4485cc536d618475d52ff".HexToByteArray(); - byte[] expectedTag1 = "5ab65624c46b8160f34e81f5".HexToByteArray(); + byte[] expectedTag1 = PlatformDetection.IsOSX ? + "5ab65624c46b8160f34e81f51fee6cd9".HexToByteArray() : + "5ab65624c46b8160f34e81f5".HexToByteArray(); byte[] expectedCiphertext2 = ( "217bed01446d731a372a2b30ac7fcd73aed7c946d9171ae9c00b1c589ca73ba2" + @@ -318,11 +334,18 @@ public static void InplaceEncryptTamperTagDecrypt() } } - [Theory] + [ConditionalTheory] [MemberData(nameof(GetNistGcmTestCases))] [ActiveIssue("https://github.com/dotnet/runtime/issues/51332", TestPlatforms.iOS | TestPlatforms.tvOS | TestPlatforms.MacCatalyst)] public static void AesGcmNistTests(AEADTest testCase) { + if (PlatformDetection.IsOSX && + testCase.Tag.Length != CryptoKitSupportedTagSizeInBytes && + !PlatformDetection.OpenSslPresentOnSystem) + { + throw new SkipTestException("Platform does not support tag sizes other than 128-bit"); + } + using (var aesGcm = new AesGcm(testCase.Key)) { byte[] ciphertext = new byte[testCase.Plaintext.Length]; @@ -337,11 +360,18 @@ public static void AesGcmNistTests(AEADTest testCase) } } - [Theory] + [ConditionalTheory] [MemberData(nameof(GetNistGcmTestCases))] [ActiveIssue("https://github.com/dotnet/runtime/issues/51332", TestPlatforms.iOS | TestPlatforms.tvOS | TestPlatforms.MacCatalyst)] public static void AesGcmNistTestsTamperTag(AEADTest testCase) { + if (PlatformDetection.IsOSX && + testCase.Tag.Length != CryptoKitSupportedTagSizeInBytes && + !PlatformDetection.OpenSslPresentOnSystem) + { + throw new SkipTestException("Platform does not support tag sizes other than 128-bit"); + } + using (var aesGcm = new AesGcm(testCase.Key)) { byte[] ciphertext = new byte[testCase.Plaintext.Length]; @@ -360,11 +390,18 @@ public static void AesGcmNistTestsTamperTag(AEADTest testCase) } } - [Theory] + [ConditionalTheory] [MemberData(nameof(GetNistGcmTestCasesWithNonEmptyPT))] [ActiveIssue("https://github.com/dotnet/runtime/issues/51332", TestPlatforms.iOS | TestPlatforms.tvOS | TestPlatforms.MacCatalyst)] public static void AesGcmNistTestsTamperCiphertext(AEADTest testCase) { + if (PlatformDetection.IsOSX && + testCase.Tag.Length != CryptoKitSupportedTagSizeInBytes && + !PlatformDetection.OpenSslPresentOnSystem) + { + throw new SkipTestException("Platform does not support tag sizes other than 128-bit"); + } + using (var aesGcm = new AesGcm(testCase.Key)) { byte[] ciphertext = new byte[testCase.Plaintext.Length]; @@ -873,7 +910,7 @@ public static void CheckIsSupported() if (PlatformDetection.IsOSX) { - expectedIsSupported = PlatformDetection.OpenSslPresentOnSystem; + expectedIsSupported = true; } else if (PlatformDetection.UsesMobileAppleCrypto) { diff --git a/src/native/libs/System.Security.Cryptography.Native.Apple/entrypoints.c b/src/native/libs/System.Security.Cryptography.Native.Apple/entrypoints.c index 4f013c4cf74843..1a880286927b6b 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Apple/entrypoints.c +++ b/src/native/libs/System.Security.Cryptography.Native.Apple/entrypoints.c @@ -26,6 +26,8 @@ static const Entry s_cryptoAppleNative[] = { + DllImportEntry(AppleCryptoNative_AesGcmEncrypt) + DllImportEntry(AppleCryptoNative_AesGcmDecrypt) DllImportEntry(AppleCryptoNative_ChaCha20Poly1305Encrypt) DllImportEntry(AppleCryptoNative_ChaCha20Poly1305Decrypt) DllImportEntry(AppleCryptoNative_DigestFree) diff --git a/src/native/libs/System.Security.Cryptography.Native.Apple/pal_swiftbindings.h b/src/native/libs/System.Security.Cryptography.Native.Apple/pal_swiftbindings.h index 42e3c22e7dad6c..9fd0f1ea0ab318 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Apple/pal_swiftbindings.h +++ b/src/native/libs/System.Security.Cryptography.Native.Apple/pal_swiftbindings.h @@ -33,3 +33,31 @@ PALEXPORT int32_t AppleCryptoNative_ChaCha20Poly1305Decrypt( int32_t plaintextBufferLength, uint8_t* aadPtr, int32_t aadLength); + +PALEXPORT int32_t AppleCryptoNative_AesGcmEncrypt( + uint8_t* keyPtr, + int32_t keyLength, + uint8_t* noncePtr, + int32_t nonceLength, + uint8_t* plaintextPtr, + int32_t plaintextLength, + uint8_t* ciphertextBuffer, + int32_t ciphertextBufferLength, + uint8_t* tagBuffer, + int32_t tagBufferLength, + uint8_t* aadPtr, + int32_t aadLength); + +PALEXPORT int32_t AppleCryptoNative_AesGcmDecrypt( + uint8_t* keyPtr, + int32_t keyLength, + uint8_t* noncePtr, + int32_t nonceLength, + uint8_t* ciphertextPtr, + int32_t ciphertextLength, + uint8_t* tagPtr, + int32_t tagLength, + uint8_t* plaintextBuffer, + int32_t plaintextBufferLength, + uint8_t* aadPtr, + int32_t aadLength); diff --git a/src/native/libs/System.Security.Cryptography.Native.Apple/pal_swiftbindings.swift b/src/native/libs/System.Security.Cryptography.Native.Apple/pal_swiftbindings.swift index 7d54df744b1f70..e1e4f9578953f6 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Apple/pal_swiftbindings.swift +++ b/src/native/libs/System.Security.Cryptography.Native.Apple/pal_swiftbindings.swift @@ -55,8 +55,7 @@ public func AppleCryptoNative_ChaCha20Poly1305Decrypt( plaintextBufferLength: Int32, aadPtr: UnsafeMutableRawPointer, aadLength: Int32 -) -> Int32 -{ +) -> Int32 { let nonceData = Data(bytesNoCopy: noncePtr, count: Int(nonceLength), deallocator: Data.Deallocator.none) let key = Data(bytesNoCopy: keyPtr, count: Int(keyLength), deallocator: Data.Deallocator.none) let ciphertext = Data(bytesNoCopy: ciphertextPtr, count: Int(ciphertextLength), deallocator: Data.Deallocator.none) @@ -86,3 +85,85 @@ public func AppleCryptoNative_ChaCha20Poly1305Decrypt( return 0 } } + +@_cdecl("AppleCryptoNative_AesGcmEncrypt") +public func AppleCryptoNative_AesGcmEncrypt( + keyPtr: UnsafeMutableRawPointer, + keyLength: Int32, + noncePtr: UnsafeMutableRawPointer, + nonceLength: Int32, + plaintextPtr: UnsafeMutableRawPointer, + plaintextLength: Int32, + ciphertextBuffer: UnsafeMutablePointer, + ciphertextBufferLength: Int32, + tagBuffer: UnsafeMutablePointer, + tagBufferLength: Int32, + aadPtr: UnsafeMutableRawPointer, + aadLength: Int32 + ) -> Int32 { + let nonceData = Data(bytesNoCopy: noncePtr, count: Int(nonceLength), deallocator: Data.Deallocator.none) + let key = Data(bytesNoCopy: keyPtr, count: Int(keyLength), deallocator: Data.Deallocator.none) + let plaintext = Data(bytesNoCopy: plaintextPtr, count: Int(plaintextLength), deallocator: Data.Deallocator.none) + let aad = Data(bytesNoCopy: aadPtr, count: Int(aadLength), deallocator: Data.Deallocator.none) + let symmetricKey = SymmetricKey(data: key) + + guard let nonce = try? AES.GCM.Nonce(data: nonceData) else { + return 0 + } + + guard let result = try? AES.GCM.seal(plaintext, using: symmetricKey, nonce: nonce, authenticating: aad) else { + return 0 + } + + assert(ciphertextBufferLength >= result.ciphertext.count) + assert(tagBufferLength >= result.tag.count) + + result.ciphertext.copyBytes(to: ciphertextBuffer, count: result.ciphertext.count) + result.tag.copyBytes(to: tagBuffer, count: result.tag.count) + return 1 + } + +@_cdecl("AppleCryptoNative_AesGcmDecrypt") +public func AppleCryptoNative_AesGcmDecrypt( + keyPtr: UnsafeMutableRawPointer, + keyLength: Int32, + noncePtr: UnsafeMutableRawPointer, + nonceLength: Int32, + ciphertextPtr: UnsafeMutableRawPointer, + ciphertextLength: Int32, + tagPtr: UnsafeMutableRawPointer, + tagLength: Int32, + plaintextBuffer: UnsafeMutablePointer, + plaintextBufferLength: Int32, + aadPtr: UnsafeMutableRawPointer, + aadLength: Int32 +) -> Int32 { + let nonceData = Data(bytesNoCopy: noncePtr, count: Int(nonceLength), deallocator: Data.Deallocator.none) + let key = Data(bytesNoCopy: keyPtr, count: Int(keyLength), deallocator: Data.Deallocator.none) + let ciphertext = Data(bytesNoCopy: ciphertextPtr, count: Int(ciphertextLength), deallocator: Data.Deallocator.none) + let aad = Data(bytesNoCopy: aadPtr, count: Int(aadLength), deallocator: Data.Deallocator.none) + let tag = Data(bytesNoCopy: tagPtr, count: Int(tagLength), deallocator: Data.Deallocator.none) + let symmetricKey = SymmetricKey(data: key) + + guard let nonce = try? AES.GCM.Nonce(data: nonceData) else { + return 0 + } + + guard let sealedBoxRestored = try? AES.GCM.SealedBox(nonce: nonce, ciphertext: ciphertext, tag: tag) else { + return 0 + } + + do { + let result = try AES.GCM.open(sealedBoxRestored, using: symmetricKey, authenticating: aad) + + assert(plaintextBufferLength >= result.count) + result.copyBytes(to: plaintextBuffer, count: result.count) + return 1 + } + catch CryptoKitError.authenticationFailure { + return -1 + } + catch { + return 0 + } +} From 8bb62106ed8cc156407626f149769374c0c71d3e Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Sun, 2 Oct 2022 12:02:08 -0400 Subject: [PATCH 2/8] Fix typo --- .../src/System/Security/Cryptography/AesGcm.macOS.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.macOS.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.macOS.cs index 5542f74210130c..c3a5988bc16d22 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.macOS.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.macOS.cs @@ -17,7 +17,7 @@ public sealed partial class AesGcm private byte[]? _key; - // CryptoKit added ChaCha20Poly1305 in macOS 10.15, which is our minimum target for macOS. We still may end + // CryptoKit added AES.GCM in macOS 10.15, which is our minimum target for macOS. We still may end // up throwing a platform not supported if a caller uses a short authentication tag and OpenSSL is not // available. But recommended use of AES-GCM with a 16-byte tag is supported. public static bool IsSupported => true; From abc8c1937b0518adc38eeb9fdcebfb47ab04a2a2 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Mon, 7 Nov 2022 12:22:28 -0500 Subject: [PATCH 3/8] Revert OpenSSL work for macOS GCM --- .../src/Resources/Strings.resx | 3 - .../src/System.Security.Cryptography.csproj | 5 +- .../Security/Cryptography/AesGcm.OpenSsl.cs | 92 +++++++++++++- .../Cryptography/AesGcmOpenSslCommon.cs | 118 ------------------ 4 files changed, 89 insertions(+), 129 deletions(-) delete mode 100644 src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcmOpenSslCommon.cs diff --git a/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx b/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx index 24df1872d61a4e..4dc40bba0b1b7a 100644 --- a/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx +++ b/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx @@ -801,9 +801,6 @@ The home directory of the current user could not be determined. - - The current platform only supports 128-bit AES-GCM tags. - Windows Cryptography Next Generation (CNG) is not supported on this platform. 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 9a3d5d7c85bacd..dadeb106d6613c 100644 --- a/src/libraries/System.Security.Cryptography/src/System.Security.Cryptography.csproj +++ b/src/libraries/System.Security.Cryptography/src/System.Security.Cryptography.csproj @@ -783,8 +783,6 @@ Link="Common\System\Text\UrlBase64Encoding.cs" /> - - @@ -872,6 +870,7 @@ + - - 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 b007d0f55fa2d2..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 @@ -16,7 +16,7 @@ public sealed partial class AesGcm [MemberNotNull(nameof(_ctxHandle))] private void ImportKey(ReadOnlySpan key) { - _ctxHandle = Interop.Crypto.EvpCipherCreatePartial(AesGcmOpenSslCommon.GetCipher(key.Length * 8)); + _ctxHandle = Interop.Crypto.EvpCipherCreatePartial(GetCipher(key.Length * 8)); Interop.Crypto.CheckValidOpenSslHandle(_ctxHandle); Interop.Crypto.EvpCipherSetKeyAndIV( @@ -32,9 +32,44 @@ private void EncryptCore( ReadOnlySpan plaintext, Span ciphertext, Span tag, - ReadOnlySpan associatedData) + ReadOnlySpan associatedData = default) { - AesGcmOpenSslCommon.Encrypt(_ctxHandle, nonce, plaintext, ciphertext, tag, associatedData); + Interop.Crypto.EvpCipherSetKeyAndIV( + _ctxHandle, + Span.Empty, + nonce, + Interop.Crypto.EvpCipherDirection.Encrypt); + + if (associatedData.Length != 0) + { + if (!Interop.Crypto.EvpCipherUpdate(_ctxHandle, Span.Empty, out _, associatedData)) + { + throw Interop.Crypto.CreateOpenSslCryptographicException(); + } + } + + if (!Interop.Crypto.EvpCipherUpdate(_ctxHandle, ciphertext, out int ciphertextBytesWritten, plaintext)) + { + throw Interop.Crypto.CreateOpenSslCryptographicException(); + } + + if (!Interop.Crypto.EvpCipherFinalEx( + _ctxHandle, + ciphertext.Slice(ciphertextBytesWritten), + out int bytesWritten)) + { + throw Interop.Crypto.CreateOpenSslCryptographicException(); + } + + ciphertextBytesWritten += bytesWritten; + + if (ciphertextBytesWritten != ciphertext.Length) + { + Debug.Fail($"GCM encrypt wrote {ciphertextBytesWritten} of {ciphertext.Length} bytes."); + throw new CryptographicException(); + } + + Interop.Crypto.EvpCipherGetGcmTag(_ctxHandle, tag); } private void DecryptCore( @@ -44,7 +79,56 @@ private void DecryptCore( Span plaintext, ReadOnlySpan associatedData) { - AesGcmOpenSslCommon.Decrypt(_ctxHandle, nonce, ciphertext, tag, plaintext, associatedData); + Interop.Crypto.EvpCipherSetKeyAndIV( + _ctxHandle, + ReadOnlySpan.Empty, + nonce, + Interop.Crypto.EvpCipherDirection.Decrypt); + + if (associatedData.Length != 0) + { + if (!Interop.Crypto.EvpCipherUpdate(_ctxHandle, Span.Empty, out _, associatedData)) + { + throw Interop.Crypto.CreateOpenSslCryptographicException(); + } + } + + if (!Interop.Crypto.EvpCipherUpdate(_ctxHandle, plaintext, out int plaintextBytesWritten, ciphertext)) + { + throw Interop.Crypto.CreateOpenSslCryptographicException(); + } + + Interop.Crypto.EvpCipherSetGcmTag(_ctxHandle, tag); + + if (!Interop.Crypto.EvpCipherFinalEx( + _ctxHandle, + plaintext.Slice(plaintextBytesWritten), + out int bytesWritten)) + { + CryptographicOperations.ZeroMemory(plaintext); + throw new AuthenticationTagMismatchException(); + } + + plaintextBytesWritten += bytesWritten; + + if (plaintextBytesWritten != plaintext.Length) + { + Debug.Fail($"GCM decrypt wrote {plaintextBytesWritten} of {plaintext.Length} bytes."); + throw new CryptographicException(); + } + } + + private static IntPtr GetCipher(int keySizeInBits) + { + switch (keySizeInBits) + { + case 128: return Interop.Crypto.EvpAes128Gcm(); + case 192: return Interop.Crypto.EvpAes192Gcm(); + case 256: return Interop.Crypto.EvpAes256Gcm(); + default: + Debug.Fail("Key size should already be validated"); + return IntPtr.Zero; + } } public void Dispose() diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcmOpenSslCommon.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcmOpenSslCommon.cs deleted file mode 100644 index 58644c640eb5fe..00000000000000 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcmOpenSslCommon.cs +++ /dev/null @@ -1,118 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Diagnostics; -using System.Diagnostics.CodeAnalysis; -using Microsoft.Win32.SafeHandles; - -namespace System.Security.Cryptography -{ - internal static class AesGcmOpenSslCommon - { - internal static void Encrypt( - SafeEvpCipherCtxHandle ctxHandle, - ReadOnlySpan nonce, - ReadOnlySpan plaintext, - Span ciphertext, - Span tag, - ReadOnlySpan associatedData = default) - { - Interop.Crypto.EvpCipherSetKeyAndIV( - ctxHandle, - Span.Empty, - nonce, - Interop.Crypto.EvpCipherDirection.Encrypt); - - if (associatedData.Length != 0) - { - if (!Interop.Crypto.EvpCipherUpdate(ctxHandle, Span.Empty, out _, associatedData)) - { - throw Interop.Crypto.CreateOpenSslCryptographicException(); - } - } - - if (!Interop.Crypto.EvpCipherUpdate(ctxHandle, ciphertext, out int ciphertextBytesWritten, plaintext)) - { - throw Interop.Crypto.CreateOpenSslCryptographicException(); - } - - if (!Interop.Crypto.EvpCipherFinalEx( - ctxHandle, - ciphertext.Slice(ciphertextBytesWritten), - out int bytesWritten)) - { - throw Interop.Crypto.CreateOpenSslCryptographicException(); - } - - ciphertextBytesWritten += bytesWritten; - - if (ciphertextBytesWritten != ciphertext.Length) - { - Debug.Fail($"GCM encrypt wrote {ciphertextBytesWritten} of {ciphertext.Length} bytes."); - throw new CryptographicException(); - } - - Interop.Crypto.EvpCipherGetGcmTag(ctxHandle, tag); - } - - internal static void Decrypt( - SafeEvpCipherCtxHandle ctxHandle, - ReadOnlySpan nonce, - ReadOnlySpan ciphertext, - ReadOnlySpan tag, - Span plaintext, - ReadOnlySpan associatedData) - { - Interop.Crypto.EvpCipherSetKeyAndIV( - ctxHandle, - ReadOnlySpan.Empty, - nonce, - Interop.Crypto.EvpCipherDirection.Decrypt); - - if (associatedData.Length != 0) - { - if (!Interop.Crypto.EvpCipherUpdate(ctxHandle, Span.Empty, out _, associatedData)) - { - throw Interop.Crypto.CreateOpenSslCryptographicException(); - } - } - - if (!Interop.Crypto.EvpCipherUpdate(ctxHandle, plaintext, out int plaintextBytesWritten, ciphertext)) - { - throw Interop.Crypto.CreateOpenSslCryptographicException(); - } - - Interop.Crypto.EvpCipherSetGcmTag(ctxHandle, tag); - - if (!Interop.Crypto.EvpCipherFinalEx( - ctxHandle, - plaintext.Slice(plaintextBytesWritten), - out int bytesWritten)) - { - CryptographicOperations.ZeroMemory(plaintext); - throw new AuthenticationTagMismatchException(); - } - - plaintextBytesWritten += bytesWritten; - - if (plaintextBytesWritten != plaintext.Length) - { - Debug.Fail($"GCM decrypt wrote {plaintextBytesWritten} of {plaintext.Length} bytes."); - throw new CryptographicException(); - } - } - - internal static IntPtr GetCipher(int keySizeInBits) - { - switch (keySizeInBits) - { - case 128: return Interop.Crypto.EvpAes128Gcm(); - case 192: return Interop.Crypto.EvpAes192Gcm(); - case 256: return Interop.Crypto.EvpAes256Gcm(); - default: - Debug.Fail("Key size should already be validated"); - return IntPtr.Zero; - } - } - } -} From 253ba44cfca285326c90b1fc3e29ed9df8f6ac00 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Mon, 7 Nov 2022 12:29:30 -0500 Subject: [PATCH 4/8] Fixup macOS --- .../src/System.Security.Cryptography.csproj | 3 +- .../Security/Cryptography/AesGcm.Android.cs | 1 + .../Cryptography/AesGcm.NotSupported.cs | 1 + .../Security/Cryptography/AesGcm.OpenSsl.cs | 1 + .../Security/Cryptography/AesGcm.Windows.cs | 1 + .../System/Security/Cryptography/AesGcm.cs | 1 - .../Security/Cryptography/AesGcm.macOS.cs | 88 ++++--------------- 7 files changed, 25 insertions(+), 71 deletions(-) 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 dadeb106d6613c..99e10abbe05951 100644 --- a/src/libraries/System.Security.Cryptography/src/System.Security.Cryptography.csproj +++ b/src/libraries/System.Security.Cryptography/src/System.Security.Cryptography.csproj @@ -783,6 +783,7 @@ Link="Common\System\Text\UrlBase64Encoding.cs" /> + @@ -870,7 +871,6 @@ - + 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 471338a8e8a76c..48f10541a5070d 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 @@ -12,6 +12,7 @@ public sealed partial class AesGcm private SafeEvpCipherCtxHandle _ctxHandle; public static bool IsSupported => true; + public static KeySizes TagByteSizes { get; } = new KeySizes(12, 16, 1); [MemberNotNull(nameof(_ctxHandle))] private void ImportKey(ReadOnlySpan key) diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.NotSupported.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.NotSupported.cs index 3e7c4b641397ac..6285492b2d413a 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.NotSupported.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.NotSupported.cs @@ -8,6 +8,7 @@ namespace System.Security.Cryptography public partial class AesGcm { public static bool IsSupported => false; + public static KeySizes TagByteSizes { get; } = new KeySizes(12, 16, 1); #pragma warning disable CA1822 private void ImportKey(ReadOnlySpan key) 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 f11936951bc9d3..05ce0353702019 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 @@ -12,6 +12,7 @@ public sealed partial class AesGcm private SafeEvpCipherCtxHandle _ctxHandle; public static bool IsSupported { get; } = Interop.OpenSslNoInit.OpenSslIsAvailable; + public static KeySizes TagByteSizes { get; } = new KeySizes(12, 16, 1); [MemberNotNull(nameof(_ctxHandle))] private void ImportKey(ReadOnlySpan key) diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.Windows.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.Windows.cs index f6326f42653f30..305fccb16188e6 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.Windows.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.Windows.cs @@ -12,6 +12,7 @@ public partial class AesGcm private SafeKeyHandle _keyHandle; public static bool IsSupported => true; + public static KeySizes TagByteSizes { get; } = new KeySizes(12, 16, 1); [MemberNotNull(nameof(_keyHandle))] private void ImportKey(ReadOnlySpan key) diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.cs index 02c7e6945eb907..104d280452482c 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.cs @@ -13,7 +13,6 @@ public sealed partial class AesGcm : IDisposable { private const int NonceSize = 12; public static KeySizes NonceByteSizes { get; } = new KeySizes(NonceSize, NonceSize, 1); - public static KeySizes TagByteSizes { get; } = new KeySizes(12, 16, 1); public AesGcm(ReadOnlySpan key) { diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.macOS.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.macOS.cs index c3a5988bc16d22..2313539f1e06bd 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.macOS.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.macOS.cs @@ -9,19 +9,15 @@ namespace System.Security.Cryptography { public sealed partial class AesGcm { - // Apple CryptoKit does not support short authentication tags. Since .NET originally supported AES-GCM via - // OpenSSL, which does support short tags, we need to continue to support them. If a caller supplies a short - // tag we will continue to use OpenSSL if it is available. Otherwise, use CryptoKit. - private static readonly bool s_openSslAvailable = Interop.OpenSslNoInit.OpenSslIsAvailable; - private const int CryptoKitSupportedTagSizeInBytes = 16; - private byte[]? _key; - // CryptoKit added AES.GCM in macOS 10.15, which is our minimum target for macOS. We still may end - // up throwing a platform not supported if a caller uses a short authentication tag and OpenSSL is not - // available. But recommended use of AES-GCM with a 16-byte tag is supported. + // CryptoKit added AES.GCM in macOS 10.15, which is our minimum target for macOS. public static bool IsSupported => true; + // CryptoKit only supports 16 byte tags. + public static KeySizes TagByteSizes { get; } = new KeySizes(16, 16, 1); + + [MemberNotNull(nameof(_key))] private void ImportKey(ReadOnlySpan key) { @@ -41,24 +37,13 @@ private void EncryptCore( ReadOnlySpan associatedData) { CheckDisposed(); - - if (tag.Length != CryptoKitSupportedTagSizeInBytes) - { - using (SafeEvpCipherCtxHandle ctxHandle = CreateOpenSslHandle()) - { - AesGcmOpenSslCommon.Encrypt(ctxHandle, nonce, plaintext, ciphertext, tag, associatedData); - } - } - else - { - Interop.AppleCrypto.AesGcmEncrypt( - _key, - nonce, - plaintext, - ciphertext, - tag, - associatedData); - } + Interop.AppleCrypto.AesGcmEncrypt( + _key, + nonce, + plaintext, + ciphertext, + tag, + associatedData); } private void DecryptCore( @@ -69,24 +54,13 @@ private void DecryptCore( ReadOnlySpan associatedData) { CheckDisposed(); - - if (tag.Length != CryptoKitSupportedTagSizeInBytes) - { - using (SafeEvpCipherCtxHandle ctxHandle = CreateOpenSslHandle()) - { - AesGcmOpenSslCommon.Decrypt(ctxHandle, nonce, ciphertext, tag, plaintext, associatedData); - } - } - else - { - Interop.AppleCrypto.AesGcmDecrypt( - _key, - nonce, - ciphertext, - tag, - plaintext, - associatedData); - } + Interop.AppleCrypto.AesGcmDecrypt( + _key, + nonce, + ciphertext, + tag, + plaintext, + associatedData); } public void Dispose() @@ -100,29 +74,5 @@ private void CheckDisposed() { ObjectDisposedException.ThrowIf(_key is null, this); } - - private SafeEvpCipherCtxHandle CreateOpenSslHandle() - { - Debug.Assert(_key is not null); - - // We should only get here if the tag size is not 128-bit. If that happens, and OpenSSL is not available, - // then we can't proceed. - if (!s_openSslAvailable) - { - throw new PlatformNotSupportedException(SR.PlatformNotSupported_AesGcmTagSize); - } - - IntPtr cipherHandle = AesGcmOpenSslCommon.GetCipher(_key.Length * 8); - SafeEvpCipherCtxHandle ctxHandle = Interop.Crypto.EvpCipherCreatePartial(cipherHandle); - - Interop.Crypto.CheckValidOpenSslHandle(ctxHandle); - Interop.Crypto.EvpCipherSetKeyAndIV( - ctxHandle, - _key, - Span.Empty, - Interop.Crypto.EvpCipherDirection.NoChange); - Interop.Crypto.EvpCipherSetGcmNonceLength(ctxHandle, NonceSize); - return ctxHandle; - } } } From 2f7e5d07d9be61a22f279c0f49d6e4b4686c1ea8 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Mon, 7 Nov 2022 14:13:17 -0500 Subject: [PATCH 5/8] Respond to code review feedback --- .../tests/AesGcmTests.cs | 149 ++++++++++-------- 1 file changed, 79 insertions(+), 70 deletions(-) diff --git a/src/libraries/System.Security.Cryptography/tests/AesGcmTests.cs b/src/libraries/System.Security.Cryptography/tests/AesGcmTests.cs index 01ab09fdace793..f25081224f7b99 100644 --- a/src/libraries/System.Security.Cryptography/tests/AesGcmTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/AesGcmTests.cs @@ -135,22 +135,11 @@ public static void ValidTagSize(int tagSize) using (var aesGcm = new AesGcm(key)) { - if (PlatformDetection.IsOSX && - tagSize != CryptoKitSupportedTagSizeInBytes && - !PlatformDetection.OpenSslPresentOnSystem) - { - byte[] decrypted = new byte[dataLength]; - Assert.Throws(() => aesGcm.Encrypt(nonce, plaintext, ciphertext, tag)); - Assert.Throws(() => aesGcm.Decrypt(nonce, ciphertext, tag, decrypted)); - } - else - { - aesGcm.Encrypt(nonce, plaintext, ciphertext, tag); + aesGcm.Encrypt(nonce, plaintext, ciphertext, tag); - byte[] decrypted = new byte[dataLength]; - aesGcm.Decrypt(nonce, ciphertext, tag, decrypted); - Assert.Equal(plaintext, decrypted); - } + byte[] decrypted = new byte[dataLength]; + aesGcm.Decrypt(nonce, ciphertext, tag, decrypted); + Assert.Equal(plaintext, decrypted); } } @@ -334,59 +323,75 @@ public static void InplaceEncryptTamperTagDecrypt() } } - [ConditionalTheory] + [Theory] [MemberData(nameof(GetNistGcmTestCases))] [ActiveIssue("https://github.com/dotnet/runtime/issues/51332", TestPlatforms.iOS | TestPlatforms.tvOS | TestPlatforms.MacCatalyst)] public static void AesGcmNistTests(AEADTest testCase) { - if (PlatformDetection.IsOSX && - testCase.Tag.Length != CryptoKitSupportedTagSizeInBytes && - !PlatformDetection.OpenSslPresentOnSystem) - { - throw new SkipTestException("Platform does not support tag sizes other than 128-bit"); - } - using (var aesGcm = new AesGcm(testCase.Key)) { byte[] ciphertext = new byte[testCase.Plaintext.Length]; byte[] tag = new byte[testCase.Tag.Length]; - aesGcm.Encrypt(testCase.Nonce, testCase.Plaintext, ciphertext, tag, testCase.AssociatedData); - Assert.Equal(testCase.Ciphertext, ciphertext); - Assert.Equal(testCase.Tag, tag); - byte[] plaintext = new byte[testCase.Plaintext.Length]; - aesGcm.Decrypt(testCase.Nonce, ciphertext, tag, plaintext, testCase.AssociatedData); - Assert.Equal(testCase.Plaintext, plaintext); + if (PlatformDetection.IsOSX && testCase.Tag.Length != CryptoKitSupportedTagSizeInBytes) + { + Assert.Throws("tag", () => + { + aesGcm.Encrypt(testCase.Nonce, testCase.Plaintext, ciphertext, tag, testCase.AssociatedData); + }); + Assert.Throws("tag", () => + { + byte[] plaintext = new byte[testCase.Plaintext.Length]; + aesGcm.Decrypt(testCase.Nonce, ciphertext, tag, testCase.Ciphertext, testCase.AssociatedData); + }); + } + else + { + aesGcm.Encrypt(testCase.Nonce, testCase.Plaintext, ciphertext, tag, testCase.AssociatedData); + Assert.Equal(testCase.Ciphertext, ciphertext); + Assert.Equal(testCase.Tag, tag); + + byte[] plaintext = new byte[testCase.Plaintext.Length]; + aesGcm.Decrypt(testCase.Nonce, ciphertext, tag, plaintext, testCase.AssociatedData); + Assert.Equal(testCase.Plaintext, plaintext); + } } } - [ConditionalTheory] + [Theory] [MemberData(nameof(GetNistGcmTestCases))] [ActiveIssue("https://github.com/dotnet/runtime/issues/51332", TestPlatforms.iOS | TestPlatforms.tvOS | TestPlatforms.MacCatalyst)] public static void AesGcmNistTestsTamperTag(AEADTest testCase) { - if (PlatformDetection.IsOSX && - testCase.Tag.Length != CryptoKitSupportedTagSizeInBytes && - !PlatformDetection.OpenSslPresentOnSystem) - { - throw new SkipTestException("Platform does not support tag sizes other than 128-bit"); - } - using (var aesGcm = new AesGcm(testCase.Key)) { - byte[] ciphertext = new byte[testCase.Plaintext.Length]; - byte[] tag = new byte[testCase.Tag.Length]; - aesGcm.Encrypt(testCase.Nonce, testCase.Plaintext, ciphertext, tag, testCase.AssociatedData); - Assert.Equal(testCase.Ciphertext, ciphertext); - Assert.Equal(testCase.Tag, tag); - - tag[0] ^= 1; - - byte[] plaintext = new byte[testCase.Plaintext.Length]; - RandomNumberGenerator.Fill(plaintext); - Assert.Throws( - () => aesGcm.Decrypt(testCase.Nonce, ciphertext, tag, plaintext, testCase.AssociatedData)); - Assert.Equal(new byte[plaintext.Length], plaintext); + if (PlatformDetection.IsOSX && testCase.Tag.Length != CryptoKitSupportedTagSizeInBytes) + { + byte[] plaintext = new byte[testCase.Plaintext.Length]; + byte[] tamperedTag = testCase.Tag.AsSpan().ToArray(); + tamperedTag[0] ^= 1; + + Assert.Throws("tag", () => + { + aesGcm.Decrypt(testCase.Nonce, testCase.Ciphertext, tamperedTag, plaintext, testCase.AssociatedData); + }); + } + else + { + byte[] ciphertext = new byte[testCase.Plaintext.Length]; + byte[] tag = new byte[testCase.Tag.Length]; + aesGcm.Encrypt(testCase.Nonce, testCase.Plaintext, ciphertext, tag, testCase.AssociatedData); + Assert.Equal(testCase.Ciphertext, ciphertext); + Assert.Equal(testCase.Tag, tag); + + tag[0] ^= 1; + + byte[] plaintext = new byte[testCase.Plaintext.Length]; + RandomNumberGenerator.Fill(plaintext); + Assert.Throws( + () => aesGcm.Decrypt(testCase.Nonce, ciphertext, tag, plaintext, testCase.AssociatedData)); + Assert.Equal(new byte[plaintext.Length], plaintext); + } } } @@ -395,28 +400,32 @@ public static void AesGcmNistTestsTamperTag(AEADTest testCase) [ActiveIssue("https://github.com/dotnet/runtime/issues/51332", TestPlatforms.iOS | TestPlatforms.tvOS | TestPlatforms.MacCatalyst)] public static void AesGcmNistTestsTamperCiphertext(AEADTest testCase) { - if (PlatformDetection.IsOSX && - testCase.Tag.Length != CryptoKitSupportedTagSizeInBytes && - !PlatformDetection.OpenSslPresentOnSystem) - { - throw new SkipTestException("Platform does not support tag sizes other than 128-bit"); - } - using (var aesGcm = new AesGcm(testCase.Key)) { - byte[] ciphertext = new byte[testCase.Plaintext.Length]; - byte[] tag = new byte[testCase.Tag.Length]; - aesGcm.Encrypt(testCase.Nonce, testCase.Plaintext, ciphertext, tag, testCase.AssociatedData); - Assert.Equal(testCase.Ciphertext, ciphertext); - Assert.Equal(testCase.Tag, tag); - - ciphertext[0] ^= 1; - - byte[] plaintext = new byte[testCase.Plaintext.Length]; - RandomNumberGenerator.Fill(plaintext); - Assert.Throws( - () => aesGcm.Decrypt(testCase.Nonce, ciphertext, tag, plaintext, testCase.AssociatedData)); - Assert.Equal(new byte[plaintext.Length], plaintext); + if (PlatformDetection.IsOSX && testCase.Tag.Length != CryptoKitSupportedTagSizeInBytes) + { + byte[] tamperedCiphertext = testCase.Ciphertext.AsSpan().ToArray(); + tamperedCiphertext[0] ^= 1; + Assert.Throws("tag", () => + { + aesGcm.Decrypt(testCase.Nonce, tamperedCiphertext, testCase.Tag, testCase.Plaintext, testCase.AssociatedData); + }); + } + else + { + byte[] ciphertext = new byte[testCase.Plaintext.Length]; + byte[] tag = new byte[testCase.Tag.Length]; + aesGcm.Encrypt(testCase.Nonce, testCase.Plaintext, ciphertext, tag, testCase.AssociatedData); + Assert.Equal(testCase.Ciphertext, ciphertext); + Assert.Equal(testCase.Tag, tag); + + ciphertext[0] ^= 1; + + byte[] plaintext = RandomNumberGenerator.GetBytes(testCase.Plaintext.Length); + Assert.Throws( + () => aesGcm.Decrypt(testCase.Nonce, ciphertext, tag, plaintext, testCase.AssociatedData)); + AssertExtensions.FilledWith(0, plaintext); + } } } From a6921ee84e80f7fcfe7c9dc65f694566d4edb61d Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Mon, 7 Nov 2022 14:17:58 -0500 Subject: [PATCH 6/8] Remove conditional from tests since it's not needed anymore --- .../System.Security.Cryptography/tests/AesGcmTests.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Security.Cryptography/tests/AesGcmTests.cs b/src/libraries/System.Security.Cryptography/tests/AesGcmTests.cs index f25081224f7b99..3932f8fe9fe0fb 100644 --- a/src/libraries/System.Security.Cryptography/tests/AesGcmTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/AesGcmTests.cs @@ -3,7 +3,6 @@ using System.Collections.Generic; using System.Linq; -using Microsoft.DotNet.XUnitExtensions; using Test.Cryptography; using Xunit; @@ -14,7 +13,7 @@ public class AesGcmTests : CommonAEADTests { private const int CryptoKitSupportedTagSizeInBytes = 16; - [ConditionalTheory] + [Theory] [ActiveIssue("https://github.com/dotnet/runtime/issues/51332", TestPlatforms.iOS | TestPlatforms.tvOS | TestPlatforms.MacCatalyst)] [MemberData(nameof(EncryptTamperAADDecryptTestInputs))] public static void EncryptTamperAADDecrypt(int dataLength, int additionalDataLength) @@ -395,7 +394,7 @@ public static void AesGcmNistTestsTamperTag(AEADTest testCase) } } - [ConditionalTheory] + [Theory] [MemberData(nameof(GetNistGcmTestCasesWithNonEmptyPT))] [ActiveIssue("https://github.com/dotnet/runtime/issues/51332", TestPlatforms.iOS | TestPlatforms.tvOS | TestPlatforms.MacCatalyst)] public static void AesGcmNistTestsTamperCiphertext(AEADTest testCase) From 59de3e3406a11c52c3864cb48b57a8d9737deb15 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Mon, 7 Nov 2022 18:38:38 -0500 Subject: [PATCH 7/8] Code review feedback --- .../Interop.Aead.cs | 1 + .../src/System/Security/Cryptography/AesGcm.macOS.cs | 1 - .../System.Security.Cryptography/tests/AesGcmTests.cs | 4 ++-- .../pal_swiftbindings.swift | 8 ++++---- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Aead.cs b/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Aead.cs index d2a23ba54b48c6..edadae0ea60ecf 100644 --- a/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Aead.cs +++ b/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Aead.cs @@ -86,6 +86,7 @@ internal static unsafe void ChaCha20Poly1305Decrypt( } } } + internal static unsafe void AesGcmEncrypt( ReadOnlySpan key, ReadOnlySpan nonce, diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.macOS.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.macOS.cs index 2313539f1e06bd..5b5b2638164f7a 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.macOS.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.macOS.cs @@ -17,7 +17,6 @@ public sealed partial class AesGcm // CryptoKit only supports 16 byte tags. public static KeySizes TagByteSizes { get; } = new KeySizes(16, 16, 1); - [MemberNotNull(nameof(_key))] private void ImportKey(ReadOnlySpan key) { diff --git a/src/libraries/System.Security.Cryptography/tests/AesGcmTests.cs b/src/libraries/System.Security.Cryptography/tests/AesGcmTests.cs index 3932f8fe9fe0fb..741b28020281f9 100644 --- a/src/libraries/System.Security.Cryptography/tests/AesGcmTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/AesGcmTests.cs @@ -25,7 +25,7 @@ public static void EncryptTamperAADDecrypt(int dataLength, int additionalDataLen byte[] ciphertext = new byte[dataLength]; byte[] key = new byte[16]; byte[] nonce = new byte[AesGcm.NonceByteSizes.MinSize]; - byte[] tag = new byte[AesGcm.TagByteSizes.MaxSize]; + byte[] tag = new byte[AesGcm.TagByteSizes.MinSize]; RandomNumberGenerator.Fill(key); RandomNumberGenerator.Fill(nonce); @@ -84,7 +84,7 @@ public static void ValidNonceSize(int nonceSize) byte[] ciphertext = new byte[dataLength]; byte[] key = new byte[16]; byte[] nonce = new byte[nonceSize]; - byte[] tag = new byte[AesGcm.TagByteSizes.MaxSize]; + byte[] tag = new byte[AesGcm.TagByteSizes.MinSize]; RandomNumberGenerator.Fill(key); RandomNumberGenerator.Fill(nonce); diff --git a/src/native/libs/System.Security.Cryptography.Native.Apple/pal_swiftbindings.swift b/src/native/libs/System.Security.Cryptography.Native.Apple/pal_swiftbindings.swift index e1e4f9578953f6..7b04d52504fa24 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Apple/pal_swiftbindings.swift +++ b/src/native/libs/System.Security.Cryptography.Native.Apple/pal_swiftbindings.swift @@ -67,12 +67,12 @@ public func AppleCryptoNative_ChaCha20Poly1305Decrypt( return 0 } - guard let sealedBoxRestored = try? ChaChaPoly.SealedBox(nonce: nonce, ciphertext: ciphertext, tag: tag) else { + guard let sealedBox = try? ChaChaPoly.SealedBox(nonce: nonce, ciphertext: ciphertext, tag: tag) else { return 0 } do { - let result = try ChaChaPoly.open(sealedBoxRestored, using: symmetricKey, authenticating: aad) + let result = try ChaChaPoly.open(sealedBox, using: symmetricKey, authenticating: aad) assert(plaintextBufferLength >= result.count) result.copyBytes(to: plaintextBuffer, count: result.count) @@ -149,12 +149,12 @@ public func AppleCryptoNative_AesGcmDecrypt( return 0 } - guard let sealedBoxRestored = try? AES.GCM.SealedBox(nonce: nonce, ciphertext: ciphertext, tag: tag) else { + guard let sealedBox = try? AES.GCM.SealedBox(nonce: nonce, ciphertext: ciphertext, tag: tag) else { return 0 } do { - let result = try AES.GCM.open(sealedBoxRestored, using: symmetricKey, authenticating: aad) + let result = try AES.GCM.open(sealedBox, using: symmetricKey, authenticating: aad) assert(plaintextBufferLength >= result.count) result.copyBytes(to: plaintextBuffer, count: result.count) From ef9b4735cb28287d4f9adbebd354d05d023a5ebd Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Tue, 8 Nov 2022 10:46:16 -0500 Subject: [PATCH 8/8] Restart CI