Add IAN TCP connections

[pinkisemils]

Due to 🍎 🔨 , we can't just use regular sockets to reach hosts inside the tunnel from the packet tunnel process. With these changes, we integrate the recent additions to wireguard-apple to use userspace networking stack to send TCP packets back and forth through the tunnel without asking for Apple's permission.

These changes also provide some general improvements to mullvad-ios, and asynchronize the ephemeral peer exchange pipelines, making them wait to start a new negotiation until after the tunnel has been reconfigured. Previously, the peer exchange connection was initiated before the tunnel was reconfigured, causing many a slowdown and instability.

There is a peculiarity - due to how we'd prefer not to link two whole tokio runtimes, the FFI for the TCP connections is exposed via function pointers to rust. Ideally, we'd be able to produce a static Rust library that weak links against all the rust dependencies it needs (as they'd be provided by mullvad-ios at link time), and link this into the packet tunnel executable - alas, linker plumbing requires levels of willpower this weak willed magpie cannot maintain given all the other changes.

Bear in mind, the TMP commits will be dropped before merging to main, they are there to aid with testing and development.

---

This change is 

[dlon]

Reviewed 6 of 27 files at r1, all commit messages.
Reviewable status: 6 of 27 files reviewed, 2 unresolved discussions (waiting on @pinkisemils)

---

mullvad-ios/src/ephemeral_peer_proxy/peer_exchange.rs line 14 at r1 (raw file):

};

const GRPC_HOST_CSTR: &'static CStr = unsafe { CStr::from_ptr(GRPC_HOST_PTR) };

You could use a CStr literal here.

---

mullvad-ios/src/ephemeral_peer_proxy/peer_exchange.rs line 38 at r1 (raw file):

                // CODE STENCH:
                // Swift can call this function from a tokio context. That *will* crash.
                // context.

"context."?

---

