From 0640a954a7d3f04a8a1e6ec6f19e24769c1ce668 Mon Sep 17 00:00:00 2001
From: Michael Vogt <mvo@ubuntu.com>
Date: Wed, 8 Nov 2017 17:32:47 +0100
Subject: [PATCH] cmd: fix re-exec bug with classic confinement for host snapd
 < 2.28

Older version of snapd (before 2.28) did use the SNAPD_DID_REEXEC
env to check if they should re-exec or not. We need
to unset it after re-exec because the host snap tool may be old and
using this key.

So if e.g. the host has snapd 2.27 and snapd re-execs to 2.29 then
`snap run --shell classic-snap` will go into an environment where the
snapd 2.27 sees this key and stops re-execing and things break in
suble ways.

C.f.
https://forum.snapcraft.io/t/seccomp-error-calling-snap-from-another-classic-snap-on-core-candidate/2736/7
---
 cmd/cmd.go                            |  9 +++++++++
 cmd/cmd_test.go                       | 15 +++++++++++++++
 tests/regression/lp-1704860/task.yaml |  5 ++++-
 3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/cmd/cmd.go b/cmd/cmd.go
index 0a6714c1b07..95e49f6fa50 100644
--- a/cmd/cmd.go
+++ b/cmd/cmd.go
@@ -176,6 +176,15 @@ func ExecInCoreSnap() {
 
 	// Did we already re-exec?
 	if strings.HasPrefix(exe, dirs.SnapMountDir) {
+		// Older version of snapd (before 2.28) did use this env
+		// to check if they should re-exec or not. We still need
+		// to unset it because the host snap tool may be old and
+		// using this key. So if e.g. the host has snapd 2.27 and
+		// snapd re-execs to 2.29 then `snap run --shell classic-snap`
+		// will go into an environment where the snapd 2.27 sees
+		// this key and stops re-execing - which is not what we
+		// want. C.f. https://forum.snapcraft.io/t/seccomp-error-calling-snap-from-another-classic-snap-on-core-candidate/2736/7
+		mustUnsetenv("SNAP_DID_REEXEC")
 		return
 	}
 
diff --git a/cmd/cmd_test.go b/cmd/cmd_test.go
index 54bca0b385a..89ccb90b52b 100644
--- a/cmd/cmd_test.go
+++ b/cmd/cmd_test.go
@@ -293,6 +293,7 @@ func (s *cmdSuite) TestExecInCoreSnapNoDouble(c *C) {
 	selfExe := filepath.Join(s.fakeroot, "proc/self/exe")
 	err := os.Symlink(filepath.Join(s.fakeroot, "/snap/core/42/usr/lib/snapd"), selfExe)
 	c.Assert(err, IsNil)
+	cmd.MockSelfExe(selfExe)
 
 	cmd.ExecInCoreSnap()
 	c.Check(s.execCalled, Equals, 0)
@@ -307,3 +308,17 @@ func (s *cmdSuite) TestExecInCoreSnapDisabled(c *C) {
 	cmd.ExecInCoreSnap()
 	c.Check(s.execCalled, Equals, 0)
 }
+
+func (s *cmdSuite) TestExecInCoreSnapUnsetsDidReexec(c *C) {
+	os.Setenv("SNAP_DID_REEXEC", "1")
+	defer os.Unsetenv("SNAP_DID_REEXEC")
+
+	selfExe := filepath.Join(s.fakeroot, "proc/self/exe")
+	err := os.Symlink(filepath.Join(s.fakeroot, "/snap/core/42/usr/lib/snapd"), selfExe)
+	c.Assert(err, IsNil)
+	cmd.MockSelfExe(selfExe)
+
+	cmd.ExecInCoreSnap()
+	c.Check(s.execCalled, Equals, 0)
+	c.Check(os.Getenv("SNAP_DID_REEXEC"), Equals, "")
+}
diff --git a/tests/regression/lp-1704860/task.yaml b/tests/regression/lp-1704860/task.yaml
index 0a074b17629..dba902bde50 100644
--- a/tests/regression/lp-1704860/task.yaml
+++ b/tests/regression/lp-1704860/task.yaml
@@ -19,4 +19,7 @@ execute: |
     . $TESTSLIB/snaps.sh
     install_local_classic test-snapd-classic-confinement
     # We don't want to see SNAP_DID_REEXEC being set.
-    snap run --shell test-snapd-classic-confinement ./snap-env-query.sh | MATCH -v 'SNAP_DID_REEXEC='
+    if snap run --shell test-snapd-classic-confinement ./snap-env-query.sh | grep 'SNAP_DID_REEXEC='; then
+        echo "SNAP_DID_REEXEC environment is not reset as it should be"
+        exit 1
+    fi