Handle A/B images and refuse unread options

cmd/cbprov/apply.go

@@ -27,7 +27,7 @@ func applyCmd() *cobra.Command {
 			if err != nil {
 				return err
 			}
-			allowed := []string{"RO_VPD", "RW_VPD", options.Region, provision.ROCBFS}
+			allowed := append([]string{"RO_VPD", "RW_VPD", options.Region}, provision.CBFSRegions...)
 			return out.edit(args[0], allowed, func(im *image.Image) (provision.Warnings, error) {
 				return provision.Apply(im, spec, filepath.Dir(file), force)
 			})

cmd/cbprov/payload.go

@@ -67,7 +67,7 @@ func payloadCmd() *cobra.Command {
 				return err
 			}
 			spec.Data = data
-			return out.edit(args[0], []string{provision.ROCBFS}, func(im *image.Image) (provision.Warnings, error) {
+			return out.edit(args[0], provision.CBFSRegions, func(im *image.Image) (provision.Warnings, error) {
 				return provision.ReplacePayload(im, spec)
 			})
 		},
@@ -95,7 +95,7 @@ type payloadInfo struct {
 
 func payloads(im *image.Image) ([]payloadInfo, error) {
 	var out []payloadInfo
-	for _, region := range []string{provision.ROCBFS, "FW_MAIN_A", "FW_MAIN_B"} {
+	for _, region := range []string{provision.ROCBFS, "COREBOOT_B", "FW_MAIN_A", "FW_MAIN_B"} {
 		if !im.HasRegion(region) {
 			continue
 		}

cmd/cbprov/schema.go

@@ -59,7 +59,7 @@ func schemaCmd() *cobra.Command {
 			if err != nil {
 				return err
 			}
-			return embedOut.edit(args[0], []string{provision.ROCBFS}, func(im *image.Image) (provision.Warnings, error) {
+			return embedOut.edit(args[0], provision.CBFSRegions, func(im *image.Image) (provision.Warnings, error) {
 				return provision.EmbedSchema(im, raw)
 			})
 		},

cmd/cbprov/show.go

@@ -211,7 +211,7 @@ func printReport(r showReport) {
 	var rs []string
 	for _, rg := range r.Regions {
 		switch rg.Name {
-		case "RO_VPD", "RW_VPD", "SMMSTORE", "COREBOOT", "FW_MAIN_A", "FW_MAIN_B":
+		case "RO_VPD", "RW_VPD", "SMMSTORE", "COREBOOT", "COREBOOT_B", "FW_MAIN_A", "FW_MAIN_B":
 			s := rg.Name + " " + size(uint64(rg.Size))
 			if rg.Name == "COREBOOT" {
 				s += " (" + size(uint64(r.Image.CBFSFree)) + " free)"

pkg/cbfs/verify.go

@@ -23,7 +23,7 @@ type anchorContainer struct {
 
 var anchorContainers = []anchorContainer{
 	{cbfs: "COREBOOT", bootblock: "BOOTBLOCK"},
-	{cbfs: "COREBOOT_B", bootblock: "BOOTBLOCK_B", secondary: true}, // Intel Top Swap
+	{cbfs: "COREBOOT_B", bootblock: "BOOTBLOCK_B", secondary: true}, // Intel Top Swap, AMD PSP A/B recovery
 }
 
 // anchor is a struct metadata_hash_anchor inside the image data.

pkg/provision/ops.go

@@ -19,6 +19,11 @@ import (
 // ROCBFS is the FMAP region of the read-only CBFS.
 const ROCBFS = "COREBOOT"
 
+// CBFSRegions are the FMAP regions a CBFS change may touch: the RO CBFS, its
+// copy COREBOOT_B (Intel Top Swap, AMD PSP A/B recovery) and the BOOTBLOCK
+// regions holding their metadata hash anchors.
+var CBFSRegions = []string{ROCBFS, "COREBOOT_B", "BOOTBLOCK", "BOOTBLOCK_B"}
+
 // KV is an ordered key/value pair from the command line or a provisioning file.
 type KV struct {
 	Key   string
@@ -91,6 +96,9 @@ func OpenOptions(im *image.Image, schemaOverride []byte) (*options.Set, string,
 
 // CommitOptions writes the option changes back into the image.
 func CommitOptions(im *image.Image, set *options.Set) error {
+	if cfg := BuildConfig(im); len(cfg) > 0 && !cfg.Bool("USE_UEFI_VARIABLE_STORE") {
+		return errors.New("coreboot in this image does not read options from SMMSTORE (CONFIG_USE_UEFI_VARIABLE_STORE is not set)")
+	}
 	region, err := im.RegionData(options.Region)
 	if err != nil {
 		return err
@@ -192,7 +200,7 @@ type PayloadSpec struct {
 // ErrSigned is returned when a change would break a signature cbprov cannot renew.
 var ErrSigned = errors.New("image is signed")
 
-// ReplacePayload swaps a payload in the RO CBFS.
+// ReplacePayload swaps a payload in the RO CBFS and its COREBOOT_B copy.
 func ReplacePayload(im *image.Image, p PayloadSpec) (Warnings, error) {
 	var w Warnings
 	if p.Name == "" {
@@ -205,53 +213,62 @@ func ReplacePayload(im *image.Image, p PayloadSpec) (Warnings, error) {
 	if ver.Vboot && !p.Force {
 		return nil, fmt.Errorf("%w: vboot RW slots (FW_MAIN_A/B) present; payload changes need re-signing, which cbprov does not do yet (--force changes the RO CBFS only)", ErrSigned)
 	}
-	fs, err := cbfs.Open(im, ROCBFS)
+	// Only the ELF magic is checked: cbfstool accepts ELFs with broken section
+	// headers and so does the SELF converter, which reads program headers only.
+	if !p.Flat && !bytes.HasPrefix(p.Data, []byte(elf.ELFMAG)) {
+		return nil, errors.New("payload is not an ELF file (use --flat with --load and --entry for raw binaries)")
+	}
+	// coreboot adds the payload to every RO CBFS (CBFS_REGIONS).
+	regions := []string{ROCBFS}
+	if im.HasRegion("COREBOOT_B") {
+		regions = append(regions, "COREBOOT_B")
+	}
+	for _, region := range regions {
+		if err := replacePayloadIn(im, region, p, &w); err != nil {
+			return nil, err
+		}
+	}
+	if ver.MetadataHash {
+		if err := cbfs.UpdateMetadataHash(im); err != nil {
+			return nil, err
+		}
+		w.add("CBFS metadata hash in the bootblock updated; if the bootblock is verified by Boot Guard or the AMD PSP, re-sign the image")
+	}
+	if ver.Vboot {
+		w.add("only the RO CBFS was changed; vboot RW slots still boot the old payload")
+	}
+	return w, nil
+}
+
+func replacePayloadIn(im *image.Image, region string, p PayloadSpec, w *Warnings) error {
+	fs, err := cbfs.Open(im, region)
 	if err != nil {
-		return nil, err
+		return err
 	}
 	comp := cbfs.CompressLZMA
 	old, found, err := findFile(fs, p.Name)
 	if err != nil {
-		return nil, err
+		return err
 	}
 	if found {
 		comp = PayloadCompression(old)
 	} else {
-		w.add("%s did not exist; adding it", p.Name)
+		w.add("%s did not exist in %s; adding it", p.Name, region)
 	}
 	if p.Compression != "" {
 		if comp, err = ParseCompression(p.Compression); err != nil {
-			return nil, err
+			return err
 		}
 	}
-	// Only the ELF magic is checked: cbfstool accepts ELFs with broken section
-	// headers and so does the SELF converter, which reads program headers only.
-	if !p.Flat && !bytes.HasPrefix(p.Data, []byte(elf.ELFMAG)) {
-		return nil, errors.New("payload is not an ELF file (use --flat with --load and --entry for raw binaries)")
-	}
 	if found {
 		if err := fs.Remove(p.Name); err != nil {
-			return nil, err
+			return err
 		}
 	}
 	if p.Flat {
-		err = fs.AddPayloadFlat(p.Name, p.Data, p.LoadAddr, p.Entry, comp)
-	} else {
-		err = fs.AddPayloadELF(p.Name, p.Data, comp)
-	}
-	if err != nil {
-		return nil, err
-	}
-	if ver.MetadataHash {
-		if err := cbfs.UpdateMetadataHash(im); err != nil {
-			return nil, err
-		}
-		w.add("CBFS metadata hash in the bootblock updated; if the bootblock is verified by Boot Guard or the AMD PSP, re-sign the image")
-	}
-	if ver.Vboot {
-		w.add("only the RO CBFS was changed; vboot RW slots still boot the old payload")
+		return fs.AddPayloadFlat(p.Name, p.Data, p.LoadAddr, p.Entry, comp)
 	}
-	return w, nil
+	return fs.AddPayloadELF(p.Name, p.Data, comp)
 }
 
 // EmbedSchema stores a CFR schema in the RO CBFS so later commands find it.

pkg/provision/provision_test.go

@@ -2,12 +2,15 @@ package provision
 
 import (
 	"bytes"
+	"encoding/binary"
 	"os"
 	"path/filepath"
 	"testing"
 
 	"github.com/linuxboot/fiano/pkg/fmap"
 
+	"github.com/9elements/cbprov/pkg/buildconfig"
+	"github.com/9elements/cbprov/pkg/cbfs"
 	"github.com/9elements/cbprov/pkg/efivars"
 	"github.com/9elements/cbprov/pkg/image"
 	"github.com/9elements/cbprov/pkg/options"
@@ -27,14 +30,14 @@ func (w *sliceWriter) Write(p []byte) (int, error) {
 
 func (w *sliceWriter) Seek(off int64, _ int) (int64, error) { w.off = off; return off, nil }
 
-// testImage has an FMAP, RO_VPD and an erased SMMSTORE but no CBFS.
-func testImage(t *testing.T) *image.Image {
+type testArea struct {
+	name      string
+	off, size uint32
+}
+
+func buildImage(t *testing.T, size int, areas []testArea) *image.Image {
 	t.Helper()
-	data := bytes.Repeat([]byte{0xff}, 512*1024)
-	areas := []struct {
-		name      string
-		off, size uint32
-	}{{"FMAP", 0x0, 0x1000}, {"RO_VPD", 0x4000, 0x4000}, {"SMMSTORE", 0x40000, 0x40000}}
+	data := bytes.Repeat([]byte{0xff}, size)
 	f := &fmap.FMap{Header: fmap.Header{VerMajor: 1, VerMinor: 1, Size: uint32(len(data)), NAreas: uint16(len(areas))}}
 	copy(f.Signature[:], "__FMAP__")
 	for _, a := range areas {
@@ -52,6 +55,45 @@ func testImage(t *testing.T) *image.Image {
 	return im
 }
 
+// testImage has an FMAP, RO_VPD and an erased SMMSTORE but no CBFS.
+func testImage(t *testing.T) *image.Image {
+	return buildImage(t, 512*1024, []testArea{{"FMAP", 0x0, 0x1000}, {"RO_VPD", 0x4000, 0x4000}, {"SMMSTORE", 0x40000, 0x40000}})
+}
+
+// abImage is laid out like an AMD PSP A/B image: two RO CBFS whose metadata
+// hash anchors sit in separate BOOTBLOCK regions. config is the CBFS build config.
+func abImage(t *testing.T, config string) *image.Image {
+	t.Helper()
+	im := buildImage(t, 1<<20, []testArea{{"FMAP", 0x0, 0x1000}, {"BOOTBLOCK", 0x1000, 0x1000},
+		{"BOOTBLOCK_B", 0x2000, 0x1000}, {"SMMSTORE", 0x40000, 0x40000},
+		{"COREBOOT", 0x80000, 0x40000}, {"COREBOOT_B", 0xc0000, 0x40000}})
+	for _, bb := range []string{"BOOTBLOCK", "BOOTBLOCK_B"} {
+		b, _ := im.RegionData(bb)
+		copy(b[0x100:], append([]byte("\xadMdtHsh\x15\x00\x00\x00\x02"), make([]byte, 64)...)) // SHA-256 anchor
+	}
+	for _, region := range []string{"COREBOOT", "COREBOOT_B"} {
+		b, _ := im.RegionData(region)
+		// Empty CBFS as cbfstool creates it: one null file spanning the region.
+		copy(b, "LARCHIVE")
+		binary.BigEndian.PutUint32(b[8:], uint32(len(b)-28))
+		binary.BigEndian.PutUint32(b[12:], 0xffffffff)
+		binary.BigEndian.PutUint32(b[16:], 0)
+		binary.BigEndian.PutUint32(b[20:], 28)
+		clear(b[24:28])
+		fs, err := cbfs.Open(im, region)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if err := fs.AddRaw(buildconfig.FileName, []byte(config), cbfs.CompressNone); err != nil {
+			t.Fatal(err)
+		}
+		if err := fs.AddPayloadFlat("fallback/payload", []byte("old payload"), 0x100000, 0x100000, cbfs.CompressNone); err != nil {
+			t.Fatal(err)
+		}
+	}
+	return im
+}
+
 func TestSetVPD(t *testing.T) {
 	im := testImage(t)
 	w, err := SetVPD(im, "RO_VPD", []KV{
@@ -168,3 +210,71 @@ func vpdErased(b []byte) bool {
 	_, err := vpd.Decode(b)
 	return err == nil && bytes.Count(b, []byte{0xff}) == len(b)
 }
+
+func TestReplacePayloadAB(t *testing.T) {
+	im := abImage(t, "CONFIG_PSP_AB_RECOVERY=y\n")
+	orig := bytes.Clone(im.Data)
+	payload := bytes.Repeat([]byte("new payload "), 1000)
+	if _, err := ReplacePayload(im, PayloadSpec{Data: payload, Flat: true, LoadAddr: 0x200000, Entry: 0x200000}); err != nil {
+		t.Fatal(err)
+	}
+	for _, region := range []string{"COREBOOT", "COREBOOT_B"} {
+		fs, _ := cbfs.Open(im, region)
+		f, found, err := findFile(fs, "fallback/payload")
+		if err != nil || !found || len(f.Segments) == 0 || f.Segments[0].LoadAddr != 0x200000 {
+			t.Errorf("%s: payload not replaced: %+v %v", region, f, err)
+		}
+	}
+	var allowed []image.Region
+	for _, n := range CBFSRegions {
+		r, _ := im.Region(n)
+		allowed = append(allowed, r)
+	}
+	for i := range orig {
+		if orig[i] == im.Data[i] {
+			continue
+		}
+		inside := false
+		for _, r := range allowed {
+			inside = inside || (uint32(i) >= r.Offset && uint32(i) < r.Offset+r.Size)
+		}
+		if !inside {
+			t.Fatalf("byte 0x%x outside %v changed", i, CBFSRegions)
+		}
+	}
+	for _, bb := range []string{"BOOTBLOCK", "BOOTBLOCK_B"} {
+		r, _ := im.Region(bb)
+		if bytes.Equal(orig[r.Offset:r.Offset+r.Size], im.Data[r.Offset:r.Offset+r.Size]) {
+			t.Errorf("%s: metadata hash not updated", bb)
+		}
+	}
+	check, err := image.Parse(bytes.Clone(im.Data))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := cbfs.UpdateMetadataHash(check); err != nil || !bytes.Equal(check.Data, im.Data) {
+		t.Errorf("metadata hashes stale after replace (%v)", err)
+	}
+}
+
+func TestCommitOptionsNeedsBackend(t *testing.T) {
+	for _, tc := range []struct {
+		config string
+		ok     bool
+	}{
+		{"CONFIG_SMMSTORE=y\n", false},
+		{"CONFIG_SMMSTORE=y\nCONFIG_DRIVERS_EFI_VARIABLE_STORE=y\nCONFIG_USE_UEFI_VARIABLE_STORE=y\n", true},
+	} {
+		im := abImage(t, tc.config)
+		set, _, err := OpenOptions(im, nil)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if err := set.SetRaw("boot_delay", 3); err != nil {
+			t.Fatal(err)
+		}
+		if err := CommitOptions(im, set); (err == nil) != tc.ok {
+			t.Errorf("config %q: err = %v", tc.config, err)
+		}
+	}
+}