From 71e70e7ea3960864c180b54fda7e7ffb79ac8c92 Mon Sep 17 00:00:00 2001
From: Aaron Hope <aaron.hope@xemed.com>
Date: Mon, 13 Mar 2017 16:44:00 -0400
Subject: [PATCH 01/13] 01-add-autotools

Minimal autotools build system

Added basic autoconf and automake (but not libtool) build support.
Generated files not included. To use, one should first run "autoreconf -is".
---
 Makefile.am  |  4 ++++
 configure.ac | 43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)
 create mode 100644 Makefile.am
 create mode 100644 configure.ac

diff --git a/Makefile.am b/Makefile.am
new file mode 100644
index 0000000..f0402ec
--- /dev/null
+++ b/Makefile.am
@@ -0,0 +1,4 @@
+
+bin_PROGRAMS = choparp
+choparp_SOURCES = src/choparp.c
+man_MANS = src/choparp.8
diff --git a/configure.ac b/configure.ac
new file mode 100644
index 0000000..4f90c9f
--- /dev/null
+++ b/configure.ac
@@ -0,0 +1,43 @@
+# This is based on David A. Wheeler's template for configure.ac
+
+# Process this file with autoconf to produce a configure script.
+
+# Initialize autoconf.
+AC_INIT([choparp], [20150613])
+# Force autoconf to be at least this version number:
+AC_PREREQ([2.68])
+# Safety check - list a source file that wouldn't be in other directories:
+AC_CONFIG_SRCDIR([src/choparp.c])
+# Put configuration results here, so we can easily #include them:
+AC_CONFIG_HEADERS([config.h])
+# Put autotools auxiliary files in subdirectories to reduce clutter:
+AC_CONFIG_AUX_DIR([build-aux])
+AC_CONFIG_MACRO_DIR([m4])
+
+# Enable "automake" to simplify creating makefiles:
+AM_INIT_AUTOMAKE([subdir-objects foreign -Wall -Werror])
+AC_CONFIG_FILES([Makefile])
+
+
+# Checks for programs, e.g., AC_PROG_CC
+AC_PROG_CC
+# only needef for automake < 1.14
+AM_PROG_CC_C_O
+
+# Checks for libraries.
+AC_CHECK_LIB([pcap], [pcap_lookupdev], [], [AC_MSG_ERROR([missing required library libpcap])])
+
+# Checks for header files.
+AC_CHECK_HEADERS([arpa/inet.h fcntl.h netinet/in.h stdlib.h string.h sys/ioctl.h sys/socket.h unistd.h],
+    [], [AC_MSG_ERROR([missing required platform headers])])
+AC_CHECK_HEADER([pcap.h], [], [AC_MSG_ERROR([missing required header pcap.h])])
+
+# Checks for typedefs, structures, and compiler characteristics.
+
+# Checks for library functions.
+AC_FUNC_MALLOC
+AC_FUNC_REALLOC
+AC_CHECK_FUNCS([ftruncate inet_ntoa memset socket strchr strtol])
+
+# Do final output.
+AC_OUTPUT

From 35b8ebb417bcfad73c5c84bf03cc5a9743b775ec Mon Sep 17 00:00:00 2001
From: Aaron Hope <aaron.hope@xemed.com>
Date: Mon, 13 Mar 2017 16:44:01 -0400
Subject: [PATCH 02/13] 02-add-tests

Add unit tests

Add tests for core functionality as automake TAP tests. To execute, run "make
check". Requires support for the linux "veth" virtual ethernet tunnel and
permissions required to manipulate such.

Effort has been made to make these safe to run on a live system. Tests will
only be executed in a private network namespace. In theory, this means that all
changes will be properly issolated, but as always, buyer beware. Systems with
support for unprivileged containers (unprivileged_userns_clone=1) will allow
tests to be run as non-root. Otherwise, root will be required.
---
 Makefile.am        |  16 ++-
 configure.ac       |   3 +
 test/linux-veth.sh | 269 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 287 insertions(+), 1 deletion(-)
 create mode 100644 test/linux-veth.sh

diff --git a/Makefile.am b/Makefile.am
index f0402ec..675c999 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1,4 +1,18 @@
-
 bin_PROGRAMS = choparp
 choparp_SOURCES = src/choparp.c
 man_MANS = src/choparp.8
+
+AM_TESTS_ENVIRONMENT = \
+	choparp_abspath='$(abs_top_builddir)'/choparp; \
+	export choparp_abspath;
+
+LOG_DRIVER = \
+	env AM_TAP_AWK='$(AWK)' $(SHELL) $(top_srcdir)/build-aux/tap-driver.sh
+
+TEST_EXTENSIONS = .sh
+
+SH_LOG_DRIVER = $(LOG_DRIVER)
+SH_LOG_COMPILER = $(SHELL)
+
+TESTS = test/linux-veth.sh
+EXTRA_DIST = $(TESTS)
diff --git a/configure.ac b/configure.ac
index 4f90c9f..83b4a51 100644
--- a/configure.ac
+++ b/configure.ac
@@ -18,6 +18,9 @@ AC_CONFIG_MACRO_DIR([m4])
 AM_INIT_AUTOMAKE([subdir-objects foreign -Wall -Werror])
 AC_CONFIG_FILES([Makefile])
 
+# TAP test driver support
+AC_PROG_AWK
+AC_REQUIRE_AUX_FILE([tap-driver.sh])
 
 # Checks for programs, e.g., AC_PROG_CC
 AC_PROG_CC
