Add key negotiation state

[buggmagnet]

This PR adds the ability to dynamically toggle the PQC on and off

---

This change is 

[acb-mv]

Reviewed 15 of 15 files at r1, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved

---

ios/MullvadPostQuantum/PostQuantumKeyNegotiatior.swift line 36 at r1 (raw file):

            opaqueConnection
        )
        guard token?.hashValue != 0.hashValue else {

It looks like in Swift, UnsafeMutableRawPointer is not nullable, so we should probably see if we can make the FFI take an optional of this.

[pinkisemils]

Reviewable status: 8 of 19 files reviewed, 1 unresolved discussion (waiting on @acb-mv and @buggmagnet)

---

.github/workflows/git-commit-message-style.yml line 32 at r2 (raw file):

          max-subject-line-length: '72'
          # The action's wordlist is a bit short. Add more accepted verbs
          additional-verbs: 'restart, coalesce, tidy'

Did you ask @faern or anyone else for their opinion on this? I don't disagree with the change, but everyone should be on board with changes here 🙂

[acb-mv]

Reviewable status: 8 of 20 files reviewed, 1 unresolved discussion (waiting on @faern)

---

.github/workflows/git-commit-message-style.yml line 32 at r2 (raw file):

Previously, pinkisemils (Emīls Piņķis) wrote…

Did you ask @faern or anyone else for their opinion on this? I don't disagree with the change, but everyone should be on board with changes here 🙂

I messaged him on Slack with a link to the commit, though after the fact.

[acb-mv]

Reviewable status: 8 of 20 files reviewed, 1 unresolved discussion (waiting on @faern)

---

.github/workflows/git-commit-message-style.yml line 32 at r2 (raw file):

Previously, acb-mv wrote…

I messaged him on Slack with a link to the commit, though after the fact.

He replied, and seems OK with the addition. https://mullvad.slack.com/archives/D06M20B17F0/p1711547132418499

[faern]

Reviewable status: 8 of 20 files reviewed, 1 unresolved discussion (waiting on @acb-mv and @pinkisemils)

---

.github/workflows/git-commit-message-style.yml line 32 at r2 (raw file):

Previously, acb-mv wrote…

He replied, and seems OK with the addition. https://mullvad.slack.com/archives/D06M20B17F0/p1711547132418499

Adding reasonable imperative verbs is OK! The list is not complete. However, given that I have scraped all imperative verbs from our 20k+ commit history I hope there won't be too many more 😅.

Nit: I think you should add this in a separate PR. Separate things separate. And you can get that PR done and merged in a few minutes.

[pinkisemils]

Reviewable status: 8 of 20 files reviewed, 1 unresolved discussion (waiting on @acb-mv and @buggmagnet)

---

ios/MullvadPostQuantum/talpid-tunnel-config-client/src/ios_ffi.rs line 13 at r2 (raw file):

pub unsafe extern "C" fn cancel_post_quantum_key_exchange(sender: *const c_void) {
    // Try to take the value, if there is a value, we can safely send the message, otherwise, assume it has been dropped and nothing happens
    let send_tx: Weak<mpsc::Sender<()>> = unsafe { Weak::from_raw(sender as _) };

We're using an rc::Weak whereas the pointer that is actually being passed here is pointing to an sync::Weak. This is not sound.

[pinkisemils]

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

---

ios/MullvadPostQuantum/talpid-tunnel-config-client/src/lib.rs line 28 at r2 (raw file):

    packet_tunnel: *const c_void,
    tcp_connection: *const c_void,
) -> *const c_void {

Instead of returning a c_void, I'd define a struct that wraps the pointer so that the Swift side can be somewhat more typesafe. Using void pointers gets confusing quickly.

[pinkisemils]

Reviewable status: 3 of 160 files reviewed, 3 unresolved discussions (waiting on @acb-mv and @buggmagnet)

---

ios/build-rust-library.sh line 43 at r6 (raw file):

# When this happens, cargo will be tricked into building for the wrong architecture, which will lead to linker issues down the line.
# cargo does not need to know about all this, therefore, set the path to the bare minimum
export PATH="${HOME}/.cargo/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/Library/Apple/usr/bin:"

Instead of hardcoding these paths, let's source ~/.bashrc or ~/.zshenv.

[faern]

Reviewable status: 3 of 160 files reviewed, 4 unresolved discussions (waiting on @acb-mv and @buggmagnet)

---

Cargo.toml line 14 at r6 (raw file):

    "ios/MullvadREST/Transport/Shadowsocks/shadowsocks-proxy",
    "ios/TunnelObfuscation/tunnel-obfuscator-proxy",
    "ios/MullvadPostQuantum/talpid-tunnel-config-client",

Might not be ideal to have multiple Rust crates with the exact same name. If a crate is needed here, consider naming it uniquely. But if they have a relationship, make that apparent in a good way 🙏

[faern]

Reviewable status: 3 of 160 files reviewed, 6 unresolved discussions (waiting on @acb-mv and @buggmagnet)

---

ios/MullvadPostQuantum/talpid-tunnel-config-client/build.rs line 2 at r6 (raw file):

fn main() {
    tonic_build::compile_protos("../../../talpid-tunnel-config-client/proto/tunnel_config.proto")

There should be no reason to compile these proto files twice. This crate depend on talpid-tunnel-config-client and that crate alread build this proto file. Both should definitely not do it.

---

ios/MullvadPostQuantum/talpid-tunnel-config-client/Cargo.toml line 2 at r6 (raw file):

[package]
name = "talpid-tunnel-config-client-proxy"

As discussed in the office. I'm very hesitant towards having this wrapper crate or whatever you want to call it. It should at least not depend on all the crypt libs again. There is no reason to depend on them twice.