efi: change how HostEnvironment overrides EFI variables

The HostEnvironment interface contains a ReadVar method for abstracting
reads of EFI variables. The default environment just calls
efi.ReadVariable, with other implementations providing mocked variables.

The github.com/canonical/go-efilib package provides a lot more APIs now
that read from (and some that write to) EFI variables and some of these
will be used from the new efi/preinstall package (these APIs are mostly
boot and secure boot related). I wanted a way to be able to override the
environment for the preinstall.RunChecks call (particularly as this is
very useful for testing), but the existing HostEnvironment approach doesn't
work well for this.

I initially considered a couple of options:
- Make it possible to mock each of the individual calls to go-efilib
  functions via the HostEnvironment interface.
- Export varsBackend from go-efilib and provide a function in to mock
  the backend by overriding the system one.

The first approach scales poorly, so my initial approach was going to be
the second one. The problem with this though is that HostEnvironment
will be part of a public, production API, and mocking the backend is global
to the whole process. I don't think this is appropriate in production code
where it might be possible that there may be one goroutine reading EFI
variables from the system when another goroutine decides to mock the backend
based on a supplied HostEnvironment.

I've taken a different approach instead, by modifying every go-efilib
function that interacts with EFI variables to accept a context.Context
argument. The context contains a VarsBackend keyed from it, and the package
exports a DefaultVarContext symbol corresponding to the default system
backend (efivarfs if it is detected, or a null backend if it isn't
detected).

This approach permits individual call sites to override the backend to
use for individual functions that interact with EFI variables (not just
ReadVariable, WriteVariable and ListVariables, but there's a whole set
of extra functions related to secure boot and boot configuration that
make use of reading and writing EFI variables).

The ReadVar method on the HostEnvironment interface has gone, and a new
method that returns a context has been added. The internal.DefaultEnv
implementation just returns efi.DefaultVarContext.

The MockVars type in internal/efitest has been updated to implement
efi.VarsBackend, and the MockHostEnvironment implementation in
internal/efitest returns a context that keys to this implementation.

This should hopefully allow preinstall.RunChecks to be able to take a
HostEnvironment argument and benefit from mocked variables even when
interacting with variables indirectly using higher level functions
exported from go-efilib.

efi/efi_test.go

