Convert our CryptoKit bindings to use Swift structs at the boundary (#…

…102583) * Try converting our CryptoKit bindings to use more Swift types. Also refactor our cryptokit bindings to reduce duplication. * Fix build on macos * Remove target-typed new and other style changes * Don't pass down null with UnsafeBufferPointer * Call copyBytes with a pointer because everything else seems to fail some assert for some reason I can't figure out. * Make copies and use copyBytes(to: buffer) again. * Fix generic value types condition * Put SwiftError parameter first to try working around Mono register allocation bug * Comma
dotnet · Jun 4, 2024 · 43dbffa · 43dbffa
1 parent 35aef5c
commit 43dbffa
Show file tree

Hide file tree

Showing 8 changed files with 300 additions and 255 deletions.
diff --git a/src/libraries/Common/src/Interop/OSX/Swift.Runtime/UnsafeBufferPointer.cs b/src/libraries/Common/src/Interop/OSX/Swift.Runtime/UnsafeBufferPointer.cs
@@ -0,0 +1,39 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace Swift.Runtime
+{
+    // <summary>
+    // Represents Swift UnsafeBufferPointer in C#.
+    // </summary>
+    internal readonly unsafe struct UnsafeBufferPointer<T> where T : unmanaged
+    {
+        private readonly T* _baseAddress;
+        private readonly nint _count;
+        public UnsafeBufferPointer(T* baseAddress, nint count)
+        {
+            _baseAddress = baseAddress;
+            _count = count;
+        }
+
+        public T* BaseAddress => _baseAddress;
+        public nint Count => _count;
+    }
+
+    // <summary>
+    // Represents Swift UnsafeMutableBufferPointer in C#.
+    // </summary>
+    internal readonly unsafe struct UnsafeMutableBufferPointer<T> where T : unmanaged
+    {
+        private readonly T* _baseAddress;
+        private readonly nint _count;
+        public UnsafeMutableBufferPointer(T* baseAddress, nint count)
+        {
+            _baseAddress = baseAddress;
+            _count = count;
+        }
+
+        public T* BaseAddress => _baseAddress;
+        public nint Count => _count;
+    }
+}
diff --git a/...ibraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Aead.cs b/...ibraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Aead.cs
@@ -5,15 +5,28 @@
 using System.Diagnostics;
 using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
+using System.Runtime.InteropServices.Swift;
 using System.Security.Cryptography;
 using System.Security.Cryptography.Apple;
