Show pcapd errors in the app UI

pcapd is now run without the daemonize option, allowing PCAPdroid to retrieve the
exit code and avoiding an unnecessary fork. Code-based error reporting is now
implemented in pcapd. Errors are now shown in the UI and common ones are localized.

app/src/main/java/com/emanuelef/remote_capture/CaptureService.java

@@ -1416,7 +1416,35 @@ public class CaptureService extends VpnService implements Runnable {
 
     public void reportError(String msg) {
         HAS_ERROR = true;
-        mHandler.post(() -> Toast.makeText(this, msg, Toast.LENGTH_LONG).show());
+
+        mHandler.post(() -> {
+            String err = msg;
+
+            // Try to get a translated string (see errors.h)
+            switch (msg) {
+                case "Invalid PCAP file":
+                    err = getString(R.string.invalid_pcap_file);
+                    break;
+                case "Could not open the capture interface":
+                    err = getString(R.string.capture_interface_open_error);
+                    break;
+                case "Unsupported datalink":
+                    err = getString(R.string.unsupported_pcap_datalink);
+                    break;
+                case "The specified PCAP file does not exist":
+                    err = getString(R.string.pcap_file_not_exists);
+                    break;
+                case "pcapd daemon did not spawn":
+                    if(mSettings.root_capture)
+                        err = getString(R.string.root_capture_start_failed);
+                    break;
+                case "PCAP read error":
+                    err = getString(R.string.pcap_read_error);
+                    break;
+            }
+
+            Toast.makeText(this, err, Toast.LENGTH_LONG).show();
+        });
     }
 
     public String getWorkingDir() {

app/src/main/jni/common/utils.c

@@ -199,9 +199,12 @@ void hexdump(const char *buf, size_t bufsize) {
 
 /* ******************************************************* */
 
-int run_shell_cmd(const char *prog, const char *args, bool as_root, bool check_error) {
+// Start a sub-process, running a command with some arguments, either as root or as the current user.
+// On success, the out_fd parameter will receive an open file descriptor to read the command output
+// Returns the pid of the child process, or -1 on failure
+// NOTE: the caller MUST call waitpid or equivalent to prevent process zombification and close the out_fd
+int start_subprocess(const char *prog, const char *args, bool as_root, int* out_fd) {
     int in_p[2], out_p[2];
-    int rv = -1;
     pid_t pid;
 
     if((pipe(in_p) != 0) || (pipe(out_p) != 0)) {
@@ -225,7 +228,7 @@ int run_shell_cmd(const char *prog, const char *args, bool as_root, bool check_e
         exit(1);
     } else if(pid > 0) {
         // parent
-        int out = out_p[0];
+        *out_fd = out_p[0];
         close(in_p[0]);
         close(out_p[1]);
 
@@ -236,30 +239,6 @@ int run_shell_cmd(const char *prog, const char *args, bool as_root, bool check_e
         write(in_p[1], args, strlen(args));
         write(in_p[1], "\n", 1);
         close(in_p[1]);
-
-        waitpid(pid, &rv, 0);
-
-        if(check_error && (rv != 0)) {
-            char buf[128];
-            struct timeval timeout = {0};
-            fd_set fds;
-
-            buf[0] = '\0';
-            FD_ZERO(&fds);
-            FD_SET(out, &fds);
-
-            select(out + 1, &fds, NULL, NULL, &timeout);
-            if (FD_ISSET(out, &fds)) {
-                int num = read(out, buf, sizeof(buf) - 1);
-                if (num > 0)
-                    buf[num] = '\0';
-            }
-
-            log_f("su \"%s\" invocation failed: %s", prog, buf);
-            rv = -1;
-        }
-
-        close(out_p[0]);
     } else {
         log_f("fork() failed[%d]: %s", errno, strerror(errno));
         close(in_p[0]);
@@ -269,5 +248,42 @@ int run_shell_cmd(const char *prog, const char *args, bool as_root, bool check_e
         return -1;
     }
 
+    return pid;
+}
+
+/* ******************************************************* */
+
+int run_shell_cmd(const char *prog, const char *args, bool as_root, bool check_error) {
+    int out_fd;
+    int pid = start_subprocess(prog, args, as_root, &out_fd);
+    if(pid <= 0)
+        return -1;
+
+    int rv;
+    if(waitpid(pid, &rv, 0) <= 0) {
+        log_f("waitpid %d failed[%d]: %s", pid, errno, strerror(errno));
+        return -1;
+    }
+
+    if(check_error && (rv != 0)) {
+        char buf[128];
+        struct timeval timeout = {0};
+        fd_set fds;
+
+        buf[0] = '\0';
+        FD_ZERO(&fds);
+        FD_SET(out_fd, &fds);
+
+        select(out_fd + 1, &fds, NULL, NULL, &timeout);
+        if (FD_ISSET(out_fd, &fds)) {
+            int num = read(out_fd, buf, sizeof(buf) - 1);
+            if (num > 0)
+                buf[num] = '\0';
+        }
+
+        log_f("\"%s\" invocation failed: %s", prog, buf);
+    }
+
+    close(out_fd);
     return rv;
 }

app/src/main/jni/common/utils.h

@@ -63,6 +63,7 @@ void tupleSwapPeers(zdtun_5tuple_t *tuple);
 char loglvl2char(int lvl);
 char* humanSize(char *buf, int bufsize, double bytes);
 void hexdump(const char *buf, size_t bufsize);
+int start_subprocess(const char *prog, const char *args, bool as_root, int* out_fd);
 int run_shell_cmd(const char *prog, const char *args, bool as_root, bool check_error);
 
 #endif // __LOG_UTILS_H__

app/src/main/jni/core/capture_libpcap.c

@@ -18,8 +18,10 @@
  */
 
 #include <sys/un.h>
+#include <sys/wait.h>
 #include <linux/limits.h>
 #include "pcapdroid.h"
+#include "errors.h"
 #include "pcapd/pcapd.h"
 #include "common/utils.h"
 #include "third_party/uthash.h"
@@ -60,15 +62,11 @@ static int get_pcapd_pid() {
 
 /* ******************************************************* */
 
-static void kill_pcapd(pcapdroid_t *pd) {
+static void kill_process(int pid, bool as_root, int signum) {
-    int pid = get_pcapd_pid();
+    char args[16];
-    if(pid > 0) {
-        char pid_s[8];
-        snprintf(pid_s, sizeof(pid_s), "%d", pid);
 
-        log_i("Killing old pcapd with pid %d", pid);
+    snprintf(args, sizeof(args), "-%d %d", signum, pid);
-        run_shell_cmd("kill", pid_s, pd->pcap.as_root, false);
+    run_shell_cmd("kill", args, as_root, false);
-    }
 }
 
 /* ******************************************************* */
@@ -138,7 +136,13 @@ static int connectPcapd(pcapdroid_t *pd) {
     addr.sun_family = AF_UNIX;
     strcpy(addr.sun_path, PCAPD_SOCKET_PATH);
 
-    kill_pcapd(pd);
+    int pid = get_pcapd_pid();
+    if(pid > 0) {
+        log_i("Killing old pcapd with pid %d", pid);
+        kill_process(pid, pd->pcap.as_root, SIGTERM);
+        pid = -1;
+    }
+
     unlink(PCAPD_PID);
     unlink(PCAPD_SOCKET_PATH);
 
@@ -164,7 +168,7 @@ static int connectPcapd(pcapdroid_t *pd) {
     if(pd->pcap_file_capture) {
         // must be a file path
         if(access(pd->pcap.capture_interface, F_OK)) {
-            log_f("The specified PCAP file does not exist: %s", pd->pcap.capture_interface);
+            log_f(PD_ERR_PCAP_DOES_NOT_EXIST);
             goto cleanup;
         }
     } else {
@@ -177,10 +181,14 @@ static int connectPcapd(pcapdroid_t *pd) {
 
     // Start the daemon
     char args[256];
-    snprintf(args, sizeof(args), "-l pcapd.log -L %u -i '%s' -d -u %d -t -b '%s'",
+    snprintf(args, sizeof(args), "-l pcapd.log -L %u -i '%s' -u %d -t -b '%s'",
              getuid(), pd->pcap.capture_interface, pd->tls_decryption.enabled ? -1 : pd->app_filter, bpf);
-    if(run_shell_cmd(pcapd, args, pd->pcap.as_root, true) != 0)
+
+    int pcapd_out;
+    pid = start_subprocess(pcapd, args, pd->pcap.as_root, &pcapd_out);
+    if(pid <= 0)
         goto cleanup;
+    close(pcapd_out);
 
     // Wait for pcapd to start
     struct timeval timeout = {.tv_sec = 3, .tv_usec = 0};
@@ -190,7 +198,7 @@ static int connectPcapd(pcapdroid_t *pd) {
     select(sock + 1, &selfds, NULL, NULL, &timeout);
 
     if(!FD_ISSET(sock, &selfds)) {
-        log_f("pcapd daemon did not spawn");
+        log_f(PD_ERR_PCAPD_NOT_SPAWNED);
         goto cleanup;
     }
 
@@ -200,9 +208,15 @@ static int connectPcapd(pcapdroid_t *pd) {
         goto cleanup;
     }
 
-    log_i("Connected to pcapd");
+    log_i("Connected to pcapd (pid=%d)", pid);
+    pd->pcap.pcapd_pid = pid;
 
 cleanup:
+    if((client < 0) && (pid > 0)) {
+        int rv;
+        kill_process(pid, pd->pcap.as_root, SIGKILL);
+        waitpid(pid, &rv, 0);
+    }
     unlink(PCAPD_SOCKET_PATH);
     close(sock);
 
@@ -487,6 +501,32 @@ void libpcap_iter_connections(pcapdroid_t *pd, conn_cb cb) {
 
 /* ******************************************************* */
 
+static void process_pcapd_rv(pcapdroid_t *pd, int rv) {
+    pcapd_rv rrv = (pcapd_rv) rv;
+    log_i("pcapd exit code: %d", rv);
+
+    switch (rrv) {
+        case PCAPD_OK:
+            break;
+        case PCAPD_INTERFACE_OPEN_FAILED:
+            if(pd->pcap_file_capture)
+                log_f(PD_ERR_INVALID_PCAP_FILE);
+            else
+                log_f(PD_ERR_INTERFACE_OPEN_ERROR);
+            break;
+        case PCAPD_UNSUPPORTED_DATALINK:
+            log_f(PD_ERR_UNSUPPORTED_DATALINK);
+            break;
+        case PCAPD_PCAP_READ_ERROR:
+            log_f(PD_ERR_PCAP_READ);
+            break;
+        default:
+            log_f("pcapd daemon exited with code %d", rv);
+    }
+}
+
+/* ******************************************************* */
+
 int run_libpcap(pcapdroid_t *pd) {
     int sock = -1;
     int rv = -1;
@@ -619,5 +659,15 @@ cleanup:
         run_shell_cmd("iptables", get_mitm_redirection_args(pd, args, false), true, false);
     }
 
+    if(pd->pcap.pcapd_pid > 0) {
+        int status = PCAPD_ERROR;
+
+        if(waitpid(pd->pcap.pcapd_pid, &status, 0) <= 0)
+            log_e("waitpid %d failed[%d]: %s", pd->pcap.pcapd_pid, errno, strerror(errno));
+
+        if(WIFEXITED(status))
+            process_pcapd_rv(pd, WEXITSTATUS(status));
+    }
+
     return rv;
 }

app/src/main/jni/core/errors.h

@@ -0,0 +1,14 @@
+#ifndef __PCAPDROID_ERRORS_H__
+#define __PCAPDROID_ERRORS_H__
+
+// This file contains a set of error strings which may be returned from the native code.
+// These should be translated in CaptureService.reportError
+
+#define PD_ERR_INVALID_PCAP_FILE        "Invalid PCAP file"
+#define PD_ERR_INTERFACE_OPEN_ERROR     "Could not open the capture interface"
+#define PD_ERR_UNSUPPORTED_DATALINK     "Unsupported datalink"
+#define PD_ERR_PCAP_DOES_NOT_EXIST      "The specified PCAP file does not exist"
+#define PD_ERR_PCAPD_NOT_SPAWNED        "pcapd daemon did not spawn"
+#define PD_ERR_PCAP_READ                "PCAP read error"
+
+#endif

app/src/main/jni/core/pcapdroid.h

@@ -223,6 +223,7 @@ typedef struct pcapdroid {
             bool as_root;
             char *bpf;
             char *capture_interface;
+            int pcapd_pid;
         } pcap;
     };
 

app/src/main/jni/pcapd/pcapd.c

@@ -204,7 +204,7 @@ static void sighandler(__unused int signo) {
   } else {
     log_w("Exit now");
     unlink(PCAPD_PID);
-    exit(1);
+    exit(PCAPD_ERROR);
   }
 }
 
@@ -255,12 +255,12 @@ static int init_pcapd_capture(pcapd_runtime_t *rt, pcapd_conf_t *conf) {
       // parent
       exit(0);
     }
-
-    // SIGPIPE will be generated as in su_cmd PCAPdroid performs a dup2 stdout/stderr to a pipe
-    // which is then closed
-    signal(SIGPIPE, SIG_IGN);
   }
 
+  // SIGPIPE will be generated as in su_cmd PCAPdroid performs a dup2 stdout/stderr to a pipe
+  // which is then closed
+  signal(SIGPIPE, SIG_IGN);
+
   rt->nlroute_sock = -1;
   rt->nldiag_sock = -1;
   rt->client = -1;
@@ -346,7 +346,7 @@ static void init_interface(pcapd_iface_t *iface) {
 
 /* ******************************************************* */
 
-static int open_interface(pcapd_iface_t *iface, pcapd_runtime_t *rt, const char *ifname, int ifid) {
+static pcapd_rv open_interface(pcapd_iface_t *iface, pcapd_runtime_t *rt, const char *ifname, int ifid) {
 #ifndef READ_FROM_PCAP
   int is_file = 0;
   pcap_t *pd;
@@ -371,7 +371,7 @@ static int open_interface(pcapd_iface_t *iface, pcapd_runtime_t *rt, const char
 
     if(!pd) {
       log_e("pcap_open(%s) failed: %s", ifname, errbuf);
-      return -1;
+      return PCAPD_INTERFACE_OPEN_FAILED;
     }
 
     is_file = 1;
@@ -386,7 +386,7 @@ static int open_interface(pcapd_iface_t *iface, pcapd_runtime_t *rt, const char
 
   if(!pd) {
     log_i("pcap_open_offline(%s) failed: %s", READ_FROM_PCAP, errbuf);
-    return -1;
+    return PCAPD_INTERFACE_OPEN_FAILED;
   }
 
   strcpy(ifname, "pcap");
@@ -422,7 +422,7 @@ static int open_interface(pcapd_iface_t *iface, pcapd_runtime_t *rt, const char
     default:
       log_i("[%s] unsupported datalink: %d", ifname, dlink);
       pcap_close(pd);
-      return -1;
+      return PCAPD_UNSUPPORTED_DATALINK;
   }
 
   struct bpf_program fcode;
@@ -431,14 +431,14 @@ static int open_interface(pcapd_iface_t *iface, pcapd_runtime_t *rt, const char
   if(pcap_compile(pd, &fcode, rt->bpf, 1, PCAP_NETMASK_UNKNOWN) < 0) {
     log_i("[%s] could not set capture filter: %s", ifname, pcap_geterr(pd));
     pcap_close(pd);
-    return -1;
+    return PCAPD_INTERFACE_OPEN_FAILED;
   }
 
   if(pcap_setfilter(pd, &fcode) < 0) {
     log_e("[%s] pcap_setfilter failed: %s", ifname, pcap_geterr(pd));
     pcap_freecode(&fcode);
     pcap_close(pd);
-    return -1;
+    return PCAPD_INTERFACE_OPEN_FAILED;
   }
   pcap_freecode(&fcode);
 
@@ -479,7 +479,7 @@ static int open_interface(pcapd_iface_t *iface, pcapd_runtime_t *rt, const char
 
   log_d("%s(%d): datalink=%s(%d)", iface->name, iface->ifidx, dlink_s, dlink);
 
-  return 0;
+  return PCAPD_OK;
 }
 
 /* ******************************************************* */
@@ -524,7 +524,7 @@ static void check_inet_interface(pcapd_runtime_t *rt) {
 
   pcap_t *old_pd = rt->inet_iface->pd;
 
-  if(open_interface(rt->inet_iface, rt, ifname, rt->conf->inet_ifid) < 0)
+  if(open_interface(rt->inet_iface, rt, ifname, rt->conf->inet_ifid) != PCAPD_OK)
     return;
 
   // Success
@@ -710,7 +710,7 @@ static void get_selectable_fds(pcapd_runtime_t *rt, fd_set *fds) {
 
 /* ******************************************************* */
 
-static int read_pkt(pcapd_runtime_t *rt, pcapd_iface_t *iface, time_t now) {
+static pcapd_rv read_pkt(pcapd_runtime_t *rt, pcapd_iface_t *iface, time_t now) {
   struct pcap_pkthdr *hdr;
   const u_char *pkt;
   int to_skip = iface->ipoffset;
@@ -723,20 +723,15 @@ static int read_pkt(pcapd_runtime_t *rt, pcapd_iface_t *iface, time_t now) {
 
       if(iface == rt->inet_iface)
         // Do not abort, just wait for route/interface changes
-        return 0;
+        return PCAPD_OK;
 
       // abort, resuming other interfaces is not supported yet
-      return -1;
+      return PCAPD_PCAP_READ_ERROR;
     } else if(rv == PCAP_ERROR_BREAK)
-      // TODO handle EOF without error
+      return PCAPD_EOF;
-#ifndef READ_FROM_PCAP
-      return -1;
-#else
-      return 0;
-#endif
 
     // can be reached when the packet buffer timeout expires
-    return 0;
+    return PCAPD_OK;
   }
 
   if(hdr->caplen >= to_skip) {
@@ -806,7 +801,7 @@ static int read_pkt(pcapd_runtime_t *rt, pcapd_iface_t *iface, time_t now) {
         if((xwrite(rt->client, &phdr, sizeof(phdr)) < 0) ||
            (xwrite(rt->client, pkt, phdr.len) < 0)) {
           log_e("write failed[%d]: %s", errno, strerror(errno));
-          return -1;
+          return PCAPD_SOCKET_WRITE_ERROR;
         }
       } else if(!rt->conf->quiet) {
         char buf[512];
@@ -831,13 +826,13 @@ static int read_pkt(pcapd_runtime_t *rt, pcapd_iface_t *iface, time_t now) {
     iface->next_stats_update = now + 3;
   }
 
-  return 0;
+  return PCAPD_OK;
 }
 
 /* ******************************************************* */
 
-int run_pcap_dump(pcapd_conf_t *conf) {
+pcapd_rv run_pcap_dump(pcapd_conf_t *conf) {
-  int rv = -1;
+  pcapd_rv rv = PCAPD_ERROR;
   struct timespec ts = {0};
   pcapd_runtime_t rt = {0};
   time_t next_interface_recheck = 0;
@@ -863,11 +858,11 @@ int run_pcap_dump(pcapd_conf_t *conf) {
 
   for(int i=0; i<conf->num_interfaces; i++) {
     if((strcmp(conf->ifnames[i], "@inet") != 0)
-          && open_interface(&rt.ifaces[i], &rt, conf->ifnames[i], i) < 0)
+          && (rv = open_interface(&rt.ifaces[i], &rt, conf->ifnames[i], i)) != PCAPD_OK)
       goto cleanup;
   }
 
-  rv = 0;
+  rv = PCAPD_OK;
   running = 1;
   get_selectable_fds(&rt, &rt.sel_fds);
 
@@ -878,7 +873,7 @@ int run_pcap_dump(pcapd_conf_t *conf) {
     if(select(rt.maxfd + 1, &fds, NULL, NULL, &timeout) < 0) {
       if(errno != EINTR) {
         log_e("select failed[%d]: %s", errno, strerror(errno));
-        rv = -1;
+        rv = PCAPD_ERROR;
       }
       break;
     }
@@ -891,7 +886,7 @@ int run_pcap_dump(pcapd_conf_t *conf) {
       break;
     } else if((rt.nlroute_sock > 0) && FD_ISSET(rt.nlroute_sock, &fds)) {
       if(handle_nl_message(&rt) < 0) {
-        rv = -1;
+        rv = PCAPD_NETLINK_ERROR;
         break;
       }
 
@@ -900,8 +895,9 @@ int run_pcap_dump(pcapd_conf_t *conf) {
     } else {
       for(int i=0; i<rt.conf->num_interfaces; i++) {
         if((rt.ifaces[i].pf != -1) && FD_ISSET(rt.ifaces[i].pf, &fds)) {
-          if(read_pkt(&rt, &rt.ifaces[i], now) < 0) {
+          if((rv = read_pkt(&rt, &rt.ifaces[i], now)) != PCAPD_OK) {
-            rv = -1;
+            if(rv == PCAPD_EOF)
+              rv = PCAPD_OK;
             running = 0;
             break;
           }
@@ -961,7 +957,7 @@ static void usage() {
     " -q             suppress non-error output\n"
   );
 
-  exit(1);
+  exit(PCAPD_ERROR);
 }
 
 /* ******************************************************* */
@@ -985,12 +981,12 @@ static void parse_args(pcapd_conf_t *conf, int argc, char **argv) {
       case 'i':
         if(conf->num_interfaces >= PCAPD_MAX_INTERFACES) {
           fprintf(stderr, "Maximum number of interfaces reached (%d)\n", PCAPD_MAX_INTERFACES);
-          exit(1);
+          exit(PCAPD_ERROR);
         }
         if(strcmp(optarg, "@inet") == 0) {
           if(conf->inet_ifid != -1) {
             fprintf(stderr, "@inet interface already specified\n");
-            exit(1);
+            exit(PCAPD_ERROR);
           }
           conf->inet_ifid = conf->num_interfaces;
         }
@@ -1009,7 +1005,7 @@ static void parse_args(pcapd_conf_t *conf, int argc, char **argv) {
         conf->uid_filter = atoi(optarg);
         if(conf->uid_filter < -1) {
           fprintf(stderr, "Invalid UID: %s\n", optarg);
-          exit(1);
+          exit(PCAPD_ERROR);
         }
         break;
       case 'b':
@@ -1064,15 +1060,12 @@ static void parse_args(pcapd_conf_t *conf, int argc, char **argv) {
 
 int main(int argc, char *argv[]) {
   pcapd_conf_t conf;
-  int rv;
 
   logtag = "pcapd";
   logcallback = logcb;
   parse_args(&conf, argc, argv);
 
-  rv = run_pcap_dump(&conf);
+  return run_pcap_dump(&conf);
-
-  return rv;
 }
 
 #endif

app/src/main/jni/pcapd/pcapd.h

@@ -39,4 +39,15 @@ typedef struct {
   uint8_t pad[2];           // padding for 64bit alignment of the payload
 } __attribute__((packed)) pcapd_hdr_t;
 
+typedef enum {
+    PCAPD_OK = 0,
+    PCAPD_ERROR,
+    PCAPD_INTERFACE_OPEN_FAILED,
+    PCAPD_NETLINK_ERROR,
+    PCAPD_PCAP_READ_ERROR,
+    PCAPD_SOCKET_WRITE_ERROR,
+    PCAPD_UNSUPPORTED_DATALINK,
+    PCAPD_EOF
+} pcapd_rv;
+
 #endif

app/src/main/jni/pcapd/pcapd_priv.h

@@ -29,6 +29,7 @@
 #include "common/uid_lru.h"
 #include "common/uid_resolver.h"
 #include "common/utils.h"
+#include "pcapd.h"
 #include "zdtun.h"
 
 #define PCAPD_MAX_INTERFACES 16
@@ -83,6 +84,6 @@ typedef struct {
 } pcapd_runtime_t;
 
 void init_conf(pcapd_conf_t *conf);
-int run_pcap_dump(pcapd_conf_t *conf);
+pcapd_rv run_pcap_dump(pcapd_conf_t *conf);
 
 #endif

app/src/main/res/values/strings.xml

@@ -479,4 +479,10 @@
     <string name="injected">Injected</string>
     <string name="open_pcap_file">Open PCAP file…</string>
     <string name="pcap_load_success">PCAP file loaded</string>
+    <string name="invalid_pcap_file">Invalid PCAP file</string>
+    <string name="capture_interface_open_error">Could not open the capture interface</string>
+    <string name="unsupported_pcap_datalink">PCAP file has an unsupported datalink</string>
+    <string name="pcap_file_not_exists">The specified PCAP file does not exist</string>
+    <string name="root_capture_start_failed">Capture start failure. Ensure that the device is rooted with Magisk</string>
+    <string name="pcap_read_error">PCAP read error</string>
 </resources>