@@ -21,6 +21,7 @@ package efi_test
 
 import (
 	"bytes"
+	"context"
 	"crypto"
 	"crypto/x509"
 	"errors"
@@ -770,3 +771,15 @@ func (v *mockPcrProfileOptionVisitor) SetEnvironment(env internal.HostEnvironmen
 func (v *mockPcrProfileOptionVisitor) AddInitialVariablesModifier(fn internal.InitialVariablesModifier) {
 	v.varModifiers = append(v.varModifiers, fn)
 }
+
+type mockVarReader struct {
+	ctx context.Context
+}
+
+func newMockVarReader(env HostEnvironment) *mockVarReader {
+	return &mockVarReader{ctx: env.VarContext()}
+}
+
+func (r *mockVarReader) ReadVar(name string, guid efi.GUID) ([]byte, efi.VariableAttributes, error) {
+	return efi.ReadVariable(r.ctx, name, guid)
+}

efi/env.go

@@ -21,6 +21,7 @@ package efi
 
 import (
 	"bytes"
+	"context"
 	"crypto"
 	"crypto/sha1"
 	"encoding/binary"
@@ -37,8 +38,10 @@ import (
 // consumers of the API can provide a custom mechanism to read EFI variables or parse
 // the TCG event log.
 type HostEnvironment interface {
-	// ReadVar reads the specified EFI variable
-	ReadVar(name string, guid efi.GUID) ([]byte, efi.VariableAttributes, error)
+	// VarContext returns a context containing a VarsBackend, keyed by efi.VarsBackendKey,
+	// for interacting with EFI variables via go-efilib. This context can be passed to any
+	// go-efilib function that interacts with EFI variables.
+	VarContext() context.Context
 
 	// ReadEventLog reads the TCG event log
 	ReadEventLog() (*tcglog.Log, error)
@@ -60,7 +63,8 @@ func (o *hostEnvironmentOption) ApplyOptionTo(visitor internal.PCRProfileOptionV
 	return nil
 }
 
-// varReader is a subset of HostEnvironment that is just for EFI variables
+// varReader is an interface that provides access to reading EFI variables during
+// profile generation.
 type varReader interface {
 	// ReadVar reads the specified EFI variable
 	ReadVar(name string, guid efi.GUID) ([]byte, efi.VariableAttributes, error)
@@ -158,7 +162,7 @@ type varContents struct {
 // It consists of the host environment and updates that have been generated by previous
 // iterations of the profile generation.
 type initialVarReader struct {
-	host      varReader
+	varsCtx   context.Context
 	overrides map[efi.VariableDescriptor]varContents
 }
 
@@ -168,7 +172,7 @@ func (r *initialVarReader) ReadVar(name string, guid efi.GUID) ([]byte, efi.Vari
 	if exists {
 		return override.data, override.attrs, nil
 	}
-	return r.host.ReadVar(name, guid)
+	return efi.ReadVariable(r.varsCtx, name, guid)
 }
 
 // Key returns the unique key for this reader, and is based on the set of updates on the
@@ -208,7 +212,7 @@ func (r *initialVarReader) Key() initialVarReaderKey {
 // Copy returns a copy of this initialVarReader.
 func (r *initialVarReader) Copy() *initialVarReader {
 	out := &initialVarReader{
-		host:      r.host,
+		varsCtx:   r.varsCtx,
 		overrides: make(map[efi.VariableDescriptor]varContents)}
 	for k, v := range r.overrides {
 		out.overrides[k] = v
@@ -226,7 +230,7 @@ func (r *initialVarReader) ApplyUpdates(updates *varUpdate) error {
 
 	for _, update := range updateSlice {
 		r.overrides[update.name] = varContents{attrs: update.attrs, data: update.data}
-		origData, _, err := r.host.ReadVar(update.name.Name, update.name.GUID)
+		origData, _, err := efi.ReadVariable(r.varsCtx, update.name.Name, update.name.GUID)
 		switch {
 		case err == efi.ErrVarNotExist:
 			// ok
@@ -247,7 +251,7 @@ func (r *initialVarReader) ApplyUpdates(updates *varUpdate) error {
 // add more starting states as some branches have paths that update EFI variables
 // (such as applying SBAT updates) which may have an effect on other branches.
 type variableSetCollector struct {
-	host varReader // the externally supplied host environment
+	varsCtx context.Context // a context containing the backend for reading EFI variables
 
 	seen map[initialVarReaderKey]struct{} // known starting states
 
@@ -256,17 +260,18 @@ type variableSetCollector struct {
 	todo []*initialVarReader
 }
 
-func newVariableSetCollector(vars varReader) *variableSetCollector {
+func newVariableSetCollector(env HostEnvironment) *variableSetCollector {
+	varsCtx := env.VarContext()
 	return &variableSetCollector{
-		host: vars,
-		seen: map[initialVarReaderKey]struct{}{initialVarReaderKey{}: struct{}{}}, // add the current environment
+		varsCtx: varsCtx,
+		seen:    map[initialVarReaderKey]struct{}{initialVarReaderKey{}: struct{}{}}, // add the current environment
 		todo: []*initialVarReader{
 			&initialVarReader{
-				host:      vars,
+				varsCtx:   varsCtx,
 				overrides: make(map[efi.VariableDescriptor]varContents)}}} // add the current environment
 }
 
-// registerUpdateFor is called when a branch updates an EFI variable. This will
+// registerUpdateFor is called when a branch updxates an EFI variable. This will
 // queue new starting states to process if the updated state hasn't already been
 // processed. This should be called on every update - this ensures that if a branch
 // makes more than one update, the generated profile will be valid for intermediate

efi/export_test.go

@@ -192,7 +192,7 @@ func MockSnapdenvTesting(testing bool) (restore func()) {
 
 func NewInitialVarReader(host HostEnvironment) *initialVarReader {
 	return &initialVarReader{
-		host:      host,
+		varsCtx:   host.VarContext(),
 		overrides: make(map[efi.VariableDescriptor]varContents)}
 }
 

efi/internal/default_env.go

@@ -20,6 +20,7 @@
 package internal
 
 import (
+	"context"
 	"os"
 
 	efi "github.com/canonical/go-efilib"
@@ -28,15 +29,16 @@ import (
 
 var (
 	eventLogPath = "/sys/kernel/security/tpm0/binary_bios_measurements" // Path of the TCG event log for the default TPM, in binary form
-	readVar      = efi.ReadVariable
 )
 
 type defaultEnvImpl struct{}
 
-func (e defaultEnvImpl) ReadVar(name string, guid efi.GUID) ([]byte, efi.VariableAttributes, error) {
-	return readVar(name, guid)
+// VarContext implements [HostEnvironment.VarContext].
+func (e defaultEnvImpl) VarContext() context.Context {
+	return efi.DefaultVarContext
 }
 
+// ReadEventLog implements [HostEnvironment.ReadEventLog].
 func (e defaultEnvImpl) ReadEventLog() (*tcglog.Log, error) {
 	f, err := os.Open(eventLogPath)
 	if err != nil {

efi/internal/default_env_test.go

@@ -30,91 +30,16 @@ import (
 	"github.com/canonical/tcglog-parser"
 	. "github.com/snapcore/secboot/efi/internal"
 	"github.com/snapcore/secboot/internal/efitest"
-	"github.com/snapcore/secboot/internal/testutil"
 
 	. "gopkg.in/check.v1"
 )
 
-var (
-	//go:embed MicrosoftKEK.crt
-	msKEKCertPEM []byte
-
-	msKEKCert []byte
-)
-
-func init() {
-	msKEKCert = testutil.MustDecodePEMType("CERTIFICATE", msKEKCertPEM)
-}
-
 type defaultEnvSuite struct{}
 
 var _ = Suite(&defaultEnvSuite{})
 
-type testReadVarData struct {
-	name string
-	guid efi.GUID
-}
-
-func (s *defaultEnvSuite) testReadVar(c *C, data *testReadVarData) {
-	ownerGuid := efi.MakeGUID(0x77fa9abd, 0x0359, 0x4d32, 0xbd60, [...]uint8{0x28, 0xf4, 0xe7, 0x8f, 0x78, 0x4b})
-	kek := &efi.SignatureList{
-		Type: efi.CertX509Guid,
-		Signatures: []*efi.SignatureData{
-			{
-				Owner: ownerGuid,
-				Data:  msKEKCert,
-			},
-		},
-	}
-	dbx := efitest.NewSignatureListNullSHA256(ownerGuid)
-	vars := efitest.MakeMockVars()
-	vars.SetSecureBoot(true)
-	vars.SetKEK(c, efi.SignatureDatabase{kek})
-	vars.SetDbx(c, efi.SignatureDatabase{dbx})
-
-	restore := MockReadVar(func(name string, guid efi.GUID) ([]byte, efi.VariableAttributes, error) {
-		entry, exists := vars[efi.VariableDescriptor{Name: name, GUID: guid}]
-		if !exists {
-			return nil, 0, efi.ErrVarNotExist
-		}
-		return entry.Payload, entry.Attrs, nil
-	})
-	defer restore()
-
-	payload, attrs, err := DefaultEnv.ReadVar(data.name, data.guid)
-
-	entry, exists := vars[efi.VariableDescriptor{Name: data.name, GUID: data.guid}]
-	if !exists {
-		c.Check(err, Equals, efi.ErrVarNotExist)
-	} else {
-		c.Check(err, IsNil)
-		c.Check(attrs, Equals, entry.Attrs)
-		c.Check(payload, DeepEquals, entry.Payload)
-	}
-}
-
-func (s *defaultEnvSuite) TestReadVar1(c *C) {
-	s.testReadVar(c, &testReadVarData{
-		name: "SecureBoot",
-		guid: efi.GlobalVariable})
-}
-
-func (s *defaultEnvSuite) TestReadVar2(c *C) {
-	s.testReadVar(c, &testReadVarData{
-		name: "KEK",
-		guid: efi.GlobalVariable})
-}
-
-func (s *defaultEnvSuite) TestReadVar3(c *C) {
-	s.testReadVar(c, &testReadVarData{
-		name: "dbx",
-		guid: efi.ImageSecurityDatabaseGuid})
-}
-
-func (s *defaultEnvSuite) TestReadVarNotExist(c *C) {
-	s.testReadVar(c, &testReadVarData{
-		name: "SecureBoot",
-		guid: efi.ImageSecurityDatabaseGuid})
+func (s *defaultEnvSuite) TestVarContext(c *C) {
+	c.Check(DefaultEnv.VarContext(), Equals, efi.DefaultVarContext)
 }
 
 func (s *defaultEnvSuite) testReadEventLog(c *C, opts *efitest.LogOptions) {

efi/internal/env.go

@@ -0,0 +1,39 @@
+// -*- Mode: Go; indent-tabs-mode: t -*-
+
+/*
+ * Copyright (C) 2024 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 internal
+
+import (
+	"context"
+
+	"github.com/canonical/tcglog-parser"
+)
+
+// HostEnvironment is an interface that abstracts out an EFI environment, so that
+// consumers of the API can provide a custom mechanism to read EFI variables or parse
+// the TCG event log. This needs to be kept in sync with [efi.HostEnvironment].
+type HostEnvironment interface {
+	// VarContext returns a context containing a VarsBackend, keyed by efi.VarsBackendKey,
+	// for interacting with EFI variables via go-efilib. This context can be passed to any
+	// go-efilib function that interacts with EFI variables.
+	VarContext() context.Context
+
+	// ReadEventLog reads the TCG event log
+	ReadEventLog() (*tcglog.Log, error)
+}

efi/internal/export_test.go

@@ -19,20 +19,10 @@
 
 package internal
 
-import efi "github.com/canonical/go-efilib"
-
 func MockEventLogPath(path string) (restore func()) {
 	origPath := eventLogPath
 	eventLogPath = path
 	return func() {
 		eventLogPath = origPath
 	}
 }
-
-func MockReadVar(fn func(string, efi.GUID) ([]byte, efi.VariableAttributes, error)) (restore func()) {
-	origReadVar := readVar
-	readVar = fn
-	return func() {
-		readVar = origReadVar
-	}
-}

efi/internal/options.go

@@ -22,7 +22,6 @@ package internal
 import (
 	efi "github.com/canonical/go-efilib"
 	"github.com/canonical/go-tpm2"
-	"github.com/canonical/tcglog-parser"
 )
 
 type InitialVariablesModifier func(VariableSet) error
@@ -39,17 +38,6 @@ type PCRProfileOptionVisitor interface {
 	AddInitialVariablesModifier(fn InitialVariablesModifier)
 }
 
-// HostEnvironment is an interface that abstracts out an EFI environment, so that
-// consumers of the API can provide a custom mechanism to read EFI variables or parse
-// the TCG event log. This needs to be kept in sync with [efi.HostEnvironment].
-type HostEnvironment interface {
-	// ReadVar reads the specified EFI variable
-	ReadVar(name string, guid efi.GUID) ([]byte, efi.VariableAttributes, error)
-
-	// ReadEventLog reads the TCG event log
-	ReadEventLog() (*tcglog.Log, error)
-}
-
 // VariableSet corresponds to a set of EFI variables.
 type VariableSet interface {
 	// ReadVar reads the specified EFI variable for this set.

efi/shim_test.go

@@ -39,7 +39,7 @@ func (s *shimSuite) testReadShimSbatPolicy(c *C, data []byte, expected ShimSbatP
 	env := efitest.NewMockHostEnvironment(efitest.MockVars{
 		{Name: "SbatPolicy", GUID: ShimGuid}: {Payload: data, Attrs: efi.AttributeNonVolatile | efi.AttributeBootserviceAccess | efi.AttributeRuntimeAccess},
 	}, nil)
-	policy, err := ReadShimSbatPolicy(env)
+	policy, err := ReadShimSbatPolicy(newMockVarReader(env))
 	if err != nil {
 		return err
 	}
@@ -59,7 +59,7 @@ func (s *shimSuite) TestReadShimSbatPolicyLatest(c *C) {
 
 func (s *shimSuite) TestReadShimSbatPolicyNotExist(c *C) {
 	env := efitest.NewMockHostEnvironment(nil, nil)
-	policy, err := ReadShimSbatPolicy(env)
+	policy, err := ReadShimSbatPolicy(newMockVarReader(env))
 	c.Check(err, IsNil)
 	c.Check(policy, Equals, ShimSbatPolicyPrevious)
 }
@@ -425,7 +425,7 @@ func (s *shimSuite) TestShimSbatPolicyLatestFromPrevious(c *C) {
 
 	c.Assert(visitor.varModifiers, HasLen, 1)
 
-	collector := NewVariableSetCollector(efitest.NewMockHostEnvironment(efitest.MakeMockVars().Set("SbatPolicy", ShimGuid, efi.AttributeNonVolatile|efi.AttributeBootserviceAccess|efi.AttributeRuntimeAccess, []byte{0x2}), nil))
+	collector := NewVariableSetCollector(efitest.NewMockHostEnvironment(efitest.MakeMockVars().AddVar("SbatPolicy", ShimGuid, efi.AttributeNonVolatile|efi.AttributeBootserviceAccess|efi.AttributeRuntimeAccess, []byte{0x2}), nil))
 	c.Check(visitor.varModifiers[0](collector.PeekAll()[0]), IsNil)
 
 	c.Assert(collector.More(), testutil.IsTrue)
@@ -452,7 +452,7 @@ func (s *shimSuite) TestShimSbatPolicyLatestFromLatest(c *C) {
 
 	c.Assert(visitor.varModifiers, HasLen, 1)
 
-	collector := NewVariableSetCollector(efitest.NewMockHostEnvironment(efitest.MakeMockVars().Set("SbatPolicy", ShimGuid, efi.AttributeNonVolatile|efi.AttributeBootserviceAccess|efi.AttributeRuntimeAccess, []byte{0x1}), nil))
+	collector := NewVariableSetCollector(efitest.NewMockHostEnvironment(efitest.MakeMockVars().AddVar("SbatPolicy", ShimGuid, efi.AttributeNonVolatile|efi.AttributeBootserviceAccess|efi.AttributeRuntimeAccess, []byte{0x1}), nil))
 	c.Check(visitor.varModifiers[0](collector.PeekAll()[0]), IsNil)
 
 	c.Assert(collector.More(), testutil.IsTrue)

efi/vars_test.go

@@ -126,13 +126,13 @@ func withTestSecureBootConfig() mockVarsConfig {
 
 func withSbatLevel(level []byte) mockVarsConfig {
 	return func(c *C, vars efitest.MockVars) {
-		vars.Set("SbatLevelRT", ShimGuid, efi.AttributeBootserviceAccess|efi.AttributeRuntimeAccess, level)
+		vars.AddVar("SbatLevelRT", ShimGuid, efi.AttributeBootserviceAccess|efi.AttributeRuntimeAccess, level)
 	}
 }
 
 func withSbatPolicy(policy ShimSbatPolicy) mockVarsConfig {
 	return func(c *C, vars efitest.MockVars) {
-		vars.Set("SbatPolicy", ShimGuid, efi.AttributeNonVolatile|efi.AttributeBootserviceAccess|efi.AttributeRuntimeAccess, []byte{uint8(policy)})
+		vars.AddVar("SbatPolicy", ShimGuid, efi.AttributeNonVolatile|efi.AttributeBootserviceAccess|efi.AttributeRuntimeAccess, []byte{uint8(policy)})
 	}
 }