Added more comprehensive support to inspect bootloader configuration (#404)

Author: magerstam

internal/image/imageinspect/bootloader_config.go

@@ -17,7 +17,7 @@ func ParsePEFromBytes(p string, blob []byte) (EFIBinaryEvidence, error) {
 		Kind:            BootloaderUnknown, // set after we have more evidence
 	}
 
-	ev.SHA256 = sha256Hex(blob)
+	ev.SHA256 = hashBytesHex(blob)
 
 	r := bytes.NewReader(blob)
 	f, err := pe.NewFile(r)
@@ -61,7 +61,7 @@ func ParsePEFromBytes(p string, blob []byte) (EFIBinaryEvidence, error) {
 			ev.Notes = append(ev.Notes, fmt.Sprintf("read section %s: %v", name, err))
 			continue
 		}
-		ev.SectionSHA256[name] = sha256Hex(data)
+		ev.SectionSHA256[name] = hashBytesHex(data)
 
 		switch name {
 		case ".linux":

internal/image/imageinspect/bootloader_config_test.go

@@ -178,7 +178,8 @@ type ModifiedEFIBinaryEvidence struct {
 	To      EFIBinaryEvidence `json:"to"`
 	Changes []FieldChange     `json:"changes,omitempty"`
 
-	UKI *UKIDiff `json:"uki,omitempty"`
+	UKI        *UKIDiff              `json:"uki,omitempty"`
+	BootConfig *BootloaderConfigDiff `json:"bootConfig,omitempty"`
 }
 
 // UKIDiff represents differences in the UKI-related fields of an EFI binary.
@@ -201,6 +202,54 @@ type SectionMapDiff struct {
 	Modified map[string]ValueDiff[string] `json:"modified,omitempty"`
 }
 
+// BootloaderConfigDiff represents differences in bootloader configuration.
+type BootloaderConfigDiff struct {
+	ConfigFileChanges    []ConfigFileChange `json:"configFileChanges,omitempty"`
+	BootEntryChanges     []BootEntryChange  `json:"bootEntryChanges,omitempty"`
+	KernelRefChanges     []KernelRefChange  `json:"kernelRefChanges,omitempty"`
+	UUIDReferenceChanges []UUIDRefChange    `json:"uuidReferenceChanges,omitempty"`
+	NotesAdded           []string           `json:"notesAdded,omitempty"`
+	NotesRemoved         []string           `json:"notesRemoved,omitempty"`
+}
+
+// ConfigFileChange represents a change to a bootloader config file.
+type ConfigFileChange struct {
+	Path     string `json:"path" yaml:"path"`
+	Status   string `json:"status" yaml:"status"` // "added", "removed", "modified"
+	HashFrom string `json:"hashFrom,omitempty" yaml:"hashFrom,omitempty"`
+	HashTo   string `json:"hashTo,omitempty" yaml:"hashTo,omitempty"`
+}
+
+// BootEntryChange represents a change to a boot entry.
+type BootEntryChange struct {
+	Name        string `json:"name" yaml:"name"`
+	Status      string `json:"status" yaml:"status"` // "added", "removed", "modified"
+	KernelFrom  string `json:"kernelFrom,omitempty" yaml:"kernelFrom,omitempty"`
+	KernelTo    string `json:"kernelTo,omitempty" yaml:"kernelTo,omitempty"`
+	InitrdFrom  string `json:"initrdFrom,omitempty" yaml:"initrdFrom,omitempty"`
+	InitrdTo    string `json:"initrdTo,omitempty" yaml:"initrdTo,omitempty"`
+	CmdlineFrom string `json:"cmdlineFrom,omitempty" yaml:"cmdlineFrom,omitempty"`
+	CmdlineTo   string `json:"cmdlineTo,omitempty" yaml:"cmdlineTo,omitempty"`
+}
+
+// KernelRefChange represents a change to a kernel reference.
+type KernelRefChange struct {
+	Path     string `json:"path" yaml:"path"`
+	Status   string `json:"status" yaml:"status"` // "added", "removed", "modified"
+	UUIDFrom string `json:"uuidFrom,omitempty" yaml:"uuidFrom,omitempty"`
+	UUIDTo   string `json:"uuidTo,omitempty" yaml:"uuidTo,omitempty"`
+}
+
+// UUIDRefChange represents a change to a UUID reference.
+type UUIDRefChange struct {
+	UUID         string `json:"uuid" yaml:"uuid"`
+	Status       string `json:"status" yaml:"status"` // "added", "removed", "modified"
+	ContextFrom  string `json:"contextFrom,omitempty" yaml:"contextFrom,omitempty"`
+	ContextTo    string `json:"contextTo,omitempty" yaml:"contextTo,omitempty"`
+	MismatchFrom bool   `json:"mismatchFrom,omitempty" yaml:"mismatchFrom,omitempty"`
+	MismatchTo   bool   `json:"mismatchTo,omitempty" yaml:"mismatchTo,omitempty"`
+}
+
 // CompareImages compares two ImageSummary objects and returns a structured diff.
 func CompareImages(from, to *ImageSummary) ImageCompareResult {
 	if from == nil || to == nil {
@@ -797,7 +846,6 @@ func tallyDiffs(d ImageDiff) diffTally {
 	if d.PartitionTable.MisalignedParts != nil {
 		t.addMeaningful(1, "PT MisalignedParts")
 	}
-
 	if len(d.Partitions.Added) > 0 {
 		t.addMeaningful(len(d.Partitions.Added), "Partitions Added")
 	}
@@ -930,32 +978,35 @@ func tallyEFIBinaryDiff(t *diffTally, d EFIBinaryDiff) {
 		}
 
 		// UKI diffs
-		if m.UKI == nil || !m.UKI.Changed {
-			continue
-		}
-
-		if m.UKI.KernelSHA256 != nil {
-			t.addMeaningful(1, "EFI "+m.Key+" UKI KernelSHA")
-		}
-		if m.UKI.OSRelSHA256 != nil {
-			t.addMeaningful(1, "EFI "+m.Key+" UKI OSRelSHA")
-		}
-		if m.UKI.UnameSHA256 != nil {
-			t.addMeaningful(1, "EFI "+m.Key+" UKI UnameSHA")
-		}
+		if m.UKI != nil && m.UKI.Changed {
+			if m.UKI.KernelSHA256 != nil {
+				t.addMeaningful(1, "EFI "+m.Key+" UKI KernelSHA")
+			}
+			if m.UKI.OSRelSHA256 != nil {
+				t.addMeaningful(1, "EFI "+m.Key+" UKI OSRelSHA")
+			}
+			if m.UKI.UnameSHA256 != nil {
+				t.addMeaningful(1, "EFI "+m.Key+" UKI UnameSHA")
+			}
 
-		otherSectionChanged := false
-		for sec := range m.UKI.SectionSHA256.Modified {
-			secL := strings.ToLower(strings.TrimSpace(sec))
-			if secL == ".cmdline" || secL == "cmdline" ||
-				secL == ".initrd" || secL == "initrd" {
-				continue
+			otherSectionChanged := false
+			for sec := range m.UKI.SectionSHA256.Modified {
+				secL := strings.ToLower(strings.TrimSpace(sec))
+				if secL == ".cmdline" || secL == "cmdline" ||
+					secL == ".initrd" || secL == "initrd" {
+					continue
+				}
+				otherSectionChanged = true
+				break
+			}
+			if otherSectionChanged {
+				t.addMeaningful(1, "EFI "+m.Key+" UKI otherSectionChanged")
 			}
-			otherSectionChanged = true
-			break
 		}
-		if otherSectionChanged {
-			t.addMeaningful(1, "EFI "+m.Key+" UKI otherSectionChanged")
+
+		// Bootloader config diffs
+		if m.BootConfig != nil {
+			tallyBootloaderConfigDiff(t, m.BootConfig, m.Key)
 		}
 	}
 }
@@ -1009,3 +1060,96 @@ func tallyFilesystemChange(t *diffTally, fs *FilesystemChange) {
 		}
 	}
 }
+
+func tallyBootloaderConfigDiff(t *diffTally, diff *BootloaderConfigDiff, efiKey string) {
+	if diff == nil {
+		return
+	}
+
+	// Config file changes are meaningful (actual bootloader configuration changed)
+	for _, cf := range diff.ConfigFileChanges {
+		switch cf.Status {
+		case "added":
+			t.addMeaningful(1, "BootConfig["+efiKey+"] config file added: "+cf.Path)
+		case "removed":
+			t.addMeaningful(1, "BootConfig["+efiKey+"] config file removed: "+cf.Path)
+		case "modified":
+			t.addMeaningful(1, "BootConfig["+efiKey+"] config file modified: "+cf.Path)
+		}
+	}
+
+	// Boot entry changes are meaningful (boot menu changed)
+	for _, be := range diff.BootEntryChanges {
+		switch be.Status {
+		case "added":
+			t.addMeaningful(1, "BootConfig["+efiKey+"] boot entry added: "+be.Name)
+		case "removed":
+			t.addMeaningful(1, "BootConfig["+efiKey+"] boot entry removed: "+be.Name)
+		case "modified":
+			// Check if kernel path or cmdline actually changed (meaningful)
+			// vs just the display name changed
+			if be.KernelFrom != be.KernelTo {
+				t.addMeaningful(1, "BootConfig["+efiKey+"] boot entry kernel changed: "+be.Name)
+			} else if be.InitrdFrom != be.InitrdTo {
+				t.addMeaningful(1, "BootConfig["+efiKey+"] boot entry initrd changed: "+be.Name)
+			} else if normalizeKernelCmdline(be.CmdlineFrom) != normalizeKernelCmdline(be.CmdlineTo) {
+				t.addMeaningful(1, "BootConfig["+efiKey+"] boot entry cmdline changed: "+be.Name)
+			} else {
+				// Only cosmetic/metadata changes
+				t.addVolatile(1, "BootConfig["+efiKey+"] boot entry metadata changed: "+be.Name)
+			}
+		}
+	}
+
+	// Kernel reference changes
+	for _, kr := range diff.KernelRefChanges {
+		switch kr.Status {
+		case "added":
+			t.addMeaningful(1, "BootConfig["+efiKey+"] kernel ref added: "+kr.Path)
+		case "removed":
+			t.addMeaningful(1, "BootConfig["+efiKey+"] kernel ref removed: "+kr.Path)
+		case "modified":
+			// UUID change is typically volatile (regenerated each build)
+			if kr.UUIDFrom != kr.UUIDTo {
+				t.addVolatile(1, "BootConfig["+efiKey+"] kernel ref UUID changed: "+kr.Path)
+			} else {
+				t.addMeaningful(1, "BootConfig["+efiKey+"] kernel ref modified: "+kr.Path)
+			}
+		}
+	}
+
+	// UUID reference changes - typically volatile (UUIDs regenerate)
+	for _, ur := range diff.UUIDReferenceChanges {
+		switch ur.Status {
+		case "added":
+			// New UUID reference found - could be meaningful or volatile depending on context
+			if ur.MismatchTo {
+				// UUID mismatch is a potential issue - meaningful
+				t.addMeaningful(1, "BootConfig["+efiKey+"] UUID ref mismatch added: "+ur.UUID)
+			} else {
+				t.addVolatile(1, "BootConfig["+efiKey+"] UUID ref added: "+ur.UUID)
+			}
+		case "removed":
+			if ur.MismatchFrom {
+				t.addMeaningful(1, "BootConfig["+efiKey+"] UUID ref mismatch removed: "+ur.UUID)
+			} else {
+				t.addVolatile(1, "BootConfig["+efiKey+"] UUID ref removed: "+ur.UUID)
+			}
+		case "modified":
+			// UUID context changed - typically volatile unless introducing/fixing mismatch
+			if ur.MismatchFrom != ur.MismatchTo {
+				t.addMeaningful(1, "BootConfig["+efiKey+"] UUID ref mismatch status changed: "+ur.UUID)
+			} else {
+				t.addVolatile(1, "BootConfig["+efiKey+"] UUID ref context changed: "+ur.UUID)
+			}
+		}
+	}
+
+	// Notes changes are informational - count as volatile
+	if len(diff.NotesAdded) > 0 {
+		t.addVolatile(len(diff.NotesAdded), "BootConfig["+efiKey+"] notes added")
+	}
+	if len(diff.NotesRemoved) > 0 {
+		t.addVolatile(len(diff.NotesRemoved), "BootConfig["+efiKey+"] notes removed")
+	}
+}