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.

Sign up for GitHub

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
merged 5 commits into from
Jul 10, 2024
Merged

efi: change how HostEnvironment overrides EFI variables #312

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
aaba2c5
efi: change how HostEnvironment overrides EFI variables
chrisccoulson Jun 21, 2024
1c78a22
efi: Allow HostEnvironment.VarContext to take a parent context
chrisccoulson Jul 8, 2024
35d03b3
Merge branch 'master' into efi-mock-vars-backend-with-hostenv
chrisccoulson Jul 8, 2024
04110a4
efi: update doc comment for HostEnvironment.VarContext
chrisccoulson Jul 9, 2024
935869c
efi: Make sure HostEnvironment.VarContext returns a context with the …
chrisccoulson Jul 9, 2024
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
2 changes: 1 addition & 1 deletion efi/efi_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
21 changes: 18 additions & 3 deletions efi/env.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
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 Down Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion efi/export_test.go
View file
Open in desktop
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{
varsCtx: host.VarContext(),
varsCtx: host.VarContext(context.TODO()),
overrides: make(map[efi.VariableDescriptor]varContents)}
}

Expand Down
4 changes: 2 additions & 2 deletions efi/internal/default_env.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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].
Expand Down
7 changes: 6 additions & 1 deletion efi/internal/default_env_test.go
View file
Open in desktop
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 @@ -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())
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) testReadEventLog(c *C, opts *efitest.LogOptions) {
Expand Down
9 changes: 7 additions & 2 deletions efi/internal/env.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
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)
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
4 changes: 2 additions & 2 deletions internal/efitest/hostenv.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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].
Expand Down