diff --git a/test/linux-veth.sh b/test/linux-veth.sh
new file mode 100644
index 0000000..2b5285c
--- /dev/null
+++ b/test/linux-veth.sh
@@ -0,0 +1,269 @@
+#!/bin/sh
+
+if [ "x$choparp_abspath" = x ]
+then
+	echo "1..0 # Skipped: missing \$choparp_abspath (did you run me as 'make check'?) "
+	exit
+fi
+
+CHOPARP_NETNS="$(ls -ld /proc/$$/ns/net)"
+CHOPARP_NETNS="${CHOPARP_NETNS#*->}"
+
+if [ -z "$TAP_RUNNER_NETNS" ]
+then
+	export LC_CTYPE=C
+	export TAP_RUNNER_NETNS="$CHOPARP_NETNS"
+	exec 4>&1
+	GOT_NETNS=$(unshare --user --map-root-user --net /bin/sh $0 5>&1 1>&4)
+	ev=$?
+	if [ "$GOT_NETNS" != "GOT_NETNS" ]
+	then
+		echo "1..0 # Skipped: unable to create private network namespace (need root?)"
+	fi
+	exit
+fi
+if [ "$TAP_RUNNER_NETNS" = "$CHOPARP_NETNS" ]
+then
+	# sanity check, should never occur
+	exit 1
+fi
+
+echo GOT_NETNS >&5
+
+rnd_byte="$(dd if=/dev/urandom bs=1 count=1 2>/dev/null | od -A n -t d)"
+
+lladdr() {
+	printf '02:00:c0:a8:%02x:%02x' "$rnd_byte" "$1"
+}
+
+ipaddr() {
+	printf '192.168.%d.%d' "$rnd_byte" "$1"
+}
+
+hex_ipaddr() {
+	printf '0xc0a8%02x%02x' "$rnd_byte" "$1"
+}
+
+arp_for() {
+	(
+		for i
+		do
+			ping -I who-has -c 1 -w 1 $(ipaddr $i) &
+		done
+		wait # for all jobs in this subshell
+	) > /dev/null
+}
+
+found() {
+	ip -4 neigh show dev who-has | \
+		grep -i -q "$(ipaddr $1) lladdr ${2:-.*} REACHABLE"
+}
+
+echo "1..9"
+
+set -x
+
+ip link add is-at type veth peer name who-has
+ip addr add 192.168.1.200/24 dev who-has
+ip link set is-at up
+ip link set who-has up
+
+#######################################################################
+
+test_desc="1 - Base case, static hardware address and single-host"
+"$choparp_abspath" is-at $(lladdr 1) $(ipaddr 1) &
+chopid=$!
+sleep 1
+
+arp_for 1
+kill $chopid
+
+if ! wait $chopid
+then
+	echo "not ok $test_desc # abnormal exit"
+elif ! found 1
+then
+	echo "not ok $test_desc # MAC resolution failure"
+else
+	echo "ok $test_desc"
+fi
+
+#######################################################################
+
+test_desc="2 - Target IP by legacy subnet"
+"$choparp_abspath" is-at $(lladdr 2) $(ipaddr 2)/255.255.255.254 &
+chopid=$!
+sleep 1
+
+arp_for 2 3
+kill $chopid
+
+if ! wait $chopid
+then
+	echo "not ok $test_desc # abnormal exit"
+elif ! (found 2 && found 3)
+then
+	echo "not ok $test_desc # MAC resolution failure"
+else
+	echo "ok $test_desc"
+fi
+
+#######################################################################
+
+test_desc="3 - Target IP by BSD-style hex"
+"$choparp_abspath" is-at $(lladdr 4) $(hex_ipaddr 4)/0xfffffffe &
+chopid=$!
+sleep 1
+
+arp_for 4
+arp_for 5
+kill $chopid
+
+if ! wait $chopid
+then
+	echo "not ok $test_desc # abnormal exit"
+elif ! (found 4 && found 5)
+then
+	echo "not ok $test_desc # MAC resolution failure"
+else
+	echo "ok $test_desc"
+fi
+
+#######################################################################
+
+test_desc="4 - Target IP by IP list"
+"$choparp_abspath" is-at $(lladdr 6) $(ipaddr 6) $(ipaddr 7) &
+chopid=$!
+sleep 1
+
+arp_for 6 7
+kill $chopid
+
+if ! wait $chopid
+then
+	echo "not ok $test_desc # abnormal exit"
+elif ! (found 6 && found 7)
+then
+	echo "not ok $test_desc # MAC resolution failure"
+else
+	echo "ok $test_desc"
+fi
+
+#######################################################################
+
+test_desc="5 - Target IP by CIDR subnet and exclusion"
+"$choparp_abspath" is-at $(lladdr 8) $(ipaddr 8)/30 -$(ipaddr 10) &
+chopid=$!
+sleep 1
+
+arp_for 8 9 10 11
+kill $chopid
+
+if ! wait $chopid
+then
+	echo "not ok $test_desc # abnormal exit"
+elif ! (found 8 && found 9 && ! found 10 && found 11)
+then
+	echo "not ok $test_desc # MAC resolution failure"
+else
+	echo "ok $test_desc"
+fi
+
+#######################################################################
+
+test_desc="6 - Hardware address detection by \"auto\" keyword"
+"$choparp_abspath" is-at auto $(ipaddr 12) &
+chopid=$!
+sleep 1
+
+arp_for 12
+kill $chopid
+
+if ! wait $chopid
+then
+	echo "not ok $test_desc # abnormal exit"
+elif ! found 12
+then
+	echo "not ok $test_desc # MAC resolution failure"
+else
+	echo "ok $test_desc"
+fi
+
+#######################################################################
+
+test_desc="7 - Hardware address by \"vhid\" keyword, decimal"
+"$choparp_abspath" is-at vhid:13 $(ipaddr 13) &
+chopid=$!
+sleep 1
+
+arp_for 13
+kill $chopid
+
+ip neigh show dev who-has
+
+if ! wait $chopid
+then
+	echo "not ok $test_desc # abnormal exit"
+elif ! found 13 00:00:5e:00:01:0d
+then
+	echo "not ok $test_desc # MAC resolution failure"
+else
+	echo "ok $test_desc"
+fi
+
+#######################################################################
+
+test_desc="8 - Hardware address by \"vhid\" keyword, hex"
+"$choparp_abspath" is-at vhid:0x0e $(ipaddr 14) &
+chopid=$!
+sleep 1
+
+arp_for 14
+kill $chopid
+
+ip neigh show dev who-has
+
+if ! wait $chopid
+then
+	echo "not ok $test_desc # abnormal exit"
+elif ! found 14 00:00:5e:00:01:0e
+then
+	echo "not ok $test_desc # MAC resolution failure"
+else
+	echo "ok $test_desc"
+fi
+
+#######################################################################
+
+test_desc="9 - Pidfile with -p"
+pidfile=$(mktemp /tmp/choparp.pid-XXXXXXXX)
+"$choparp_abspath" -p $pidfile is-at auto $(ipaddr 15) &
+chopid=$!
+sleep 1
+
+chopid_from_file=$(cat $pidfile)
+arp_for 15
+kill $chopid
+
+if ! wait $chopid
+then
+	echo "not ok $test_desc # abnormal exit"
+elif ! found 15
+then
+	echo "not ok $test_desc # MAC resolution failure"
+elif [ "$chopid" -ne "$chopid_from_file" ]
+then
+	echo "not ok $test_desc # invalid pidfile"
+elif [ -f "$pidfile" ]
+then
+	echo "not ok $test_desc # pidfile not removed after exit"
+else
+	echo "ok $test_desc"
+fi
+
+#######################################################################
+
+# Cleanup
+ip link delete is-at
+
+exit 0

From d45ef25f115c1b9bc1ad4eadf3ad47385669ea54 Mon Sep 17 00:00:00 2001
From: Aaron Hope <aaron.hope@xemed.com>
Date: Mon, 13 Mar 2017 16:44:01 -0400
Subject: [PATCH 03/13] 03-string-bounds-checking

More rigorous string manipulation

Fix improper use of bounds-checked strncpy and snprintf (return values were
ignored).  Replace wth input validation and classic strcpy as needed.

Replace hand-written buffer management with asprintf(3) appends.

Fix off-by-one error in vhid parsing.
---
 configure.ac  |  2 +-
 src/choparp.c | 51 ++++++++++++++++++++++-----------------------------
 2 files changed, 23 insertions(+), 30 deletions(-)

diff --git a/configure.ac b/configure.ac
index 83b4a51..de1ce2d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -40,7 +40,7 @@ AC_CHECK_HEADER([pcap.h], [], [AC_MSG_ERROR([missing required header pcap.h])])
 # Checks for library functions.
 AC_FUNC_MALLOC
 AC_FUNC_REALLOC
-AC_CHECK_FUNCS([ftruncate inet_ntoa memset socket strchr strtol])
+AC_CHECK_FUNCS([ftruncate inet_ntop memset socket strchr strtol])
 
 # Do final output.
 AC_OUTPUT
