sts-sponsors team mailing list archive

[Anton Troyanov <mp+442457@xxxxxxxxxxxxxxxxxx>]

Anton Troyanov has proposed merging ~troyanov/maas:use-netmon-binary into maas:master.

Commit message:
feat(netmon): replace tcpdump with maas-netmon

Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~troyanov/maas/+git/maas/+merge/442457
-- 
Your team MAAS Committers is subscribed to branch maas:master.

diff --git a/Makefile b/Makefile
index 8caf1e9..68e0262 100644
--- a/Makefile
+++ b/Makefile
@@ -202,7 +202,6 @@ lint-go-fix: lint-go
 lint-shell:
 	@shellcheck -x \
 		package-files/usr/lib/maas/beacon-monitor \
-		package-files/usr/lib/maas/network-monitor \
 		package-files/usr/lib/maas/unverified-ssh \
 		snap/hooks/* \
 		snap/local/tree/bin/* \
diff --git a/debian/extras/99-maas-common-sudoers b/debian/extras/99-maas-common-sudoers
index 412c60b..c00627c 100644
--- a/debian/extras/99-maas-common-sudoers
+++ b/debian/extras/99-maas-common-sudoers
@@ -2,7 +2,6 @@ maas ALL= NOPASSWD: /usr/bin/lshw
 maas ALL= NOPASSWD: /sbin/blockdev
 
 # MAAS network monitoring tools.
-maas ALL= NOPASSWD: /usr/lib/maas/network-monitor
 maas ALL= NOPASSWD: /usr/lib/maas/beacon-monitor
 
 # Control of the HTTP server: MAAS needs to reconfigure it after editing
diff --git a/debian/maas-common.install b/debian/maas-common.install
index 7caf604..5d6f0f5 100644
--- a/debian/maas-common.install
+++ b/debian/maas-common.install
@@ -4,7 +4,6 @@ package-files/usr/lib/maas/maas-delete-file usr/lib/maas
 package-files/usr/lib/maas/maas-write-file usr/lib/maas
 
 # Install network monitoring scripts
-package-files/usr/lib/maas/network-monitor usr/lib/maas
 package-files/usr/lib/maas/beacon-monitor usr/lib/maas
 
 # Install unverified-ssh
diff --git a/package-files/usr/lib/maas/network-monitor b/package-files/usr/lib/maas/network-monitor
deleted file mode 100755
index 2b6d362..0000000
--- a/package-files/usr/lib/maas/network-monitor
+++ /dev/null
@@ -1,21 +0,0 @@
-#!/bin/sh -euf
-# Copyright 2016 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-# Utility script to wrap `tcpdump`, so that this script can be called with
-# `sudo` without allowing MAAS access to read arbitrary network traffic.
-# This script is designed to be as minimal as possible, to prevent arbitrary
-# code execution.
-
-if [ $# -ne 1 ]; then
-    echo "Writes ARP traffic (and ARP traffic on tagged VLANs) to stdout" >&2
-    echo "using tcpdump's binary PCAP format." >&2
-    echo "" >&2
-    echo "Usage:" >&2
-    echo "    $0 <interface>" >&2
-    exit 32
-fi
-
-exec "${SNAP:-}/usr/bin/tcpdump" -Z root --interface "$1" --no-promiscuous-mode \
-    --packet-buffered --immediate-mode --snapshot-length=64 -n -w - \
-    "arp or (vlan and arp)"
diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml
index 6889cda..77cd0c3 100644
--- a/snap/snapcraft.yaml
+++ b/snap/snapcraft.yaml
@@ -248,8 +248,10 @@ parts:
       - OUT_PREFIX=maas-
     build-packages:
       - golang-go
+    organize:
+      bin/maas-netmon: usr/sbin/maas-netmon
     prime:
-      - bin/maas-netmon
+      - usr/sbin/maas-netmon
 
   tree:
     plugin: dump
diff --git a/src/provisioningserver/utils/arp.py b/src/provisioningserver/utils/arp.py
index 86172fb..e0f26dc 100644
--- a/src/provisioningserver/utils/arp.py
+++ b/src/provisioningserver/utils/arp.py
@@ -389,7 +389,7 @@ def add_arguments(parser):
         type=str,
         required=False,
         help="File to read PCAP output from. Use - for stdin. Default is to "
-        "call `sudo /usr/lib/maas/network-monitor` to get input.",
+        "call `sudo /usr/sbin/maas-netmon` to get input.",
     )
 
 
@@ -406,7 +406,7 @@ def run(
     if args.input_file is None:
         if args.interface is None:
             raise ActionScriptError("Required argument: interface")
-        cmd = [get_path("/usr/lib/maas/network-monitor"), args.interface]
+        cmd = [get_path("/usr/sbin/maas-netmon"), args.interface]
         cmd = sudo(cmd)
         network_monitor = subprocess.Popen(
             cmd, stdin=subprocess.DEVNULL, stdout=subprocess.PIPE
diff --git a/src/provisioningserver/utils/tests/test_arp.py b/src/provisioningserver/utils/tests/test_arp.py
index cf26e68..52b28a2 100644
--- a/src/provisioningserver/utils/tests/test_arp.py
+++ b/src/provisioningserver/utils/tests/test_arp.py
@@ -585,7 +585,7 @@ class TestObserveARPCommand(MAASTestCase):
         self.assertThat(
             popen,
             MockCalledOnceWith(
-                ["sudo", "-n", "/usr/lib/maas/network-monitor", "eth0"],
+                ["sudo", "-n", "/usr/sbin/maas-netmon", "eth0"],
                 stdin=subprocess.DEVNULL,
                 stdout=subprocess.PIPE,
             ),
@@ -604,7 +604,7 @@ class TestObserveARPCommand(MAASTestCase):
         self.assertThat(
             popen,
             MockCalledOnceWith(
-                ["sudo", "-n", "/usr/lib/maas/network-monitor", "eth0"],
+                ["sudo", "-n", "/usr/sbin/maas-netmon", "eth0"],
                 stdin=subprocess.DEVNULL,
                 stdout=subprocess.PIPE,
             ),