fde: extract new runFDEinitramfsHelper() helper

4 files changed, 175 insertions, 144 deletions

diff --git a/kernel/fde/cmd_helper.go b/kernel/fde/cmd_helper.go
new file mode 100644
index 0000000000..d02fc940f4
--- /dev/null
+++ b/kernel/fde/cmd_helper.go
@@ -0,0 +1,154 @@
+// -*- Mode: Go; indent-tabs-mode: t -*-
+
+/*
+ * Copyright (C) 2021 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+package fde
+
+import (
+	"bytes"
+	"fmt"
+	"io/ioutil"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"time"
+
+	"github.com/snapcore/snapd/dirs"
+	"github.com/snapcore/snapd/logger"
+	"github.com/snapcore/snapd/osutil"
+)
+
+// fdeInitramfsHelperRuntimeMax is the maximum runtime a helper can execute
+// XXX: what is a reasonable default here?
+var fdeInitramfsHelperRuntimeMax = 2 * time.Minute
+
+// 50 ms means we check at a frequency 20 Hz, fast enough to not hold
+// up boot, but not too fast that we are hogging the CPU from the
+// thing we are waiting to finish running
+var fdeInitramfsHelperPollWait = 50 * time.Millisecond
+
+// fdeInitramfsHelperPollWaitParanoiaFactor controls much longer we wait
+// then fdeInitramfsHelperRuntimeMax before stopping to poll for results
+var fdeInitramfsHelperPollWaitParanoiaFactor = 2
+
+// overridden in tests
+var fdeInitramfsHelperCommandExtra []string
+
+func runFDEinitramfsHelper(name string, stdin []byte) (output []byte, err error) {
+	runDir := filepath.Join(dirs.GlobalRootDir, "/run", name)
+	if err := os.MkdirAll(runDir, 0700); err != nil {
+	return nil, fmt.Errorf("cannot create tmp dir for %s: %v", name, err)
+	}
+
+	// delete and re-create the std{in,out,err} stream files that we use for the
+	// hook to be robust against bugs where the files are created with too
+	// permissive permissions or not properly deleted afterwards since the hook
+	// will be invoked multiple times during the initrd and we want to be really
+	// careful since the stdout file will contain the unsealed encryption key
+	for _, stream := range []string{"stdin", "stdout", "stderr"} {
+	streamFile := filepath.Join(runDir, name+"."+stream)
+	// we want to make sure that the file permissions for stdout are always
+	// 0600, so to ensure this is the case and be robust against bugs, we
+	// always delete the file and re-create it with 0600
+
+	// note that if the file already exists, WriteFile will not change the
+	// permissions, so deleting first is the right thing to do
+	os.Remove(streamFile)
+	if stream == "stdin" {
+	err = ioutil.WriteFile(streamFile, stdin, 0600)
+	} else {
+	err = ioutil.WriteFile(streamFile, nil, 0600)
+	}
+	if err != nil {
+	return nil, fmt.Errorf("cannot create %s for %s: %v", name, stream, err)
+	}
+	}
+
+	// TODO: put this into a new "systemd/run" package
+	cmd := exec.Command(
+	"systemd-run",
+	"--collect",
+	"--service-type=exec",
+	"--quiet",
+	// ensure we get some result from the hook within a
+	// reasonable timeout and output from systemd if
+	// things go wrong
+	fmt.Sprintf("--property=RuntimeMaxSec=%s", fdeInitramfsHelperRuntimeMax),
+	// Do not allow mounting, this ensures hooks in initrd
+	// can not mess around with ubuntu-data.
+	//
+	// Note that this is not about perfect confinement, more about
+	// making sure that people using the hook know that we do not
+	// want them to mess around outside of just providing unseal.
+	"--property=SystemCallFilter=~@mount",
+	// WORKAROUNDS
+	// workaround the lack of "--pipe"
+	fmt.Sprintf("--property=StandardInput=file:%s/%s.stdin", runDir, name),
+	// NOTE: these files are manually created above with 0600 because by
+	// default systemd will create them 0644 and we want to be paranoid here
+	fmt.Sprintf("--property=StandardOutput=file:%s/%s.stdout", runDir, name),
+	fmt.Sprintf("--property=StandardError=file:%s/%s.stderr", runDir, name),
+	// this ensures we get useful output for e.g. segfaults
+	fmt.Sprintf(`--property=ExecStopPost=/bin/sh -c 'if [ "$EXIT_STATUS" = 0 ]; then touch %[1]s/%[2]s.success; else echo "service result: $SERVICE_RESULT" >%[1]s/%[2]s.failed; fi'`, runDir, name),
+	)
+	if fdeInitramfsHelperCommandExtra != nil {
+	cmd.Args = append(cmd.Args, fdeInitramfsHelperCommandExtra...)
+	}
+	// "name" is what we actually need to run
+	cmd.Args = append(cmd.Args, name)
+
+	// ensure we cleanup our tmp files
+	defer func() {
+	if err := os.RemoveAll(runDir); err != nil {
+	logger.Noticef("cannot remove tmp dir: %v", err)
+	}
+	}()
+
+	// run the command
+	output, err = cmd.CombinedOutput()
+	if err != nil {
+	return output, err
+	}
+
+	// This loop will be terminate by systemd-run, either because
+	// "name" exists or it gets killed when it reaches the
+	// fdeInitramfsHelperRuntimeMax defined above.
+	//
+	// However we are paranoid and exit this loop if systemd
+	// did not terminate the process after twice the allocated
+	// runtime
+	maxLoops := int(fdeInitramfsHelperRuntimeMax/fdeInitramfsHelperPollWait) * fdeInitramfsHelperPollWaitParanoiaFactor
+	for i := 0; i < maxLoops; i++ {
+	switch {
+	case osutil.FileExists(filepath.Join(runDir, name+".failed")):
+	stderr, _ := ioutil.ReadFile(filepath.Join(runDir, name+".stderr"))
+	systemdErr, _ := ioutil.ReadFile(filepath.Join(runDir, name+".failed"))
+	buf := bytes.NewBuffer(stderr)
+	buf.Write(systemdErr)
+	return buf.Bytes(), fmt.Errorf("%s failed", name)
+	case osutil.FileExists(filepath.Join(runDir, name+".success")):
+	return ioutil.ReadFile(filepath.Join(runDir, name+".stdout"))
+	default:
+	time.Sleep(fdeInitramfsHelperPollWait)
+	}
+	}
+
+	// this should never happen, the loop above should be terminated
+	// via systemd
+	return nil, fmt.Errorf("internal error: systemd-run did not honor RuntimeMax=%s setting", fdeInitramfsHelperRuntimeMax)
+}
diff --git a/kernel/fde/export_test.go b/kernel/fde/export_test.go
index f2e9596a03..dbd1003630 100644
--- a/kernel/fde/export_test.go
+++ b/kernel/fde/export_test.go
@@ -23,26 +23,26 @@ import (
	"time"
 )
 
-func MockFdeRevealKeyCommandExtra(args []string) (restore func()) {
-	oldFdeRevealKeyCommandExtra := fdeRevealKeyCommandExtra
-	fdeRevealKeyCommandExtra = args
+func MockFdeInitramfsHelperCommandExtra(args []string) (restore func()) {
+	oldFdeRevealKeyCommandExtra := fdeInitramfsHelperCommandExtra
+	fdeInitramfsHelperCommandExtra = args
	return func() {
-	fdeRevealKeyCommandExtra = oldFdeRevealKeyCommandExtra
+	fdeInitramfsHelperCommandExtra = oldFdeRevealKeyCommandExtra
	}
 }
 
 func MockFdeRevealKeyRuntimeMax(d time.Duration) (restore func()) {
-	oldFdeRevealKeyRuntimeMax := fdeRevealKeyRuntimeMax
-	fdeRevealKeyRuntimeMax = d
+	oldFdeRevealKeyRuntimeMax := fdeInitramfsHelperRuntimeMax
+	fdeInitramfsHelperRuntimeMax = d
	return func() {
-	fdeRevealKeyRuntimeMax = oldFdeRevealKeyRuntimeMax
+	fdeInitramfsHelperRuntimeMax = oldFdeRevealKeyRuntimeMax
	}
 }
 
 func MockFdeRevealKeyPollWaitParanoiaFactor(n int) (restore func()) {
-	oldFdeRevealKeyPollWaitParanoiaFactor := fdeRevealKeyPollWaitParanoiaFactor
-	fdeRevealKeyPollWaitParanoiaFactor = n
+	oldFdeRevealKeyPollWaitParanoiaFactor := fdeInitramfsHelperPollWaitParanoiaFactor
+	fdeInitramfsHelperPollWaitParanoiaFactor = n
	return func() {
-	fdeRevealKeyPollWaitParanoiaFactor = oldFdeRevealKeyPollWaitParanoiaFactor
+	fdeInitramfsHelperPollWaitParanoiaFactor = oldFdeRevealKeyPollWaitParanoiaFactor
	}
 }
diff --git a/kernel/fde/fde_test.go b/kernel/fde/fde_test.go
index a84aa4dc94..ac2907a939 100644
--- a/kernel/fde/fde_test.go
+++ b/kernel/fde/fde_test.go
@@ -178,7 +178,7 @@ func checkSystemdRunOrSkip(c *C) {
 func (s *fdeSuite) TestLockSealedKeysCallsFdeReveal(c *C) {
	checkSystemdRunOrSkip(c)
 
-	restore := fde.MockFdeRevealKeyCommandExtra([]string{"--user"})
+	restore := fde.MockFdeInitramfsHelperCommandExtra([]string{"--user"})
	defer restore()
	fdeRevealKeyStdin := filepath.Join(c.MkDir(), "stdin")
	mockSystemdRun := testutil.MockCommand(c, "fde-reveal-key", fmt.Sprintf(`
@@ -200,7 +200,7 @@ cat - > %s
 func (s *fdeSuite) TestLockSealedKeysHonorsRuntimeMax(c *C) {
	checkSystemdRunOrSkip(c)
 
-	restore := fde.MockFdeRevealKeyCommandExtra([]string{"--user"})
+	restore := fde.MockFdeInitramfsHelperCommandExtra([]string{"--user"})
	defer restore()
	mockSystemdRun := testutil.MockCommand(c, "fde-reveal-key", "sleep 60")
	defer mockSystemdRun.Restore()
@@ -218,7 +218,7 @@ func (s *fdeSuite) TestLockSealedKeysHonorsRuntimeMax(c *C) {
 func (s *fdeSuite) TestLockSealedKeysHonorsParanoia(c *C) {
	checkSystemdRunOrSkip(c)
 
-	restore := fde.MockFdeRevealKeyCommandExtra([]string{"--user"})
+	restore := fde.MockFdeInitramfsHelperCommandExtra([]string{"--user"})
	defer restore()
	mockSystemdRun := testutil.MockCommand(c, "fde-reveal-key", "sleep 60")
	defer mockSystemdRun.Restore()
@@ -243,7 +243,7 @@ func (s *fdeSuite) TestReveal(c *C) {
	sealedKey := []byte("sealed-v2-payload")
	v2payload := []byte("unsealed-v2-payload")
 
-	restore := fde.MockFdeRevealKeyCommandExtra([]string{"--user"})
+	restore := fde.MockFdeInitramfsHelperCommandExtra([]string{"--user"})
	defer restore()
	fdeRevealKeyStdin := filepath.Join(c.MkDir(), "stdin")
	mockSystemdRun := testutil.MockCommand(c, "fde-reveal-key", fmt.Sprintf(`
@@ -277,7 +277,7 @@ func (s *fdeSuite) TestRevealV1(c *C) {
	// fix randutil outcome
	rand.Seed(1)
 
-	restore := fde.MockFdeRevealKeyCommandExtra([]string{"--user"})
+	restore := fde.MockFdeInitramfsHelperCommandExtra([]string{"--user"})
	defer restore()
	fdeRevealKeyStdin := filepath.Join(c.MkDir(), "stdin")
	mockSystemdRun := testutil.MockCommand(c, "fde-reveal-key", fmt.Sprintf(`
@@ -311,7 +311,7 @@ func (s *fdeSuite) TestRevealV2PayloadV1Hook(c *C) {
	sealedKey := []byte("sealed-v2-payload")
	v2payload := []byte("unsealed-v2-payload")
 
-	restore := fde.MockFdeRevealKeyCommandExtra([]string{"--user"})
+	restore := fde.MockFdeInitramfsHelperCommandExtra([]string{"--user"})
	defer restore()
	fdeRevealKeyStdin := filepath.Join(c.MkDir(), "stdin")
	mockSystemdRun := testutil.MockCommand(c, "fde-reveal-key", fmt.Sprintf(`
@@ -347,7 +347,7 @@ func (s *fdeSuite) TestRevealV2BadJSON(c *C) {
 
	sealedKey := []byte("sealed-v2-payload")
 
-	restore := fde.MockFdeRevealKeyCommandExtra([]string{"--user"})
+	restore := fde.MockFdeInitramfsHelperCommandExtra([]string{"--user"})
	defer restore()
	fdeRevealKeyStdin := filepath.Join(c.MkDir(), "stdin")
	mockSystemdRun := testutil.MockCommand(c, "fde-reveal-key", fmt.Sprintf(`
@@ -381,7 +381,7 @@ func (s *fdeSuite) TestRevealV1BadOutputSize(c *C) {
	// fix randutil outcome
	rand.Seed(1)
 
-	restore := fde.MockFdeRevealKeyCommandExtra([]string{"--user"})
+	restore := fde.MockFdeInitramfsHelperCommandExtra([]string{"--user"})
	defer restore()
	fdeRevealKeyStdin := filepath.Join(c.MkDir(), "stdin")
	mockSystemdRun := testutil.MockCommand(c, "fde-reveal-key", fmt.Sprintf(`
@@ -467,7 +467,7 @@ echo "making the hook always fail for simpler test code" 1>&2
 exit 1
 `, streamFiles[0], streamFiles[1], streamFiles[2]))
	defer mockSystemdRun.Restore()
-	restore := fde.MockFdeRevealKeyCommandExtra([]string{"--user"})
+	restore := fde.MockFdeInitramfsHelperCommandExtra([]string{"--user"})
	defer restore()
 
	sealedKey := []byte{1, 2, 3, 4}
@@ -497,7 +497,7 @@ func (s *fdeSuite) TestRevealErr(c *C) {
 
	mockSystemdRun := testutil.MockCommand(c, "systemd-run", `echo failed 1>&2; false`)
	defer mockSystemdRun.Restore()
-	restore := fde.MockFdeRevealKeyCommandExtra([]string{"--user"})
+	restore := fde.MockFdeInitramfsHelperCommandExtra([]string{"--user"})
	defer restore()
 
	sealedKey := []byte{1, 2, 3, 4}
diff --git a/kernel/fde/reveal_key.go b/kernel/fde/reveal_key.go
index 7e39512ba9..7625eb3055 100644
--- a/kernel/fde/reveal_key.go
+++ b/kernel/fde/reveal_key.go
@@ -23,14 +23,7 @@ import (
	"bytes"
	"encoding/json"
	"fmt"
-	"io/ioutil"
-	"os"
-	"os/exec"
-	"path/filepath"
-	"time"
 
-	"github.com/snapcore/snapd/dirs"
-	"github.com/snapcore/snapd/logger"
	"github.com/snapcore/snapd/osutil"
	"github.com/snapcore/snapd/randutil"
 )
@@ -48,22 +41,6 @@ type RevealKeyRequest struct {
	// TODO: add VolumeName,SourceDevicePath later
 }
 
-// fdeRevealKeyRuntimeMax is the maximum runtime a fde-reveal-key can execute
-// XXX: what is a reasonable default here?
-var fdeRevealKeyRuntimeMax = 2 * time.Minute
-
-// 50 ms means we check at a frequency 20 Hz, fast enough to not hold
-// up boot, but not too fast that we are hogging the CPU from the
-// thing we are waiting to finish running
-var fdeRevealKeyPollWait = 50 * time.Millisecond
-
-// fdeRevealKeyPollWaitParanoiaFactor controls much longer we wait
-// then fdeRevealKeyRuntimeMax before stopping to poll for results
-var fdeRevealKeyPollWaitParanoiaFactor = 2
-
-// overridden in tests
-var fdeRevealKeyCommandExtra []string
-
 // runFDERevealKeyCommand returns the output of fde-reveal-key run
 // with systemd.
 //
@@ -76,107 +53,7 @@ func runFDERevealKeyCommand(req *RevealKeyRequest) (output []byte, err error) {
	return nil, fmt.Errorf(`cannot build request for fde-reveal-key %q: %v`, req.Op, err)
	}
 
-	runDir := filepath.Join(dirs.GlobalRootDir, "/run/fde-reveal-key")
-	if err := os.MkdirAll(runDir, 0700); err != nil {
-	return nil, fmt.Errorf("cannot create tmp dir for fde-reveal-key: %v", err)
-	}
-
-	// delete and re-create the std{in,out,err} stream files that we use for the
-	// hook to be robust against bugs where the files are created with too
-	// permissive permissions or not properly deleted afterwards since the hook
-	// will be invoked multiple times during the initrd and we want to be really
-	// careful since the stdout file will contain the unsealed encryption key
-	for _, stream := range []string{"stdin", "stdout", "stderr"} {
-	streamFile := filepath.Join(runDir, "fde-reveal-key."+stream)
-	// we want to make sure that the file permissions for stdout are always
-	// 0600, so to ensure this is the case and be robust against bugs, we
-	// always delete the file and re-create it with 0600
-
-	// note that if the file already exists, WriteFile will not change the
-	// permissions, so deleting first is the right thing to do
-	os.Remove(streamFile)
-	if stream == "stdin" {
-	err = ioutil.WriteFile(streamFile, stdin, 0600)
-	} else {
-	err = ioutil.WriteFile(streamFile, nil, 0600)
-	}
-	if err != nil {
-	return nil, fmt.Errorf("cannot create %s for fde-reveal-key: %v", stream, err)
-	}
-	}
-
-	// TODO: put this into a new "systemd/run" package
-	cmd := exec.Command(
-	"systemd-run",
-	"--collect",
-	"--service-type=exec",
-	"--quiet",
-	// ensure we get some result from the hook within a
-	// reasonable timeout and output from systemd if
-	// things go wrong
-	fmt.Sprintf("--property=RuntimeMaxSec=%s", fdeRevealKeyRuntimeMax),
-	// Do not allow mounting, this ensures hooks in initrd
-	// can not mess around with ubuntu-data.
-	//
-	// Note that this is not about perfect confinement, more about
-	// making sure that people using the hook know that we do not
-	// want them to mess around outside of just providing unseal.
-	"--property=SystemCallFilter=~@mount",
-	// WORKAROUNDS
-	// workaround the lack of "--pipe"
-	fmt.Sprintf("--property=StandardInput=file:%s/fde-reveal-key.stdin", runDir),
-	// NOTE: these files are manually created above with 0600 because by
-	// default systemd will create them 0644 and we want to be paranoid here
-	fmt.Sprintf("--property=StandardOutput=file:%s/fde-reveal-key.stdout", runDir),
-	fmt.Sprintf("--property=StandardError=file:%s/fde-reveal-key.stderr", runDir),
-	// this ensures we get useful output for e.g. segfaults
-	fmt.Sprintf(`--property=ExecStopPost=/bin/sh -c 'if [ "$EXIT_STATUS" = 0 ]; then touch %[1]s/fde-reveal-key.success; else echo "service result: $SERVICE_RESULT" >%[1]s/fde-reveal-key.failed; fi'`, runDir),
-	)
-	if fdeRevealKeyCommandExtra != nil {
-	cmd.Args = append(cmd.Args, fdeRevealKeyCommandExtra...)
-	}
-	// fde-reveal-key is what we actually need to run
-	cmd.Args = append(cmd.Args, "fde-reveal-key")
-
-	// ensure we cleanup our tmp files
-	defer func() {
-	if err := os.RemoveAll(runDir); err != nil {
-	logger.Noticef("cannot remove tmp dir: %v", err)
-	}
-	}()
-
-	// run the command
-	output, err = cmd.CombinedOutput()
-	if err != nil {
-	return output, err
-	}
-
-	// This loop will be terminate by systemd-run, either because
-	// fde-reveal-key exists or it gets killed when it reaches the
-	// fdeRevealKeyRuntimeMax defined above.
-	//
-	// However we are paranoid and exit this loop if systemd
-	// did not terminate the process after twice the allocated
-	// runtime
-	maxLoops := int(fdeRevealKeyRuntimeMax/fdeRevealKeyPollWait) * fdeRevealKeyPollWaitParanoiaFactor
-	for i := 0; i < maxLoops; i++ {
-	switch {
-	case osutil.FileExists(filepath.Join(runDir, "fde-reveal-key.failed")):
-	stderr, _ := ioutil.ReadFile(filepath.Join(runDir, "fde-reveal-key.stderr"))
-	systemdErr, _ := ioutil.ReadFile(filepath.Join(runDir, "fde-reveal-key.failed"))
-	buf := bytes.NewBuffer(stderr)
-	buf.Write(systemdErr)
-	return buf.Bytes(), fmt.Errorf("fde-reveal-key failed")
-	case osutil.FileExists(filepath.Join(runDir, "fde-reveal-key.success")):
-	return ioutil.ReadFile(filepath.Join(runDir, "fde-reveal-key.stdout"))
-	default:
-	time.Sleep(fdeRevealKeyPollWait)
-	}
-	}
-
-	// this should never happen, the loop above should be terminated
-	// via systemd
-	return nil, fmt.Errorf("internal error: systemd-run did not honor RuntimeMax=%s setting", fdeRevealKeyRuntimeMax)
+	return runFDEinitramfsHelper("fde-reveal-key", stdin)
 }
 
 var runFDERevealKey = runFDERevealKeyCommand