From ba7291c12f01878ae9d6e7849a2715065ecd5214 Mon Sep 17 00:00:00 2001 From: emanuele-f Date: Fri, 18 Aug 2023 01:09:19 +0200 Subject: [PATCH] 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. --- .../remote_capture/CaptureService.java | 30 ++++++- app/src/main/jni/common/utils.c | 70 ++++++++++------- app/src/main/jni/common/utils.h | 1 + app/src/main/jni/core/capture_libpcap.c | 78 +++++++++++++++---- app/src/main/jni/core/errors.h | 14 ++++ app/src/main/jni/core/pcapdroid.h | 1 + app/src/main/jni/pcapd/pcapd.c | 75 ++++++++---------- app/src/main/jni/pcapd/pcapd.h | 11 +++ app/src/main/jni/pcapd/pcapd_priv.h | 3 +- app/src/main/res/values/strings.xml | 6 ++ 10 files changed, 205 insertions(+), 84 deletions(-) create mode 100644 app/src/main/jni/core/errors.h diff --git a/app/src/main/java/com/emanuelef/remote_capture/CaptureService.java b/app/src/main/java/com/emanuelef/remote_capture/CaptureService.java index f1d4c9b0..891c104b 100644 --- a/app/src/main/java/com/emanuelef/remote_capture/CaptureService.java +++ b/app/src/main/java/com/emanuelef/remote_capture/CaptureService.java @@ -1416,7 +1416,35 @@ public void stopPcapDump() { 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() { diff --git a/app/src/main/jni/common/utils.c b/app/src/main/jni/common/utils.c index a532346a..8a0cf0f3 100644 --- a/app/src/main/jni/common/utils.c +++ b/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; } \ No newline at end of file diff --git a/app/src/main/jni/common/utils.h b/app/src/main/jni/common/utils.h index 68927f54..651d9c96 100644 --- a/app/src/main/jni/common/utils.h +++ b/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__ diff --git a/app/src/main/jni/core/capture_libpcap.c b/app/src/main/jni/core/capture_libpcap.c index 0cfa2356..fec935b8 100644 --- a/app/src/main/jni/core/capture_libpcap.c +++ b/app/src/main/jni/core/capture_libpcap.c @@ -18,8 +18,10 @@ */ #include +#include #include #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) { - int pid = get_pcapd_pid(); - if(pid > 0) { - char pid_s[8]; - snprintf(pid_s, sizeof(pid_s), "%d", pid); +static void kill_process(int pid, bool as_root, int signum) { + char args[16]; - log_i("Killing old pcapd with pid %d", pid); - run_shell_cmd("kill", pid_s, pd->pcap.as_root, false); - } + snprintf(args, sizeof(args), "-%d %d", signum, pid); + 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 @@ int run_libpcap(pcapdroid_t *pd) { 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; } diff --git a/app/src/main/jni/core/errors.h b/app/src/main/jni/core/errors.h new file mode 100644 index 00000000..77e0a7f1 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/app/src/main/jni/core/pcapdroid.h b/app/src/main/jni/core/pcapdroid.h index acffb670..b542d16d 100644 --- a/app/src/main/jni/core/pcapdroid.h +++ b/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; }; diff --git a/app/src/main/jni/pcapd/pcapd.c b/app/src/main/jni/pcapd/pcapd.c index 1333ff6d..dd4c7461 100644 --- a/app/src/main/jni/pcapd/pcapd.c +++ b/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 -#ifndef READ_FROM_PCAP - return -1; -#else - return 0; -#endif + return PCAPD_EOF; // 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) { - int rv = -1; +pcapd_rv run_pcap_dump(pcapd_conf_t *conf) { + 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; inum_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; inum_interfaces; i++) { if((rt.ifaces[i].pf != -1) && FD_ISSET(rt.ifaces[i].pf, &fds)) { - if(read_pkt(&rt, &rt.ifaces[i], now) < 0) { - rv = -1; + if((rv = read_pkt(&rt, &rt.ifaces[i], now)) != PCAPD_OK) { + 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 rv; + return run_pcap_dump(&conf); } #endif diff --git a/app/src/main/jni/pcapd/pcapd.h b/app/src/main/jni/pcapd/pcapd.h index 786f8920..63665a46 100644 --- a/app/src/main/jni/pcapd/pcapd.h +++ b/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 diff --git a/app/src/main/jni/pcapd/pcapd_priv.h b/app/src/main/jni/pcapd/pcapd_priv.h index 0ddaf774..65368a79 100644 --- a/app/src/main/jni/pcapd/pcapd_priv.h +++ b/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 diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index b871cbc6..ccc6ad14 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -479,4 +479,10 @@ Injected Open PCAP file… PCAP file loaded + Invalid PCAP file + Could not open the capture interface + PCAP file has an unsupported datalink + The specified PCAP file does not exist + Capture start failure. Ensure that the device is rooted with Magisk + PCAP read error