Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

efi: change how HostEnvironment overrides EFI variables #312

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions efi/efi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ package efi_test

import (
"bytes"
"context"
"crypto"
"crypto/x509"
"errors"
Expand Down Expand Up @@ -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(context.Background())}
}

func (r *mockVarReader) ReadVar(name string, guid efi.GUID) ([]byte, efi.VariableAttributes, error) {
return efi.ReadVariable(r.ctx, name, guid)
}
46 changes: 33 additions & 13 deletions efi/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ package efi

import (
"bytes"
"context"
"crypto"
"crypto/sha1"
"encoding/binary"
Expand All @@ -37,8 +38,15 @@ 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 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
// 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.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should the doc say something about parent?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated the documentation now.

VarContext(parent context.Context) context.Context

// ReadEventLog reads the TCG event log
ReadEventLog() (*tcglog.Log, error)
Expand All @@ -60,7 +68,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)
Expand Down Expand Up @@ -158,7 +167,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
}

Expand All @@ -168,7 +177,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
Expand Down Expand Up @@ -208,7 +217,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
Expand All @@ -226,7 +235,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
Expand All @@ -247,7 +256,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

Expand All @@ -256,17 +265,28 @@ type variableSetCollector struct {
todo []*initialVarReader
}

func newVariableSetCollector(vars varReader) *variableSetCollector {
func newVariableSetCollector(env HostEnvironment) *variableSetCollector {
// 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{
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
Expand Down
4 changes: 3 additions & 1 deletion efi/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -192,7 +194,7 @@ func MockSnapdenvTesting(testing bool) (restore func()) {

func NewInitialVarReader(host HostEnvironment) *initialVarReader {
return &initialVarReader{
host: host,
varsCtx: host.VarContext(context.TODO()),
overrides: make(map[efi.VariableDescriptor]varContents)}
}

Expand Down
8 changes: 5 additions & 3 deletions efi/internal/default_env.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package internal

import (
"context"
"os"

efi "github.com/canonical/go-efilib"
Expand All @@ -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(parent context.Context) context.Context {
return efi.WithDefaultVarsBackend(parent)
}

// ReadEventLog implements [HostEnvironment.ReadEventLog].
func (e defaultEnvImpl) ReadEventLog() (*tcglog.Log, error) {
f, err := os.Open(eventLogPath)
if err != nil {
Expand Down
87 changes: 14 additions & 73 deletions efi/internal/default_env_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package internal_test

import (
"context"
_ "embed"
"io"
"os"
Expand All @@ -35,86 +36,26 @@ import (
. "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})
}
type testKey struct{}

func (s *defaultEnvSuite) TestReadVar2(c *C) {
s.testReadVar(c, &testReadVarData{
name: "KEK",
guid: efi.GlobalVariable})
}
func (s *defaultEnvSuite) TestVarContext(c *C) {
ctx := DefaultEnv.VarContext(context.WithValue(context.Background(), testKey{}, int64(10)))
c.Assert(ctx, NotNil)

func (s *defaultEnvSuite) TestReadVar3(c *C) {
s.testReadVar(c, &testReadVarData{
name: "dbx",
guid: efi.ImageSecurityDatabaseGuid})
}
expected := efi.WithDefaultVarsBackend(context.Background())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe a good idea to pass something with an attached value already and check it can be retrieved?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added this to the test

c.Check(ctx.Value(efi.VarsBackendKey{}), Equals, expected.Value(efi.VarsBackendKey{}))

func (s *defaultEnvSuite) TestReadVarNotExist(c *C) {
s.testReadVar(c, &testReadVarData{
name: "SecureBoot",
guid: efi.ImageSecurityDatabaseGuid})
// 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) {
Expand Down
44 changes: 44 additions & 0 deletions efi/internal/env.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// -*- 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 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
// 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.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same about parent

VarContext(parent context.Context) context.Context

// ReadEventLog reads the TCG event log
ReadEventLog() (*tcglog.Log, error)
}
10 changes: 0 additions & 10 deletions efi/internal/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
12 changes: 0 additions & 12 deletions efi/internal/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down
Loading
Loading