Merge ~alfonsosanchezbeato/network-manager:add-wifi-ap-support into network-manager:snap-1.10

[Alfonso Sanchez-Beato]

Add missing bits to enable sharing connections and be able to access points. Fixes LP: #1832494.

Add missing bits to enable sharing connections and be able to access points. Fixes LP: #1832494.

[System Enablement Bot (system-enablement-ci-bot)]

FAILED: Continuous integration, rev:18ee3b1c16386cbff7a84f9369bc15510c8f3e8a
https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build-prepare/402/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=amd64/435/console
    FAILURE: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=arm64/435/console
    FAILURE: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=armhf/435/console
    FAILURE: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=i386/435/console
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-docs/1227
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-cleanup/969
    None: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-update-mp/1015/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build-prepare/402/rebuild

review: Needs Fixing (continuous-integration)

[System Enablement Bot (system-enablement-ci-bot)]

FAILED: Continuous integration, rev:18ee3b1c16386cbff7a84f9369bc15510c8f3e8a
https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build-prepare/403/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=amd64/436/console
    FAILURE: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=arm64/436/console
    FAILURE: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=armhf/436/console
    FAILURE: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=i386/436/console
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-docs/1228
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-cleanup/970
    None: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-update-mp/1016/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build-prepare/403/rebuild

review: Needs Fixing (continuous-integration)

[System Enablement Bot (system-enablement-ci-bot)]

PASSED: Continuous integration, rev:18ee3b1c16386cbff7a84f9369bc15510c8f3e8a
https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build-prepare/404/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=amd64/437
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=arm64/437
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=armhf/437
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=i386/437
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-docs/1229
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-cleanup/971
    None: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-update-mp/1017/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build-prepare/404/rebuild

review: Approve (continuous-integration)

[Sebastien Bacher (seb128)]

Thanks for the work, I'm not maintaining that snap/having real knowledge about it but since we want to consolidate the deb&snap packaging at some point I'm going to comment from that angle

Please don't add undocumented patches, set-ld-library-path-for-iptables.patch should have some header with a description on why the change is needed and references to upstream/downstream bugs as appropriate

It would be also good to describe if the patch would be acceptable for the deb and see if that's something can could be upstreamed in some way

review: Needs Fixing

[Alfonso Sanchez-Beato (alfonsosanchezbeato)]

@Sebastien thanks for the review, I will a description to the NM patch.

Re: deb, no, I do not think this patch should be used by the deb package, as what it does is making sure that iptables is able to locate the right libraries coming in the snap, which is not needed for the deb.

[Konrad Zapałowicz (kzapalowicz)]

review: Approve

[System Enablement Bot (system-enablement-ci-bot)]

FAILED: Continuous integration, rev:7e79ba7d7a32bb4747cddd37a04ebc7eeaac7b6d
https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build-prepare/415/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=amd64/447/console
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=arm64/447
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=armhf/447
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=i386/447
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-docs/1239
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-cleanup/981
    None: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-update-mp/1028/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build-prepare/415/rebuild

review: Needs Fixing (continuous-integration)

[Sebastien Bacher (seb128)]

Looks fine to me but I know little about the snap so it should probably get another review from someone who does understand it better and knows how to do the testing

review: Approve

[System Enablement Bot (system-enablement-ci-bot)]

FAILED: Continuous integration, rev:7e79ba7d7a32bb4747cddd37a04ebc7eeaac7b6d
https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build-prepare/416/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=amd64/448/console
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=arm64/448
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=armhf/448
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=i386/448
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-docs/1240
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-cleanup/982
    None: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-update-mp/1029/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build-prepare/416/rebuild

review: Needs Fixing (continuous-integration)

[System Enablement Bot (system-enablement-ci-bot)]