+using Swift.Runtime;
 
 #pragma warning disable CS3016 // Arrays as attribute arguments are not CLS Compliant
 
 internal static partial class Interop
 {
     internal static partial class AppleCrypto
     {
+        private static byte NullSentinel;
+
+        // CryptoKit doesn't do well with a null pointer for the buffer data,
+        // so provide a sentinel pointer instead.
+        private static ref readonly byte GetSwiftRef(ReadOnlySpan<byte> b)
+        {
+            return ref (b.Length == 0
+                ? ref NullSentinel
+                : ref MemoryMarshal.GetReference(b));
+        }
+
         internal static unsafe void ChaCha20Poly1305Encrypt(
             ReadOnlySpan<byte> key,
             ReadOnlySpan<byte> nonce,
@@ -24,23 +37,22 @@ internal static unsafe void ChaCha20Poly1305Encrypt(
         {
             fixed (byte* keyPtr = key)
             fixed (byte* noncePtr = nonce)
-            fixed (byte* plaintextPtr = plaintext)
-            fixed (byte* ciphertextPtr = ciphertext)
+            fixed (byte* plaintextPtr = &GetSwiftRef(plaintext))
+            fixed (byte* ciphertextPtr = &GetSwiftRef(ciphertext))
             fixed (byte* tagPtr = tag)
-            fixed (byte* aadPtr = aad)
+            fixed (byte* aadPtr = &GetSwiftRef(aad))
             {
-                const int Success = 1;
-                int result = AppleCryptoNative_ChaCha20Poly1305Encrypt(
-                    keyPtr, key.Length,
-                    noncePtr, nonce.Length,
-                    plaintextPtr, plaintext.Length,
-                    ciphertextPtr, ciphertext.Length,
-                    tagPtr, tag.Length,
-                    aadPtr, aad.Length);
-
-                if (result != Success)
+                AppleCryptoNative_ChaCha20Poly1305Encrypt(
+                    out SwiftError error,
+                    new UnsafeBufferPointer<byte>(keyPtr, key.Length),
+                    new UnsafeBufferPointer<byte>(noncePtr, nonce.Length),
+                    new UnsafeBufferPointer<byte>(plaintextPtr, plaintext.Length),
+                    new UnsafeMutableBufferPointer<byte>(ciphertextPtr, ciphertext.Length),
+                    new UnsafeMutableBufferPointer<byte>(tagPtr, tag.Length),
+                    new UnsafeBufferPointer<byte>(aadPtr, aad.Length));
+
+                if (error.Value != null)
                 {
-                    Debug.Assert(result == 0);
                     CryptographicOperations.ZeroMemory(ciphertext);
                     CryptographicOperations.ZeroMemory(tag);
                     throw new CryptographicException();
@@ -58,32 +70,30 @@ internal static unsafe void ChaCha20Poly1305Decrypt(
         {
             fixed (byte* keyPtr = key)
             fixed (byte* noncePtr = nonce)
-            fixed (byte* ciphertextPtr = ciphertext)
+            fixed (byte* ciphertextPtr = &GetSwiftRef(ciphertext))
             fixed (byte* tagPtr = tag)
-            fixed (byte* plaintextPtr = plaintext)
-            fixed (byte* aadPtr = aad)
+            fixed (byte* plaintextPtr = &GetSwiftRef(plaintext))
+            fixed (byte* aadPtr = &GetSwiftRef(aad))
             {
-                const int Success = 1;
-                const int AuthTagMismatch = -1;
-                int result = AppleCryptoNative_ChaCha20Poly1305Decrypt(
-                    keyPtr, key.Length,
-                    noncePtr, nonce.Length,
-                    ciphertextPtr, ciphertext.Length,
-                    tagPtr, tag.Length,
-                    plaintextPtr, plaintext.Length,
-                    aadPtr, aad.Length);
-
-                if (result != Success)
+                AppleCryptoNative_ChaCha20Poly1305Decrypt(
+                    out SwiftError error,
+                    new UnsafeBufferPointer<byte>(keyPtr, key.Length),
+                    new UnsafeBufferPointer<byte>(noncePtr, nonce.Length),
+                    new UnsafeBufferPointer<byte>(ciphertextPtr, ciphertext.Length),
+                    new UnsafeBufferPointer<byte>(tagPtr, tag.Length),
+                    new UnsafeMutableBufferPointer<byte>(plaintextPtr, plaintext.Length),
+                    new UnsafeBufferPointer<byte>(aadPtr, aad.Length));
+
+                if (error.Value != null)
                 {
                     CryptographicOperations.ZeroMemory(plaintext);
 
-                    if (result == AuthTagMismatch)
+                    if (AppleCryptoNative_IsAuthenticationFailure(error.Value))
                     {
                         throw new AuthenticationTagMismatchException();
                     }
                     else
                     {
-                        Debug.Assert(result == 0);
                         throw new CryptographicException();
                     }
                 }
@@ -100,23 +110,22 @@ internal static unsafe void AesGcmEncrypt(
         {
             fixed (byte* keyPtr = key)
             fixed (byte* noncePtr = nonce)
-            fixed (byte* plaintextPtr = plaintext)
-            fixed (byte* ciphertextPtr = ciphertext)
+            fixed (byte* plaintextPtr = &GetSwiftRef(plaintext))
+            fixed (byte* ciphertextPtr = &GetSwiftRef(ciphertext))
             fixed (byte* tagPtr = tag)
-            fixed (byte* aadPtr = aad)
+            fixed (byte* aadPtr = &GetSwiftRef(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)
+                AppleCryptoNative_AesGcmEncrypt(
+                    out SwiftError error,
+                    new UnsafeBufferPointer<byte>(keyPtr, key.Length),
+                    new UnsafeBufferPointer<byte>(noncePtr, nonce.Length),
+                    new UnsafeBufferPointer<byte>(plaintextPtr, plaintext.Length),
+                    new UnsafeMutableBufferPointer<byte>(ciphertextPtr, ciphertext.Length),
+                    new UnsafeMutableBufferPointer<byte>(tagPtr, tag.Length),
+                    new UnsafeBufferPointer<byte>(aadPtr, aad.Length));
+
+                if (error.Value != null)
                 {
-                    Debug.Assert(result == 0);
                     CryptographicOperations.ZeroMemory(ciphertext);
                     CryptographicOperations.ZeroMemory(tag);
                     throw new CryptographicException();
@@ -134,32 +143,30 @@ internal static unsafe void AesGcmDecrypt(
         {
             fixed (byte* keyPtr = key)
             fixed (byte* noncePtr = nonce)
-            fixed (byte* ciphertextPtr = ciphertext)
+            fixed (byte* ciphertextPtr = &GetSwiftRef(ciphertext))
             fixed (byte* tagPtr = tag)
-            fixed (byte* plaintextPtr = plaintext)
-            fixed (byte* aadPtr = aad)
+            fixed (byte* plaintextPtr = &GetSwiftRef(plaintext))
+            fixed (byte* aadPtr = &GetSwiftRef(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)
+                AppleCryptoNative_AesGcmDecrypt(
+                    out SwiftError error,
+                    new UnsafeBufferPointer<byte>(keyPtr, key.Length),
+                    new UnsafeBufferPointer<byte>(noncePtr, nonce.Length),
+                    new UnsafeBufferPointer<byte>(ciphertextPtr, ciphertext.Length),
+                    new UnsafeBufferPointer<byte>(tagPtr, tag.Length),
+                    new UnsafeMutableBufferPointer<byte>(plaintextPtr, plaintext.Length),
+                    new UnsafeBufferPointer<byte>(aadPtr, aad.Length));
+
+                if (error.Value != null)
                 {
                     CryptographicOperations.ZeroMemory(plaintext);
 
-                    if (result == AuthTagMismatch)
+                    if (AppleCryptoNative_IsAuthenticationFailure(error.Value))
                     {
                         throw new AuthenticationTagMismatchException();
                     }
                     else
                     {
-                        Debug.Assert(result == 0);
                         throw new CryptographicException();
                     }
                 }
@@ -168,66 +175,51 @@ internal static unsafe void AesGcmDecrypt(
 
         [LibraryImport(Libraries.AppleCryptoNative)]
         [UnmanagedCallConv(CallConvs = [ typeof(CallConvSwift) ])]
-        private static unsafe partial int AppleCryptoNative_ChaCha20Poly1305Encrypt(
-            byte* keyPtr,
-            int keyLength,
-            byte* noncePtr,
-            int nonceLength,
-            byte* plaintextPtr,
-            int plaintextLength,
-            byte* ciphertextPtr,
-            int ciphertextLength,
-            byte* tagPtr,
-            int tagLength,
-            byte* aadPtr,
-            int aadLength);
+        private static unsafe partial void AppleCryptoNative_ChaCha20Poly1305Encrypt(
+            out SwiftError error,
+            UnsafeBufferPointer<byte> key,
+            UnsafeBufferPointer<byte> nonce,
+            UnsafeBufferPointer<byte> plaintext,
+            UnsafeMutableBufferPointer<byte> ciphertext,
+            UnsafeMutableBufferPointer<byte> tag,
+            UnsafeBufferPointer<byte> aad);
 
         [LibraryImport(Libraries.AppleCryptoNative)]
         [UnmanagedCallConv(CallConvs = [ typeof(CallConvSwift) ])]
-        private static unsafe partial int AppleCryptoNative_ChaCha20Poly1305Decrypt(
-            byte* keyPtr,
-            int keyLength,
-            byte* noncePtr,
-            int nonceLength,
-            byte* ciphertextPtr,
-            int ciphertextLength,
-            byte* tagPtr,
-            int tagLength,
-            byte* plaintextPtr,
-            int plaintextLength,
-            byte* aadPtr,
-            int aadLength);
+        private static unsafe partial void AppleCryptoNative_ChaCha20Poly1305Decrypt(
+            out SwiftError error,
+            UnsafeBufferPointer<byte> key,
+            UnsafeBufferPointer<byte> nonce,
+            UnsafeBufferPointer<byte> ciphertext,
+            UnsafeBufferPointer<byte> tag,
+            UnsafeMutableBufferPointer<byte> plaintext,
+            UnsafeBufferPointer<byte> aad);
 
         [LibraryImport(Libraries.AppleCryptoNative)]
         [UnmanagedCallConv(CallConvs = [ typeof(CallConvSwift) ])]
-        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);
+        private static unsafe partial void AppleCryptoNative_AesGcmEncrypt(
+            out SwiftError error,
+            UnsafeBufferPointer<byte> key,
+            UnsafeBufferPointer<byte> nonce,
+            UnsafeBufferPointer<byte> plaintext,
+            UnsafeMutableBufferPointer<byte> ciphertext,
+            UnsafeMutableBufferPointer<byte> tag,
+            UnsafeBufferPointer<byte> aad);
 
         [LibraryImport(Libraries.AppleCryptoNative)]
         [UnmanagedCallConv(CallConvs = [ typeof(CallConvSwift) ])]
-        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);
+        private static unsafe partial void AppleCryptoNative_AesGcmDecrypt(
+            out SwiftError error,
+            UnsafeBufferPointer<byte> key,
+            UnsafeBufferPointer<byte> nonce,
+            UnsafeBufferPointer<byte> ciphertext,
+            UnsafeBufferPointer<byte> tag,
+            UnsafeMutableBufferPointer<byte> plaintext,
+            UnsafeBufferPointer<byte> aad);
+
+        [LibraryImport(Libraries.AppleCryptoNative)]
+        [UnmanagedCallConv(CallConvs = new[] { typeof(CallConvSwift) })]
+        [return: MarshalAs(UnmanagedType.U1)]
+        private static unsafe partial bool AppleCryptoNative_IsAuthenticationFailure(void* error);
     }
 }
diff --git a/src/libraries/System.Security.Cryptography/src/System.Security.Cryptography.csproj b/src/libraries/System.Security.Cryptography/src/System.Security.Cryptography.csproj
@@ -1210,6 +1210,8 @@
              Link="Common\Interop\Unix\System.Security.Cryptography.Native\Interop.EvpPkey.Rsa.cs" />
     <Compile Include="$(CommonPath)Interop\Unix\System.Security.Cryptography.Native\Interop.OpenSslVersion.cs"
              Link="Common\Interop\Unix\System.Security.Cryptography.Native\Interop.OpenSslVersion.cs" />
+    <Compile Include="$(CommonPath)Interop\OSX\Swift.Runtime\UnsafeBufferPointer.cs"
+             Link="Common\Interop\OSX\Swift.Runtime\UnsafeBufferPointer.cs" />
     <Compile Include="$(CommonPath)Interop\OSX\System.Security.Cryptography.Native.Apple\Interop.Aead.cs"
              Link="Common\Interop\OSX\System.Security.Cryptography.Native.Apple\Interop.Aead.cs" />
     <Compile Include="$(CommonPath)Interop\OSX\System.Security.Cryptography.Native.Apple\Interop.Keychain.macOS.cs"

diff --git a/src/mono/mono/metadata/marshal.c b/src/mono/mono/metadata/marshal.c
@@ -6818,9 +6818,11 @@ mono_marshal_get_swift_physical_lowering (MonoType *type, gboolean native_layout
 	}
 
 	// Non-value types are illegal at the interop boundary.
-	if (type->type == MONO_TYPE_GENERICINST && !mono_type_generic_inst_is_valuetype (type)) {
-		lowering.by_reference = TRUE;
-		return lowering;
+	if (type->type == MONO_TYPE_GENERICINST) {
+		if (!mono_type_generic_inst_is_valuetype (type)) {
+			lowering.by_reference = TRUE;
+			return lowering;
+		}
 	} else if (type->type != MONO_TYPE_VALUETYPE && !mono_type_is_primitive(type)) {
 		lowering.by_reference = TRUE;
 		return lowering;

diff --git a/src/native/libs/System.Security.Cryptography.Native.Apple/CMakeLists.txt b/src/native/libs/System.Security.Cryptography.Native.Apple/CMakeLists.txt
@@ -51,7 +51,7 @@ endif()
 
 add_custom_command(
     OUTPUT pal_swiftbindings.o
-    COMMAND xcrun swiftc -emit-object -static -parse-as-library -runtime-compatibility-version none -sdk ${CMAKE_OSX_SYSROOT} -target ${SWIFT_COMPILER_TARGET} ${CMAKE_CURRENT_SOURCE_DIR}/pal_swiftbindings.swift -o pal_swiftbindings.o
+    COMMAND xcrun swiftc -emit-object -static -parse-as-library -enable-library-evolution -g -runtime-compatibility-version none -sdk ${CMAKE_OSX_SYSROOT} -target ${SWIFT_COMPILER_TARGET} ${CMAKE_CURRENT_SOURCE_DIR}/pal_swiftbindings.swift -o pal_swiftbindings.o
     MAIN_DEPENDENCY ${CMAKE_CURRENT_SOURCE_DIR}/pal_swiftbindings.swift
     COMMENT "Compiling Swift file pal_swiftbindings.swift"
 )

diff --git a/src/native/libs/System.Security.Cryptography.Native.Apple/entrypoints.c b/src/native/libs/System.Security.Cryptography.Native.Apple/entrypoints.c
@@ -48,6 +48,7 @@ static const Entry s_cryptoAppleNative[] =
     DllImportEntry(AppleCryptoNative_HmacFinal)
     DllImportEntry(AppleCryptoNative_HmacCurrent)
     DllImportEntry(AppleCryptoNative_HmacOneShot)
+    DllImportEntry(AppleCryptoNative_IsAuthenticationFailure)
     DllImportEntry(AppleCryptoNative_SecKeychainItemCopyKeychain)
     DllImportEntry(AppleCryptoNative_SecKeychainCopyDefault)
     DllImportEntry(AppleCryptoNative_SecKeychainCreate)

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
@@ -10,3 +10,4 @@ EXTERN_C void* AppleCryptoNative_ChaCha20Poly1305Encrypt;
 EXTERN_C void* AppleCryptoNative_ChaCha20Poly1305Decrypt;
 EXTERN_C void* AppleCryptoNative_AesGcmEncrypt;
 EXTERN_C void* AppleCryptoNative_AesGcmDecrypt;
+EXTERN_C void* AppleCryptoNative_IsAuthenticationFailure;