mullvad-ios/src/ephemeral_peer_proxy/peer_exchange.rs line 106 at r1 (raw file):

                async move {
                    provider
                        .connect(&GRPC_HOST_CSTR)

Nit: Pointless &.

---

mullvad-ios/src/ephemeral_peer_proxy/mod.rs line 37 at r1 (raw file):

// This is safe as long as the PacketTunnel class outlives the instance of the PacketTunnelBridge,
// and thus the ephemeral peer exchange. Since the peer exchange takes place in the packet tunnel
// process on iOS, it is certain _enough_ this will be the case.

I'm not sure this belongs here. Seems like whoever constructs a PacketTunnelBridge should have to promise that packet_tunnel will outlive it.

---

mullvad-ios/src/ephemeral_peer_proxy/mod.rs line 38 at r1 (raw file):

// and thus the ephemeral peer exchange. Since the peer exchange takes place in the packet tunnel
// process on iOS, it is certain _enough_ this will be the case.
// It is safe to implement Send for PacketTunnelBridge because the packet_tunnel

Should this line be removed?

[dlon]

Reviewed 3 of 27 files at r1.
Reviewable status: 9 of 27 files reviewed, 2 unresolved discussions (waiting on @pinkisemils)

[dlon]

Reviewable status: 9 of 27 files reviewed, 2 unresolved discussions (waiting on @pinkisemils)

---

mullvad-ios/src/ephemeral_peer_proxy/mod.rs line 37 at r1 (raw file):

Previously, dlon (David Lönnhager) wrote…

I'm not sure this belongs here. Seems like whoever constructs a PacketTunnelBridge should have to promise that packet_tunnel will outlive it.

And (somewhat unrelatedly) point out (at the FFI boundary?) that whatever function is passed packet_tunnel may be called from different threads.

[dlon]

Reviewed 1 of 27 files at r1.
Reviewable status: 10 of 27 files reviewed, 2 unresolved discussions (waiting on @pinkisemils)

---

mullvad-ios/src/ephemeral_peer_proxy/mod.rs line 37 at r1 (raw file):

Previously, dlon (David Lönnhager) wrote…

And (somewhat unrelatedly) point out (at the FFI boundary?) that whatever function is passed packet_tunnel may be called from different threads.

Then you could simply state here that the caller has promised that it is sendable.

[dlon]

Reviewable status: 10 of 27 files reviewed, 3 unresolved discussions (waiting on @pinkisemils)

---

mullvad-ios/src/ephemeral_peer_proxy/ios_tcp_connection.rs line 39 at r1 (raw file):

impl WgTcpConnectionFuncs {
    pub fn open(&self, tunnel_handle: i32, address: *const u8, timeout: u64) -> i32 {

I don't think this is actually safe at all. You could assign anything to the public function pointers, and dereference anything here.

A suggestion is to wrap WgTcpConnectionFuncs in another type, make it private, add an unsafe constructor that takes WgTcpConnectionFuncs and state as a precondition that the function pointers must be 'static and valid.

Or simply make them unsafe.

[pinkisemils]

Reviewable status: 6 of 29 files reviewed, 1 unresolved discussion (waiting on @dlon)

---

mullvad-ios/src/ephemeral_peer_proxy/ios_tcp_connection.rs line 39 at r1 (raw file):

Previously, dlon (David Lönnhager) wrote…

I don't think this is actually safe at all. You could assign anything to the public function pointers, and dereference anything here.

A suggestion is to wrap WgTcpConnectionFuncs in another type, make it private, add an unsafe constructor that takes WgTcpConnectionFuncs and state as a precondition that the function pointers must be 'static and valid.

Or simply make them unsafe.

The public visibility is needed for cbindgen to add those fields to the header file. This does remain inherently unsafe, so I have marked them unsafe now.

[dlon]

Reviewed 6 of 6 files at r2.
Reviewable status: 12 of 29 files reviewed, all discussions resolved

[pinkisemils]

Lints and tests have now all been fixed, I believe this is ready for the final review now.

[buggmagnet]

Reviewed 22 of 27 files at r1.
Reviewable status: 16 of 36 files reviewed, 1 unresolved discussion (waiting on @dlon and @pinkisemils)

---

mullvad-ios/src/ephemeral_peer_proxy/peer_exchange.rs line 36 at r1 (raw file):

            if let Some(task) = inner.task.take() {
                task.abort();
                // CODE STENCH:

nit
I've been toying with the idea that we could have our own global executor on the swift side that is used to create an async runtime context for exchanging things with rust.

In theory, that would let us do cool things like control which execution context to use when doing FFI, and potentially have a clean way to call async functions across FFI, but at this point, I'd need to spend time grinding my teeth on it to see if it's viable, or even doable.

[dlon]

Reviewed 12 of 20 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: 29 of 36 files reviewed, 1 unresolved discussion (waiting on @buggmagnet and @pinkisemils)

[acb-mv]

Reviewed 1 of 2 files at r5, all commit messages.
Reviewable status: 28 of 36 files reviewed, 2 unresolved discussions (waiting on @buggmagnet, @dlon, and @pinkisemils)

---

ios/MullvadTypes/Protocols/EphemeralPeerReceiver.swift line 33 at r6 (raw file):

    public func receivePostQuantumKey(_ key: PreSharedKey, ephemeralKey: PrivateKey) {
        let semaphore = DispatchSemaphore(value: 0)

Is there a reason to use semaphores rather than running the key reception on a serial DispatchQueue?

[pinkisemils]

Reviewable status: 28 of 36 files reviewed, 2 unresolved discussions (waiting on @acb-mv, @buggmagnet, and @dlon)

---

ios/MullvadTypes/Protocols/EphemeralPeerReceiver.swift line 33 at r6 (raw file):

Previously, acb-mv wrote…

Is there a reason to use semaphores rather than running the key reception on a serial DispatchQueue?

I did it this way to wait for an asynchronous task to finish from a blocking call. I'll gladly improve this if you have a good suggestion 🙂 I do not feel like this is an ideal solution, in fact it feels a bit fragile now.

[dlon]

lgtm

Reviewed 3 of 11 files at r7, all commit messages.
Reviewable status: 24 of 36 files reviewed, 2 unresolved discussions (waiting on @acb-mv, @buggmagnet, and @pinkisemils)

[rablador]

Reviewed 13 of 27 files at r1, 1 of 6 files at r2, 12 of 20 files at r3, 1 of 1 files at r4, 11 of 11 files at r7, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @acb-mv and @pinkisemils)

---

ios/MullvadTypes/Promise.swift line 51 at r7 (raw file):

}

public struct OneshotChannel {

This struct should be documented I think.

[pinkisemils]

Reviewable status: 35 of 36 files reviewed, 2 unresolved discussions (waiting on @acb-mv, @buggmagnet, and @rablador)

---

ios/MullvadTypes/Promise.swift line 51 at r7 (raw file):

Previously, rablador (Jon Petersson) wrote…

This struct should be documented I think.

Done.

[rablador]

Reviewed 1 of 1 files at r8, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @acb-mv)

[github-actions]

🚨 End to end tests failed. Please check the failed workflow run.