Add experimental Swift bindings for CryptoKit

[kotlarmilos]

Description

This PR introduces experimental Swift bindings for CryptoKit, aiming to eliminate the Swift wrapper. It validates the Swift interop and demonstrates the Swift bindings surface for end-users.

The bindings are manually created as a proof of concept.

Current limitations:

• Non-frozen type payload is manually retrieved from metadata
• Protocol descriptor symbols required for protocol witness tables are missing in the JSON ABI file
• SwiftSelf must be the last parameter per calling convention which is against the existing implementation

If these changes are considered as low-risk, we will proceed with the PR. Otherwise, we will close it after passing tests. The next steps include updating the projection tooling to automate these bindings.

Resolves dotnet/runtimelab#2580

[matouskozak]

Good job Milos and Jeremi with the CryptoKit bindings. I put a few comments/questions to clarify my thoughts.

[stephen-hawley]

What a great effort and I'm so happy to see how this has come together.
Can't wait to get to the point where we can do this in a way that's automated.

[stephen-hawley]

We don't know if CryptoKit.framework will move over time or will live in different locations on different platforms. Generally speaking, what you want to do is build the library referencing CryptoKit.framework and then rely on a post-build step to run install_name_tool to set paths that point to the appropriate location for the platform.

[kotlarmilos]

Absolute paths are used in this setup: https://github.com/dotnet/runtime/blob/main/src/libraries/Common/src/Interop/OSX/Interop.Libraries.cs

Can a mangled name differ per architecture? There are test failures:

System.EntryPointNotFoundException : Unable to find an entry point named '$s10Foundation4DataV5bytes5countACSV_SitcfC' in shared library '/System/Library/Frameworks/Foundation.framework/Foundation'

[stephen-hawley]

I know you're trying to get a lot of infrastructure in place, but I would prefer the ISwiftType interface to be safe rather than unsafe. To that end, we can define a type like this:

public struct SwiftMetadata {
    public SwiftMetadata (NativeHandle handle) { // or IntPtr if we want
        this.handle = handle;
    }
    NativeHandle handle;
    // other methods as are needed
}

In addition, using a strong type for this is a way to ensure that we don't have conflicts between function signatures (for example foo(nint x) conflicts with foo(IntPtr x)) and we can add useful methods onto it, such as asking what the underlying type is, getting the nominal type descriptor, getting the size of the instance, etc.

[stephen-hawley]

Same idea. If we're going to have a protocol interface, let's not make it safe and public. We can use a placeholder struct for the return value.

[stephen-hawley]

The pointers all need some revision. Swift has very specific semantics that surround pointers that are used to ensure that they play nicely with the swift memory model. It's is not OK to initialize the pointer value to a T *.
Swift unmanaged pointers have the following states:

1. Unallocated
2. Allocated
3. Initialized
4. Deinitialized
5. Deallocated

On top of that, allocation and deallocation should always be owned by Swift and not by us.

Imagine this scenario in Swift:

public func consumePointerContents<T>(x: UnsafeMutablePointer<T>) {
    print ("consuming \(x.pointee)");
    x.deinitialize ();
    x.deallocate ();
}

If we call this from C# with a non-swift allocated and initialized UnsafeMutablePointer, we're in a load of trouble.

Similarly, in the mutable versions of these, Pointee should be get/set, but we should call swift to make the changes because the Swift runtime will take care of using the appropriate witness table methods to ensure that reference counting works properly.

[jkoritzinsky]

Could we define a wrapper of the P/Invokes that takes two ISwiftType values and calls the WitnessTable functions in the wrapper so we don't have to do it manually?

[kotlarmilos]

I've moved Swift-specific features to the bindings to simulate projection tooling behavior. I believe it's reasonable to handle marshalling in the projections rather than in the user-facing APIs. Here's an example of the API:

Data symmetricKeyData = new Data(keyPtr, key.Length);
SymmetricKey symmetricKey = new SymmetricKey(symmetricKeyData);

Data nonceData = new Data(noncePtr, nonce.Length);
ChaChaPoly.Nonce chaChaPolyNonce = new ChaChaPoly.Nonce(nonceData);

