refactor: efivarfs mock and tests

Refactor efivarfs and add tests.

Signed-off-by: Andrey Smirnov <andrey.smirnov@siderolabs.com>
Signed-off-by: Noel Georgi <git@frezbo.dev>
(cherry picked from commit ea4ed16)

Author: smira

internal/app/machined/pkg/runtime/v1alpha1/bootloader/sdboot/efivars.go

@@ -12,7 +12,6 @@ import (
 
 	"github.com/siderolabs/gen/xslices"
 	"github.com/siderolabs/go-blockdevice/v2/blkid"
-	"golang.org/x/sys/unix"
 
 	"github.com/siderolabs/talos/internal/pkg/efivarfs"
 	"github.com/siderolabs/talos/pkg/machinery/constants"
@@ -37,7 +36,14 @@ const (
 
 // ReadVariable reads a SystemdBoot EFI variable.
 func ReadVariable(name string) (string, error) {
-	data, _, err := efivarfs.Read(efivarfs.ScopeSystemd, name)
+	efi, err := efivarfs.NewFilesystemReaderWriter(false)
+	if err != nil {
+		return "", fmt.Errorf("failed to create efivarfs reader/writer: %w", err)
+	}
+
+	defer efi.Close() //nolint:errcheck
+
+	data, _, err := efi.Read(efivarfs.ScopeSystemd, name)
 	if err != nil {
 		// if the variable does not exist, return an empty string
 		if errors.Is(err, os.ErrNotExist) {
@@ -65,12 +71,12 @@ func ReadVariable(name string) (string, error) {
 
 // WriteVariable reads a SystemdBoot EFI variable.
 func WriteVariable(name, value string) error {
-	// mount EFI vars as rw
-	if err := unix.Mount("efivarfs", constants.EFIVarsMountPoint, "efivarfs", unix.MS_REMOUNT, ""); err != nil {
-		return err
+	efi, err := efivarfs.NewFilesystemReaderWriter(true)
+	if err != nil {
+		return fmt.Errorf("failed to create efivarfs reader/writer: %w", err)
 	}
 
-	defer unix.Mount("efivarfs", constants.EFIVarsMountPoint, "efivarfs", unix.MS_REMOUNT|unix.MS_RDONLY, "") //nolint:errcheck
+	defer efi.Close() //nolint:errcheck
 
 	out := make([]byte, (len(value)+1)*2)
 
@@ -83,7 +89,7 @@ func WriteVariable(name, value string) error {
 
 	out = append(out[:n], 0, 0)
 
-	return efivarfs.Write(efivarfs.ScopeSystemd, name, efivarfs.AttrBootserviceAccess|efivarfs.AttrRuntimeAccess|efivarfs.AttrNonVolatile, out)
+	return efi.Write(efivarfs.ScopeSystemd, name, efivarfs.AttrBootserviceAccess|efivarfs.AttrRuntimeAccess|efivarfs.AttrNonVolatile, out)
 }
 
 // CreateBootEntry creates a UEFI boot entry named "Talos Linux UKI" and sets it as the first in the `BootOrder`
@@ -92,14 +98,14 @@ func WriteVariable(name, value string) error {
 //
 //nolint:gocyclo
 func CreateBootEntry(installDisk, sdBootFilePath string) error {
-	// mount EFI vars as rw
-	if err := unix.Mount("efivarfs", constants.EFIVarsMountPoint, "efivarfs", unix.MS_REMOUNT, ""); err != nil {
-		return err
+	efi, err := efivarfs.NewFilesystemReaderWriter(true)
+	if err != nil {
+		return fmt.Errorf("failed to create efivarfs reader/writer: %w", err)
 	}
 
-	defer unix.Mount("efivarfs", constants.EFIVarsMountPoint, "efivarfs", unix.MS_REMOUNT|unix.MS_RDONLY, "") //nolint:errcheck
+	defer efi.Close() //nolint:errcheck
 
-	rawBootOrderData, _, err := efivarfs.Read(efivarfs.ScopeGlobal, "BootOrder")
+	rawBootOrderData, _, err := efi.Read(efivarfs.ScopeGlobal, "BootOrder")
 	if err != nil {
 		return fmt.Errorf("failed to read BootOrder: %w", err)
 	}
@@ -113,7 +119,7 @@ func CreateBootEntry(installDisk, sdBootFilePath string) error {
 	talosBootIndex := len(bootOrder)
 
 	for _, idx := range bootOrder {
-		bootEntry, err := efivarfs.GetBootEntry(int(idx))
+		bootEntry, err := efivarfs.GetBootEntry(efi, int(idx))
 		if err != nil {
 			return fmt.Errorf("failed to get boot entry %d: %w", idx, err)
 		}
@@ -150,7 +156,7 @@ func CreateBootEntry(installDisk, sdBootFilePath string) error {
 		return fmt.Errorf("EFI partition UUID not found on install disk %q", installDisk)
 	}
 
-	if err := efivarfs.SetBootEntry(talosBootIndex, &efivarfs.LoadOption{
+	if err := efivarfs.SetBootEntry(efi, talosBootIndex, &efivarfs.LoadOption{
 		Description: TalosBootEntryDescription,
 		FilePath: efivarfs.DevicePath{
 			&efivarfs.HardDrivePath{
@@ -167,7 +173,7 @@ func CreateBootEntry(installDisk, sdBootFilePath string) error {
 		return fmt.Errorf("failed to set boot entry %d: %w", talosBootIndex, err)
 	}
 
-	currentBootOrder, err := efivarfs.GetBootOrder()
+	currentBootOrder, err := efivarfs.GetBootOrder(efi)
 	if err != nil {
 		return fmt.Errorf("failed to get current BootOrder: %w", err)
 	}
@@ -177,7 +183,7 @@ func CreateBootEntry(installDisk, sdBootFilePath string) error {
 		return nil
 	}
 
-	if err := efivarfs.SetBootOrder(slices.Concat([]uint16{uint16(talosBootIndex)}, currentBootOrder)); err != nil {
+	if err := efivarfs.SetBootOrder(efi, slices.Concat([]uint16{uint16(talosBootIndex)}, currentBootOrder)); err != nil {
 		return fmt.Errorf("failed to set BootOrder: %w", err)
 	}
 

internal/pkg/efivarfs/boot.go

@@ -74,7 +74,7 @@ func (e *LoadOption) Marshal() ([]byte, error) {
 		attrs |= 0x01
 	}
 
-	data = append32(data, attrs)
+	data = binary.LittleEndian.AppendUint32(data, attrs)
 
 	filePathRaw, err := e.FilePath.Marshal()
 	if err != nil {
@@ -94,7 +94,7 @@ func (e *LoadOption) Marshal() ([]byte, error) {
 		return nil, fmt.Errorf("failed marshaling FilePath/ExtraPath: value too big (%d)", len(filePathRaw))
 	}
 
-	data = append16(data, uint16(len(filePathRaw)))
+	data = binary.LittleEndian.AppendUint16(data, uint16(len(filePathRaw)))
 
 	if strings.IndexByte(e.Description, 0x00) != -1 {
 		return nil, fmt.Errorf("failed to encode Description: contains invalid null bytes")
@@ -178,8 +178,9 @@ type BootOrder []uint16
 // Marshal generates the binary representation of a BootOrder.
 func (t *BootOrder) Marshal() []byte {
 	var out []byte
+
 	for _, v := range *t {
-		out = append16(out, v)
+		out = binary.LittleEndian.AppendUint16(out, v)
 	}
 
 	return out
@@ -195,24 +196,8 @@ func UnmarshalBootOrder(d []byte) (BootOrder, error) {
 
 	out := make(BootOrder, l)
 	for i := range l {
-		out[i] = uint16(d[2*i]) | uint16(d[2*i+1])<<8
+		out[i] = binary.LittleEndian.Uint16(d[i*2:])
 	}
 
 	return out, nil
 }
-
-func append16(d []byte, v uint16) []byte {
-	return append(d,
-		byte(v&0xFF),
-		byte(v>>8&0xFF),
-	)
-}
-
-func append32(d []byte, v uint32) []byte {
-	return append(d,
-		byte(v&0xFF),
-		byte(v>>8&0xFF),
-		byte(v>>16&0xFF),
-		byte(v>>24&0xFF),
-	)
-}

internal/pkg/efivarfs/devicepath.go

@@ -50,7 +50,7 @@ type PartitionMBR struct {
 func (p PartitionMBR) partitionSignature() (sig [16]byte) {
 	copy(sig[:4], p.DiskSignature[:])
 
-	return
+	return sig
 }
 
 func (p PartitionMBR) partitionFormat() uint8 {
@@ -283,7 +283,7 @@ func (d DevicePath) Marshal() ([]byte, error) {
 			return nil, fmt.Errorf("path element payload over maximum size")
 		}
 
-		buf = append16(buf, uint16(len(elemBuf)+4))
+		buf = binary.LittleEndian.AppendUint16(buf, uint16(len(elemBuf)+4))
 		buf = append(buf, elemBuf...)
 	}
 	// End of device path (Type 0x7f, SubType 0xFF)

internal/pkg/efivarfs/efivarfs.go

@@ -23,7 +23,10 @@ import (
 
 	"github.com/g0rbe/go-chattr"
 	"github.com/google/uuid"
+	"golang.org/x/sys/unix"
 	"golang.org/x/text/encoding/unicode"
+
+	"github.com/siderolabs/talos/pkg/machinery/constants"
 )
 
 const (
@@ -87,8 +90,48 @@ func varPath(scope uuid.UUID, varName string) string {
 	return fmt.Sprintf("/sys/firmware/efi/efivars/%s-%s", varName, scope.String())
 }
 
+// ReaderWriter is an interface for reading and writing EFI variables.
+type ReaderWriter interface {
+	Write(scope uuid.UUID, varName string, attrs Attribute, value []byte) error
+	Delete(scope uuid.UUID, varName string) error
+	Read(scope uuid.UUID, varName string) ([]byte, Attribute, error)
+	List(scope uuid.UUID) ([]string, error)
+}
+
+// FilesystemReaderWriter implements ReaderWriter using the efivars Linux filesystem.
+type FilesystemReaderWriter struct {
+	write bool
+}
+
+// NewFilesystemReaderWriter creates a new FilesystemReaderWriter.
+func NewFilesystemReaderWriter(write bool) (*FilesystemReaderWriter, error) {
+	if write {
+		if err := unix.Mount("efivarfs", constants.EFIVarsMountPoint, "efivarfs", unix.MS_REMOUNT, ""); err != nil {
+			return nil, err
+		}
+	}
+
+	return &FilesystemReaderWriter{
+		write: write,
+	}, nil
+}
+
+// Close unmounts efivarfs if the FilesystemReaderWriter was created with write
+// access.
+func (rw *FilesystemReaderWriter) Close() error {
+	if rw.write {
+		return unix.Mount("efivarfs", constants.EFIVarsMountPoint, "efivarfs", unix.MS_REMOUNT|unix.MS_RDONLY, "")
+	}
+
+	return nil
+}
+
 // Write writes the value of the named variable in the given scope.
-func Write(scope uuid.UUID, varName string, attrs Attribute, value []byte) error {
+func (rw *FilesystemReaderWriter) Write(scope uuid.UUID, varName string, attrs Attribute, value []byte) error {
+	if !rw.write {
+		return errors.New("efivarfs was opened read-only")
+	}
+
 	// Ref: https://docs.kernel.org/filesystems/efivarfs.html
 	// Remove immutable attribute from the efivarfs file if it exists
 	if _, err := os.Stat(varPath(scope, varName)); err == nil {
@@ -140,7 +183,7 @@ func Write(scope uuid.UUID, varName string, attrs Attribute, value []byte) error
 }
 
 // Read reads the value of the named variable in the given scope.
-func Read(scope uuid.UUID, varName string) ([]byte, Attribute, error) {
+func (rw *FilesystemReaderWriter) Read(scope uuid.UUID, varName string) ([]byte, Attribute, error) {
 	val, err := os.ReadFile(varPath(scope, varName))
 	if err != nil {
 		e := err
@@ -162,7 +205,7 @@ func Read(scope uuid.UUID, varName string) ([]byte, Attribute, error) {
 
 // List lists all variable names present for a given scope sorted by their names
 // in Go's "native" string sort order.
-func List(scope uuid.UUID) ([]string, error) {
+func (rw *FilesystemReaderWriter) List(scope uuid.UUID) ([]string, error) {
 	vars, err := os.ReadDir(Path)
 	if err != nil {
 		return nil, fmt.Errorf("failed to list variable directory: %w", err)
@@ -189,6 +232,10 @@ func List(scope uuid.UUID) ([]string, error) {
 
 // Delete deletes the given variable name in the given scope. Use with care,
 // some firmware fails to boot if variables it uses are deleted.
-func Delete(scope uuid.UUID, varName string) error {
+func (rw *FilesystemReaderWriter) Delete(scope uuid.UUID, varName string) error {
+	if !rw.write {
+		return errors.New("efivarfs was opened read-only")
+	}
+
 	return os.Remove(varPath(scope, varName))
 }

internal/pkg/efivarfs/efivarfs_test.go

@@ -0,0 +1,129 @@
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+package efivarfs_test
+
+import (
+	"encoding/binary"
+	"io/fs"
+	"testing"
+
+	"github.com/google/uuid"
+	"github.com/stretchr/testify/require"
+
+	"github.com/siderolabs/talos/internal/pkg/efivarfs"
+)
+
+func TestBootOrder(t *testing.T) {
+	t.Parallel()
+
+	var bootOrderEntries []byte
+
+	for _, entry := range []int{1, 0, 2, 3} {
+		bootOrderEntries = binary.LittleEndian.AppendUint16(bootOrderEntries, uint16(entry))
+	}
+
+	efiRW := efivarfs.Mock{
+		Variables: map[uuid.UUID]map[string]efivarfs.MockVariable{
+			efivarfs.ScopeGlobal: {
+				"BootOrder": {
+					Attrs: 0,
+					Data:  bootOrderEntries,
+				},
+			},
+		},
+	}
+
+	vars, err := efiRW.List(efivarfs.ScopeGlobal)
+	require.NoError(t, err)
+
+	require.Contains(t, vars, "BootOrder", "variable BootOrder not found")
+
+	bootOrder, err := efivarfs.GetBootOrder(&efiRW)
+	require.NoError(t, err)
+
+	require.Equal(t, efivarfs.BootOrder([]uint16{1, 0, 2, 3}), bootOrder, "BootOrder does not match expected value")
+
+	require.NoError(t, efivarfs.SetBootOrder(&efiRW, efivarfs.BootOrder([]uint16{1, 0, 3})))
+
+	bootOrder, err = efivarfs.GetBootOrder(&efiRW)
+	require.NoError(t, err)
+
+	require.Equal(t, efivarfs.BootOrder([]uint16{1, 0, 3}), bootOrder, "BootOrder does not match expected value after SetBootOrder")
+}
+
+func TestBootEntries(t *testing.T) {
+	t.Parallel()
+
+	efiRW := efivarfs.Mock{}
+
+	// no entries yet
+	entries, err := efivarfs.ListBootEntries(&efiRW)
+	require.NoError(t, err)
+	require.Empty(t, len(entries), "expected no boot entries in empty mock")
+
+	// create first entry
+	idx, err := efivarfs.AddBootEntry(&efiRW, &efivarfs.LoadOption{
+		Description: "First Entry",
+		FilePath: efivarfs.DevicePath{
+			efivarfs.FilePath("/first.efi"),
+		},
+	})
+	require.NoError(t, err)
+	require.Equal(t, 0, idx, "first boot entry index should be 0")
+
+	// verify first entry
+	entry, err := efivarfs.GetBootEntry(&efiRW, idx)
+	require.NoError(t, err)
+	require.Equal(t, "First Entry", entry.Description, "first boot entry description does not match")
+	require.Equal(t, efivarfs.DevicePath{efivarfs.FilePath("/first.efi")}, entry.FilePath, "first boot entry file path does not match")
+
+	// create second entry
+	require.NoError(t, efivarfs.SetBootEntry(&efiRW, 1, &efivarfs.LoadOption{
+		Description: "Second Entry",
+		FilePath: efivarfs.DevicePath{
+			efivarfs.FilePath("/second.efi"),
+		},
+	}), "failed to set second boot entry")
+
+	// verify second entry
+	entry, err = efivarfs.GetBootEntry(&efiRW, 1)
+	require.NoError(t, err)
+	require.Equal(t, "Second Entry", entry.Description, "second boot entry description does not match")
+	require.Equal(t, efivarfs.DevicePath{efivarfs.FilePath("/second.efi")}, entry.FilePath, "second boot entry file path does not match")
+
+	// list all entries
+	entries, err = efivarfs.ListBootEntries(&efiRW)
+	require.NoError(t, err)
+	require.Len(t, entries, 2, "expected exactly two boot entries after adding two")
+
+	// try overwrite first entry
+	require.NoError(t, efivarfs.SetBootEntry(&efiRW, idx, &efivarfs.LoadOption{
+		Description: "First Entry Overwritten",
+		FilePath: efivarfs.DevicePath{
+			efivarfs.FilePath("/first_overwritten.efi"),
+		},
+	}), "failed to overwrite first boot entry")
+
+	// verify first entry after overwrite
+	entry, err = efivarfs.GetBootEntry(&efiRW, idx)
+	require.NoError(t, err)
+	require.Equal(t, "First Entry Overwritten", entry.Description, "first boot entry description does not match after overwrite")
+	require.Equal(t, efivarfs.DevicePath{efivarfs.FilePath("/first_overwritten.efi")}, entry.FilePath, "first boot entry file path does not match after overwrite")
+
+	// verify delete non-existing entry
+	require.ErrorIs(t, efivarfs.DeleteBootEntry(&efiRW, 42), fs.ErrNotExist, "expected ErrNoSuchEntry when deleting non-existing entry")
+
+	// delete second entry
+	require.NoError(t, efivarfs.DeleteBootEntry(&efiRW, 1), "failed to delete second boot entry")
+
+	// verify second entry is gone
+	_, err = efivarfs.GetBootEntry(&efiRW, 1)
+	require.ErrorIs(t, err, fs.ErrNotExist, "expected ErrNoSuchEntry when getting deleted entry")
+
+	// list entries
+	entries, err = efivarfs.ListBootEntries(&efiRW)
+	require.NoError(t, err)
+	require.Len(t, entries, 1, "expected exactly one boot entry after deleting one of two")
+}