Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TCP knocks not being received over mobile Internet #23

Open
solemnwarning opened this issue Jun 30, 2023 · 11 comments
Open

TCP knocks not being received over mobile Internet #23

solemnwarning opened this issue Jun 30, 2023 · 11 comments

Comments

@solemnwarning
Copy link

I'm trying to set this up to knock on a sequence of TCP ports, it works fine if I send to a host over the LAN, but when I switch to my carrier's connection none of the packets come through. I see packets on the correct ports if I attempt to connect using a Telnet client from my phone, so my ISP isn't blocking those ports.

I had a quick look at the source and can see it initiates a TCP connection, then immediately shuts it down, so my only thought is maybe the immediate shut-down after the connection causes my carrier's network to drop the connection before the server sees the SYN. From several Linux PCs I've been knocking using a shell script that attempts to initiate a connection with nc (which fails, as the target explicitly rejects the connection), this is to ensure the ports are knocked in sequence and there is no out-of-order issues from the Linux PCs.

So my ideas for improvements:

  • Don't cancel the connect() request until the inter-packet interval has been reached.
  • Add an option to wait for the connect() to complete (or fail) when using TCP.
@impalex
Copy link
Owner

impalex commented Jun 30, 2023

Most likely, your device that receives knocks is incorrectly configured. Check the interface that is listening. If this works on the local network, then you are listening on the local interface. You need to listen to another interface that looks "outside". Or both, if you want it to work both on the local network and outside.

App is fine. The proposed improvements will break the application - the resulting packet sequences will be incorrect.

@solemnwarning
Copy link
Author

The device that receives the knocks is not misconfigured. I'm verifying the activity by monitoring the incoming interface with tcpdump, and as stated, when I use another application to initiate a TCP connection from the phone to the device via the carrier's network I can see them.

I don't see how adding a delay before closing the initiated connection could possibly cause out-of-order packets, since it would still be closed before starting the next connection in the sequence. For more safety it could wait delay / 2 before closing the connection, then delay / 2 before advancing to the next packet in the sequence.

@impalex
Copy link
Owner

impalex commented Jun 30, 2023

Just check it with another port knocking app. For example, this one.

As for the delay... connect() and delay will send more than one SYN packet. Let's say we need to send packets to ports 222, 333, 444 in this order. If we add a delay after connect(), the resulting sequence will look like "222, 222, 222, 333, 333, 333, 444, 444" instead of "222, 333, 444".

@solemnwarning
Copy link
Author

I hadn't considered the duplicated SYN issue, although that will depend on how long the delay is. If you want to be really pedantic the kernel could technically queue up multiple SYNs for transmission before the shutdown() and close() syscalls get processed. Additionally, if the server explicitly sends an ICMP reject OR accepts the knock connection, then there won't be multiple SYNs.

As far as I can tell, the immediate FIN/RST packets being generated are causing the initial SYN to be discarded by my carrier's network before it makes it through to my server.

Now I'm thinking about it, there is also another case where an immediate close might cause the packet to be lost - if you're on-link with the target and don't have its MAC address cached, or any of the routers along the path need to do an ARP lookup before they can forward the packet (and they're keeping track of the TCP state in that buffer).

So I still think adding a delay before doing the shutdown/close (with a predefined upper limit to avoid SYN retransmissions... I wonder if we can get the retransmit delay from the kernel).

For my use case a "wait for connect to complete/fail" tickbox in the profile settings would do it, I might try getting it compiling and prototype either/both to verify it works.

@impalex
Copy link
Owner

impalex commented Jun 30, 2023

The TCP part is a bit clunky, but there's no better way to do it. Android is very strict. I recommend using UDP or ICMP.

As far as I can tell, the immediate FIN/RST packets being generated are causing the initial SYN to be discarded by my carrier's network before it makes it through to my server.

You are first and only one who reports this.
On the other hand, I know many people who use TCP without any problems.

So I still think adding a delay before doing the shutdown/close (with a predefined upper limit to avoid SYN retransmissions... I wonder if we can get the retransmit delay from the kernel).

As far as I remember, there was a delay there at one time. And quite a few reports about duplicate packets that break the sequence.

For my use case a "wait for connect to complete/fail" tickbox in the profile settings would do it, I might try getting it compiling and prototype either/both to verify it works.

Feel free to try.

Oh, and btw keep in mind that the recipient is not required to respond to the packet.

@impalex
Copy link
Owner

impalex commented Jun 30, 2023

As far as I can tell, the immediate FIN/RST packets being generated are causing the initial SYN to be discarded by my carrier's network before it makes it through to my server.