Data plaintextData = new Data(plaintextPtr, plaintext.Length);
Data aadData = new Data(aadPtr, aad.Length);

ChaChaPoly.SealedBox sealedBox = ChaChaPoly.seal(
    plaintextData,
    symmetricKey,
    chaChaPolyNonce,
    aadData,
    out SwiftError error);

if (error.Value != null)
{
    CryptographicOperations.ZeroMemory(ciphertext);
    CryptographicOperations.ZeroMemory(tag);
    throw new CryptographicException();
}

Data resultCiphertext = sealedBox.Ciphertext;
Data resultTag = sealedBox.Tag;

resultCiphertext.CopyBytes(ciphertextPtr, resultCiphertext.Count);
resultTag.CopyBytes(tagPtr, resultTag.Count);

Do you think the P/Invoke wrapper would still be beneficial in this case?

[bartonjs]

While I think it's good that the runtime is supporting more interop; I do not believe we want to change to this style for the crypto/AEAD constructions. It's too noisy, and too fragile-looking. The "We have one function that performs GCM-Encrypt" and "We have one function that performs GCM-Decrypt" (x2 because s/GCM/ChaChaPoly) is much more easily audited code.

That might mean that the correct button for me to hit right now is "close", but I'll hold off a bit.

[jkoritzinsky]

The goal of this work is to enable us to remove all Swift code from our interop PAL, as that code requires additional infrastructure that's more fragile and difficult to audit and debug than directly calling into the Swift APIs themselves.

I see this work as similar to how we call into CNG on Windows. We call directly into the native APIs and we don't go through a native shim.

[bartonjs]

I see this work as similar to how we call into CNG on Windows

That's actually my least preferred crypto interop code.

more fragile and difficult to audit and debug than directly calling into the Swift APIs themselves.

No. This code has a bunch of magic consts and pointers/allocations; that makes it very very bad to understand/audit.

CNG/Win32 handles "we added more fields" by putting a cbSize on their structs and using it in lieu of a version number. I don't see anything in this Swift interop code that is version-tolerable should Apple change the layout of the types in the future.

Maybe Swift inherently says changing a type is breaking and can't ever be done, but I'd like to see something written down about that, ironclad enough that when we get it wrong and it leads to an OOB-write and a subsequent security incident that we can sic our lawyers on Apple. Absent that, this version-intolerant code is entirely unacceptable.

[jkoritzinsky]

I see this work as similar to how we call into CNG on Windows

That's actually my least preferred crypto interop code.

more fragile and difficult to audit and debug than directly calling into the Swift APIs themselves.

No. This code has a bunch of magic consts and pointers/allocations; that makes it very very bad to understand/audit.

I don't really see how this code is much different than writing Swift code that we don't understand how it's compiled down anyway. We're just doing the same thing (barring PR feedback that still needs to be addressed) that the Swift compiler does.

CNG/Win32 handles "we added more fields" by putting a cbSize on their structs and using it in lieu of a version number. I don't see anything in this Swift interop code that is version-tolerable should Apple change the layout of the types in the future.

Maybe Swift inherently says changing a type is breaking and can't ever be done, but I'd like to see something written down about that, ironclad enough that when we get it wrong and it leads to an OOB-write and a subsequent security incident that we can sic our lawyers on Apple. Absent that, this version-intolerant code is entirely unacceptable.

For types that are @frozen, their shape (size, alignment, lowering) can never change. A few of the types here (ChaChaPoly.SealedBox, Foundation.Data, etc) are frozen. Here's the most official thing I could find that we could point to if Apple were to break the ecosystem: https://docs.swift.org/swift-book/documentation/the-swift-programming-language/attributes/#frozen.

For non-frozen types (the ChaChaPoly.Nonce type for example), the value witness table defines the struct size, alignment, etc. The code above should (one of the changes that needs to be made) fetch this size from CryptoKit at runtime instead of embedding the size on the .NET side.

[agocke]

