From aaba2c578f0179601096543385e679cd53ca2336 Mon Sep 17 00:00:00 2001 From: Chris Coulson Date: Fri, 21 Jun 2024 19:26:47 +0100 Subject: [PATCH 1/4] 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 | 13 +++++ efi/env.go | 31 +++++++----- efi/export_test.go | 2 +- efi/internal/default_env.go | 8 ++-- efi/internal/default_env_test.go | 79 +------------------------------ efi/internal/env.go | 39 +++++++++++++++ efi/internal/export_test.go | 10 ---- efi/internal/options.go | 12 ----- efi/shim_test.go | 8 ++-- efi/vars_test.go | 4 +- go.mod | 2 +- go.sum | 2 + internal/efitest/hostenv.go | 14 ++---- internal/efitest/vars.go | 51 ++++++++++++++++---- tools/gen-compattest-data/main.go | 5 +- 15 files changed, 137 insertions(+), 143 deletions(-) create mode 100644 efi/internal/env.go diff --git a/efi/efi_test.go b/efi/efi_test.go index 5be3c25a..2d14699e 100644 --- a/efi/efi_test.go +++ b/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) +} diff --git a/efi/env.go b/efi/env.go index 793bbcad..2019c23a 100644 --- a/efi/env.go +++ b/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 diff --git a/efi/export_test.go b/efi/export_test.go index 983d2f15..b6dcaabb 100644 --- a/efi/export_test.go +++ b/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)} } diff --git a/efi/internal/default_env.go b/efi/internal/default_env.go index e792cfde..849c5a3a 100644 --- a/efi/internal/default_env.go +++ b/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 { diff --git a/efi/internal/default_env_test.go b/efi/internal/default_env_test.go index 0fbc2215..dadb8dc2 100644 --- a/efi/internal/default_env_test.go +++ b/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) { diff --git a/efi/internal/env.go b/efi/internal/env.go new file mode 100644 index 00000000..376540a5 --- /dev/null +++ b/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 . + * + */ + +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) +} diff --git a/efi/internal/export_test.go b/efi/internal/export_test.go index f2191a36..d31a4fde 100644 --- a/efi/internal/export_test.go +++ b/efi/internal/export_test.go @@ -19,8 +19,6 @@ package internal -import efi "github.com/canonical/go-efilib" - func MockEventLogPath(path string) (restore func()) { origPath := eventLogPath eventLogPath = path @@ -28,11 +26,3 @@ func MockEventLogPath(path string) (restore func()) { eventLogPath = origPath } } - -func MockReadVar(fn func(string, efi.GUID) ([]byte, efi.VariableAttributes, error)) (restore func()) { - origReadVar := readVar - readVar = fn - return func() { - readVar = origReadVar - } -} diff --git a/efi/internal/options.go b/efi/internal/options.go index 3402dac4..05b19dc3 100644 --- a/efi/internal/options.go +++ b/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. diff --git a/efi/shim_test.go b/efi/shim_test.go index 3fba7798..f232850c 100644 --- a/efi/shim_test.go +++ b/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) diff --git a/efi/vars_test.go b/efi/vars_test.go index 6c4a2f93..bf335818 100644 --- a/efi/vars_test.go +++ b/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)}) } } diff --git a/go.mod b/go.mod index ad02119a..8ec9bb5d 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/snapcore/secboot go 1.18 require ( - github.com/canonical/go-efilib v0.9.10-0.20240618094320-cc6dd01c07dc + github.com/canonical/go-efilib v1.0.0 github.com/canonical/go-sp800.108-kdf v0.0.0-20210315104021-ead800bbf9a0 github.com/canonical/go-sp800.90a-drbg v0.0.0-20210314144037-6eeb1040d6c3 github.com/canonical/go-tpm2 v1.3.0 diff --git a/go.sum b/go.sum index ef8a63a5..68fb7f47 100644 --- a/go.sum +++ b/go.sum @@ -10,6 +10,8 @@ github.com/canonical/go-efilib v0.9.9 h1:MAVp6SOrAIsymEfUpYKOyQfu2hIm8dQ46zaG+8F github.com/canonical/go-efilib v0.9.9/go.mod h1:n0Ttsy1JuHAvqaFbZBs6PAzoiiJdfkHsAmDOEbexYEQ= github.com/canonical/go-efilib v0.9.10-0.20240618094320-cc6dd01c07dc h1:U2xZoSBOhzYtWFJ6MhSSbIlrG8wAcqSqZhejLEimJMg= github.com/canonical/go-efilib v0.9.10-0.20240618094320-cc6dd01c07dc/go.mod h1:n0Ttsy1JuHAvqaFbZBs6PAzoiiJdfkHsAmDOEbexYEQ= +github.com/canonical/go-efilib v1.0.0 h1:da3shbdfbJgIUzq4qspkNdWccV5lNMi0ZeYksVCoCvg= +github.com/canonical/go-efilib v1.0.0/go.mod h1:n0Ttsy1JuHAvqaFbZBs6PAzoiiJdfkHsAmDOEbexYEQ= github.com/canonical/go-sp800.108-kdf v0.0.0-20210314145419-a3359f2d21b9/go.mod h1:Zrs3YjJr+w51u0R/dyLh/oWt/EcBVdLPCVFYC4daW5s= github.com/canonical/go-sp800.108-kdf v0.0.0-20210315104021-ead800bbf9a0 h1:ZE2XMRFHcwlib3uU9is37+pKkkMloVoEPWmgQ6GK1yo= github.com/canonical/go-sp800.108-kdf v0.0.0-20210315104021-ead800bbf9a0/go.mod h1:Zrs3YjJr+w51u0R/dyLh/oWt/EcBVdLPCVFYC4daW5s= diff --git a/internal/efitest/hostenv.go b/internal/efitest/hostenv.go index 0ced4a2a..82bed7fd 100644 --- a/internal/efitest/hostenv.go +++ b/internal/efitest/hostenv.go @@ -20,6 +20,7 @@ package efitest import ( + "context" "errors" efi "github.com/canonical/go-efilib" @@ -39,16 +40,9 @@ func NewMockHostEnvironment(vars MockVars, log *tcglog.Log) *MockHostEnvironment Log: log} } -// ReadVar implements [github.com/snapcore/secboot/efi.HostEnvironment.ReadVar]. -func (e *MockHostEnvironment) ReadVar(name string, guid efi.GUID) ([]byte, efi.VariableAttributes, error) { - if e.Vars == nil { - return nil, 0, efi.ErrVarNotExist - } - entry, found := e.Vars[efi.VariableDescriptor{Name: name, GUID: guid}] - if !found { - return nil, 0, efi.ErrVarNotExist - } - return entry.Payload, entry.Attrs, nil +// VarContext implements [github.com/snapcore/secboot/efi.HostEnvironment.VarContext]/ +func (e *MockHostEnvironment) VarContext() context.Context { + return context.WithValue(context.Background(), efi.VarsBackendKey{}, e.Vars) } // ReadEventLog implements [github.com/snapcore/secboot/efi.HostEnvironment.ReadEventLog]. diff --git a/internal/efitest/vars.go b/internal/efitest/vars.go index 795073b7..b4d179ec 100644 --- a/internal/efitest/vars.go +++ b/internal/efitest/vars.go @@ -22,6 +22,7 @@ package efitest import ( "bytes" _ "crypto/sha256" + "errors" "io" efi "github.com/canonical/go-efilib" @@ -54,12 +55,35 @@ func MakeMockVars() MockVars { return make(MockVars) } -func (v MockVars) Set(name string, guid efi.GUID, attrs efi.VariableAttributes, data []byte) MockVars { +// Get implements [efi.VarsBackend.Get]. +func (v MockVars) Get(name string, guid efi.GUID) (efi.VariableAttributes, []byte, error) { + entry, found := v[efi.VariableDescriptor{Name: name, GUID: guid}] + if !found { + return 0, nil, efi.ErrVarNotExist + } + return entry.Attrs, entry.Payload, nil +} + +// Set implements [efi.VarsBackend.Set]. +func (v MockVars) Set(name string, guid efi.GUID, attrs efi.VariableAttributes, data []byte) error { + return errors.New("not implemented") +} + +// List implements [efi.VarsBackend.List]. +func (v MockVars) List() ([]efi.VariableDescriptor, error) { + return nil, errors.New("not implemented") +} + +// AddVar adds the specified mock variable. If the attributes indicate that the variable is authenticated, +// the data should not include the authentication header. +func (v MockVars) AddVar(name string, guid efi.GUID, attrs efi.VariableAttributes, data []byte) MockVars { v[efi.VariableDescriptor{Name: name, GUID: guid}] = &VarEntry{Attrs: attrs, Payload: data} return v } -func (v MockVars) Append(name string, guid efi.GUID, attrs efi.VariableAttributes, data []byte) MockVars { +// AppendVar appends data to the specified mock variable. If the attributes indicate that the variable is +// authenticated, the data should not include the authentication header. +func (v MockVars) AppendVar(name string, guid efi.GUID, attrs efi.VariableAttributes, data []byte) MockVars { desc := efi.VariableDescriptor{Name: name, GUID: guid} entry, exists := v[desc] if !exists { @@ -70,34 +94,43 @@ func (v MockVars) Append(name string, guid efi.GUID, attrs efi.VariableAttribute return v } +// SetSecureBoot sets the status of the SecureBoot global variable. func (v MockVars) SetSecureBoot(enabled bool) MockVars { var sbVal byte if enabled { sbVal = 0x01 } - return v.Set("SecureBoot", efi.GlobalVariable, efi.AttributeBootserviceAccess|efi.AttributeRuntimeAccess, []byte{sbVal}) + return v.AddVar("SecureBoot", efi.GlobalVariable, efi.AttributeBootserviceAccess|efi.AttributeRuntimeAccess, []byte{sbVal}) } +// SetPK sets the PK global variable. func (v MockVars) SetPK(c *C, pk *efi.SignatureList) MockVars { - return v.Set("PK", efi.GlobalVariable, efi.AttributeNonVolatile|efi.AttributeBootserviceAccess|efi.AttributeRuntimeAccess|efi.AttributeTimeBasedAuthenticatedWriteAccess, MakeVarPayload(c, pk)) + return v.AddVar("PK", efi.GlobalVariable, efi.AttributeNonVolatile|efi.AttributeBootserviceAccess|efi.AttributeRuntimeAccess|efi.AttributeTimeBasedAuthenticatedWriteAccess, MakeVarPayload(c, pk)) } +// SetKEK sets the KEK global variable. func (v MockVars) SetKEK(c *C, kek efi.SignatureDatabase) MockVars { - return v.Set("KEK", efi.GlobalVariable, efi.AttributeNonVolatile|efi.AttributeBootserviceAccess|efi.AttributeRuntimeAccess|efi.AttributeTimeBasedAuthenticatedWriteAccess, MakeVarPayload(c, kek)) + return v.AddVar("KEK", efi.GlobalVariable, efi.AttributeNonVolatile|efi.AttributeBootserviceAccess|efi.AttributeRuntimeAccess|efi.AttributeTimeBasedAuthenticatedWriteAccess, MakeVarPayload(c, kek)) } +// SetDb sets the db image authentication variable. func (v MockVars) SetDb(c *C, db efi.SignatureDatabase) MockVars { - return v.Set("db", efi.ImageSecurityDatabaseGuid, efi.AttributeNonVolatile|efi.AttributeBootserviceAccess|efi.AttributeRuntimeAccess|efi.AttributeTimeBasedAuthenticatedWriteAccess, MakeVarPayload(c, db)) + return v.AddVar("db", efi.ImageSecurityDatabaseGuid, efi.AttributeNonVolatile|efi.AttributeBootserviceAccess|efi.AttributeRuntimeAccess|efi.AttributeTimeBasedAuthenticatedWriteAccess, MakeVarPayload(c, db)) } +// AppendDb appends to the db image authentication variable. Note that this just appends +// bytes - it does no de-duplication of signatures. func (v MockVars) AppendDb(c *C, db efi.SignatureDatabase) MockVars { - return v.Append("db", efi.ImageSecurityDatabaseGuid, efi.AttributeNonVolatile|efi.AttributeBootserviceAccess|efi.AttributeRuntimeAccess|efi.AttributeTimeBasedAuthenticatedWriteAccess, MakeVarPayload(c, db)) + return v.AppendVar("db", efi.ImageSecurityDatabaseGuid, efi.AttributeNonVolatile|efi.AttributeBootserviceAccess|efi.AttributeRuntimeAccess|efi.AttributeTimeBasedAuthenticatedWriteAccess, MakeVarPayload(c, db)) } +// SetDbx sets the dbx image authentication variable. func (v MockVars) SetDbx(c *C, dbx efi.SignatureDatabase) MockVars { - return v.Set("dbx", efi.ImageSecurityDatabaseGuid, efi.AttributeNonVolatile|efi.AttributeBootserviceAccess|efi.AttributeRuntimeAccess|efi.AttributeTimeBasedAuthenticatedWriteAccess, MakeVarPayload(c, dbx)) + return v.AddVar("dbx", efi.ImageSecurityDatabaseGuid, efi.AttributeNonVolatile|efi.AttributeBootserviceAccess|efi.AttributeRuntimeAccess|efi.AttributeTimeBasedAuthenticatedWriteAccess, MakeVarPayload(c, dbx)) } +// AppendDbx appends to the dbx image authentication variable. Note that this just appends +// bytes - it does no de-duplication of signatures. func (v MockVars) AppendDbx(c *C, dbx efi.SignatureDatabase) MockVars { - return v.Append("dbx", efi.ImageSecurityDatabaseGuid, efi.AttributeNonVolatile|efi.AttributeBootserviceAccess|efi.AttributeRuntimeAccess|efi.AttributeTimeBasedAuthenticatedWriteAccess, MakeVarPayload(c, dbx)) + return v.AppendVar("dbx", efi.ImageSecurityDatabaseGuid, efi.AttributeNonVolatile|efi.AttributeBootserviceAccess|efi.AttributeRuntimeAccess|efi.AttributeTimeBasedAuthenticatedWriteAccess, MakeVarPayload(c, dbx)) } diff --git a/tools/gen-compattest-data/main.go b/tools/gen-compattest-data/main.go index 3e4705c1..2652fed2 100644 --- a/tools/gen-compattest-data/main.go +++ b/tools/gen-compattest-data/main.go @@ -154,7 +154,10 @@ func run() int { // TODO: This data was deleted in https://github.com/snapcore/secboot/pull/156 and // https://github.com/snapcore/secboot/pull/274. - env := &mockEFIEnvironment{"efi/testdata/efivars2", "efi/testdata/eventlog1.bin"} + // The env is only used in computePCRProtectionProfile, which currently just returns + // an error. + //env := &mockEFIEnvironment{"efi/testdata/efivars2", "efi/testdata/eventlog1.bin"} + var env secboot_efi.HostEnvironment tpm, err := secboot_tpm2.ConnectToDefaultTPM() if err != nil { From 1c78a22ff4688a080a9d362fe74a39bd498394d6 Mon Sep 17 00:00:00 2001 From: Chris Coulson Date: Mon, 8 Jul 2024 21:30:19 +0100 Subject: [PATCH 2/4] efi: Allow HostEnvironment.VarContext to take a parent context --- efi/efi_test.go | 2 +- efi/env.go | 21 ++++++++++++++++++--- efi/export_test.go | 4 +++- efi/internal/default_env.go | 4 ++-- efi/internal/default_env_test.go | 7 ++++++- efi/internal/env.go | 9 +++++++-- go.mod | 2 +- go.sum | 4 ++++ internal/efitest/hostenv.go | 4 ++-- 9 files changed, 44 insertions(+), 13 deletions(-) diff --git a/efi/efi_test.go b/efi/efi_test.go index 2d14699e..5f33c29a 100644 --- a/efi/efi_test.go +++ b/efi/efi_test.go @@ -777,7 +777,7 @@ type mockVarReader struct { } func newMockVarReader(env HostEnvironment) *mockVarReader { - return &mockVarReader{ctx: env.VarContext()} + return &mockVarReader{ctx: env.VarContext(context.Background())} } func (r *mockVarReader) ReadVar(name string, guid efi.GUID) ([]byte, efi.VariableAttributes, error) { diff --git a/efi/env.go b/efi/env.go index 2019c23a..885ffee2 100644 --- a/efi/env.go +++ b/efi/env.go @@ -40,8 +40,13 @@ import ( 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 + // go-efilib function that interacts with EFI variables. Right now, go-efilib doesn't + // support any other uses of the context such as cancelation or deadlines. The efivarfs + // backend will support this eventually for variable writes because it currently implements + // a retry loop that has a potential to race with other processes. Cancelation and / or + // deadlines for sections of code that performs multiple reads using efivarfs may be useful + // in the future because the kernel rate-limits reads per-user. + VarContext(parent context.Context) context.Context // ReadEventLog reads the TCG event log ReadEventLog() (*tcglog.Log, error) @@ -261,7 +266,17 @@ type variableSetCollector struct { } func newVariableSetCollector(env HostEnvironment) *variableSetCollector { - varsCtx := env.VarContext() + // We pass the TODO context here for now, as the context just essentially + // a container for holding the variable backend, so there's no need for + // the caller to override it. + // + // In the future, the efivarfs backend will make use of cancellation and / or + // deadlines it because it runs a retry loop that can race with other processes, + // although this package is doing no writing. Cancellation and / or deadlines + // may also still be useful in the read path especially where a section of code + // performs multiple reads via efivarfs - these reads are rate-limitted per-user + // in the kernel. + varsCtx := env.VarContext(context.TODO()) return &variableSetCollector{ varsCtx: varsCtx, seen: map[initialVarReaderKey]struct{}{initialVarReaderKey{}: struct{}{}}, // add the current environment diff --git a/efi/export_test.go b/efi/export_test.go index b6dcaabb..c0da28d8 100644 --- a/efi/export_test.go +++ b/efi/export_test.go @@ -20,6 +20,8 @@ package efi import ( + "context" + efi "github.com/canonical/go-efilib" "github.com/canonical/tcglog-parser" "github.com/snapcore/secboot/efi/internal" @@ -192,7 +194,7 @@ func MockSnapdenvTesting(testing bool) (restore func()) { func NewInitialVarReader(host HostEnvironment) *initialVarReader { return &initialVarReader{ - varsCtx: host.VarContext(), + varsCtx: host.VarContext(context.TODO()), overrides: make(map[efi.VariableDescriptor]varContents)} } diff --git a/efi/internal/default_env.go b/efi/internal/default_env.go index 849c5a3a..7b35c201 100644 --- a/efi/internal/default_env.go +++ b/efi/internal/default_env.go @@ -34,8 +34,8 @@ var ( type defaultEnvImpl struct{} // VarContext implements [HostEnvironment.VarContext]. -func (e defaultEnvImpl) VarContext() context.Context { - return efi.DefaultVarContext +func (e defaultEnvImpl) VarContext(parent context.Context) context.Context { + return efi.WithDefaultVarsBackend(parent) } // ReadEventLog implements [HostEnvironment.ReadEventLog]. diff --git a/efi/internal/default_env_test.go b/efi/internal/default_env_test.go index dadb8dc2..a9f42a04 100644 --- a/efi/internal/default_env_test.go +++ b/efi/internal/default_env_test.go @@ -20,6 +20,7 @@ package internal_test import ( + "context" _ "embed" "io" "os" @@ -39,7 +40,11 @@ type defaultEnvSuite struct{} var _ = Suite(&defaultEnvSuite{}) func (s *defaultEnvSuite) TestVarContext(c *C) { - c.Check(DefaultEnv.VarContext(), Equals, efi.DefaultVarContext) + ctx := DefaultEnv.VarContext(context.Background()) + c.Assert(ctx, NotNil) + + expected := efi.WithDefaultVarsBackend(context.Background()) + c.Check(ctx.Value(efi.VarsBackendKey{}), Equals, expected.Value(efi.VarsBackendKey{})) } func (s *defaultEnvSuite) testReadEventLog(c *C, opts *efitest.LogOptions) { diff --git a/efi/internal/env.go b/efi/internal/env.go index 376540a5..453c3961 100644 --- a/efi/internal/env.go +++ b/efi/internal/env.go @@ -31,8 +31,13 @@ import ( 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 + // go-efilib function that interacts with EFI variables. Right now, go-efilib doesn't + // support any other uses of the context such as cancelation or deadlines. The efivarfs + // backend will support this eventually for variable writes because it currently implements + // a retry loop that has a potential to race with other processes. Cancelation and / or + // deadlines for sections of code that performs multiple reads using efivarfs may be useful + // in the future because the kernel rate-limits reads per-user. + VarContext(parent context.Context) context.Context // ReadEventLog reads the TCG event log ReadEventLog() (*tcglog.Log, error) diff --git a/go.mod b/go.mod index 8ec9bb5d..86d54cc9 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/snapcore/secboot go 1.18 require ( - github.com/canonical/go-efilib v1.0.0 + github.com/canonical/go-efilib v1.2.0 github.com/canonical/go-sp800.108-kdf v0.0.0-20210315104021-ead800bbf9a0 github.com/canonical/go-sp800.90a-drbg v0.0.0-20210314144037-6eeb1040d6c3 github.com/canonical/go-tpm2 v1.3.0 diff --git a/go.sum b/go.sum index 68fb7f47..e9a0914d 100644 --- a/go.sum +++ b/go.sum @@ -12,6 +12,10 @@ github.com/canonical/go-efilib v0.9.10-0.20240618094320-cc6dd01c07dc h1:U2xZoSBO github.com/canonical/go-efilib v0.9.10-0.20240618094320-cc6dd01c07dc/go.mod h1:n0Ttsy1JuHAvqaFbZBs6PAzoiiJdfkHsAmDOEbexYEQ= github.com/canonical/go-efilib v1.0.0 h1:da3shbdfbJgIUzq4qspkNdWccV5lNMi0ZeYksVCoCvg= github.com/canonical/go-efilib v1.0.0/go.mod h1:n0Ttsy1JuHAvqaFbZBs6PAzoiiJdfkHsAmDOEbexYEQ= +github.com/canonical/go-efilib v1.1.0 h1:14CRXLfczZFthTfp2MQBLRz91F2zx8F5jP/YOlJaeDk= +github.com/canonical/go-efilib v1.1.0/go.mod h1:n0Ttsy1JuHAvqaFbZBs6PAzoiiJdfkHsAmDOEbexYEQ= +github.com/canonical/go-efilib v1.2.0 h1:+fvJdkj3oVyURFtfk8gSft6pdKyVzzdzNn9GC1kMJw8= +github.com/canonical/go-efilib v1.2.0/go.mod h1:n0Ttsy1JuHAvqaFbZBs6PAzoiiJdfkHsAmDOEbexYEQ= github.com/canonical/go-sp800.108-kdf v0.0.0-20210314145419-a3359f2d21b9/go.mod h1:Zrs3YjJr+w51u0R/dyLh/oWt/EcBVdLPCVFYC4daW5s= github.com/canonical/go-sp800.108-kdf v0.0.0-20210315104021-ead800bbf9a0 h1:ZE2XMRFHcwlib3uU9is37+pKkkMloVoEPWmgQ6GK1yo= github.com/canonical/go-sp800.108-kdf v0.0.0-20210315104021-ead800bbf9a0/go.mod h1:Zrs3YjJr+w51u0R/dyLh/oWt/EcBVdLPCVFYC4daW5s= diff --git a/internal/efitest/hostenv.go b/internal/efitest/hostenv.go index 82bed7fd..d9ec5e0b 100644 --- a/internal/efitest/hostenv.go +++ b/internal/efitest/hostenv.go @@ -41,8 +41,8 @@ func NewMockHostEnvironment(vars MockVars, log *tcglog.Log) *MockHostEnvironment } // VarContext implements [github.com/snapcore/secboot/efi.HostEnvironment.VarContext]/ -func (e *MockHostEnvironment) VarContext() context.Context { - return context.WithValue(context.Background(), efi.VarsBackendKey{}, e.Vars) +func (e *MockHostEnvironment) VarContext(parent context.Context) context.Context { + return context.WithValue(parent, efi.VarsBackendKey{}, e.Vars) } // ReadEventLog implements [github.com/snapcore/secboot/efi.HostEnvironment.ReadEventLog]. From 04110a4e09e4d9f5eeda791c770c8c4071c5425b Mon Sep 17 00:00:00 2001 From: Chris Coulson Date: Tue, 9 Jul 2024 21:34:15 +0100 Subject: [PATCH 3/4] efi: update doc comment for HostEnvironment.VarContext --- efi/env.go | 2 +- efi/internal/env.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/efi/env.go b/efi/env.go index 885ffee2..76894b0c 100644 --- a/efi/env.go +++ b/efi/env.go @@ -38,7 +38,7 @@ import ( // consumers of the API can provide a custom mechanism to read EFI variables or parse // the TCG event log. type HostEnvironment interface { - // VarContext returns a context containing a VarsBackend, keyed by efi.VarsBackendKey, + // VarContext returns a copy of parent 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. Right now, go-efilib doesn't // support any other uses of the context such as cancelation or deadlines. The efivarfs diff --git a/efi/internal/env.go b/efi/internal/env.go index 453c3961..1d742d11 100644 --- a/efi/internal/env.go +++ b/efi/internal/env.go @@ -29,7 +29,7 @@ import ( // 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, + // VarContext returns a copy of parent 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. Right now, go-efilib doesn't // support any other uses of the context such as cancelation or deadlines. The efivarfs From 935869cb953bab8a1504c37cd00c0007f4fccfd6 Mon Sep 17 00:00:00 2001 From: Chris Coulson Date: Tue, 9 Jul 2024 22:15:03 +0100 Subject: [PATCH 4/4] efi: Make sure HostEnvironment.VarContext returns a context with the right parent --- efi/internal/default_env_test.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/efi/internal/default_env_test.go b/efi/internal/default_env_test.go index a9f42a04..1c5e6c51 100644 --- a/efi/internal/default_env_test.go +++ b/efi/internal/default_env_test.go @@ -31,6 +31,7 @@ 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" ) @@ -39,12 +40,22 @@ type defaultEnvSuite struct{} var _ = Suite(&defaultEnvSuite{}) +type testKey struct{} + func (s *defaultEnvSuite) TestVarContext(c *C) { - ctx := DefaultEnv.VarContext(context.Background()) + ctx := DefaultEnv.VarContext(context.WithValue(context.Background(), testKey{}, int64(10))) c.Assert(ctx, NotNil) expected := efi.WithDefaultVarsBackend(context.Background()) c.Check(ctx.Value(efi.VarsBackendKey{}), Equals, expected.Value(efi.VarsBackendKey{})) + + // Make sure that the returned context has the right parent by testing the + // value we attached to it. + testVal := ctx.Value(testKey{}) + c.Assert(testVal, NotNil) + testVali64, ok := testVal.(int64) + c.Assert(ok, testutil.IsTrue) + c.Check(testVali64, Equals, int64(10)) } func (s *defaultEnvSuite) testReadEventLog(c *C, opts *efitest.LogOptions) {