I strongly doubt this statement because there are no FIN/RST packets. What other packets could be mentioned if there is no connection? Wireshark logs confirm that nothing besides SYN is being sent.

image

@solemnwarning
Copy link
Author

You're right about that. I was jumping to conclusions as it explained (in my mind) how the packets arrived fine over Wi-Fi but not via the carrier's network.

I got it building and confirmed that adding a usleep(10000); call between connect() and shutdown() generates TCP traffic at the receivers end when knocking from my carrier's network, while the unmodified code has no traffic.

I've uploaded a dump of the first few packets received tcp.pcap, it looks to me like they're operating some kind of transparent TCP proxy, especially as the SYN keeps getting retransmitted for a while even though the phone closed the socket after only 10ms, so sending just a SYN packet via my carrier is probably impossible.

I'll continue looking to implement an option to wait for the connection to actively complete/fail, since that should still work for my application.

@solemnwarning
Copy link
Author

As a side note, I'm using TCP specifically because I can ensure each packet is acknowledged before proceeding to the next one. When I initially set it up with UDP I had issues with the packets not arriving in order without a long wait, especially when using a flakey wireless connection (#18 suggests I'm not the only one who has this problem).

@impalex
Copy link
Owner

impalex commented Jul 1, 2023

Adding a delay after connect() does only one thing - it increases the number of retransmitted SYN packets. Strictly speaking, it results in an incorrectly generated sequence. If increasing the number of sent packets somehow helps, it may indicate an unstable connection and packet loss.

As a side note, I'm using TCP specifically because I can ensure each packet is acknowledged before proceeding to the next one.

The recipient is not obligated to respond to packets. Neither confirming nor denying. In most cases, the received packets are simply discarded. This is normal and expected behavior.

@impalex
Copy link
Owner

impalex commented Jul 1, 2023

In any case, I am planning a major refactoring of the application. It's not happening today or tomorrow... Maybe I'll start working on it in a month or so, something like that. I will keep your issue in mind. If increasing the number of sent packets really does help in some way, I will consider how to incorporate this feature into the application without breaking anything for other users.

Just to clarify, are you absolutely certain, 100% sure, that packet retransmission actually helps in your case?

@solemnwarning
Copy link
Author

Its not the retransmission that is helping in this case, but allowing the connection to complete.

As far as I've been able to figure out, my carrier operates a transparent TCP proxy between my phone and the target system.

If I initiate a connection and then immediately close the socket my phone doesn't get to complete the handshake with the proxy, so the proxy never attempts to initiate a connection with the target system and it never sees anything.

If I allow the handshake to complete, THEN the TCP proxy in the carrier's network starts its own connection attempt to the target system, and "helpfully" continues doing so even after my phone has terminated its connection to the proxy, and the server has replied with an ICMP error (port unreachable).

Adding usleep(10000); between the connect/shutdown calls and setting the inter-packet delay to zero works in my case (the delay breaks the sequence due to the aforementioned stupid TCP proxy continuing to try and connect in the background).

So, I'm inclined to think either adding a "TCP connect mode" checkbox to the "Advanced" screen, or adding "TCP SYN" and "TCP connect" as different port types makes more sense than the delay. As well as my iptables/ipset-based port knocking, this would work with a port knocking server written using plain TCP sockets rather than packet sniffing (which would be reliable against packet loss, since TCP would guarantee a complete exchange before moving on).

I was going to add a PR with something like the above, but if you're intending to do major refactoring anyway I'll leave it with you. Here are the changes I've made to my build to get it working:

diff --git a/app/src/main/cpp/netutil.cpp b/app/src/main/cpp/netutil.cpp
index 3a9934a..fc15f13 100644
--- a/app/src/main/cpp/netutil.cpp
+++ b/app/src/main/cpp/netutil.cpp
@@ -21,6 +21,7 @@
 #include <cstring>
 #include <algorithm>
 #include <unistd.h>
+#include <poll.h>
 
 const int DUMMY_PORT = 1337;
 
@@ -153,7 +154,7 @@ int ping(int family, const char *host, const int size, const int count, jbyte* p
     return EXIT_SUCCESS;
 }
 
