johnno1962 / InjectionIII

Re-write of Injection for Xcode in (mostly) Swift
MIT License
4.08k stars 319 forks source link

Idea for enhancment: use broadcast to allow iOS17 devices to discover connected host #463

Closed oryonatan closed 11 months ago

oryonatan commented 1 year ago

Hi @johnno1962 I have an enhancement suggestion for the app, that can really help me, I've ran a POC for that and it seems to work.

Currently when you try to use injection on a device, the device tries to find the connected computer using multiple methods, which alas in my organization all fail - as we have our iOS devices running on different subnet than our mac developer machines.

In iOS17, Apple introduced a new feature that creates a private network between each iOS device connected via USB, and the connected developer machine.

I tried making Injection connect via vs network, but faced an issue where the iPhone was not able to find the ip address of the developer machine.

Hardcoding the developer machine ip address does work, but this is an APIPA random address that is regenerated every time the cable is reconnected, so this is quite complex to manage, and requires me to add a build phase that changes the app code to hardcode the ip address on every rebuild.

There is another option however - that I did manage to get working - which is to open a port on the developer machine that responds to UDP broadcast, and than send a UDP broadcast from Injection code, recognizing the mac host ip by its response.

I've tested this code, and indeed it works and enables the iPhone to detect the developer machine over the private network:

On the developer machine - I ran the following python server:

import socket

def main():
    server_socket = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
    server_socket.setsockopt(socket.SOL_SOCKET, socket.SO_BROADCAST, 1)
    server_socket.bind(('', 4242))  # Listening on port 4242

    while True:
        message, client_address = server_socket.recvfrom(1024)
        response_message = "I am here!"
        server_socket.sendto(response_message.encode(), client_address)

if __name__ == "__main__":
    main()

In the iPhone side - I added this code to the ClientBoot.m file

char* find_devices() {
    int sockfd = socket(AF_INET, SOCK_DGRAM, 0);
    if (sockfd < 0) {
        perror("socket");
        exit(EXIT_FAILURE);
    }

    struct sockaddr_in myaddr;
    memset(&myaddr, 0, sizeof(myaddr));
    myaddr.sin_family = AF_INET;
    myaddr.sin_addr.s_addr = htonl(INADDR_ANY);
    myaddr.sin_port = htons(0);

    if (bind(sockfd, (struct sockaddr *) &myaddr, sizeof(myaddr)) < 0) {
        perror("bind");
        close(sockfd);
        exit(EXIT_FAILURE);
    }

    int broadcastEnable = 1;
    if (setsockopt(sockfd, SOL_SOCKET, SO_BROADCAST, &broadcastEnable, sizeof(broadcastEnable)) < 0) {
        perror("setsockopt (SO_BROADCAST)");
        close(sockfd);
        exit(EXIT_FAILURE);
    }

    struct sockaddr_in broadcastAddr;
    memset(&broadcastAddr, 0, sizeof(broadcastAddr));
    broadcastAddr.sin_family = AF_INET;
    broadcastAddr.sin_addr.s_addr = inet_addr("169.254.255.255");
    broadcastAddr.sin_port = htons(4242);

    const char *message = "Hello, is anyone there?";
    if (sendto(sockfd, message, strlen(message), 0, (struct sockaddr *) &broadcastAddr, sizeof(broadcastAddr)) < 0) {
        perror("sendto");
        close(sockfd);
        exit(EXIT_FAILURE);
    }

    char buffer[1024];
    struct sockaddr_in responseAddr;
    socklen_t responseAddrLen = sizeof(responseAddr);
    int recvlen = recvfrom(sockfd, buffer, sizeof(buffer) - 1, 0, (struct sockaddr *) &responseAddr, &responseAddrLen);
    if (recvlen > 0) {
        buffer[recvlen] = 0;
        char *responderIP = (char *)malloc(INET_ADDRSTRLEN);
        if (responderIP == NULL) {
            perror("malloc");
            close(sockfd);
            exit(EXIT_FAILURE);
        }
        inet_ntop(AF_INET, &(responseAddr.sin_addr), responderIP, INET_ADDRSTRLEN);
        close(sockfd);
        return responderIP;
    } else if (recvlen < 0) {
        perror("recvfrom");
    }

    close(sockfd);
    return NULL;
}