PASSED: Continuous integration, rev:7e79ba7d7a32bb4747cddd37a04ebc7eeaac7b6d
https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build-prepare/417/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=amd64/449
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=arm64/449
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=armhf/449
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build/ARCHITECTURE=i386/449
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-docs/1241
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-cleanup/983
    None: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-update-mp/1030/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-build-prepare/417/rebuild

review: Approve (continuous-integration)

[Alfonso Sanchez-Beato (alfonsosanchezbeato)]

@Sebastien, well, that was already done by Konrad :)

Note also that automatic tests are passing: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-test/2205/console

diff --git a/debian/patches/series b/debian/patches/series
index 3267d3c..3b31e47 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -17,3 +17,4 @@ disable-libnm-glib-test-devices-array.patch
 add-snap-support.patch
 disable-tests-that-fail-under-launchpad-builds.patch
 snap-support-ppp.patch
+set-ld-library-path-for-iptables.patch
diff --git a/debian/patches/set-ld-library-path-for-iptables.patch b/debian/patches/set-ld-library-path-for-iptables.patch
new file mode 100644
index 0000000..065816c
--- /dev/null
+++ b/debian/patches/set-ld-library-path-for-iptables.patch
@@ -0,0 +1,34 @@
+From: =?UTF-8?q?Alfonso=20S=C3=A1nchez-Beato?= <[e-mail]>
+Date: Thu, 18 Jul 2019 17:48:39 +0200
+Subject: [PATCH] snap: pass NM env vars when calling iptables
+
+This is necessary so LD_LIBRARY_PATH is passed down to iptables so it is
+able to find the libraries included in the snap package.
+---
+ src/nm-act-request.c | 3 +--
+ 1 file changed, 1 insertion(+), 2 deletions(-)
+
+diff --git a/src/nm-act-request.c b/src/nm-act-request.c
+index 87070794..bcf3e0bb 100644
+--- a/src/nm-act-request.c
++++ b/src/nm-act-request.c
+@@ -309,7 +309,6 @@ nm_act_request_set_shared (NMActRequest *req, gboolean shared)
+ 	/* Send the rules to iptables */
+ 	for (iter = list; iter; iter = g_slist_next (iter)) {
+ 		ShareRule *rule = (ShareRule *) iter->data;
+-		char *envp[1] = { NULL };
+ 		gs_strfreev char **argv = NULL;
+ 		gs_free char *cmd = NULL;
+ 
+@@ -327,7 +326,7 @@ nm_act_request_set_shared (NMActRequest *req, gboolean shared)
+ 			GError *error = NULL;
+ 
+ 			nm_log_info (LOGD_SHARING, "Executing: %s", cmd);
+-			if (!g_spawn_sync ("/", argv, envp, G_SPAWN_STDOUT_TO_DEV_NULL | G_SPAWN_STDERR_TO_DEV_NULL,
++			if (!g_spawn_sync ("/", argv, NULL, G_SPAWN_STDOUT_TO_DEV_NULL | G_SPAWN_STDERR_TO_DEV_NULL,
+ 			                   NULL, NULL, NULL, NULL, &status, &error)) {
+ 				nm_log_warn (LOGD_SHARING, "Error executing command: %s",
+ 				             error->message);
+-- 
+2.17.1
+
diff --git a/snap-common/bin/networkmanager b/snap-common/bin/networkmanager
index 0ee19a1..bb8131b 100755
--- a/snap-common/bin/networkmanager
+++ b/snap-common/bin/networkmanager
@@ -31,6 +31,11 @@ mkdir -p "$SNAP_DATA"/conf.d
 mkdir -p "$SNAP_DATA"/state
 mkdir -p "$SNAP_DATA"/state/dhcp
 
+# Folders needed by dnsmasq
+mkdir -p "$SNAP_DATA"/var/lib/misc
+mkdir -p "$SNAP_DATA"/etc
+mkdir -p "$SNAP_DATA"/var/run
+
 # Apply current snapctl settings
 . "$SNAP"/bin/snap-config.sh
 apply_snap_config
diff --git a/snap-common/usr/share/doc/dnsmasq/copyright b/snap-common/usr/share/doc/dnsmasq/copyright
new file mode 100644
index 0000000..d516e43
--- /dev/null
+++ b/snap-common/usr/share/doc/dnsmasq/copyright
@@ -0,0 +1,21 @@
+dnsmasq is Copyright (c) 2000-2018 Simon Kelley
+
+It was downloaded from: http://www.thekelleys.org.uk/dnsmasq/
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; version 2 dated June, 1991, or
+   (at your option) version 3 dated 29 June, 2007.
+ 
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+On Debian GNU/Linux systems, the text of the GNU general public license is 
+available in the file /usr/share/common-licenses/GPL-2 or 
+/usr/share/common-licenses/GPL-3
+
+The Debian package of dnsmasq was created by Simon Kelley with assistance 
+from Lars Bahner.
+
diff --git a/snap-patch/dnsmasq.patch b/snap-patch/dnsmasq.patch
new file mode 100644
index 0000000..fbd6904
--- /dev/null
+++ b/snap-patch/dnsmasq.patch
@@ -0,0 +1,102 @@
+From 8e829e18fc739f05b1a25cc40ce1722f65831950 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Alfonso=20S=C3=A1nchez-Beato?=
+ <[e-mail]>
+Date: Tue, 25 Jun 2019 17:36:41 +0200
+Subject: [PATCH] snap: do not tinker with uid/gid, change fixed paths
+
+---
+ src/config.h  |  6 +++---
+ src/dnsmasq.c | 30 +++++++++++++++---------------
+ 2 files changed, 18 insertions(+), 18 deletions(-)
+
+diff --git a/src/config.h b/src/config.h
+index ecefb87..25b92ba 100644
+--- a/src/config.h
++++ b/src/config.h
+@@ -190,7 +190,7 @@ RESOLVFILE
+ #   elif defined(__ANDROID__)
+ #      define LEASEFILE "/data/misc/dhcp/dnsmasq.leases"
+ #   else
+-#      define LEASEFILE "/var/lib/misc/dnsmasq.leases"
++#      define LEASEFILE "/var/snap/network-manager/current/var/lib/misc/dnsmasq.leases"
+ #   endif
+ #endif
+ 
+@@ -198,7 +198,7 @@ RESOLVFILE
+ #   if defined(__FreeBSD__)
+ #      define CONFFILE "/usr/local/etc/dnsmasq.conf"
+ #   else
+-#      define CONFFILE "/etc/dnsmasq.conf"
++#      define CONFFILE "/var/snap/network-manager/current/etc/dnsmasq.conf"
+ #   endif
+ #endif
+ 
+@@ -214,7 +214,7 @@ RESOLVFILE
+ #   if defined(__ANDROID__)
+ #      define RUNFILE "/data/dnsmasq.pid"
+ #    else
+-#      define RUNFILE "/var/run/dnsmasq.pid"
++#      define RUNFILE "/var/snap/network-manager/current/var/run/dnsmasq.pid"
+ #    endif
+ #endif
+ 
+diff --git a/src/dnsmasq.c b/src/dnsmasq.c
+index ce44809..17c451b 100644
+--- a/src/dnsmasq.c
++++ b/src/dnsmasq.c
+@@ -548,8 +548,8 @@ int main (int argc, char **argv)
+ 		 of the directory containing the file. That directory will
+ 		 need to by owned by the dnsmasq user, and the ownership of the
+ 		 file has to match, to keep systemd >273 happy. */
+-	      if (getuid() == 0 && ent_pw && ent_pw->pw_uid != 0 && fchown(fd, ent_pw->pw_uid, ent_pw->pw_gid) == -1)
+-		chown_warn = errno;
++	      /* if (getuid() == 0 && ent_pw && ent_pw->pw_uid != 0 && fchown(fd, ent_pw->pw_uid, ent_pw->pw_gid) == -1) */
++	      /*   chown_warn = errno; */
+ 
+ 	      if (!read_write(fd, (unsigned char *)daemon->namebuff, strlen(daemon->namebuff), 0))
+ 		err = 1;
+@@ -595,16 +595,16 @@ int main (int argc, char **argv)
+   if (!option_bool(OPT_DEBUG) && getuid() == 0)   
+     {
+       int bad_capabilities = 0;
+-      gid_t dummy;
++      /* gid_t dummy; */
+       
+       /* remove all supplementary groups */
+-      if (gp && 
+-	  (setgroups(0, &dummy) == -1 ||
+-	   setgid(gp->gr_gid) == -1))
+-	{
+-	  send_event(err_pipe[1], EVENT_GROUP_ERR, errno, daemon->groupname);
+-	  _exit(0);
+-	}
++      /* if (gp &&  */
++      /*     (setgroups(0, &dummy) == -1 || */
++      /*      setgid(gp->gr_gid) == -1)) */
++      /*   { */
++      /*     send_event(err_pipe[1], EVENT_GROUP_ERR, errno, daemon->groupname); */
++      /*     _exit(0); */
++      /*   } */
+   
+       if (ent_pw && ent_pw->pw_uid != 0)
+ 	{     
+@@ -654,11 +654,11 @@ int main (int argc, char **argv)
+ 	    }
+ 	  
+ 	  /* finally drop root */
+-	  if (setuid(ent_pw->pw_uid) == -1)
+-	    {
+-	      send_event(err_pipe[1], EVENT_USER_ERR, errno, daemon->username);
+-	      _exit(0);
+-	    }     
++	  /* if (setuid(ent_pw->pw_uid) == -1) */
++	  /*   { */
++	  /*     send_event(err_pipe[1], EVENT_USER_ERR, errno, daemon->username); */
++	  /*     _exit(0); */
++	  /*   }      */
+ 
+ #ifdef HAVE_LINUX_NETWORK
+ 	  if (is_dad_listeners() || option_bool(OPT_CLEVERBIND))
+-- 
+2.17.1
+
diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml
index da71965..7480dc5 100644
--- a/snap/snapcraft.yaml
+++ b/snap/snapcraft.yaml
@@ -50,11 +50,35 @@ apps:
     command: bin/networkmanager
     daemon: simple
     slots: [service]
-    plugs: [modem-manager, ppp, network-setup-observe, wpa, firewall-control, hardware-observe]
+    plugs: [modem-manager, ppp, network-setup-observe, wpa, firewall-control, hardware-observe, kernel-module-control]
+
+layout:
+  /usr/lib/x86_64-linux-gnu/xtables:
+    bind: $SNAP/usr/lib/x86_64-linux-gnu/xtables
+
 parts:
   networkmanager-common:
     plugin: dump
     source: snap-common
+
+  dnsmasq:
+    plugin: make
+    source: https://git.launchpad.net/ubuntu/+source/dnsmasq
+    source-type: git
+    source-branch: applied/ubuntu/bionic
+    build-packages:
+      - build-essential
+    make-parameters:
+      - PREFIX=/
+    override-build: |
+      git apply ../../../snap-patch/dnsmasq.patch
+      snapcraftctl build
+    filesets:
+      binaries:
+        - sbin/dnsmasq
+    prime:
+      - $binaries
+
   #
   # TODO: investigate whether this HACK is still needed. The script
   # dhcp-lease-mover relies on inotifywait to determine if any DHCP
@@ -288,6 +312,7 @@ parts:
         - usr/lib/*/libteamdctl*
         - usr/lib/*/libwind*
         - usr/lib/*/libxtables*
+        - usr/lib/*/xtables/*
       unwanted:
         # We don't use dhclient so we don't need this helper
         - -usr/lib/NetworkManager/nm-dhcp-helper