@bartonjs I'm not sure I understand your concern. It looks like the previous code made lots of ABI assumptions, and the new code also makes a lot of ABI assumptions. The main difference I see in the PR is trading Swift code for C# code. Broadly, that seems like a good thing for our team. It is unfortunate that the bindings are manually written at the moment, opening us up to typos. Would auto-generation solve the problem?

[jkotas]

Would auto-generation solve the problem?

The extra code related to the convenience (related to my comment #108318 (comment)) is both introducing unnecessary perf overhead, and it makes the code more complex. I think simpler, less convenient bindings, would be more palatable. Auto-generating them is nice, but it is not strictly required.

[bartonjs]

It is unfortunate that the bindings are manually written at the moment, opening us up to typos.

My main dislike of this (and with "CS-Win32") is that we're injecting a human step of transliterating things from the library's supported interface methodology (.h, whatever swift's version of .h is) into C#.

@frozen means that the size of the structure can't ever change (on this processor)... OK... you know who already knows that? A swift compiler. What's a witness table? I don't know. Is it important? Not to me... but it is to a compiler. This approach means that now I (as a reviewer, or a code author) need to be compiler-dev fluent in Swift in order to do my job.

Before:

                AppleCryptoNative_ChaCha20Poly1305Encrypt(
                    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),
                    out SwiftError error);

After:

                var sealedBox = CryptoKit.PInvoke_ChaChaPoly_Seal(
                    messagePtr,
                    swiftKey.payload,
                    swiftNonce.payload,
                    authenticatedDataPtr,
                    plaintextData.Metadata(),
                    aadData.Metadata(),
                    default(DataProtocol).WitnessTable(plaintextData),
                    default(DataProtocol).WitnessTable(aadData),
                    out SwiftError error);

What's default(DataProtocol).WitnessTable(plaintextData) do? Beats me. Is it important? Beats me. ChaChaPoly.Seal is defined as taking 4 parameters. This passes 8 in and 1 out. I have literally no ability to reason about that.

The original version used a tightly controlled C layer as the contract between C# and the swift code.

                int result = AppleCryptoNative_ChaCha20Poly1305Encrypt(
                    keyPtr, key.Length,
                    noncePtr, nonce.Length,
                    plaintextPtr, plaintext.Length,
                    ciphertextPtr, ciphertext.Length,
                    tagPtr, tag.Length,
                    aadPtr, aad.Length);

Changing that to UnsafeBufferPointer and UnsafeMutableBufferPointer was fine (though I remain upset by the introduction of custom types to handle it instead of the clear (though slightly repetitive) code that it replaced).

And, while ranting, the compiler is also responsible for knowing "s9CryptoKit03ChaC4PolyO4seal_5using5nonce14authenticatingAC9SealedBoxVx_AA12SymmetricKeyVAC5NonceVSgq_tK10Foundation12DataProtocolRzAoPR_r0_lFZ" is the name of the function to jump to.

So...

Would auto-generation solve the problem?

It would improve it. My favorite auto-generation, though, would be from the swift compiler.

[bartonjs]

And, for what it's worth, it's increasing any/all P/Invoke machinery (GC suspend, et al) at least 5x, since what we used to do in one P/invoke is now 5.

[agocke]

It would improve it. My favorite auto-generation, though, would be from the swift compiler.

In this case we would be generating the binding from the output of the Swift compiler, so it's effectively the same.

But more broadly, it looks like this is an optimization problem with four independent variables that we're trying to minimize:

1. Code complexity
2. Library/abstraction overhead
3. P/Invokes
4. Swift code

It sounds like we have improvements that can be made for (1) and (2). It seems plausible that this isn't even an area where automatic projection is beneficial and we might want to handle this one manually.

(3) and (4) seem to be in tension. It doesn't sound like there's a single Swift API that does everything we want. So we have the choice of massaging things together using multiple calls into Swift, or writing a wrapper function in Swift to perform this action on the Swift side.

I'd like to better understand which tradeoff you would prefer to make here. It sounds like you'd prioritize minimizing P/Invokes, which somewhat surprises me. That means that you prefer to sign-off as an owner of this Swift code, and also a Swift expert in the subset used in this code -- and you also want to commit to maintaining that expertise in all newly released Swift language and compiler versions (we automatically update our toolset to latest every release). My assumption would have been that you would want to minimize Swift ownership. Could you describe why you prefer that to handing over validation of the ABI compat to the JIT/interop teams?

[bartonjs]

It sounds like you'd prioritize minimizing P/Invokes

Not prioritizing, but pointing out that this changes the landscape. Let's consider PBKDF2. There's a step that basically boils down to

Span<byte> block = ...;

for (int i = 0; i < iterationCount; i++) {
   Hash(block, block);
}

Since iterationCount is probably between 100k and 200k, that's a lot of P/Invokes if that is written in the "P/Invoke every function" style. Assuming a wrapper library already exists, we'd instead move the loop to the other side of the P/Invoke, and get substantially faster for the same work.

5 calls vs 1 isn't as bad as n-hundred-thousand vs 1; but it's still something we're conscious of.

It sounds like you'd prioritize minimizing P/Invokes

OK, that might actually be true. One of the things I have to do every year is update our documentation of "for each cryptographic operation you {perform|expose} performed in terms of some other library, how do you call that library?". The shim having a very tight "this is how we encrypt" -- and it being written in the language relevant to the system library -- makes that job easier.

That means that you prefer to sign-off as an owner of this Swift code, and also a Swift expert in the subset used in this code -- and you also want to commit to maintaining that expertise in all newly released Swift language and compiler versions (we automatically update our toolset to latest every release).

Over what this change looks like? Absolutely. I don't want to see "clever" Swift code; but I have no objection to the use of Swift to call Swift.

[stephen-hawley]

It is unfortunate that the bindings are manually written at the moment, opening us up to typos.

My main dislike of this (and with "CS-Win32") is that we're injecting a human step of transliterating things from the library's supported interface methodology (.h, whatever swift's version of .h is) into C#.

@frozen means that the size of the structure can't ever change (on this processor)... OK... you know who already knows that? A swift compiler. What's a witness table? I don't know. Is it important? Not to me... but it is to a compiler. This approach means that now I (as a reviewer, or a code author) need to be compiler-dev fluent in Swift in order to do my job.

There will always be a dividing line between Swift and C# because while the languages are semantically very similar, the implementation is very, very different. Swift has a deconstructed type system in that the elements that make the type system work live in separate objects that are only esoterically connected. So the same way that when you're calling a C++ method you can get away with knowing that there is a first argument that is implicit, Swift does the same thing with generics and PATs. That knowledge can't all live on the Swift side unless we want to make some very fundamental changes in the way that C# looks at pinvokes.

So if you're calling a function in Swift of the form public func someFunc<T: SomePAT>(x: T) there are now two extra implicit arguments that the Swift compiler knows about that C# doesn't: the type metadata for T and the protocol witness table for T with respect to SomePAT. In order to call the func from C# with a pinvoke, those arguments need to be there but we can't make the C# pinvoke into someFunc generic unless and able to understand the Swift implicit arguments without a fundamental change to C# because C# doesn't let pinvokes be generic.

Further, I would suggest that attention to the pinvokes is warranted only inasmuch as we have all the various argument passing cases working and tested for automated binding. Past that, we've put our knowledge of Swift's calling conventions into two places: to the greatest extent possible in the runtime and in the binding code. If one of our customers is calling a generated pinvoke themselves then we have done something very wrong.

[bartonjs]

I didn't really understand that. But, that's fine. I don't need to P/Invoke into a Swift function. I need to a) P/Invoke, and b) end up in a Swift function. Making a wrapper that exposes a clean C API seems the best answer. Which is what we started with.

[bartonjs]

This change is unacceptably brittle, and is not desired by the cryptography team.

[agocke]

Over what this change looks like? Absolutely. I don't want to see "clever" Swift code; but I have no objection to the use of Swift to call Swift.

Sure, that's your choice. In that case, I think you're right -- there's not a lot of value in increasing the use of more complicated Swift interop in this code. The tradeoff is more complex interop vs understanding and owning more Swift, and you're choosing owning more Swift. It's a fine choice, as long as it's made deliberately.