Finally - I changed const char *envHost = find_devices(); to const char *envHost = find_devices();

As this POC seems to work - my enhancement suggestion is to add a server to the InjectionIII app that will respond to UDP broadcasts, and adding a code in ClientBoot.m that'll utilize this server to find the development machine IP.

Thanks!

johnno1962 commented 1 year ago

Hi, thanks very much for spending the time on this. There is some very similar code at the end of SimpleSocket.mm though it no longer seems to work. My first impression is that I'd be reluctant to build something iOS 17 specific into the app for now but if you come up with a PR that is backward compatible I'll take a look after testing it out. As it stands I don't think the environment variable override variable is an entirely bad solution for the minority of people who are using device injection (which didn't even work 6 months ago though the number using it does seem to be steadily increasing.) Apologies in advance for the time you spend refactoring ClientBoot.mm. It's overdue for a rewrite.

oryonatan commented 1 year ago

no worries, seems like the current solution with multicasts no longer works, but this solution with broadcasts does work ... not sure why, I can try to dig into the current code and see if I can fix it.

johnno1962 commented 1 year ago

I look forward to the reviewing it; You write quality C. I may have a try myself now you've suggested using broadcast instead of multicast addresses.

oryonatan commented 1 year ago

hi @johnno1962 seems like this idea have some issues:

  1. It seems like the iOS17 network ip address is only set up after around 30-60 seconds of connection over USB.
  2. no support for older devices.
  3. the device fails to find the host if it connected to wifi.

A colleague of mine suggested looking at the Peertalk library and try using it instead, looking at the library and the examples.

Am I right to assume that all of the communication in Injection/Hotreloading is done by the SimpleSocket file is that right? seems like behind the scenes, all of the network code there boils down to this method:


- (BOOL)perform:(io_func)io ofBytes:(const void *)buffer
         length:(size_t)length cmd:(SEL)cmd {
    size_t rd, ptr = 0;
    SLog(@"#%d %s %lu [%p] %s", clientSocket, io == read ?
         "<-" : "->", length, buffer, sel_getName(cmd));
    while (ptr < length &&
       (rd = io(clientSocket, (char *)buffer+ptr, length-ptr)) > 0)
        ptr += rd;
    if (ptr < length) {
        NSLog(@"[%@ %s:%p length:%lu] error: %lu %s",
              self, sel_getName(cmd), buffer, length, ptr, strerror(errno));
        return FALSE;
    }
    return TRUE;
}

so maybe we can re-implement it over their network stack?

johnno1962 commented 1 year ago

Hi again, my first impression is I'd be reluctant to change the network "stack" at this stage so I'd be wanting to try other alternatives first. I also note that project hasn't been updated for three years which is either very good or very not good when one is thinking of adopting it. I've a tried few simple broadcast things this morning but the problem is getting the local address(es) of the device to construct the broadcast addresses to try. I'll try a bit harder later on unless you can suggest other alternatives. Thanks for continuing to look into this. It would be nice to find a better solution for devices.

johnno1962 commented 1 year ago

I haven't found any easy solutions to device discovery. I may try dusting off some old "Bonjour" code tomorrow but unless that suddenly starts working (I think I've already tried it) I'll probably be leaving things as they are unless you present something both functional and compatible. This is a niche requirement for which there are already workarounds.

zenangst commented 1 year ago

@johnno1962 have you looked into Multipeer Connectivity? https://developer.apple.com/documentation/multipeerconnectivity

Or the Network.framework - https://developer.apple.com/documentation/network

johnno1962 commented 1 year ago

Thanks @zenangst, I tried Bonjour but couldn't get some very old code to work from a phone though it's probably the "right" way to do this. I gave up too soon though, in end I went back and got the broadcasting code above to work instead of trying to use multicast groups. @oryonatan, can you update to the latest release candidate and HotReloading, "filelist" branch and give it a try for me please? https://github.com/johnno1962/HotReloading/commit/c3ec5e5c8140006ff5e652742fedef9c4d89e918

johnno1962 commented 1 year ago

Erk, I just saw the following which is probably why Bonjour didn't work:

Important Apps that use the local network must provide a usage string in their Info.plist with the key NSLocalNetworkUsageDescription. Apps that use Bonjour must also declare the services they browse, using the NSBonjourServices key.