-int send_tcp_packet(int family, const char *host, const int port) {
+int send_tcp_packet(int family, const char *host, const int port, int timeout) {
     void *addr;
     int addr_size;
 
@@ -180,6 +181,11 @@ int send_tcp_packet(int family, const char *host, const int port) {
         return EXIT_FAILURE;
     }
     connect(sock, (struct sockaddr *)addr, addr_size); // NOLINT(bugprone-unused-return-value)
+
+    /* wait until connection succeeds/fails/times out */
+    struct pollfd pfd = { sock, POLLOUT };
+    poll(&pfd, 1, timeout);
+
     shutdown(sock, SHUT_RDWR);
     close(sock);
 
@@ -216,20 +222,20 @@ extern "C" jint Java_me_impa_knockonports_service_Knocker_ping6(JNIEnv *env, job
     return result;
 }
 
-extern "C" jint Java_me_impa_knockonports_service_Knocker_sendtcp(JNIEnv *env, jobject  __unused thiz, jstring host, jint port) {
+extern "C" jint Java_me_impa_knockonports_service_Knocker_sendtcp(JNIEnv *env, jobject  __unused thiz, jstring host, jint port, jint timeout) {
     const char *n_host = env->GetStringUTFChars(host, NULL);
 
-    int result = send_tcp_packet(AF_INET, n_host, port);
+    int result = send_tcp_packet(AF_INET, n_host, port, timeout);
 
     (*env).ReleaseStringUTFChars(host, n_host);
 
     return result;
 }
 
-extern "C" jint Java_me_impa_knockonports_service_Knocker_sendtcp6(JNIEnv *env, jobject  __unused thiz, jstring host, jint port) {
+extern "C" jint Java_me_impa_knockonports_service_Knocker_sendtcp6(JNIEnv *env, jobject  __unused thiz, jstring host, jint port, jint timeout) {
     const char *n_host = env->GetStringUTFChars(host, NULL);
 
-    int result = send_tcp_packet(AF_INET6, n_host, port);
+    int result = send_tcp_packet(AF_INET6, n_host, port, timeout);
 
     (*env).ReleaseStringUTFChars(host, n_host);
 
diff --git a/app/src/main/cpp/netutil.h b/app/src/main/cpp/netutil.h
index 9076c51..a19a93c 100644
--- a/app/src/main/cpp/netutil.h
+++ b/app/src/main/cpp/netutil.h
@@ -17,10 +17,10 @@ JNIEXPORT jint JNICALL Java_me_impa_knockonports_service_Knocker_ping6(
 
 extern "C"
 JNIEXPORT jint JNICALL Java_me_impa_knockonports_service_Knocker_sendtcp(
-        JNIEnv *, jobject __unused, jstring, jint);
+        JNIEnv *, jobject __unused, jstring, jint, jint);
 
 extern "C"
 JNIEXPORT jint JNICALL Java_me_impa_knockonports_service_Knocker_sendtcp6(
-        JNIEnv *, jobject __unused, jstring, jint);
+        JNIEnv *, jobject __unused, jstring, jint, jint);
 
 #endif //KNOCKONPORTS_NETUTIL_H
diff --git a/app/src/main/java/me/impa/knockonports/service/Knocker.kt b/app/src/main/java/me/impa/knockonports/service/Knocker.kt
index 851a909..184a97b 100644
--- a/app/src/main/java/me/impa/knockonports/service/Knocker.kt
+++ b/app/src/main/java/me/impa/knockonports/service/Knocker.kt
@@ -174,9 +174,9 @@ class Knocker(val context: Context, private val sequence: Sequence): Logging {
                                 debug { "Knock TCP ${it.port}" }
                                 address.run {
                                     if (this is Inet4Address)
-                                        sendtcp(this.hostAddress, it.port!!)
+                                        sendtcp(this.hostAddress, it.port!!, delay.toInt())
                                     else
-                                        sendtcp6(this.hostAddress, it.port!!)
+                                        sendtcp6(this.hostAddress, it.port!!, delay.toInt())
                                 }
                             }
                             SequenceStepType.ICMP -> {
@@ -198,7 +198,7 @@ class Knocker(val context: Context, private val sequence: Sequence): Logging {
                     }
                     if (icmpTasks.size>0)
                         runBlocking { icmpTasks.awaitAll() }
-                    if (delay > 0 && it.type != SequenceStepType.ICMP)
+                    if (delay > 0 && it.type != SequenceStepType.ICMP && it.type != SequenceStepType.TCP)
                         Thread.sleep(delay)
                 } catch (e: Exception) {
                     warn("Error while sending knock", e)
@@ -279,8 +279,8 @@ class Knocker(val context: Context, private val sequence: Sequence): Logging {
 
     private external fun ping(address: String, size: Int, count: Int, pattern: ByteArray, sleep: Int): Int
     private external fun ping6(address: String, size: Int, count: Int, pattern: ByteArray, sleep: Int): Int
-    private external fun sendtcp(host: String, port: Int): Int
-    private external fun sendtcp6(host: String, port: Int): Int
+    private external fun sendtcp(host: String, port: Int, timeout: Int): Int
+    private external fun sendtcp6(host: String, port: Int, timeout: Int): Int
 
     companion object {
         init {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants