From 61727a4dc807d37b7046fc7b98be19212fb4949c Mon Sep 17 00:00:00 2001
From: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Date: Sat, 10 Nov 2018 10:01:16 +0100
Subject: [PATCH 1/8] interfaces: add Repository.Backend

This patch adds a helper method to obtain a given security backend by
name.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
---
 interfaces/repo.go      | 13 +++++++++++++
 interfaces/repo_test.go | 10 ++++++++++
 2 files changed, 23 insertions(+)

diff --git a/interfaces/repo.go b/interfaces/repo.go
index 6bea71d0f06..81156cf7a8b 100644
--- a/interfaces/repo.go
+++ b/interfaces/repo.go
@@ -888,6 +888,19 @@ func (r *Repository) Backends() []SecurityBackend {
 	return result
 }
 
+// Backend returns the backend with the given security system name, if any.
+func (r *Repository) Backend(securitySystem SecuritySystem) SecurityBackend {
+	r.m.Lock()
+	defer r.m.Unlock()
+
+	for _, backend := range r.backends {
+		if backend.Name() == securitySystem {
+			return backend
+		}
+	}
+	return nil
+}
+
 // Interfaces returns object holding a lists of all the plugs and slots and their connections.
 func (r *Repository) Interfaces() *Interfaces {
 	r.m.Lock()
diff --git a/interfaces/repo_test.go b/interfaces/repo_test.go
index 5b9b61eab99..f0233aacacb 100644
--- a/interfaces/repo_test.go
+++ b/interfaces/repo_test.go
@@ -225,6 +225,16 @@ func (s *RepositorySuite) TestBackends(c *C) {
 	c.Assert(s.emptyRepo.Backends(), DeepEquals, []SecurityBackend{b2, b1})
 }
 
+func (s *RepositorySuite) TestBackend(c *C) {
+	b1 := &ifacetest.TestSecurityBackend{BackendName: "b1"}
+	b2 := &ifacetest.TestSecurityBackend{BackendName: "b2"}
+	c.Assert(s.emptyRepo.AddBackend(b2), IsNil)
+	c.Assert(s.emptyRepo.AddBackend(b1), IsNil)
+	c.Assert(s.emptyRepo.Backend("b1"), Equals, b1)
+	c.Assert(s.emptyRepo.Backend("b2"), Equals, b2)
+	c.Assert(s.emptyRepo.Backend("b3"), IsNil)
+}
+
 // Tests for Repository.Interface()
 
 func (s *RepositorySuite) TestInterface(c *C) {

From b59f5a4b42b6dc782b225982baa06000f68c12ee Mon Sep 17 00:00:00 2001
From: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Date: Sat, 10 Nov 2018 10:01:43 +0100
Subject: [PATCH 2/8] overlord/ifacestate: add setupPhasedSecurity

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
---
 overlord/ifacestate/helpers.go | 45 ++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/overlord/ifacestate/helpers.go b/overlord/ifacestate/helpers.go
index fdcd7fc307d..e052cf866cf 100644
--- a/overlord/ifacestate/helpers.go
+++ b/overlord/ifacestate/helpers.go
@@ -310,6 +310,51 @@ func (m *InterfaceManager) reloadConnections(snapName string) ([]string, error)
 	return result, nil
 }
 
+func (m *InterfaceManager) setupPhasedSecurity(task *state.Task, snaps []*snap.Info, opts []interfaces.ConfinementOptions) error {
+	st := task.State()
+
+	// Setup the systemd security backend for all the snaps first.
+	// See LP: #1802581 for rationale.
+
+	// NOTE: Unlike the loop in the next paragraph, the order is per backend
+	// and per snap. This order is deliberate. It minimizes change in behavior
+	// of how the security setup is performed.
+	//
+	// Once fully dependency aware security setup is implemented, with
+	// topological sorting and dependencies between interface endpoints, this
+	// code can be unified with the code below.
+	priorityBackend := interfaces.SecuritySystemd
+	if backend := m.repo.Backend(priorityBackend); backend != nil {
+		for i, snapInfo := range snaps {
+			st.Unlock()
+			err := backend.Setup(snapInfo, opts[i], m.repo)
+			st.Lock()
+			if err != nil {
+				task.Errorf("cannot setup %s for snap %q: %s", backend.Name(), snapInfo.InstanceName(), err)
+				return err
+			}
+		}
+	}
+
+	// Setup the remaining backends using per snap and per backend ordering.
+	for i, snapInfo := range snaps {
+		for _, backend := range m.repo.Backends() {
+			if backend.Name() == priorityBackend {
+				continue
+			}
+			st.Unlock()
+			err := backend.Setup(snapInfo, opts[i], m.repo)
+			st.Lock()
+			if err != nil {
+				task.Errorf("cannot setup %s for snap %q: %s", backend.Name(), snapInfo.InstanceName(), err)
+				return err
+			}
+		}
+	}
+
+	return nil
+}
+
 func (m *InterfaceManager) setupSnapSecurity(task *state.Task, snapInfo *snap.Info, opts interfaces.ConfinementOptions) error {
 	st := task.State()
 	instanceName := snapInfo.InstanceName()

From 6127cb0d4fdabef01abd86545975ce66cd557738 Mon Sep 17 00:00:00 2001
From: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Date: Sat, 10 Nov 2018 10:34:08 +0100
Subject: [PATCH 3/8] overlord/ifacestate: use setupPhasedSecurity

Fixes: https://bugs.launchpad.net/tillamook/+bug/1802581
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
---
 overlord/ifacestate/handlers.go | 53 +++++++++++++++++++++++++++------
 1 file changed, 44 insertions(+), 9 deletions(-)

diff --git a/overlord/ifacestate/handlers.go b/overlord/ifacestate/handlers.go
index 19414f9911b..52ddf142d20 100644
--- a/overlord/ifacestate/handlers.go
+++ b/overlord/ifacestate/handlers.go
@@ -105,6 +105,8 @@ func (m *InterfaceManager) doSetupProfiles(task *state.Task, tomb *tomb.Tomb) er
 }
 
 func (m *InterfaceManager) setupProfilesForSnap(task *state.Task, _ *tomb.Tomb, snapInfo *snap.Info, opts interfaces.ConfinementOptions) error {
+	st := task.State()
+
 	if err := addImplicitSlots(task.State(), snapInfo); err != nil {
 		return err
 	}
@@ -137,13 +139,17 @@ func (m *InterfaceManager) setupProfilesForSnap(task *state.Task, _ *tomb.Tomb,
 		task.Logf("%s", snap.BadInterfacesSummary(snapInfo))
 	}
 
+	// Reload the connections and compute the set of affected snaps. The set
+	// affectedSet set contains name of all the affected snap instances.  The
+	// arrays affectedNames and affectedSnaps contain, arrays of snap names and
+	// snapInfo's, respectively. The arrays are sorted by name with the special
+	// exception that the snap being setup is always first. The affectedSnaps
+	// array may be shorter than the set of affected snaps in case any of the
+	// snaps cannot be found in the state.
 	reconnectedSnaps, err := m.reloadConnections(snapName)
 	if err != nil {
 		return err
 	}
-	if err := m.setupSnapSecurity(task, snapInfo, opts); err != nil {
-		return err
-	}
 	affectedSet := make(map[string]bool)
 	for _, name := range disconnectedSnaps {
 		affectedSet[name] = true
@@ -151,14 +157,43 @@ func (m *InterfaceManager) setupProfilesForSnap(task *state.Task, _ *tomb.Tomb,
 	for _, name := range reconnectedSnaps {
 		affectedSet[name] = true
 	}
-	// The principal snap was already handled above.
-	delete(affectedSet, snapInfo.InstanceName())
-	affectedSnaps := make([]string, 0, len(affectedSet))
+
+	// Sort the set of affected names, ensuring that the snap being setup
+	// is first regardless of the name it has.
+	affectedNames := make([]string, 0, len(affectedSet))
 	for name := range affectedSet {
-		affectedSnaps = append(affectedSnaps, name)
+		if name != snapName {
+			affectedNames = append(affectedNames, name)
+		}
+	}
+	sort.Strings(affectedNames)
+	affectedNames = append([]string{snapName}, affectedNames...)
+
+	// Obtain snap.Info for each affected snap, skipping those that cannot be
+	// found and compute the confinement options that apply to it.
+	affectedSnaps := make([]*snap.Info, 0, len(affectedSet))
+	confinementOpts := make([]interfaces.ConfinementOptions, 0, len(affectedSet))
+	// For the snap being setup we know exactly what was requested.
+	affectedSnaps = append(affectedSnaps, snapInfo)
+	confinementOpts = append(confinementOpts, opts)
+	// For remaining snaps we need to interrogate the state.
+	for _, name := range affectedNames[1:] {
+		var snapst snapstate.SnapState
+		if err := snapstate.Get(st, name, &snapst); err != nil {
+			task.Errorf("cannot obtain state of snap %s: %s", name, err)
+			continue
+		}
+		snapInfo, err := snapst.CurrentInfo()
+		if err != nil {
+			return err
+		}
+		if err := addImplicitSlots(st, snapInfo); err != nil {
+			return err
+		}
+		affectedSnaps = append(affectedSnaps, snapInfo)
+		confinementOpts = append(confinementOpts, confinementOptions(snapst.Flags))
 	}
-	sort.Strings(affectedSnaps)
-	return m.setupAffectedSnaps(task, snapName, affectedSnaps)
+	return m.setupPhasedSecurity(task, affectedSnaps, confinementOpts)
 }
 
 func (m *InterfaceManager) doRemoveProfiles(task *state.Task, tomb *tomb.Tomb) error {

From dd08e0ed21fe2dd9030887d386f0971391dd0459 Mon Sep 17 00:00:00 2001
From: Michael Vogt <mvo@ubuntu.com>
Date: Mon, 12 Nov 2018 14:28:16 +0100
Subject: [PATCH 4/8] ifstate: tweak setupPhasedSecurity()

To fix LP: #1802581 a new helper setupPhasedSecurity() was added.
This commit simplifies this helper - we already order the backends
and the systemd backend is already first so we can skip the extra
first step and instead setup security for all affected snaps at
the same time (ordered by backends).
---
 interfaces/repo.go             | 13 -------------
 interfaces/repo_test.go        | 10 ----------
 overlord/ifacestate/helpers.go | 31 +++----------------------------
 3 files changed, 3 insertions(+), 51 deletions(-)

diff --git a/interfaces/repo.go b/interfaces/repo.go
index 81156cf7a8b..6bea71d0f06 100644
--- a/interfaces/repo.go
+++ b/interfaces/repo.go
@@ -888,19 +888,6 @@ func (r *Repository) Backends() []SecurityBackend {
 	return result
 }
 
-// Backend returns the backend with the given security system name, if any.
-func (r *Repository) Backend(securitySystem SecuritySystem) SecurityBackend {
-	r.m.Lock()
-	defer r.m.Unlock()
-
-	for _, backend := range r.backends {
-		if backend.Name() == securitySystem {
-			return backend
-		}
-	}
-	return nil
-}
-
 // Interfaces returns object holding a lists of all the plugs and slots and their connections.
 func (r *Repository) Interfaces() *Interfaces {
 	r.m.Lock()
diff --git a/interfaces/repo_test.go b/interfaces/repo_test.go
index f0233aacacb..5b9b61eab99 100644
--- a/interfaces/repo_test.go
+++ b/interfaces/repo_test.go
@@ -225,16 +225,6 @@ func (s *RepositorySuite) TestBackends(c *C) {
 	c.Assert(s.emptyRepo.Backends(), DeepEquals, []SecurityBackend{b2, b1})
 }
 
-func (s *RepositorySuite) TestBackend(c *C) {
-	b1 := &ifacetest.TestSecurityBackend{BackendName: "b1"}
-	b2 := &ifacetest.TestSecurityBackend{BackendName: "b2"}
-	c.Assert(s.emptyRepo.AddBackend(b2), IsNil)
-	c.Assert(s.emptyRepo.AddBackend(b1), IsNil)
-	c.Assert(s.emptyRepo.Backend("b1"), Equals, b1)
-	c.Assert(s.emptyRepo.Backend("b2"), Equals, b2)
-	c.Assert(s.emptyRepo.Backend("b3"), IsNil)
-}
-
 // Tests for Repository.Interface()
 
 func (s *RepositorySuite) TestInterface(c *C) {
diff --git a/overlord/ifacestate/helpers.go b/overlord/ifacestate/helpers.go
index e052cf866cf..60048892796 100644
--- a/overlord/ifacestate/helpers.go
+++ b/overlord/ifacestate/helpers.go
@@ -313,18 +313,9 @@ func (m *InterfaceManager) reloadConnections(snapName string) ([]string, error)
 func (m *InterfaceManager) setupPhasedSecurity(task *state.Task, snaps []*snap.Info, opts []interfaces.ConfinementOptions) error {
 	st := task.State()
 
-	// Setup the systemd security backend for all the snaps first.
-	// See LP: #1802581 for rationale.
-
-	// NOTE: Unlike the loop in the next paragraph, the order is per backend
-	// and per snap. This order is deliberate. It minimizes change in behavior
-	// of how the security setup is performed.
-	//
-	// Once fully dependency aware security setup is implemented, with
-	// topological sorting and dependencies between interface endpoints, this
-	// code can be unified with the code below.
-	priorityBackend := interfaces.SecuritySystemd
-	if backend := m.repo.Backend(priorityBackend); backend != nil {
+	// Setup all affected snaps, start with the most important security
+	// backend and run it for all snaps. See LP: 1802581
+	for _, backend := range m.repo.Backends() {
 		for i, snapInfo := range snaps {
 			st.Unlock()
 			err := backend.Setup(snapInfo, opts[i], m.repo)
@@ -336,22 +327,6 @@ func (m *InterfaceManager) setupPhasedSecurity(task *state.Task, snaps []*snap.I
 		}
 	}
 
-	// Setup the remaining backends using per snap and per backend ordering.
-	for i, snapInfo := range snaps {
-		for _, backend := range m.repo.Backends() {
-			if backend.Name() == priorityBackend {
-				continue
-			}
-			st.Unlock()
-			err := backend.Setup(snapInfo, opts[i], m.repo)
-			st.Lock()
-			if err != nil {
-				task.Errorf("cannot setup %s for snap %q: %s", backend.Name(), snapInfo.InstanceName(), err)
-				return err
-			}
-		}
-	}
-
 	return nil
 }
 

From 9588c9ff756a0d2b91158fcac3d8ad016e6fdd6e Mon Sep 17 00:00:00 2001
From: Michael Vogt <mvo@ubuntu.com>
Date: Mon, 12 Nov 2018 16:42:03 +0100
Subject: [PATCH 5/8] tests: add regression test for lp-1802581

---
 tests/regression/lp-1802581/mock-gpio.py | 54 ++++++++++++++++++++++++
 tests/regression/lp-1802581/task.yaml    | 38 +++++++++++++++++
 2 files changed, 92 insertions(+)
 create mode 100755 tests/regression/lp-1802581/mock-gpio.py
 create mode 100644 tests/regression/lp-1802581/task.yaml

diff --git a/tests/regression/lp-1802581/mock-gpio.py b/tests/regression/lp-1802581/mock-gpio.py
new file mode 100755
index 00000000000..584e60b3a9d
--- /dev/null
+++ b/tests/regression/lp-1802581/mock-gpio.py
@@ -0,0 +1,54 @@
+#!/usr/bin/python3
+
+import os
+import selectors
+import sys
+
+
+def data_to_gpio(data):
+    return "gpio"+data.decode().strip()
+
+
+def export_ready(fd, mask):
+    data = os.read(fd, 128)
+    # allow quit
+    if data == b'quit\n':
+        return False
+    with open(data_to_gpio(data), "w"):
+        pass
+    return True
+    
+
+def unexport_ready(fd, mask):
+    os.remove(data_to_gpio(os.read(fd, 128)))
+    return True
+
+
+def dispatch(sel):
+    for key, mask in sel.select():
+        callback = key.data
+        if not callback(key.fileobj, mask):
+            return False
+    return True
+
+
+if __name__ == "__main__":
+    os.chdir(sys.argv[1])
+    
+    # fake gpio export/unexport files
+    os.mkfifo("export")
+    os.mkfifo("unexport")
+
+    # ensure that we create the right 
+    sel = selectors.DefaultSelector()
+    efd = os.open("export", os.O_RDWR | os.O_NONBLOCK)
+    ufd = os.open("unexport", os.O_RDWR | os.O_NONBLOCK)
+    sel.register(efd, selectors.EVENT_READ, export_ready)
+    sel.register(ufd, selectors.EVENT_READ, unexport_ready)
+    while True:
+        if not dispatch(sel):
+            break
+
+    # cleanup
+    os.close(efd)
+    os.close(ufd)
diff --git a/tests/regression/lp-1802581/task.yaml b/tests/regression/lp-1802581/task.yaml
new file mode 100644
index 00000000000..859396d5847
--- /dev/null
+++ b/tests/regression/lp-1802581/task.yaml
@@ -0,0 +1,38 @@
+summary: Regression test for  1802581
+
+systems: [ubuntu-core-16-64]
+
+environment:
+    GPIO_MOCK_DIR: /home/test/gpio-mock
+
+prepare: |
+    echo "Mock gpio"
+    mkdir -p "$GPIO_MOCK_DIR"
+    systemd-run --unit mock-gpio -- "$(pwd)/mock-gpio.py" "$GPIO_MOCK_DIR"
+    # shellcheck source=tests/lib/systemd.sh
+    . "$TESTSLIB/systemd.sh"
+    wait_for_service mock-gpio
+    mount --bind "$GPIO_MOCK_DIR" /sys/class/gpio
+
+restore: |
+    systemctl stop mock-gpio || true
+    umount /sys/class/gpio || true
+    rm -rf "$GPIO_MOCK_DIR"
+
+execute: |
+    echo "Install a snap that uses the gpio consumer"
+    #shellcheck source=tests/lib/snaps.sh
+    . "$TESTSLIB"/snaps.sh
+    install_local gpio-consumer
+
+    echo "And connect the gpio pin"
+    snap connect gpio-consumer:gpio :gpio-pin
+    snap interfaces | MATCH ":gpio-pin.*gpio-consumer:gpio"
+
+    # LP-1802581
+    echo "Now disable and enable the snap to ensure lp: #1802581 is fixed"
+    snap disable gpio-consumer
+    snap enable gpio-consumer
+
+    echo "Check that the connection is still here after enable"
+    snap interfaces | MATCH ":gpio-pin.*gpio-consumer:gpio"

From bd190c29964c6c22a97cce272f4b0df2b7b02491 Mon Sep 17 00:00:00 2001
From: Michael Vogt <mvo@ubuntu.com>
Date: Tue, 13 Nov 2018 09:04:19 +0100
Subject: [PATCH 6/8] tests: improve mock-gpio.py

---
 tests/regression/lp-1802581/mock-gpio.py | 45 ++++++++++++++++++------
 tests/regression/lp-1802581/task.yaml    | 17 ++++-----
 2 files changed, 44 insertions(+), 18 deletions(-)

diff --git a/tests/regression/lp-1802581/mock-gpio.py b/tests/regression/lp-1802581/mock-gpio.py
index 584e60b3a9d..42cb9f53a11 100755
--- a/tests/regression/lp-1802581/mock-gpio.py
+++ b/tests/regression/lp-1802581/mock-gpio.py
@@ -1,26 +1,41 @@
 #!/usr/bin/python3
 
+import atexit
+import logging
 import os
 import selectors
+import shutil
+import subprocess
 import sys
+import tempfile
 
 
-def data_to_gpio(data):
-    return "gpio"+data.decode().strip()
+def read_gpio_pin(fd):
+    data = os.read(fd, 128)
+    pin = data.decode().strip()
+    if not pin.isnumeric():
+        logging.warning("invalid gpio pin {}".format(pin))
+        return ""
+    return "gpio{}".format(pin)
 
 
 def export_ready(fd, mask):
-    data = os.read(fd, 128)
+    pin = read_gpio_pin(fd)
     # allow quit
-    if data == b'quit\n':
+    if pin == 'quit':
         return False
-    with open(data_to_gpio(data), "w"):
-        pass
+    if pin:
+        with open(pin, "w"):
+            pass
     return True
     
 
 def unexport_ready(fd, mask):
-    os.remove(data_to_gpio(os.read(fd, 128)))
+    pin = read_gpio_pin(fd)
+    try:
+        os.remove(pin)
+    except Exception as e:
+        logging.warning("got exception {}".format(e))
     return True
 
 
@@ -33,13 +48,23 @@ def dispatch(sel):
 
 
 if __name__ == "__main__":
-    os.chdir(sys.argv[1])
+    if os.getuid() != 0:
+        print("must run as root")
+        sys.exit(1)
+
+    # setup mock env
+    mock_gpio_dir = tempfile.mkdtemp("mock-gpio")
+    atexit.register(shutil.rmtree, mock_gpio_dir)
+    os.chdir(mock_gpio_dir)
+    subprocess.check_call(
+        ["mount", "--bind", mock_gpio_dir, "/sys/class/gpio"])
+    atexit.register(lambda: subprocess.call(["umount", "/sys/class/gpio"]))
     
     # fake gpio export/unexport files
     os.mkfifo("export")
     os.mkfifo("unexport")
 
-    # ensure that we create the right 
+    # react to the export/unexport calls
     sel = selectors.DefaultSelector()
     efd = os.open("export", os.O_RDWR | os.O_NONBLOCK)
     ufd = os.open("unexport", os.O_RDWR | os.O_NONBLOCK)
@@ -49,6 +74,6 @@ def dispatch(sel):
         if not dispatch(sel):
             break
 
-    # cleanup
+    # cleanup when we get a quit call
     os.close(efd)
     os.close(ufd)
diff --git a/tests/regression/lp-1802581/task.yaml b/tests/regression/lp-1802581/task.yaml
index 859396d5847..7e2a55b5914 100644
--- a/tests/regression/lp-1802581/task.yaml
+++ b/tests/regression/lp-1802581/task.yaml
@@ -2,22 +2,20 @@ summary: Regression test for  1802581
 
 systems: [ubuntu-core-16-64]
 
-environment:
-    GPIO_MOCK_DIR: /home/test/gpio-mock
-
 prepare: |
-    echo "Mock gpio"
-    mkdir -p "$GPIO_MOCK_DIR"
-    systemd-run --unit mock-gpio -- "$(pwd)/mock-gpio.py" "$GPIO_MOCK_DIR"
+    echo "Run mock gpio daemon"
+    systemd-run --unit mock-gpio -- "$(pwd)/mock-gpio.py"
     # shellcheck source=tests/lib/systemd.sh
     . "$TESTSLIB/systemd.sh"
     wait_for_service mock-gpio
-    mount --bind "$GPIO_MOCK_DIR" /sys/class/gpio
 
 restore: |
     systemctl stop mock-gpio || true
+    # for good measure, mock-gpio.py does this umount already on exit
     umount /sys/class/gpio || true
-    rm -rf "$GPIO_MOCK_DIR"
+
+debug: |
+    journalctl -u mock-gpio.service
 
 execute: |
     echo "Install a snap that uses the gpio consumer"
@@ -36,3 +34,6 @@ execute: |
 
     echo "Check that the connection is still here after enable"
     snap interfaces | MATCH ":gpio-pin.*gpio-consumer:gpio"
+
+    echo "Ensure that our mock service is full functional"
+    systemctl status mock-gpio.service | MATCH "Active: active"

From 071616a527c56415276e858df8554e1bf7112cbb Mon Sep 17 00:00:00 2001
From: Michael Vogt <mvo@ubuntu.com>
Date: Tue, 13 Nov 2018 09:54:56 +0100
Subject: [PATCH 7/8] mock-gpio.py: make pylint/pep8/mypy (mostly) clean

---
 tests/regression/lp-1802581/mock-gpio.py | 41 +++++++++++++++++-------
 tests/regression/lp-1802581/task.yaml    | 19 ++++++++++-
 2 files changed, 47 insertions(+), 13 deletions(-)

diff --git a/tests/regression/lp-1802581/mock-gpio.py b/tests/regression/lp-1802581/mock-gpio.py
index 42cb9f53a11..89ab4448dfd 100755
--- a/tests/regression/lp-1802581/mock-gpio.py
+++ b/tests/regression/lp-1802581/mock-gpio.py
@@ -1,4 +1,5 @@
 #!/usr/bin/python3
+"""Mock the /sys/class/gpio subsystem"""
 
 import atexit
 import logging
@@ -9,18 +10,25 @@
 import sys
 import tempfile
 
+from typing import Type
 
-def read_gpio_pin(fd):
-    data = os.read(fd, 128)
+
+def read_gpio_pin(read_fd: int) -> str:
+    """
+    read_gpio_pin reads from the given fd and return a string with the
+    numeric pin or "" if an invalid pin was specified.
+    """
+    data = os.read(read_fd, 128)
     pin = data.decode().strip()
     if not pin.isnumeric():
-        logging.warning("invalid gpio pin {}".format(pin))
+        logging.warning("invalid gpio pin %s", pin)
         return ""
     return "gpio{}".format(pin)
 
 
-def export_ready(fd, mask):
-    pin = read_gpio_pin(fd)
+def export_ready(read_fd: int, _) -> bool:
+    """export_ready is run when the "export" file is ready for reading"""
+    pin = read_gpio_pin(read_fd)
     # allow quit
     if pin == 'quit':
         return False
@@ -28,18 +36,22 @@ def export_ready(fd, mask):
         with open(pin, "w"):
             pass
     return True
-    
 
-def unexport_ready(fd, mask):
-    pin = read_gpio_pin(fd)
+
+def unexport_ready(read_fd: int, _) -> bool:
+    """export_ready is run when the "unexport" file is ready for reading"""
+    pin = read_gpio_pin(read_fd)
     try:
         os.remove(pin)
-    except Exception as e:
-        logging.warning("got exception {}".format(e))
+    except OSError as err:
+        logging.warning("got exception %s", err)
     return True
 
 
 def dispatch(sel):
+    """
+    dispatch dispatches the events from the "sel" source
+    """
     for key, mask in sel.select():
         callback = key.data
         if not callback(key.fileobj, mask):
@@ -47,7 +59,8 @@ def dispatch(sel):
     return True
 
 
-if __name__ == "__main__":
+def main():
+    """the main method"""
     if os.getuid() != 0:
         print("must run as root")
         sys.exit(1)
@@ -59,7 +72,7 @@ def dispatch(sel):
     subprocess.check_call(
         ["mount", "--bind", mock_gpio_dir, "/sys/class/gpio"])
     atexit.register(lambda: subprocess.call(["umount", "/sys/class/gpio"]))
-    
+
     # fake gpio export/unexport files
     os.mkfifo("export")
     os.mkfifo("unexport")
@@ -77,3 +90,7 @@ def dispatch(sel):
     # cleanup when we get a quit call
     os.close(efd)
     os.close(ufd)
+
+
+if __name__ == "__main__":
+    main()
diff --git a/tests/regression/lp-1802581/task.yaml b/tests/regression/lp-1802581/task.yaml
index 7e2a55b5914..80a0b8516b0 100644
--- a/tests/regression/lp-1802581/task.yaml
+++ b/tests/regression/lp-1802581/task.yaml
@@ -1,4 +1,21 @@
-summary: Regression test for  1802581
+summary: Regression test for LP 1802581
+
+description: |
+    When using the snapd GPIO interface the slot side needs to export the GPIO
+    pint so that a file appears under /sys/class/gpio/gpioNNN. That file is a
+    symbolic link to a special, platform specific device path. The symbolic
+    link needs to be evaluated so that apparmor rules for the plug side can be
+    constructed.
+
+    A customer has reported that when a snap is disabled and then re-enabled
+    the GPIO pin is not exported and apparmor backend cannot setup security
+    of the snap being enabled.
+
+    A similar issue has occurred once in the past where the order in which
+    security backends operated was not deterministic. We fixed the issue by
+    applying a fixed order so that the systemd backend, responsible for
+    exporting the pin, would always run before the apparmor backend, which
+    could now rely on the pin being exposed to userspace.
 
 systems: [ubuntu-core-16-64]
 

From 71b219f9adda72582db4add4e638c7738af0db33 Mon Sep 17 00:00:00 2001
From: Michael Vogt <mvo@ubuntu.com>
Date: Wed, 14 Nov 2018 16:31:59 +0100
Subject: [PATCH 8/8] ifacestate: rename setupPhasedSecurity ->
 setupSecurityByBackend

---
 overlord/ifacestate/handlers.go | 2 +-
 overlord/ifacestate/helpers.go  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/overlord/ifacestate/handlers.go b/overlord/ifacestate/handlers.go
index 52ddf142d20..b953ab100d9 100644
--- a/overlord/ifacestate/handlers.go
+++ b/overlord/ifacestate/handlers.go
@@ -193,7 +193,7 @@ func (m *InterfaceManager) setupProfilesForSnap(task *state.Task, _ *tomb.Tomb,
 		affectedSnaps = append(affectedSnaps, snapInfo)
 		confinementOpts = append(confinementOpts, confinementOptions(snapst.Flags))
 	}
-	return m.setupPhasedSecurity(task, affectedSnaps, confinementOpts)
+	return m.setupSecurityByBackend(task, affectedSnaps, confinementOpts)
 }
 
 func (m *InterfaceManager) doRemoveProfiles(task *state.Task, tomb *tomb.Tomb) error {
diff --git a/overlord/ifacestate/helpers.go b/overlord/ifacestate/helpers.go
index 60048892796..27f0792fa44 100644
--- a/overlord/ifacestate/helpers.go
+++ b/overlord/ifacestate/helpers.go
@@ -310,7 +310,7 @@ func (m *InterfaceManager) reloadConnections(snapName string) ([]string, error)
 	return result, nil
 }
 
-func (m *InterfaceManager) setupPhasedSecurity(task *state.Task, snaps []*snap.Info, opts []interfaces.ConfinementOptions) error {
+func (m *InterfaceManager) setupSecurityByBackend(task *state.Task, snaps []*snap.Info, opts []interfaces.ConfinementOptions) error {
 	st := task.State()
 
 	// Setup all affected snaps, start with the most important security