Think we probably want to avoid having to ask users to do that though.

johnno1962 commented 1 year ago

I've updated the filelist branch to be a little more flexible in how it tries to connect. I have a slightly unusual home network where my dev machine is not directly on our Wi-Fi network which works better using the hostname from Package.swift. For most people though the broadcast option will be more reliable so it now tries both.

oryonatan commented 1 year ago

Hi, just checked out the code at filelist at home, connected the iPhone and the development device into the different networks and it seems like it works perfectly 🎉

I will check back tomorrow at office, and see if this still works with under our office IT constraints.

oryonatan commented 1 year ago

Just wanted to update that I checked and seems to also work with one small caveat - if the device is connected to wifi the initial broadcast will fail to find the host

also - you need to set up the INJECTION_DAEMON flag for this to work.

johnno1962 commented 1 year ago

I don't know about the INJECTION_DAEMON thing but are you sure the InjectionIII app you're running is the latest release candidate (build 7889 which you can see by hovering over the "Quit InjectionIII" menu item")? Otherwise it will still be waiting for a multicast.

I've made some progress this end understanding what's going on with being/not being able to connect. In Xcode 14.3 I need to enable Wi-Fi on my dev machine and have broadcasts find a way to connect. If I use Xcode 15 and unplug and reconnect my phone the 169.254.255.255 network becomes active over USB and it can find a route to my host by resolving its hostname provided by HotReloading's Package.swift. This resolves what was a mystery for me how on earth the phone was able to connect to my dev machine which effectively has its own network; It is using the 169.254 network (though broadcasts on this network don't seem to work by themselves). It tries all three possibilities now in parallel and the first that resolves becomes the injection client.

I've just pushed a couple of aesthetic tweaks to the filelist branch if you want to update and make sure you're running the most recent release candidate of the InjectionIII.app from yesterday. Sorry I didn't explicitly mention that!

oryonatan commented 1 year ago

Hi - regarding injectionIII.app version - I've compiled it myself from commit 368ca02 - Multicast becomes Broadcast.

johnno1962 commented 1 year ago

So what is the current situation? It works but only if you define an environment variable INJECTION_DAEMON?

johnno1962 commented 1 year ago

I've pushed a commit to the InjectionIII repo picking up the most recent HotReloading changes.

oryonatan commented 1 year ago

broadcast seems to work over USB in XCode 15 and iOS17 when

  1. The dev machine is connected to the internet.
  2. The iPhone is not connected to wifi

Connecting the iPhone to another wifi (different than the dev machine) causes this to stop working (although I do gent the printed message of Broadcasting 169.254.255.255)

oryonatan commented 1 year ago

(I suppose connecting them to the same network will also work, but didn't check that)

(also I think that the INJECTION_DAEMON flag is redundant, this seems to work anyway)

oryonatan commented 1 year ago

Regarding what is going on - I tried running wireshark on the dev machine and saw this:

  1. When the iPhone is not connected to wifi - after
        if (sendto(multicastSocket, &msgbuf, sizeof msgbuf, 0,
                   (struct sockaddr *)&addr, sizeof(addr)) < 0) {

    is called I see a UDP packet originating from the iPhone on the host machine

  2. When the iPhone is connected to wifi - I don't see this packet
johnno1962 commented 1 year ago

Ah, the irony of a configuration that is only able to connect when Wi-Fi is turned off on the device! Probably to do with "service order" where it chooses which interface to route the request through, though it should be sending out the request on all interfaces. With Wi-Fi switched off is your app able to use the internet of your dev machine via NAT or the cellular connection, i.e. do vanilla network requests inside the app work? Is there a reason why you can't connect your dev machine and phone to the same network? Otherwise, broadcasts don't help us.

oryonatan commented 1 year ago
  1. when I disconnect the wifi from the iPhone - it loses connection to the internet, there is no NAT here.
  2. I don't have cellular connection on the device (it is a company device) so I am not sure.
  3. for "security" reasons our IT systems have the iPhone and the Dev devices on different networks, I guess this might sound a bit niche or ridiculous, but changing the company IT infra is quite challenging.
  4. I wonder if we can change the aforementioned "service order" in order to force multicastSocket to use a specific interface ... it seems like in Linux there is an SO_BINDTODEVICE option for a socket, and FreeBSD should have an IP_SENDIF option, but I think it is not available in Darwin, so maybe something else will work.
oryonatan commented 1 year ago

maybe this will work: https://lists.apple.com/archives/darwin-dev/2014/Jan/msg00004.html

oryonatan commented 1 year ago

ok this code snippet seems to work, just need to find out which interface to use

   [self forEachInterface:^(in_addr_t laddr, in_addr_t nmask) {
//
//        uint32_t Anet = ntohl(laddr) >> 24;
//        if (Anet == 127 || Anet == 10) return;

      struct ifaddrs *ifaddr, *ifa;
      int family, s;

      if (getifaddrs(&ifaddr) == -1) {
          perror("getifaddrs");
          exit(EXIT_FAILURE);
      }

      /* Walk through linked list, maintaining head pointer so we can free list later */
      for (ifa = ifaddr; ifa != NULL; ifa = ifa->ifa_next) {
          if (ifa->ifa_addr == NULL)
              continue;

          family = ifa->ifa_addr->sa_family;

          /* Display interface name and family (including IPv4 or IPv6) */
          printf("%s\tAF_%s\n", ifa->ifa_name,
                 (family == AF_INET) ? "INET" :
                 (family == AF_INET6) ? "INET6" : "UNKNOWN");
        int idx = if_nametoindex(ifa->ifa_name);
        setsockopt(multicastSocket, IPPROTO_IP, IP_BOUND_IF, &idx, sizeof(idx) );
        addr.sin_addr.s_addr = laddr | ~nmask;
          printf("Broadcasting %s\n", inet_ntoa(addr.sin_addr));
          if (sendto(multicastSocket, &msgbuf, sizeof msgbuf, 0,
                     (struct sockaddr *)&addr, sizeof(addr)) < 0) {
              [self error:@"Could not send multicast ping: %s"];
          }
      }

    }];
johnno1962 commented 1 year ago

Interesting, Thanks for that! I've pushed a commit to filelist branch to use if_nametoindex().

oryonatan commented 1 year ago

seems to work for me now on commit * c9674e5 -Use if_nametoindex - John Holdsworth (HEAD, origin/filelist)

oryonatan commented 1 year ago

I just wanted to add that I really appreciate what you are doing here, thanks!

johnno1962 commented 1 year ago

Thanks for your help and raising this issue in the first place! Don't be afraid to mention to your boss he can sponsor me on github if this all pans out.. I could see the broadcast to 169.254 getting through now with your change. Are we good now then? I'll take the opportunity to make the multicastHash which distinguishes between users a little more unique since multicast/broadcast are incomparable anyway and roll a new release candidate for you later.

oryonatan commented 1 year ago

Thanks for your help and raising this issue in the first place! Don't be afraid to mention to your boss he can sponsor me on github if this all pans out.. I could see the broadcast to 169.254 getting through now with your change. Are we good now then? I'll take the opportunity to make the multicastHash which distinguishes between users a little more unique since multicast/broadcast are incomparable anyway and roll a new release candidate for you later.

We are good!

oryonatan commented 1 year ago

regarding sponsoring, I will ask for it!

johnno1962 commented 1 year ago

I've merged the filelist branch onto HotReloading main, you'll need to download the latest release candidate to work with it.

oryonatan commented 11 months ago

Hi @johnno1962

First thing - regarding sponsorship, we had a bit of a technical issues with Sponsoring using the company Gihub enterprise account - but I think they managed to solve it - you should see something from LightricksSponsors I think :).

Another thing - we previously used to work with Injection by loading the injection binary bundle, this is easier for us then using SPM + Hotreloading, as this means that 1. we can internally deploy our binary injection bundle and control when it is loaded 2. We don't have to use SPM - and it seems like in SPM it is complicated to include Hotreloading just in Debug builds.

It seems like when using injection this way - by loading a bundle, the fixes you added here don't really work on device, I am getting a Injection bundle loaded but could not connect. Is InjectionIII.app running? message.

So is there a way to make the broadcast trick work also when loading a bundle? or alternatively is there an easy way to make Hotreloading be added to the app only on debug builds (seems like SPM does not support debug only packages)

johnno1962 commented 11 months ago

Hi, thanks for the sponsorship (twice)! It's appreciated and helps keep motivation up. Unfortunately there is no way to include HotReloading only on Debug builds though I have raised various radars about this. For me the easiest way to do this would be to be able to use #if DEBUG in the Package.swift but you can't alas.

You can't load the bundle on a device as it isn't on the file system of your phone so for device injection you have to use the HotReloading project. This also allows the Package.swift to build the hostname of your development Mac into the executable which is one way other than broadcasts to facilitate device discovery.

oryonatan commented 11 months ago

oh, but we can load the bundle on a device, simply by adding a build script that copies the bundle, we have something like this:

rsync -a "$SRCROOT/iOSInjection.bundle" "$BUILT_PRODUCTS_DIR/$FULL_PRODUCT_NAME/"
    codesign -f --sign "$EXPANDED_CODE_SIGN_IDENTITY" --timestamp\=none --preserve-metadata\=identifier,entitlements,flags --generate-entitlement-der "$BUILT_PRODUCTS_DIR/$FULL_PRODUCT_NAME/iOSInjection.bundle/Frameworks/SwiftTrace.framework/SwiftTrace"
    codesign -f --sign "$EXPANDED_CODE_SIGN_IDENTITY" --timestamp\=none --preserve-metadata\=identifier,entitlements,flags --generate-entitlement-der "$BUILT_PRODUCTS_DIR/$FULL_PRODUCT_NAME/iOSInjection.bundle"
    defaults write com.johnholdsworth.InjectionIII "$PROJECT_FILE_PATH" $EXPANDED_CODE_SIGN_IDENTITY
johnno1962 commented 11 months ago

Very interesting! And it loads despite being built for the simulator? I guess it wouldn't take much to change ClientBoot.mm to attempt the device connecting even in the bundle. I'll take a quick look. Thanks for all these ideas. Keep 'em coming...

oryonatan commented 11 months ago

well, it works but I am using few hacks that @byohay made to make it work on device, I'll check if I can send you his changes

oryonatan commented 11 months ago

this is his commit that made the bundle build for iPhone https://github.com/johnno1962/InjectionIII/commit/2090207ac62321165f8f8926272ef9f72030d48c

johnno1962 commented 11 months ago

You're ahead of me here. I've been using the maciOSInjection.bundle which almost works.

The changes you need to make to try connecting from the bundle are: ClientBoot.mm, line 61: #if !defined(INJECTION_III_APP) || 1 <<- enable the code. SimpleSocket.mm, line 359: if (//[self multicastHash] == msgbuf.hash && <<- comment out and rebuild the >app<

With these changes it might work but I've only been able to get a program with the maciOS bundle to run once and now I'm getting weird errors from the device. This leaves the problem of how to distinguish between more than one developer on a network sending broadcasts which is what the "multicastHash" is all about.

johnno1962 commented 11 months ago

OK, I've been able to see this working by making the following changes and using the "maciOSInjection.bundle":

diff --git a/Sources/HotReloadingGuts/ClientBoot.mm b/Sources/HotReloadingGuts/ClientBoot.mm
index 44a6475..48a2a30 100644
--- a/Sources/HotReloadingGuts/ClientBoot.mm
+++ b/Sources/HotReloadingGuts/ClientBoot.mm
@@ -58,7 +58,7 @@ + (void)tryConnect:(Class)clientClass {
         "https://github.com/johnno1962/InjectionIII/releases\n"
     APP_PREFIX"And have typed: defaults write com.johnholdsworth.InjectionIII deviceUnlock any\n";
     BOOL isVapor = dlsym(RTLD_DEFAULT, VAPOR_SYMBOL) != nullptr;
-#if !defined(INJECTION_III_APP)
+#if !defined(INJECTION_III_APP) || 1
 #if TARGET_IPHONE_SIMULATOR || TARGET_OS_OSX
     BOOL isiOSAppOnMac = false;
     if (@available(iOS 14.0, *)) {
@@ -83,7 +83,7 @@ + (void)tryConnect:(Class)clientClass {
     injectionHost = [clientClass
         getMulticastService:HOTRELOADING_MULTICAST port:HOTRELOADING_PORT
                     message:APP_PREFIX"Connecting to %s (%s)...\n"];
-    socketAddr = [injectionHost stringByAppendingString:socketAddr];
+    socketAddr = [injectionHost stringByAppendingString:@HOTRELOADING_PORT];
     if (injectionClient)
         return;
 #endif
diff --git a/Sources/HotReloadingGuts/SimpleSocket.mm b/Sources/HotReloadingGuts/SimpleSocket.mm
index 001bad0..04bd902 100644
--- a/Sources/HotReloadingGuts/SimpleSocket.mm
+++ b/Sources/HotReloadingGuts/SimpleSocket.mm
@@ -42,6 +42,8 @@

 @implementation SimpleSocket

+static BOOL isConnected;
+
 + (int)error:(NSString *)message {
     NSLog([@"%@/" stringByAppendingString:message],
           self, strerror(errno));
@@ -98,6 +100,7 @@ + (void)runServer:(NSString *)address {
                         if (v4Addr->sin_addr.s_addr == addr)
                             client.isLocalClient = TRUE;
                     }];
+                    isConnected = TRUE;
                     [client run];
                 }
             }
@@ -274,6 +277,7 @@ - (BOOL)writeCommand:(int)command withString:(NSString *)string {
 }

 - (void)dealloc {
+    isConnected = FALSE;
     close(clientSocket);
 }

@@ -356,7 +360,7 @@ + (void)multicastListen:(NSNumber *)socket {
               [self multicastHash], msgbuf.hash);

         gethostname(msgbuf.host, sizeof msgbuf.host);
-        if ([self multicastHash] == msgbuf.hash &&
+        if (!isConnected &&//[self multicastHash] == msgbuf.hash &&
             sendto(multicastSocket, &msgbuf, sizeof msgbuf, 0,
                    (struct sockaddr *)&addr, addrlen) < sizeof msgbuf) {
             [self error:@"Could not send to multicast: %s"];
diff --git a/Sources/injectiond/DeviceServer.swift b/Sources/injectiond/DeviceServer.swift
index 9076f2a..67b335b 100644
--- a/Sources/injectiond/DeviceServer.swift
+++ b/Sources/injectiond/DeviceServer.swift
@@ -21,8 +21,14 @@ class DeviceServer: InjectionServer {

     #if !SWIFT_PACKAGE
     override func validateConnection() -> Bool {
-        return readInt() == HOTRELOADING_SALT &&
-            readString()?.hasPrefix(NSHomeDirectory()) == true
+        switch readInt() {
+        case INJECTION_SALT:
+            return readString() == INJECTION_KEY
+        case HOTRELOADING_SALT:
+            return readString()?.hasPrefix(NSHomeDirectory()) == true
+        default:
+            return false
+        }
     }
     #endif

Need to think a little more about how to distinguish clients and the other changes I had to make but this looks promising. The weird device problems I could only resolve by performing a full Archive/Notarise build cycle.

oryonatan commented 11 months ago

this seems to work on the private iOS17 USB based network 🎉

johnno1962 commented 11 months ago

GM @oryonatan, I've pushed this idea through to its conclusion and released a new release candidate from branch dev-bundle. The InjectionIII.app contains a new script copy_bundle.sh which you use instead of the build phase you mentioned to copy the bundle into the app package and codesign it. I've updated the README on that branch but it's still a bit ragged. Would you like to give the new release candidate a try and let me know how you get on? I was able to differentiate users by writing the user's home directory into the Info.plist of the copied bundle in the finish.

oryonatan commented 11 months ago

thanks @johnno1962, I'll check this version out

oryonatan commented 11 months ago

Hi! it works! I didn't check the copy_bundle.sh script though

johnno1962 commented 11 months ago

Happy to hear it though I don't quite know how it can be working using broadcasts if you're not using the script. It must have found another way!

everlof commented 9 months ago

Just a totally random thought and maybe doesn't even apply, but couldn't help to notice the correlation. Lots of people have started getting problems with REALLY slow debuggers after iOS 17 just because they enforce the use of this private network between the phone and the mac. And the only way to speed it up is to disable the wifi when debugging.

And then you notice this behaviour where there's no IP assigned when you enable the wifi. Coincidence? Here a thread of it being discussed: https://developer.apple.com/forums/thread/737875