efi/preinstall: moved manual preinstall check actions after automatic actions

Author: ernestl

efi/preinstall/actions.go

@@ -60,12 +60,12 @@ const (
 	ActionRebootToFWSettings Action = "reboot-to-fw-settings"
 
 	// ActionContactOEM is a hint that the user should contact the OEM for the
-	// device because of a bug in the platform. It is a pseudo-action and cannnot
+	// device because of a bug in the platform. It is a pseudo-action and cannot
 	// be performed by this package.
 	ActionContactOEM Action = "contact-oem"
 
 	// ActionContactOSVendor is a hint that the user should contact the OS vendor
-	// because of a bug in the OS. It is a pseudo-action and cannnot be performed
+	// because of a bug in the OS. It is a pseudo-action and cannot be performed
 	// by this package.
 	ActionContactOSVendor Action = "contact-os-vendor"
 

efi/preinstall/checks_context.go

@@ -35,6 +35,13 @@ import (
 // errorKindToActions maps an error kind to one or more possible actions. The
 // slice of actions is an OR in the sense that any one of these actions can
 // be executed to attempt to resolve the associated error kind.
+//
+// The order of actions is significant as it represents the recommended order
+// in which users should attempt to resolve issues. Actions are ordered by:
+//   1. Automatic actions before manual actions e.g. automatic PPI-based actions
+//      are listed before manual actions that require rebooting to firmware settings.
+//   2. Simplest actions first e.g. ActionClearTPMViaFirmware before
+//      ActionEnableAndClearTPMViaFirmware
 var errorKindToActions map[ErrorKind][]Action
 
 // errorKindToProceedFlag maps an error kind to a flag that can be set
@@ -57,28 +64,28 @@ func init() {
 			ActionContactOEM,
 		},
 		ErrorKindTPMDeviceDisabled: []Action{
-			ActionRebootToFWSettings,           // suggest rebooting to the firmware settings UI to enable the TPM
 			ActionEnableTPMViaFirmware,         // suggest enabling the TPM via the PPI
 			ActionEnableAndClearTPMViaFirmware, // suggest enabling and clearing the TPM via the PPI
+			ActionRebootToFWSettings,           // suggest rebooting to the firmware settings UI to enable the TPM
 		},
 		ErrorKindTPMHierarchiesOwned: []Action{
-			ActionRebootToFWSettings,           // suggest rebooting to the firmware settings UI to clear the TPM
 			ActionClearTPMViaFirmware,          // suggest clearing the TPM via the PPI
 			ActionEnableAndClearTPMViaFirmware, // suggest enabling and clearing the TPM via the PPI
+			ActionRebootToFWSettings,           // suggest rebooting to the firmware settings UI to clear the TPM
 			ActionClearTPMSimple,               // suggest clearing the TPM using TPM2_Clear
 			ActionClearTPM,                     // suggest clearing the TPM using TPM2_Clear
 			// TODO: Add action to clear the authorization values / policies
 		},
 		ErrorKindTPMDeviceLockoutLockedOut: []Action{
-			ActionRebootToFWSettings,           // suggest rebooting to the firmware settings UI to clear the TPM
 			ActionClearTPMViaFirmware,          // suggest clearing the TPM via the PPI
 			ActionEnableAndClearTPMViaFirmware, // suggest enabling and clearing the TPM via the PPI
+			ActionRebootToFWSettings,           // suggest rebooting to the firmware settings UI to clear the TPM
 			// There will be no option to clear the lockout as there isn't a mechanism to do this.
 		},
 		ErrorKindInsufficientTPMStorage: []Action{
-			ActionRebootToFWSettings,           // suggest rebooting to the firmware settings UI to clear the TPM
 			ActionClearTPMViaFirmware,          // suggest clearing the TPM via the PPI
 			ActionEnableAndClearTPMViaFirmware, // suggest enabling and clearing the TPM via the PPI
+			ActionRebootToFWSettings,           // suggest rebooting to the firmware settings UI to clear the TPM
 			ActionClearTPMSimple,               // suggest clearing the TPM using TPM2_Clear
 			ActionClearTPM,                     // suggest clearing the TPM using TPM2_Clear
 		},
@@ -95,8 +102,8 @@ func init() {
 			ActionContactOEM, // suggest contacting the OEM because of a firmware bug
 		},
 		ErrorKindInsufficientDMAProtection: []Action{
-			ActionContactOEM,         // suggest contacting the OEM because of a firmware bug.
 			ActionRebootToFWSettings, // suggest rebooting to the firmware settings UI to enable DMA protection.
+			ActionContactOEM,         // suggest contacting the OEM because of a firmware bug.
 		},
 		ErrorKindNoKernelIOMMU: []Action{
 			ActionRebootToFWSettings, // suggest rebooting to the firmware settings UI to enable DMA protection.
@@ -115,8 +122,8 @@ func init() {
 			// TODO: Add an action to just disable these by erasing the SysPrepOrder and SysPrepXXXX variables
 		},
 		ErrorKindAbsolutePresent: []Action{
-			ActionContactOEM,         // suggest contacting the OEM if there's no way to disable it.
 			ActionRebootToFWSettings, // suggest rebooting to the firmware settings UI to disable it.
+			ActionContactOEM,         // suggest contacting the OEM if there's no way to disable it.
 			// TODO: Add an action to just disable this automatically on supported platforms (eg, Dell via the WMI interface)
 		},
 		ErrorKindInvalidSecureBootMode: []Action{
@@ -397,7 +404,7 @@ func (c *RunChecksContext) classifyRunChecksError(err error) (info errorInfo, ou
 		return errorInfo{kind: ErrorKindInsufficientTPMStorage}, nil
 	}
 
-	// This has to become before MeasuredBootError because that error wraps this one.
+	// This has to come before MeasuredBootError because that error wraps this one.
 	var pcrAlgErr *NoSuitablePCRAlgorithmError
 	if errors.As(err, &pcrAlgErr) {
 		// RunChecks indicates that there is no suitable PCR bank. The possibilities here:

efi/preinstall/errors.go

@@ -37,7 +37,7 @@ import (
 // to the supplied string, useful for displaying multiple errors. In a multi-line
 // string, subsequent lines in the supplied string will all be aligned with the start
 // of the first line after the marker. The indentation argument specifies the
-// indentation of the maker, in the number of characters.
+// indentation of the marker, in the number of characters.
 func makeIndentedListItem(indentation int, marker, str string) string {
 	scanner := bufio.NewScanner(bytes.NewReader([]byte(str)))
 
@@ -94,7 +94,7 @@ func makeIndentedListItem(indentation int, marker, str string) string {
 // CompoundError is an interface for accessing wrapped errors from an error type that
 // wraps more than one error. The [RunChecks] and [RunChecksContext.Run] APIs may return
 // multiple errors that are wrapped by a type implementing this interface, as an
-// alternative to aborting early and returning individual errors as the occur. This is
+// alternative to aborting early and returning individual errors as they occur. This is
 // to ensure as much information is gathered as possible.
 type CompoundError interface {
 	Unwrap() []error
@@ -185,7 +185,7 @@ func (e *EFIVariableAccessError) Unwrap() error {
 var (
 	// ErrSystemNotEFI is returned unwrapped from RunChecks if the current host
 	// system does not appear to be an EFI system.
-	ErrSystemNotEFI = errors.New("host system is not a EFI system")
+	ErrSystemNotEFI = errors.New("host system is not an EFI system")
 )
 
 // Errors related to checking platform firmware protections.
@@ -263,7 +263,7 @@ var (
 
 	// ErrTPMStartupLocalityNotProtected is returned wrapped in HostSecurityError if access to
 	// the TPM's startup locality is available to platform firmware or privileged code. This
-	// means that it's not possible to provide a mitigation against reseet attacks (see the
+	// means that it's not possible to provide a mitigation against reset attacks (see the
 	// description of DiscreteTPMDetected). This error is only relevant for discrete TPMs.
 	// It can be permitted by passing the PermitNoDiscreteTPMResetMitigation flag to RunChecks.
 	ErrTPMStartupLocalityNotProtected = errors.New("access to the discrete TPM's startup locality is available to platform firmware and privileged OS code, preventing any mitigation against reset attacks")
@@ -331,7 +331,7 @@ var (
 
 	// ErrNoPCClientTPM is returned wrapped in TPM2DeviceError if a TPM2 device exists but
 	// it doesn't claim to be meet the requirements for PC-Client. Note that swtpm used
-	// by VM's don't behave correctly here, so we account for that instead of returning
+	// by VMs don't behave correctly here, so we account for that instead of returning
 	// an error.
 	ErrNoPCClientTPM = errors.New("TPM2 device is present but it is not a PC-Client TPM")
 
@@ -344,7 +344,7 @@ var (
 	// ErrTPMFailure is returned wrapped in TPM2DeviceError is the TPM device is in
 	// failure mode. A TPM device in failure mode can only execute commands to obtain
 	// test results, or fetch a limited set of permanent properties to determine the
-	// manufactuer, vendor name or firmware version. Resetting a device in failure mode
+	// manufacturer, vendor name or firmware version. Resetting a device in failure mode
 	// may clear it but it's possible that the failure may occur again during the next
 	// boot cycle, in which case, it's likely that there is a fault somewhere with the
 	// TPM's hardware (in the case of dTPMs) or the TPM's firmware.
@@ -507,7 +507,7 @@ func (e *MeasuredBootError) Unwrap() error {
 // cannot be used to generate profiles for PCR 0.
 //
 // If an error occurs, this error will be returned wrapped in
-// [NoSuitablePCRAlgorithmError] if the PermitNolatformFirmwareProfileSupport flag
+// [NoSuitablePCRAlgorithmError] if the PermitNoPlatformFirmwareProfileSupport flag
 // is not supplied to [RunChecks].
 type PlatformFirmwarePCRError struct {
 	err error
@@ -523,7 +523,7 @@ func (e *PlatformFirmwarePCRError) Unwrap() error {
 
 // Errors related to platform config PCR checks
 
-// PlatformConfigPCR may be returned if the PCR 1 value is inconsistent with the
+// PlatformConfigPCRError may be returned if the PCR 1 value is inconsistent with the
 // value reconstructed from the TCG log.
 //
 // This error will currently always be returned as a warning in [CheckResult] if
@@ -553,7 +553,7 @@ func (e *PlatformConfigPCRError) Unwrap() error {
 
 // Errors related to drivers and apps PCR checks.
 
-// DriversAndAppsError may be returned if the PCR 2 value is inconsistent with the
+// DriversAndAppsPCRError may be returned if the PCR 2 value is inconsistent with the
 // value reconstructed from the TCG log.
 //
 // If an error occurs, this error will be returned as a warning in [CheckResult] if
@@ -631,7 +631,7 @@ func (e *DriversAndAppsConfigPCRError) Unwrap() error {
 //   - EV_EFI_BOOT_SERVICES_APPLICATION events that occur before secure boot policy
 //     is measured.
 //   - Unexpected event types before the OS-present phase.
-//   - The presence of system prepartion apps when the firmware indicates they are
+//   - The presence of system preparation apps when the firmware indicates they are
 //     not supported.
 //   - EV_EFI_BOOT_SERVICES_APPLICATION events that occur in the OS-present phase
 //     but aren't associated with the OS launch or Absolute.
@@ -823,7 +823,7 @@ var (
 	// indicates that pre-OS components were authenticated using Authenticode digests rather than a
 	// X.509 certificate. This makes PCR7 inherently fragile with regards to firmware updates because db
 	// has to be changed accordingly each time.
-	// This can be bypassed by supplying the PermitPreOSVeriricationUsingDigests flag to RunChecks, in
+	// This can be bypassed by supplying the PermitPreOSVerificationUsingDigests flag to RunChecks, in
 	// which case, the error is returned as a warning via CheckResult.
 	//
 	// The check for pre-OS components authenticated using a digest may not execute if a