diff --git a/src/choparp.c b/src/choparp.c
index b284583..8c65e6a 100644
--- a/src/choparp.c
+++ b/src/choparp.c
@@ -76,36 +76,25 @@ static char *pidfile = NULL;
 static pcap_t *pc;
 
 char* cidr_to_str(struct cidr *a) {
-    char buf[64];
-    char *res = NULL;
-    int res_alloc, res_len;
-    int len;
+    char addr[INET_ADDRSTRLEN], mask[INET_ADDRSTRLEN];
+    char *res = NULL, *oldres = NULL;
+    char *prefix = "";
+    char *sep = "";
 
     while (a) {
-        if (a->mask.s_addr == INADDR_NONE) {
-            len = snprintf(buf, sizeof buf, "dst host %s", inet_ntoa(a->addr));
-        } else {
-            len = snprintf(buf, sizeof buf, "dst net %s mask ", inet_ntoa(a->addr));
-            len += snprintf(buf + len, sizeof buf - len, "%s", inet_ntoa(a->mask));
-        }
-
-        if (!res) {
-            res_alloc = 1024;
-            res = malloc(res_alloc);
-            strncpy(res, buf, res_alloc);
-            res_len = len;
-
-        } else {
-            if (res_len + len + 5 > res_alloc) {
-                res_alloc *= 2;
-                res = realloc(res, res_alloc);
-            }
-            strncat(res, " or ", res_alloc - res_len - 1);
-            res_len += 4;
-            strncat(res, buf, res_alloc - res_len - 1);
-            res_len += len;
+        inet_ntop(AF_INET, &a->addr, addr, sizeof(addr));
+        inet_ntop(AF_INET, &a->mask, mask, sizeof(mask));
+        if (a->mask.s_addr == INADDR_NONE
+            ? asprintf(&res, "%s%sdst host %s", prefix, sep, addr) < 0
+            : asprintf(&res, "%s%sdst net %s mask %s", prefix, sep, addr, mask) < 0
+        ) {
+            perror("asprintf");
+            exit(1);
         }
 
+        free(oldres);
+        prefix = oldres = res;
+        sep = " or ";
         a = a->next;
     }
     return res;
@@ -182,13 +171,17 @@ setmac(char *addr, char *ifname){
         int fd;
         struct ifreq ifr;
 
+        if (strlen(ifname) + 1 > sizeof(ifr.ifr_name)) {
+            fprintf(stderr, "if_name too long: %s\n", ifname);
+            exit(1);
+        }
+        strcpy(ifr.ifr_name, ifname);
+
         if ((fd = socket(PF_INET, SOCK_STREAM, 0)) == -1) {
             perror("socket");
             return -1;
         }
 
-        strncpy(ifr.ifr_name, ifname, sizeof ifr.ifr_name);
-
         if (ioctl(fd, SIOCGIFHWADDR, &ifr) < 0) {
             perror("ioctl(SIOCGIFHWADDR)");
             return -1;
@@ -212,7 +205,7 @@ setmac(char *addr, char *ifname){
         fprintf(stderr, "%s: not found\n", ifname);
         return -1;
 
-    } else if (!strncmp (addr, "vhid:", 4)) {
+    } else if (!strncmp (addr, "vhid:", 5)) {
         /*
          * Virtual router mac address
          * CARP address format: 00:00:5e:00:01:<VHID>

From a2019fe7dea6490e9cc7875e119b6636c3f1a60c Mon Sep 17 00:00:00 2001
From: Aaron Hope <aaron.hope@xemed.com>
Date: Mon, 13 Mar 2017 16:44:01 -0400
Subject: [PATCH 04/13] 04-integer-conversion

Fix erroneous and non-compliant integer conversions

Replace improper pointer cast between integer types of differing width. This
fixes a bug affecting LP64 64-bit big-endian architectures.

Replace BSD legacy integer type aliases with C prmitives, to comply with scanf
requirements.
---
 src/choparp.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/src/choparp.c b/src/choparp.c
index 8c65e6a..7be2159 100644
--- a/src/choparp.c
+++ b/src/choparp.c
@@ -164,7 +164,7 @@ process_arp(u_char *user, const struct pcap_pkthdr *pkthdr, const u_char *packet
 
 int
 setmac(char *addr, char *ifname){
-    u_int m0, m1, m2, m3, m4, m5;
+    unsigned m0, m1, m2, m3, m4, m5;
 
     if (!strcmp (addr, "auto")) {
 #ifdef __linux__
@@ -234,16 +234,19 @@ setmac(char *addr, char *ifname){
 
 int
 atoip(char *buf, u_int32_t *ip_addr){
-    u_int	i0, i1, i2, i3;
+    unsigned i0, i1, i2, i3;
+    unsigned long hex_addr;
 
     if (sscanf(buf, "%u.%u.%u.%u", &i0, &i1, &i2, &i3) == 4){
-	*ip_addr = (i0 << 24) + (i1 << 16) + (i2 << 8) + i3;
-	return(0);
+        *ip_addr = (i0 << 24) + (i1 << 16) + (i2 << 8) + i3;
+        return 0;
+    }
+    if (sscanf(buf, "0x%lx", &hex_addr) >= 1) {
+        *ip_addr = (u_int32_t)hex_addr;
+        return 0;
     }
-    if (sscanf(buf, "0x%lx", (unsigned long *) ip_addr) == 1)
-	return(0);
 
-    return(-1);	
+    return -1;
 }
 
 void

From f9287239954b570337f22e778fa1f98b638a7890 Mon Sep 17 00:00:00 2001
From: Aaron Hope <aaron.hope@xemed.com>
Date: Mon, 13 Mar 2017 16:44:02 -0400
Subject: [PATCH 05/13] 05-input-validation

Input validation enhancements

Make sure the interface link type is ethernet, since that's all we support.

Add bounds-checking for integer values.

Add checks for trailing characters on input parameters.

Documentation calls for vhid: mac address to be followed by a hexadecimal
number, but implementation parsed it as decimal. Split the difference and
support hexadecimal when preceded by "0x" and decimal otherwise.
---
 src/choparp.c | 58 ++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 44 insertions(+), 14 deletions(-)

diff --git a/src/choparp.c b/src/choparp.c
index 7be2159..d1a8c23 100644
--- a/src/choparp.c
+++ b/src/choparp.c
@@ -104,6 +104,7 @@ pcap_t *
 open_pcap(char *ifname, char *filter_str) {
     pcap_t *pc = NULL;
     struct bpf_program filter;
+    int dlt;
 
     /* Set up PCAP */
     if ((pc = pcap_open_live(ifname, 128, 0,  512, errbuf))==NULL){
@@ -123,6 +124,12 @@ open_pcap(char *ifname, char *filter_str) {
        exit(1);
     }
 
+    /* wired ethernet only */
+    if ((dlt = pcap_datalink(pc)) != DLT_EN10MB) {
+       fprintf(stderr, "unsupported link type: %s\n", pcap_datalink_val_to_name(dlt));
+       exit(1);
+    }
+
     pcap_freecode(&filter);
     return pc;
 }
@@ -164,6 +171,7 @@ process_arp(u_char *user, const struct pcap_pkthdr *pkthdr, const u_char *packet
 
 int
 setmac(char *addr, char *ifname){
+    int len = 0;
     unsigned m0, m1, m2, m3, m4, m5;
 
     if (!strcmp (addr, "auto")) {
@@ -205,21 +213,29 @@ setmac(char *addr, char *ifname){
         fprintf(stderr, "%s: not found\n", ifname);
         return -1;
 
-    } else if (!strncmp (addr, "vhid:", 5)) {
+    } else if (sscanf(addr, "vhid:%n", &len) >= 0 && len) {
         /*
          * Virtual router mac address
          * CARP address format: 00:00:5e:00:01:<VHID>
          */
-        char *vhid = addr + 5;
-        if (!*vhid)
-            return(-1);
-        m0 = 0;
-        m1 = 0;
-        m2 = 0x5e;
-        m3 = 0;
-        m4 = 1;
-        m5 = atoi(vhid);
-    } else if (sscanf(addr, "%x:%x:%x:%x:%x:%x", &m0, &m1, &m2, &m3, &m4, &m5) < 6) {
+        unsigned vhid;
+        /* allow decimal and hexadecimal (leading zero is decimal, not octal) */
+        int matched =
+            sscanf(addr, "vhid:0x%x%n", &vhid, &len) > 0
+            || sscanf(addr, "vhid:%u%n", &vhid, &len) > 0;
+        if (!matched || addr[len] || vhid > 0xff) {
+            fprintf(stderr, "invalid vhid spec: %s\n", addr);
+            exit(-1);
+        }
+        target_mac[0] = 0;
+        target_mac[1] = 0;
+        target_mac[2] = 0x5e;
+        target_mac[3] = 0;
+        target_mac[4] = 1;
+        target_mac[5] = (u_char)vhid;
+        return;
+    } else if (sscanf(addr, "%2x:%2x:%2x:%2x:%2x:%2x%n", &m0, &m1, &m2, &m3, &m4, &m5, &len) < 6
+        || addr[len]) {
         fprintf(stderr, "invalid MAC address: %s", addr);
         return(-1);
     }
@@ -236,12 +252,22 @@ int
 atoip(char *buf, u_int32_t *ip_addr){
     unsigned i0, i1, i2, i3;
     unsigned long hex_addr;
-
-    if (sscanf(buf, "%u.%u.%u.%u", &i0, &i1, &i2, &i3) == 4){
+    int len = 0;
+
+    if (sscanf(buf, "%u.%u.%u.%u%n", &i0, &i1, &i2, &i3, &len) >= 4
+        && i0 <= 0xff
+        && i1 <= 0xff
+        && i2 <= 0xff
+        && i3 <= 0xff
+        && !buf[len]
+    ) {
         *ip_addr = (i0 << 24) + (i1 << 16) + (i2 << 8) + i3;
         return 0;
     }
-    if (sscanf(buf, "0x%lx", &hex_addr) >= 1) {
+    if (sscanf(buf, "0x%lx%n", &hex_addr, &len) >= 1
+        && hex_addr <= 0xffffffff
+        && !buf[len]
+    ) {
         *ip_addr = (u_int32_t)hex_addr;
         return 0;
     }
@@ -354,6 +380,10 @@ main(int argc, char **argv){
                     "and (%s)"
 
 #define EXCL_FILTER TMPL_FILTER " and not (%s)"
+    if (targets_filter == NULL) {
+        fprintf(stderr, "at least one address range must be an affirmative match\n");
+        exit(1);
+    }
     if (excludes_filter == NULL)
         asprintf (&filter, TMPL_FILTER, targets_filter);
     else

From c8ce904fc133e466ec0c1a7995c6d07afa7c861c Mon Sep 17 00:00:00 2001
From: Aaron Hope <aaron.hope@xemed.com>
Date: Mon, 13 Mar 2017 16:44:03 -0400
Subject: [PATCH 06/13] 06-error-handling

Additional error checking and reporting

Treat and report pidfile creation as a fatal error.

Report fatal pcap errors.

Check for memory allocation failures.

Distinguish general runtime errors from invalid user input using exit codes 1 and 2
respectively, in accordance with the LSB init script conventions.
---
 src/choparp.c | 120 +++++++++++++++++++++++++++++++-------------------
 1 file changed, 75 insertions(+), 45 deletions(-)

diff --git a/src/choparp.c b/src/choparp.c
index d1a8c23..f39d0ed 100644
--- a/src/choparp.c
+++ b/src/choparp.c
@@ -52,6 +52,7 @@
 #include <fcntl.h>
 #include <unistd.h>
 #include <sys/ioctl.h>
+#include <errno.h>
 
 #ifndef __linux__
 #include <ifaddrs.h>
@@ -154,13 +155,36 @@ gen_arpreply(u_char *buf) {
 }
 
 void
-cleanup(int sig){
-     if (pidfile != NULL)
-         unlink(pidfile);
+cleanup_pidfile() {
+    if (pidfile != NULL)
+        unlink(pidfile);
+}
 
-     pcap_breakloop(pc);
-     pcap_close(pc);
-     exit(0);
+void
+breakloop_pc(int sig) {
+    pcap_breakloop(pc);
+}
+
+void
+setup_pidfile() {
+    int fd;
+    FILE *fp;
+    if (pidfile == NULL)
+        return;
+
+    /* errno may or may not be set to something useful */
+    errno = 0;
+    if (
+        (fd = open(pidfile, O_WRONLY | O_CREAT | O_SYNC, 0600)) < 0
+        || atexit(cleanup_pidfile) < 0
+        || ftruncate(fd, 0) < 0
+        || (fp = fdopen(fd, "w")) == NULL
+        || fprintf(fp, "%ld\n", (long)getpid()) < 0
+        || fclose(fp) == EOF
+    ) {
+        perror("failed to create pidfile");
+        exit(1);
+    }
 }
 
 void
@@ -169,7 +193,7 @@ process_arp(u_char *user, const struct pcap_pkthdr *pkthdr, const u_char *packet
     pcap_inject((pcap_t *)user, packet, pkthdr->len);
 }
 
-int
+void
 setmac(char *addr, char *ifname){
     int len = 0;
     unsigned m0, m1, m2, m3, m4, m5;
@@ -187,19 +211,23 @@ setmac(char *addr, char *ifname){
 
         if ((fd = socket(PF_INET, SOCK_STREAM, 0)) == -1) {
             perror("socket");
-            return -1;
+            exit(1);
         }
 
         if (ioctl(fd, SIOCGIFHWADDR, &ifr) < 0) {
             perror("ioctl(SIOCGIFHWADDR)");
-            return -1;
+            exit(1);
         }
         memcpy(target_mac, ifr.ifr_hwaddr.sa_data, ETHER_ADDR_LEN);
-        return 0;
+        close(fd);
+        return;
 #else
         struct ifaddrs *ifas, *ifa;
 
-        getifaddrs (&ifas);
+        if(getifaddrs(&ifas) < 0) {
+            perror("getifaddrs");
+            exit(1);
+        }
         for (ifa = ifas; ifa != NULL; ifa = ifa->ifa_next) {
 #define SDL ((struct sockaddr_dl *)ifa->ifa_addr)
             if (strcmp (ifa->ifa_name, ifname)
@@ -207,13 +235,14 @@ setmac(char *addr, char *ifname){
               || SDL->sdl_alen != ETHER_ADDR_LEN)
                 continue;
             memcpy (target_mac, SDL->sdl_data + SDL->sdl_nlen, ETHER_ADDR_LEN);
-            return 0;
+            freeifaddrs(ifas);
+            return;
         }
-#endif
         fprintf(stderr, "%s: not found\n", ifname);
-        return -1;
-
-    } else if (sscanf(addr, "vhid:%n", &len) >= 0 && len) {
+        exit(2);
+#endif
+    }
+    if (sscanf(addr, "vhid:%n", &len) >= 0 && len) {
         /*
          * Virtual router mac address
          * CARP address format: 00:00:5e:00:01:<VHID>
@@ -225,7 +254,7 @@ setmac(char *addr, char *ifname){
             || sscanf(addr, "vhid:%u%n", &vhid, &len) > 0;
         if (!matched || addr[len] || vhid > 0xff) {
             fprintf(stderr, "invalid vhid spec: %s\n", addr);
-            exit(-1);
+            exit(2);
         }
         target_mac[0] = 0;
         target_mac[1] = 0;
@@ -234,10 +263,11 @@ setmac(char *addr, char *ifname){
         target_mac[4] = 1;
         target_mac[5] = (u_char)vhid;
         return;
-    } else if (sscanf(addr, "%2x:%2x:%2x:%2x:%2x:%2x%n", &m0, &m1, &m2, &m3, &m4, &m5, &len) < 6
+    }
+    if (sscanf(addr, "%2x:%2x:%2x:%2x:%2x:%2x%n", &m0, &m1, &m2, &m3, &m4, &m5, &len) < 6
         || addr[len]) {
         fprintf(stderr, "invalid MAC address: %s", addr);
-        return(-1);
+        exit(2);
     }
     target_mac[0] = (u_char )m0;
     target_mac[1] = (u_char )m1;
@@ -245,7 +275,6 @@ setmac(char *addr, char *ifname){
     target_mac[3] = (u_char )m3;
     target_mac[4] = (u_char )m4;
     target_mac[5] = (u_char )m5;
-    return(0);
 }
 
 int
@@ -278,12 +307,12 @@ atoip(char *buf, u_int32_t *ip_addr){
 void
 usage(void){
     fprintf(stderr,"usage: choparp [-p PIDFILE] if_name mac_addr [-]addr/mask...\n");
-    exit(-1);
+    exit(2);
 }
 
 int
 main(int argc, char **argv){
-    int pidf, opt;
+    int opt;
     char *ifname;
     char *filter, *targets_filter, *excludes_filter;
     struct cidr **targets_tail = &targets, **excludes_tail = &excludes;
@@ -320,9 +349,7 @@ main(int argc, char **argv){
 	usage();
 
     ifname = argv[0];
-    if (setmac(argv[1], ifname)) {
-        exit(1);
-    }
+    setmac(argv[1], ifname);
     argv += 2; argc -= 2;
 
     while (argc > 0) {
@@ -384,31 +411,34 @@ main(int argc, char **argv){
         fprintf(stderr, "at least one address range must be an affirmative match\n");
         exit(1);
     }
-    if (excludes_filter == NULL)
-        asprintf (&filter, TMPL_FILTER, targets_filter);
-    else
-        asprintf (&filter, EXCL_FILTER, targets_filter, excludes_filter);
+    if (excludes_filter == NULL
+        ? asprintf(&filter, TMPL_FILTER, targets_filter) < 0
+        : asprintf(&filter, EXCL_FILTER, targets_filter, excludes_filter) < 0
+    ) {
+        perror("asprintf");
+        exit(1);
+    }
+    free(targets_filter);
+    free(excludes_filter);
 
 #ifdef DEBUG
         fprintf(stderr, "Filter on %s: %s\n", ifname, filter);
 #endif
-    if ((pc = open_pcap(ifname, filter)) < 0)
-	exit(1);
+
+    pc = open_pcap(ifname, filter);
     free(filter);
 
-    if (pidfile != NULL) {
-        pidf = open(pidfile, O_RDWR | O_CREAT | O_FSYNC, 0600);
-        if (pidf > 0) {
-            ftruncate(pidf, 0);
-            dprintf(pidf, "%u\n", getpid());
-            close(pidf);
-            memset(&act, 0, sizeof(act));
-            act.sa_handler = cleanup;
-            sigaction(SIGINT, &act, NULL);
-            sigaction(SIGTERM, &act, NULL);
-        }
-    }
+    setup_pidfile();
+
+    memset(&act, 0, sizeof(act));
+    act.sa_handler = breakloop_pc;
+    sigaction(SIGINT, &act, NULL);
+    sigaction(SIGTERM, &act, NULL);
 
-    pcap_loop(pc, 0, process_arp, (u_char*)pc);
-    exit(1);
+    if (pcap_loop(pc, 0, process_arp, (u_char*)pc) == -1) {
+        pcap_perror(pc, "pcap_loop");
+        exit(1);
+    }
+    pcap_close(pc);
+    exit(0);
 }

From 1b9002d0e7bd9fe3b373f7026dde842badc3ebfc Mon Sep 17 00:00:00 2001
From: Aaron Hope <aaron.hope@xemed.com>
Date: Mon, 13 Mar 2017 16:44:03 -0400
Subject: [PATCH 07/13] 07-getopt-parsing

Fix getopt(3) usage errors

Since IP addresses/ranges are excluded with the same character as standard
options (dash, '-'), GNU getopt(3) will erroneously interpret these as invalid
options, unless the user sets the POSIXLY_CORRECT environment variable.
Specifying a '+' prefix ensures that these will be treated as regular
arguments.

With POSIX getopt, option arguments may or may not be in separate argv[] elements.
If the user bundles the option argument, such as -p/run/choparp.pid, the option
parsing code would skip over the first non-option argument.
---
 src/choparp.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/src/choparp.c b/src/choparp.c
index f39d0ed..64971ff 100644
--- a/src/choparp.c
+++ b/src/choparp.c
@@ -327,7 +327,7 @@ main(int argc, char **argv){
 	(LIST ## _tail) = &(*(LIST ## _tail))->next;		\
     } while (0)
 
-    while ((opt = getopt(argc, argv, "p:")) != -1) {
+    while ((opt = getopt(argc, argv, "+p:")) != -1) {
         switch (opt) {
         case 'p':
             pidfile = optarg;
@@ -337,13 +337,8 @@ main(int argc, char **argv){
         }
     }
 
-    if (pidfile == NULL) {
-        argv++;
-        argc--;
-    } else {
-        argv += 3;
-        argc -= 3;
-    }
+    argv += optind;
+    argc -= optind;
 
     if (argc < 3)
 	usage();

From ea4f5caf04332bb1005353815e60c3940559aaee Mon Sep 17 00:00:00 2001
From: Aaron Hope <aaron.hope@xemed.com>
Date: Mon, 13 Mar 2017 16:44:03 -0400
Subject: [PATCH 08/13] 08-update-manual

Update manual page choparp(8)

Document new pidfile feature.

Fix misleading vhid: number format mismatch.

Replace IP and MAC addresses under assigned prefixes with RFC5737 "TEST-NET"
and RFC7042 reserved values for documentation.

General attempts at clarification.
---
 src/choparp.8 | 97 +++++++++++++++++++++++++++++++--------------------
 1 file changed, 60 insertions(+), 37 deletions(-)

diff --git a/src/choparp.8 b/src/choparp.8
index 3921ceb..f520ffd 100644
--- a/src/choparp.8
+++ b/src/choparp.8
@@ -35,13 +35,14 @@
 .Nd cheap and omitted proxy ARP
 .Sh SYNOPSIS
 .Nm chpoarp
+.Op Fl p Ar file
 .Ar if_name mac_addr
 .Oo Fl Oc Ns Ar net_addr Ns
 .Op / Ns Ar net_mask
 .Ar ...
 .Sh DESCRIPTION
 .Pp
-.Nm choparp 
+.Nm choparp
 is a easy-to-use proxy ARP daemon.
 It watches ARP request packets visible on the interface specified by argument
 .Ar if_name ,
@@ -49,61 +50,83 @@ and sends proxy ARP reply to the sender if the ARP request queries the
 MAC address
 .Pq ethernet hardware address
 for the network specified by
-.Ar net_addr Ns / Ar net_mask .
+.Ar net_addr Ns / Ns Ar net_mask .
 .Pp
+.Bl -tag -width indent
+.It Fl p Ar file
+Save process id to the given file.
+.It Ar if_name
+Network interface on which to listen and respond to ARP requests.
+.It Ar mac_addr
+The MAC address to be published for the specified hosts, in one of three formats.
+.Bl -tag -width indent
+.It Ar xx:xx:xx:xx:xx:xx
+Static value specified as 6 colon-separated byte values in hexadecimal.
+.It Ar vhid: Ns Oo <nnn>|0x<xx> Oc
+Virtual router (VRRP/CARP) address with the final octet (VRID)
+given by decimal value <nnn> or hexadecimal value <xx> .
+.It Ar auto
+Usually,
 .Ar mac_addr
-is th MAC address to be published for the specified hosts.
-It is normally the address of
+is the address of
 .Ar if_name .
-The format of
-.Ar mac_addr
-must be 6 colon-separated bytes of hexadecimal value, such as
-.Ad 00:00:01:01:14:46 .
-The keyword
-.Ad auto
-can also be used to use the address of
+Specify the keyword
+.Sy auto
+to use the address of
 .Ar if_name
-from the system configuration.
-.Pp
-A
-.Xr carp 4
-virtual router address can be specified as
-.Ad vhid:<hex>
+detected from the system configuration.
+.El
 .
-.Pp
-.Ar net_addr
-must be in dotted quad notation
-.Pq for example Ad 133.138.1.134
-or be a 32 bit hexadecimal value starting with
+.It Ar net_addr
+IPv4 address in either dotted quad notation
+.Pq for example Ar 192.0.2.2
+or, BSD-style, as a 32 bit hexadecimal value starting with
 .Dq 0x
-.Pq for example Ad 0x858a0186 .
+.Pq for example Ar 0x858a0186 .
+.It Ar net_mask
+Alone,
+.Ar net_addr
+specifies a single host; to specify an IP range, include a
 .Ar net_mask
-can likewise be speficied as a dotted quad or hexadecimal value,
-or alternatively as a mask length. The following address specifications
-are therefore equivalent:
+in the same way one would specify a subnet.
+Full 32-bit values may be in either dotted-decimal and BSD-style hexadecimal format.
+Prefix lengths may be specified in CIDR notation.
+The following address specifications are equivalent:
 .Bl -item -offset indent
 .It
-.Ad 192.168.98.0/255.255.254.0
+.Ar 192.0.2.0/255.255.254.0
 .It
-.Ad 192.168.98.0/0xfffffe00
+.Ar 0xc0000200/0xfffffe00
 .It
-.Ad 192.168.98.0/23
+.Ar 192.0.2.0/0xfffffe00
+.It
+.Ar 192.0.2.0/23
 .El
 .Pp
-Multiple addresses can be specified.
-Addresses can be
-.Em excluded
-by preceding them with
-.Fl
+Address matches may be either affirmative or negative, with affirmative as the default.
+To exclude an address or range, specify a negative match with a prefix
+.Dq -
+(dash) prefix.
+Multiple matches may be specified, with at least one being affirmative.
+.El
 .Sh EXAMPLES
 If you have network interface
 .Dq ne0
 with MAC address
-.Dq 00:00:01:01:14:16 ,
-and would like to send proxy ARP reply for 192.168.0.64/26,
+.Dq 00:00:5e:00:53:01
+, and would like to send proxy ARP reply for 192.168.0.64/26,
 the argument would be as follows:
 .Bd -literal -offset indent
-choparp ne0 00:00:01:01:14:46 192.168.0.64/255.255.255.192
+choparp ne0 00:00:5e:00:53:01 192.168.0.64/26
+.Ed
+.Pp
+Were
+.Dq ne0
+assigned the IPv4 address 192.168.0.65/26,
+you could additionally exclude the the host address and all-ones and all-zeroes
+broadcast addresses as follows:
+.Bd -literal -offset indent
+choparp ne0 00:00:5e:00:53:01 192.168.0.64/26 -192.168.0.64/31 -192.168.0.127
 .Ed
 .Sh BUGS
 Supports Ethernet interfaces only.

From c7caa1849d6faf6d795dbb4014e638e2b9105166 Mon Sep 17 00:00:00 2001
From: Aaron Hope <aaron.hope@xemed.com>
Date: Mon, 13 Mar 2017 16:44:04 -0400
Subject: [PATCH 09/13] 09-runtime-diagnostics

Make compile-time diagnostics available at run time.

Uses a conventional accumulating "-v" option.

Add a conventional "-h" option to print usage.
---
 src/choparp.8 |  3 +++
 src/choparp.c | 21 ++++++++++++---------
 2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/src/choparp.8 b/src/choparp.8
index f520ffd..1df1c9e 100644
--- a/src/choparp.8
+++ b/src/choparp.8
@@ -36,6 +36,7 @@
 .Sh SYNOPSIS
 .Nm chpoarp
 .Op Fl p Ar file
+.Op Fl v
 .Ar if_name mac_addr
 .Oo Fl Oc Ns Ar net_addr Ns
 .Op / Ns Ar net_mask
@@ -55,6 +56,8 @@ for the network specified by
 .Bl -tag -width indent
 .It Fl p Ar file
 Save process id to the given file.
+.It Fl v
+Send diagnostic messages to stdout. Repeat to increase verbosity.
 .It Ar if_name
 Network interface on which to listen and respond to ARP requests.
 .It Ar mac_addr
diff --git a/src/choparp.c b/src/choparp.c
index 64971ff..3d81283 100644
--- a/src/choparp.c
+++ b/src/choparp.c
@@ -74,6 +74,7 @@ char errbuf[PCAP_ERRBUF_SIZE];
 u_char target_mac[ETHER_ADDR_LEN];	/* target MAC address */
 
 static char *pidfile = NULL;
+static int verbose = 0;
 static pcap_t *pc;
 
 char* cidr_to_str(struct cidr *a) {
@@ -327,11 +328,15 @@ main(int argc, char **argv){
 	(LIST ## _tail) = &(*(LIST ## _tail))->next;		\
     } while (0)
 
-    while ((opt = getopt(argc, argv, "+p:")) != -1) {
+    while ((opt = getopt(argc, argv, "+p:vh")) != -1) {
         switch (opt) {
         case 'p':
             pidfile = optarg;
             break;
+        case 'v':
+            ++verbose;
+            break;
+        case 'h':
         case '?':
             usage();
         }
@@ -376,7 +381,6 @@ main(int argc, char **argv){
 	argv++, argc--;
     }
 
-#ifdef DEBUG
 #define SHOW(LIST) \
     do {							\
 	struct cidr *t;						\
@@ -387,10 +391,10 @@ main(int argc, char **argv){
 	}							\
     } while (0)
 
-    SHOW(targets);
-    SHOW(excludes);
-    exit (0);
-#endif
+    if (verbose >= 3) {
+        SHOW(targets);
+        SHOW(excludes);
+    }
 
     targets_filter = cidr_to_str(targets);
     excludes_filter = cidr_to_str(excludes);
@@ -416,9 +420,8 @@ main(int argc, char **argv){
     free(targets_filter);
     free(excludes_filter);
 
-#ifdef DEBUG
-        fprintf(stderr, "Filter on %s: %s\n", ifname, filter);
-#endif
+    if (verbose)
+        printf("Filter on %s: %s\n", ifname, filter);
 
     pc = open_pcap(ifname, filter);
     free(filter);

From f25efcfa8d2627c3cb2a1447f2d0dd4d9b7cea10 Mon Sep 17 00:00:00 2001
From: Aaron Hope <ahope@xemed.com>
Date: Thu, 16 Mar 2017 16:56:39 -0400
Subject: [PATCH 10/13] Improve option parsing robustness

OpenBSD getopt can return ':' in cases where other implementations
would return '?'. Add a default case as a catch-all to avoid
undefined behavior (i.e. bugs).
---
 src/choparp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/choparp.c b/src/choparp.c
index 3d81283..43e8078 100644
--- a/src/choparp.c
+++ b/src/choparp.c
@@ -337,7 +337,7 @@ main(int argc, char **argv){
             ++verbose;
             break;
         case 'h':
-        case '?':
+        default:
             usage();
         }
     }

From a2822f27d16ba608ad2dce01ba7a6f2ac523f63b Mon Sep 17 00:00:00 2001
From: Aaron Hope <ahope@xemed.com>
Date: Sat, 25 Mar 2017 12:49:20 -0400
Subject: [PATCH 11/13] Unit test fixes

Run test script using shell selected by autoconf instead of /bin/sh
Fix recursion check
---
 Makefile.am        |  3 ++-
 test/linux-veth.sh | 16 ++++++++--------
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 675c999..dd4ae15 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -4,7 +4,8 @@ man_MANS = src/choparp.8
 
 AM_TESTS_ENVIRONMENT = \
 	choparp_abspath='$(abs_top_builddir)'/choparp; \
-	export choparp_abspath;
+	sh_test_shell='$(SHELL)' \
+	export choparp_abspath sh_test_shell;
 
 LOG_DRIVER = \
 	env AM_TAP_AWK='$(AWK)' $(SHELL) $(top_srcdir)/build-aux/tap-driver.sh
diff --git a/test/linux-veth.sh b/test/linux-veth.sh
index 2b5285c..05987b7 100644
--- a/test/linux-veth.sh
+++ b/test/linux-veth.sh
@@ -8,17 +8,18 @@ fi
 
 CHOPARP_NETNS="$(ls -ld /proc/$$/ns/net)"
 CHOPARP_NETNS="${CHOPARP_NETNS#*->}"
+STAGE="${1-stage1}"
 
-if [ -z "$TAP_RUNNER_NETNS" ]
+if [ "${TAP_RUNNER_NETNS+set}" != "set" ] && [ "$STAGE" = "stage1" ]
 then
 	export LC_CTYPE=C
 	export TAP_RUNNER_NETNS="$CHOPARP_NETNS"
+	# stash and restore stdout, pass pipeline as file descriptor 5
 	exec 4>&1
-	GOT_NETNS=$(unshare --user --map-root-user --net /bin/sh $0 5>&1 1>&4)
-	ev=$?
-	if [ "$GOT_NETNS" != "GOT_NETNS" ]
+	STAGE2_STATUS=$(unshare --user --map-root-user --net "$sh_test_shell" "$0" stage2 5>&1 1>&4)
+	if [ "$STAGE2_STATUS" != "STAGE2_REACHED" ]
 	then
-		echo "1..0 # Skipped: unable to create private network namespace (need root?)"
+		echo "1..0 # Skipped: unable to create private user/network namespace (need root?)"
 	fi
 	exit
 fi
@@ -28,7 +29,8 @@ then
 	exit 1
 fi
 
-echo GOT_NETNS >&5
+echo "1..9"
+echo STAGE2_REACHED >&5
 
 rnd_byte="$(dd if=/dev/urandom bs=1 count=1 2>/dev/null | od -A n -t d)"
 
@@ -59,8 +61,6 @@ found() {
 		grep -i -q "$(ipaddr $1) lladdr ${2:-.*} REACHABLE"
 }
 
-echo "1..9"
-
 set -x
 
 ip link add is-at type veth peer name who-has

From 18719dd5534ae2894c958da6f774bb94a5281b7a Mon Sep 17 00:00:00 2001
From: Aaron Hope <ahope@xemed.com>
Date: Sun, 21 May 2017 23:12:45 -0400
Subject: [PATCH 12/13] Split out linux sandbox setup from test script

Test diagnostics improved.

Split linux namespace sanbox setup into distinct stages each in separate files.
Multiple logical stages are required and coercing them into one souce file
required obscure shell features and compromised comprehension. Keep it simple
instead.
---
 Makefile.am                     |  11 ++--
 test/linux-ns-unshare-stage2.sh |  16 +++++
 test/linux-ns-unshare.sh        |  40 ++++++++++++
 test/linux-veth.sh              | 108 +++++++++++++++-----------------
 4 files changed, 113 insertions(+), 62 deletions(-)
 create mode 100644 test/linux-ns-unshare-stage2.sh
 create mode 100644 test/linux-ns-unshare.sh

diff --git a/Makefile.am b/Makefile.am
index dd4ae15..3ff5116 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -3,17 +3,18 @@ choparp_SOURCES = src/choparp.c
 man_MANS = src/choparp.8
 
 AM_TESTS_ENVIRONMENT = \
-	choparp_abspath='$(abs_top_builddir)'/choparp; \
-	sh_test_shell='$(SHELL)' \
-	export choparp_abspath sh_test_shell;
+	AM_BUILDDIR='$(abs_top_builddir)'; \
+	AM_SRCDIR='$(abs_top_srcdir)'; \
+	AM_SHELL='$(SHELL)'; \
+	export AM_BUILDDIR AM_SRCDIR AM_SHELL ;
 
 LOG_DRIVER = \
-	env AM_TAP_AWK='$(AWK)' $(SHELL) $(top_srcdir)/build-aux/tap-driver.sh
+	env AM_TAP_AWK='$(AWK)' $(SHELL) $(top_srcdir)/build-aux/tap-driver.sh --comments
 
 TEST_EXTENSIONS = .sh
 
 SH_LOG_DRIVER = $(LOG_DRIVER)
 SH_LOG_COMPILER = $(SHELL)
 
-TESTS = test/linux-veth.sh
+TESTS = test/linux-ns-unshare.sh
 EXTRA_DIST = $(TESTS)
diff --git a/test/linux-ns-unshare-stage2.sh b/test/linux-ns-unshare-stage2.sh
new file mode 100644
index 0000000..aff7a95
--- /dev/null
+++ b/test/linux-ns-unshare-stage2.sh
@@ -0,0 +1,16 @@
+#!/bin/sh
+
+#
+# Verify the sandbox created in stage1 and proceed with functional tests.
+#
+
+STAGE2_NETNS=$(readlink /proc/$$/ns/net | grep -E '^net:\[[0-9]+\]')
+
+if [ -z "$STAGE1_NETNS" ] || -z [ "$STAGE2_NETNS" ] ||
+	[ "$STAGE1_NETNS" '=' "$STAGE2_NETNS" ]
+then
+	echo "Bail out! Failed to confirm sandbox"
+	exit
+fi
+
+. "$AM_SRCDIR/test/linux-veth.sh"
diff --git a/test/linux-ns-unshare.sh b/test/linux-ns-unshare.sh
new file mode 100644
index 0000000..2a5b1ba
--- /dev/null
+++ b/test/linux-ns-unshare.sh
@@ -0,0 +1,40 @@
+#!/bin/sh
+
+# Meaningful tests require the freedom to break things.  Linux namespaces can
+# give us a disposable sandbox without the need for persistent changes and
+# end-user configuration.  If the host allows unprivileged userns, tests can even
+# run without root.
+
+if [ -z "$AM_BUILDDIR" ]
+then
+	echo "1..0 # Skipped: missing \$AM_BUILDDIR"
+	echo "# Did you run me under 'make check'?"
+	exit
+fi
+
+if ! ((unshare --help | grep map-root-user) && ip link show) > /dev/null
+then
+	echo "1..0 # Skipped: required linux utilities seem to be missing"
+	echo "# Test requires util-linux >= 2.25 and iproute2"
+	exit
+fi
+
+if ! unshare --user --map-root-user --net true
+then
+	echo "1..0 # Skipped: unable to create private user/network namespace"
+	echo "# Test requires root or kernel.unprivileged_userns_clone=1"
+	exit
+fi
+
+export LC_CTYPE=C
+
+STAGE1_NETNS=$(readlink /proc/$$/ns/net | grep -E '^net:\[[0-9]+\]$')
+export STAGE1_NETNS
+
+STAGE2="$AM_SRCDIR/test/linux-ns-unshare-stage2.sh"
+
+if ! unshare --user --map-root-user --net "$AM_SHELL" "$STAGE2"
+then
+	echo "Bail out! Failure in second stage"
+	exit
+fi
diff --git a/test/linux-veth.sh b/test/linux-veth.sh
index 05987b7..f9146f0 100644
--- a/test/linux-veth.sh
+++ b/test/linux-veth.sh
@@ -1,37 +1,34 @@
 #!/bin/sh
 
-if [ "x$choparp_abspath" = x ]
-then
-	echo "1..0 # Skipped: missing \$choparp_abspath (did you run me as 'make check'?) "
-	exit
-fi
+#
+# Functional tests. Do NOT run this on a production system or any environment
+# not devoted to testing. This script is normally run from linux-ns-unshare.sh
+# which configures a sandbox. Docker, GitLab, autopkgtest, etc are fine too.
+# Just don't run this script casually.
+#
 
-CHOPARP_NETNS="$(ls -ld /proc/$$/ns/net)"
-CHOPARP_NETNS="${CHOPARP_NETNS#*->}"
-STAGE="${1-stage1}"
+set -x
 
-if [ "${TAP_RUNNER_NETNS+set}" != "set" ] && [ "$STAGE" = "stage1" ]
+if [ -z "$AM_BUILDDIR" ]
 then
-	export LC_CTYPE=C
-	export TAP_RUNNER_NETNS="$CHOPARP_NETNS"
-	# stash and restore stdout, pass pipeline as file descriptor 5
-	exec 4>&1
-	STAGE2_STATUS=$(unshare --user --map-root-user --net "$sh_test_shell" "$0" stage2 5>&1 1>&4)
-	if [ "$STAGE2_STATUS" != "STAGE2_REACHED" ]
-	then
-		echo "1..0 # Skipped: unable to create private user/network namespace (need root?)"
-	fi
+	echo "1..0 # Skipped: missing \$AM_BUILDDIR"
+	echo "# Did you run me under 'make check'? "
 	exit
 fi
-if [ "$TAP_RUNNER_NETNS" = "$CHOPARP_NETNS" ]
+
+if ! ip link add is-at type veth peer name who-has ||
+	! ip addr add 192.168.1.200/24 dev who-has     ||
+	! ip link set is-at up                         ||
+	! ip link set who-has up
 then
-	# sanity check, should never occur
-	exit 1
+	echo "1..0 # Skipped: unable to set up private veth interfaces"
+	exit
 fi
 
-echo "1..9"
-echo STAGE2_REACHED >&5
+# Allow this many seconds for choparp start-up before sending test arp requests
+startup_grace=1
 
+# Any arbitrary value should do, but might as well switch it up between runs
 rnd_byte="$(dd if=/dev/urandom bs=1 count=1 2>/dev/null | od -A n -t d)"
 
 lladdr() {
@@ -61,26 +58,23 @@ found() {
 		grep -i -q "$(ipaddr $1) lladdr ${2:-.*} REACHABLE"
 }
 
-set -x
+#######################################################################
 
-ip link add is-at type veth peer name who-has
-ip addr add 192.168.1.200/24 dev who-has
-ip link set is-at up
-ip link set who-has up
+echo "1..9"
 
 #######################################################################
 
 test_desc="1 - Base case, static hardware address and single-host"
-"$choparp_abspath" is-at $(lladdr 1) $(ipaddr 1) &
+"$AM_BUILDDIR"/choparp is-at $(lladdr 1) $(ipaddr 1) &
 chopid=$!
-sleep 1
+sleep $startup_grace
 
 arp_for 1
 kill $chopid
 
 if ! wait $chopid
 then
-	echo "not ok $test_desc # abnormal exit"
+	echo "not ok $test_desc # abnormal exit $?"
 elif ! found 1
 then
 	echo "not ok $test_desc # MAC resolution failure"
@@ -91,16 +85,16 @@ fi
 #######################################################################
 
 test_desc="2 - Target IP by legacy subnet"
-"$choparp_abspath" is-at $(lladdr 2) $(ipaddr 2)/255.255.255.254 &
+"$AM_BUILDDIR"/choparp is-at $(lladdr 2) $(ipaddr 2)/255.255.255.254 &
 chopid=$!
-sleep 1
+sleep $startup_grace
 
 arp_for 2 3
 kill $chopid
 
 if ! wait $chopid
 then
-	echo "not ok $test_desc # abnormal exit"
+	echo "not ok $test_desc # abnormal exit $?"
 elif ! (found 2 && found 3)
 then
 	echo "not ok $test_desc # MAC resolution failure"
@@ -111,9 +105,9 @@ fi
 #######################################################################
 
 test_desc="3 - Target IP by BSD-style hex"
-"$choparp_abspath" is-at $(lladdr 4) $(hex_ipaddr 4)/0xfffffffe &
+"$AM_BUILDDIR"/choparp is-at $(lladdr 4) $(hex_ipaddr 4)/0xfffffffe &
 chopid=$!
-sleep 1
+sleep $startup_grace
 
 arp_for 4
 arp_for 5
@@ -121,7 +115,7 @@ kill $chopid
 
 if ! wait $chopid
 then
-	echo "not ok $test_desc # abnormal exit"
+	echo "not ok $test_desc # abnormal exit $?"
 elif ! (found 4 && found 5)
 then
 	echo "not ok $test_desc # MAC resolution failure"
@@ -132,16 +126,16 @@ fi
 #######################################################################
 
 test_desc="4 - Target IP by IP list"
-"$choparp_abspath" is-at $(lladdr 6) $(ipaddr 6) $(ipaddr 7) &
+"$AM_BUILDDIR"/choparp is-at $(lladdr 6) $(ipaddr 6) $(ipaddr 7) &
 chopid=$!
-sleep 1
+sleep $startup_grace
 
 arp_for 6 7
 kill $chopid
 
 if ! wait $chopid
 then
-	echo "not ok $test_desc # abnormal exit"
+	echo "not ok $test_desc # abnormal exit $?"
 elif ! (found 6 && found 7)
 then
 	echo "not ok $test_desc # MAC resolution failure"
@@ -152,16 +146,16 @@ fi
 #######################################################################
 
 test_desc="5 - Target IP by CIDR subnet and exclusion"
-"$choparp_abspath" is-at $(lladdr 8) $(ipaddr 8)/30 -$(ipaddr 10) &
+"$AM_BUILDDIR"/choparp is-at $(lladdr 8) $(ipaddr 8)/30 -$(ipaddr 10) &
 chopid=$!
-sleep 1
+sleep $startup_grace
 
 arp_for 8 9 10 11
 kill $chopid
 
 if ! wait $chopid
 then
-	echo "not ok $test_desc # abnormal exit"
+	echo "not ok $test_desc # abnormal exit $?"
 elif ! (found 8 && found 9 && ! found 10 && found 11)
 then
 	echo "not ok $test_desc # MAC resolution failure"
@@ -172,16 +166,16 @@ fi
 #######################################################################
 
 test_desc="6 - Hardware address detection by \"auto\" keyword"
-"$choparp_abspath" is-at auto $(ipaddr 12) &
+"$AM_BUILDDIR"/choparp is-at auto $(ipaddr 12) &
 chopid=$!
-sleep 1
+sleep $startup_grace
 
 arp_for 12
 kill $chopid
 
 if ! wait $chopid
 then
-	echo "not ok $test_desc # abnormal exit"
+	echo "not ok $test_desc # abnormal exit $?"
 elif ! found 12
 then
 	echo "not ok $test_desc # MAC resolution failure"
@@ -192,9 +186,9 @@ fi
 #######################################################################
 
 test_desc="7 - Hardware address by \"vhid\" keyword, decimal"
-"$choparp_abspath" is-at vhid:13 $(ipaddr 13) &
+"$AM_BUILDDIR"/choparp is-at vhid:13 $(ipaddr 13) &
 chopid=$!
-sleep 1
+sleep $startup_grace
 
 arp_for 13
 kill $chopid
@@ -203,7 +197,7 @@ ip neigh show dev who-has
 
 if ! wait $chopid
 then
-	echo "not ok $test_desc # abnormal exit"
+	echo "not ok $test_desc # abnormal exit $?"
 elif ! found 13 00:00:5e:00:01:0d
 then
 	echo "not ok $test_desc # MAC resolution failure"
@@ -214,9 +208,9 @@ fi
 #######################################################################
 
 test_desc="8 - Hardware address by \"vhid\" keyword, hex"
-"$choparp_abspath" is-at vhid:0x0e $(ipaddr 14) &
+"$AM_BUILDDIR"/choparp is-at vhid:0x0e $(ipaddr 14) &
 chopid=$!
-sleep 1
+sleep $startup_grace
 
 arp_for 14
 kill $chopid
@@ -225,7 +219,7 @@ ip neigh show dev who-has
 
 if ! wait $chopid
 then
-	echo "not ok $test_desc # abnormal exit"
+	echo "not ok $test_desc # abnormal exit $?"
 elif ! found 14 00:00:5e:00:01:0e
 then
 	echo "not ok $test_desc # MAC resolution failure"
@@ -237,21 +231,21 @@ fi
 
 test_desc="9 - Pidfile with -p"
 pidfile=$(mktemp /tmp/choparp.pid-XXXXXXXX)
-"$choparp_abspath" -p $pidfile is-at auto $(ipaddr 15) &
+"$AM_BUILDDIR"/choparp -p $pidfile is-at auto $(ipaddr 15) &
 chopid=$!
-sleep 1
+sleep $startup_grace
 
-chopid_from_file=$(cat $pidfile)
+chopid_from_file=$(cat "$pidfile")
 arp_for 15
 kill $chopid
 
 if ! wait $chopid
 then
-	echo "not ok $test_desc # abnormal exit"
+	echo "not ok $test_desc # abnormal exit $?"
 elif ! found 15
 then
 	echo "not ok $test_desc # MAC resolution failure"
-elif [ "$chopid" -ne "$chopid_from_file" ]
+elif ! [ "$chopid" -eq "$chopid_from_file" ]
 then
 	echo "not ok $test_desc # invalid pidfile"
 elif [ -f "$pidfile" ]

From 4a439ef303796965706b12e4ecf4a3d155b46b12 Mon Sep 17 00:00:00 2001
From: Aaron Hope <ahope@xemed.com>
Date: Mon, 22 May 2017 12:28:37 -0400
Subject: [PATCH 13/13] Build documentation update

---
 Makefile.am |  2 +-
 README.md   | 12 +++++++++++-
 m4/NOTES    |  6 ++++++
 3 files changed, 18 insertions(+), 2 deletions(-)
 create mode 100644 m4/NOTES

diff --git a/Makefile.am b/Makefile.am
index 3ff5116..a2c6771 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -17,4 +17,4 @@ SH_LOG_DRIVER = $(LOG_DRIVER)
 SH_LOG_COMPILER = $(SHELL)
 
 TESTS = test/linux-ns-unshare.sh
-EXTRA_DIST = $(TESTS)
+EXTRA_DIST = $(TESTS) m4/NOTES
diff --git a/README.md b/README.md
index 05b3677..85f9b6a 100644
--- a/README.md
+++ b/README.md
@@ -23,7 +23,17 @@ list.
 Build instructions
 ------------------
 
-Requires libpcap.
+Requires libpcap, autoconf, and automake.
+
+```
+autoreconf -is
+./configure
+make
+make check
+sudo make install
+```
+
+If autotools displease you, it is still possible to build directly.
 
 `gcc -o choparp choparp.c -lpcap`
 
diff --git a/m4/NOTES b/m4/NOTES
new file mode 100644
index 0000000..60f99fe
--- /dev/null
+++ b/m4/NOTES
@@ -0,0 +1,6 @@
+This directory was added to conform with modern autotools conventions,
+superseding acinclude.m4. Additional local Autoconf macros would
+reside here.
+
+This NOTES file exists to add the directory to git and effectively
+suppresses a spurious warning message.