[WIP] Apple: Adopt FFI changes and update the API exposed to Apple clients #20
Conversation
It just delegates all actions to its delegate. Adapter will become its delegate and handle generating network settings and applying it on the packet tunnel.
The adapter generates the network settings from the tunnel addresseses and the resources obtained from the ffi callbacks. It saves previously obtained data for use when it needs to update the settings. When starting the tunnel, we don't apply the settings until WrappedSession has started, so that we don't call setTunnelSettings multiple times (once for tunnel addresses, and once for resources).
roop
changed the title
There was a problem hiding this comment.
What's here so far looks excellent; thanks for cleaning up the Swift code so much!
Note that we're transitioning to a monorepo (excluding the clients) in firezone/firezone#1656 (that PR is based on #18 instead of connlib's master); I apologize for the lack of prior communication on this, but you'll need to recreate this PR once the transition is complete. Fortunately, #18 moved these files but didn't change them, so there will be no conflicts.
Additionally, because the macOS/iOS build is currently broken on #18, I've been working on fixing it. I'll open a PR for that as soon as firezone/firezone#1656 is merged; your PR should ultimately be based on my upcoming PR, since you won't be able to test your changes otherwise. Let me know if waiting for my PR blocks you and we'll work something out.
| // SPDX-License-Identifier: MIT | ||
| // Copyright © 2018-2023 WireGuard LLC. All Rights Reserved. |
There was a problem hiding this comment.
For documentation purposes, it'd be good to include some comments at the very top explaining the rationale for copying this file + the permanent address of the original for reference, i.e.
// (put explanation here)
// https://github.com/WireGuard/wireguard-apple/blob/7b279383d1f5aeb939bef6507b95b33afbb577e6/Sources/WireGuardKit/IPAddressRange.swift
// SPDX-License-Identifier: MIT
// Copyright © 2018-2023 WireGuard LLC. All Rights Reserved.Also, the attribution requirement of the MIT license will additionally need to be upheld in the Apple client when distributed; if we haven't decided how to handle that yet then we can put it on a list of "things we need to remember to put in an attribution for before we release".
There was a problem hiding this comment.
I'm using that as an easy way to convert "100.64.0.0/10" (from ResourceList) to {"100.64.0.0", "255.192.0.0"} (the way NEIPv4Route wants).
Will add the rationale in the file.
| ipv6Settings.includedRoutes = ipv6Routes | ||
|
|
||
| let networkSettings = NEPacketTunnelNetworkSettings(tunnelRemoteAddress: "127.0.0.1") |
There was a problem hiding this comment.
Is the deleted comment beginning with "iOS requires a tunnel endpoint" still relevant/helpful here?
There was a problem hiding this comment.
It's still relevant (comes from WireguardKit as well). I'll retain it.
| /* If the tunnel interface addresses are being updated, it's impossible for the tunnel to | ||
| stay up due to the way WireGuard works. Still, we try not to change the tunnel's routes | ||
| here Just In Case™. | ||
| */ |
There was a problem hiding this comment.
Maybe not. I think we can update the routes without bringing down the tunnel, but I'm not fully sure because I haven't tried it in practice.
Documentation and forums suggest we are allowed to call setTunnelNetworkSettings() while the tunnel is up. Have not checked it in practice.
Okay, I'll wait for your PR to get things building and then modify this PR to be on top of that. Meanwhile, I'll rework this PR to adopt your comments. Additionally, I see a weird issue in iOS, where the app fails to load connlib at app launch with this error: I'll investigate that as well. |
It's WIP because I'm investigating these issues:
In macOS:
When I run it in macOS, the tunnel process exits after a while. It seems to be because connlib cannot create a utun device in macOS.
In iOS:
dyld fails to load connlib.