efi/preinstall: refine the checks for manufacturing mode on Intel

This refines the checks that run on Intel systems so that appropriate
error messages are returned if the system is in manufacturing mode. The
changes are based on checks in the coreboot repository, specifically
https://github.com/coreboot/coreboot/blob/eb5bdf06b92534b6f66f612297a4ccb69008b4ac/src/soc/intel/common/block/cse/cse_spec.c#L15

Fixes: FR-12761
Fixes: #515

Author: chrisccoulson

efi/preinstall/check_host_security_amd64_test.go

@@ -134,11 +134,11 @@ C7E003CB
 	)
 
 	err := CheckHostSecurity(env, nil)
-	c.Check(err, ErrorMatches, `encountered an error when checking Intel BootGuard configuration: no hardware root-of-trust properly configured: ME is in manufacturing mode`)
+	c.Check(err, ErrorMatches, `encountered an error when checking Intel BootGuard configuration: no hardware root-of-trust properly configured: system is in manufacturing mode`)
 
 	var nhrotErr *NoHardwareRootOfTrustError
 	c.Check(errors.As(err, &nhrotErr), testutil.IsTrue)
-	c.Check(nhrotErr, ErrorMatches, `no hardware root-of-trust properly configured: ME is in manufacturing mode`)
+	c.Check(nhrotErr, ErrorMatches, `no hardware root-of-trust properly configured: system is in manufacturing mode`)
 }
 
 func (s *hostSecurityAMD64Suite) TestCheckHostSecurityAMDErrPSP(c *C) {

efi/preinstall/check_host_security_intel.go

@@ -61,19 +61,12 @@ func (reg hfsts1) operationMode() meOperationMode {
 }
 
 const (
-	// hfsts1MfgMode indicates that the ME is in manufacturing mode. Fwupd refers to this
-	// as manufacturing mode for CSME #11 and SPI protection mode for CSME #18. Slimbootloader
-	// refers to this as the latter for everything other than Comet Lake.
-	hfsts1MfgMode hfsts1 = 1 << 4
-
 	// hfsts1OperationMode is the ME operation mode bitmask, from fwupd and slimbootloader.
 	hfsts1OperationMode hfsts1 = 0xf0000
 
 	hfsts5BtgAcmActive hfsts5 = 1 << 0
 	hfsts5BtgAcmDone   hfsts5 = 1 << 8
 
-	hfsts6FPFSOCLock hfsts6 = 1 << 30
-
 	meOperationModeNormal         meOperationMode = 0x0
 	meOperationModeDebug          meOperationMode = 0x2
 	meOperationModeDisabled       meOperationMode = 0x3
@@ -316,24 +309,14 @@ func checkHostSecurityIntelBootGuard(env internal_efi.HostEnvironment) error {
 		return &NoHardwareRootOfTrustError{errors.New("invalid ME operation mode")}
 	}
 
-	// Check manufacturing mode is not enabled.
-	if regs.Hfsts1&hfsts1MfgMode > 0 {
-		return &NoHardwareRootOfTrustError{errors.New("ME is in manufacturing mode")}
-	}
-
 	// Check that the BootGuard ACM is active. Fwupd only checks this for CSME #18, but it
 	// appears that the same bits are defined for both versions.
 	if regs.Hfsts5&(hfsts5BtgAcmActive|hfsts5BtgAcmDone) != hfsts5BtgAcmActive|hfsts5BtgAcmDone {
 		return &NoHardwareRootOfTrustError{errors.New("BootGuard ACM is not active")}
 	}
 
-	// Check that the FPFs are locked.
-	if regs.Hfsts6&hfsts6FPFSOCLock == 0 {
-		return &NoHardwareRootOfTrustError{errors.New("BootGuard OTP fuses are not locked")}
-	}
-
 	if vers.Major < 18 {
-		return checkHostSecurityIntelBootGuardCSME11(toHfstsRegistersCsme11(regs))
+		return checkHostSecurityIntelBootGuardCSME11(vers, toHfstsRegistersCsme11(regs))
 	}
 
 	return checkHostSecurityIntelBootGuardCSME18(toHfstsRegistersCsme18(regs))

efi/preinstall/check_host_security_intel_csme11.go

@@ -83,23 +83,55 @@ func toHfstsRegistersCsme11(regs hfstsRegisters) hfstsRegistersCsme11 {
 }
 
 const (
+	// hfsts1Csme11MfgMode indicates that the system is in manufacturing mode. Note that
+	// fwupd and coreboot refer to this bit as manufacturing mode whilst slimbootloader refers
+	// to this as SPI protection mode. Based on this and the comments in coreboot, this bit
+	// is set when the SPI flash descriptor is locked.
+	hfsts1Csme11MfgMode hfsts1Csme11 = 1 << 4
+
 	hfsts6Csme11ForceBootPolicy        hfsts6Csme11 = 1 << 0
 	hfsts6Csme11CpuDebugDisable        hfsts6Csme11 = 1 << 1
 	hfsts6Csme11ProtectBIOSEnv         hfsts6Csme11 = 1 << 3
 	hfsts6Csme11ErrorEnforcementPolicy hfsts6Csme11 = 0xc0
 	hfsts6Csme11MeasuredBoot           hfsts6Csme11 = 1 << 8
 	hfsts6Csme11VerifiedBoot           hfsts6Csme11 = 1 << 9
+	hfsts6Csme11MfgLock                hfsts6Csme11 = 1 << 21
 	hfsts6Csme11BootGuardDisable       hfsts6Csme11 = 1 << 28
+	hfsts6Csme11FPFSOCLock             hfsts6Csme11 = 1 << 30
 
 	errorEnforcementPolicyCsme11Nothing        errorEnforcementPolicyCsme11 = 0
 	errorEnforcementPolicyCsme11Shutdown30Mins errorEnforcementPolicyCsme11 = 1 // fwupd defines this as 3, which I think is wrong.
 	errorEnforcementPolicyCsme11ShutdownNow    errorEnforcementPolicyCsme11 = 3 // fwupd defines this as 2, which I think is wrong.
 )
 
-func checkHostSecurityIntelBootGuardCSME11(regs hfstsRegistersCsme11) error {
+func isInManufacturingModeCSME11(vers meVersion, regs hfstsRegistersCsme11) bool {
+	// This is based on the checks from
+	// https://github.com/coreboot/coreboot/blob/eb5bdf06b92534b6f66f612297a4ccb69008b4ac/src/soc/intel/common/block/cse/cse_spec.c#L15
+	if regs.Hfsts1&hfsts1Csme11MfgMode > 0 {
+		return true
+	}
+	if vers.Major > 13 {
+		if regs.Hfsts6&hfsts6Csme11FPFSOCLock == 0 {
+			return true
+		}
+	}
+	if vers.Major > 15 {
+		if regs.Hfsts6&hfsts6Csme11MfgLock == 0 {
+			return true
+		}
+	}
+	return false
+}
+
+func checkHostSecurityIntelBootGuardCSME11(vers meVersion, regs hfstsRegistersCsme11) error {
 	// These checks are based on the HSI checks performed in the pci-mei
 	// plugin in fwupd.
 
+	// Make sure that the system is not in manufacturing mode.
+	if isInManufacturingModeCSME11(vers, regs) {
+		return &NoHardwareRootOfTrustError{errors.New("system is in manufacturing mode")}
+	}
+
 	// Check that BootGuard is enabled.
 	if regs.Hfsts6&hfsts6Csme11BootGuardDisable > 0 {
 		return &NoHardwareRootOfTrustError{errors.New("BootGuard is disabled")}

efi/preinstall/check_host_security_intel_csme11_test.go

@@ -32,35 +32,96 @@ type hostSecurityIntelCsme11Suite struct{}
 var _ = Suite(&hostSecurityIntelCsme11Suite{})
 
 func (*hostSecurityIntelCsme11Suite) TestCheckHostSecurityIntelBootGuardCSME11GoodFVMEProfile(c *C) {
-	err := CheckHostSecurityIntelBootGuardCSME11(HfstsRegistersCsme11{Hfsts6: 0xC7E003CB})
+	err := CheckHostSecurityIntelBootGuardCSME11(MeVersion{Major: 16}, HfstsRegistersCsme11{
+		Hfsts1: 0x94000245,
+		Hfsts6: 0xC7E003CB,
+	})
 	c.Check(err, IsNil)
 }
 
 func (*hostSecurityIntelCsme11Suite) TestCheckHostSecurityIntelBootGuardCSME11GoodFVEProfile(c *C) {
-	err := CheckHostSecurityIntelBootGuardCSME11(HfstsRegistersCsme11{Hfsts6: 0xC7E002CB})
+	err := CheckHostSecurityIntelBootGuardCSME11(MeVersion{Major: 16}, HfstsRegistersCsme11{
+		Hfsts1: 0x94000245,
+		Hfsts6: 0xC7E002CB,
+	})
 	c.Check(err, IsNil)
 }
 
+func (*hostSecurityIntelCsme11Suite) TestCheckHostSecurityIntelBootGuardCSME11Good13(c *C) {
+	err := CheckHostSecurityIntelBootGuardCSME11(MeVersion{Major: 13}, HfstsRegistersCsme11{
+		Hfsts1: 0x94000245,
+		Hfsts6: 0x87C003CB,
+	})
+	c.Check(err, IsNil)
+}
+
+func (*hostSecurityIntelCsme11Suite) TestCheckHostSecurityIntelBootGuardCSME11Good15(c *C) {
+	err := CheckHostSecurityIntelBootGuardCSME11(MeVersion{Major: 15}, HfstsRegistersCsme11{
+		Hfsts1: 0x94000245,
+		Hfsts6: 0xC7C003CB,
+	})
+	c.Check(err, IsNil)
+}
+
+func (*hostSecurityIntelCsme11Suite) TestCheckHostSecurityIntelBootGuardCSME11ErrMfgMode(c *C) {
+	err := CheckHostSecurityIntelBootGuardCSME11(MeVersion{Major: 13}, HfstsRegistersCsme11{
+		Hfsts1: 0x94000255,
+		Hfsts6: 0x87C003CB,
+	})
+	c.Check(err, ErrorMatches, `no hardware root-of-trust properly configured: system is in manufacturing mode`)
+	c.Check(err, FitsTypeOf, &NoHardwareRootOfTrustError{})
+}
+
+func (*hostSecurityIntelCsme11Suite) TestCheckHostSecurityIntelBootGuardCSME11ErrFPFsNotLocked(c *C) {
+	err := CheckHostSecurityIntelBootGuardCSME11(MeVersion{Major: 15}, HfstsRegistersCsme11{
+		Hfsts1: 0x94000245,
+		Hfsts6: 0x87C003CB,
+	})
+	c.Check(err, ErrorMatches, `no hardware root-of-trust properly configured: system is in manufacturing mode`)
+	c.Check(err, FitsTypeOf, &NoHardwareRootOfTrustError{})
+}
+
+func (*hostSecurityIntelCsme11Suite) TestCheckHostSecurityIntelBootGuardCSME11ErrNoManufLock(c *C) {
+	err := CheckHostSecurityIntelBootGuardCSME11(MeVersion{Major: 16}, HfstsRegistersCsme11{
+		Hfsts1: 0x94000245,
+		Hfsts6: 0x87C003CB,
+	})
+	c.Check(err, ErrorMatches, `no hardware root-of-trust properly configured: system is in manufacturing mode`)
+	c.Check(err, FitsTypeOf, &NoHardwareRootOfTrustError{})
+}
+
 func (*hostSecurityIntelCsme11Suite) TestCheckHostSecurityIntelBootGuardCSME11ErrBootGuardDisabled(c *C) {
-	err := CheckHostSecurityIntelBootGuardCSME11(HfstsRegistersCsme11{Hfsts6: 0xD7E003CB})
+	err := CheckHostSecurityIntelBootGuardCSME11(MeVersion{Major: 16}, HfstsRegistersCsme11{
+		Hfsts1: 0x94000245,
+		Hfsts6: 0xD7E003CB,
+	})
 	c.Check(err, ErrorMatches, `no hardware root-of-trust properly configured: BootGuard is disabled`)
 	c.Check(err, FitsTypeOf, &NoHardwareRootOfTrustError{})
 }
 
 func (*hostSecurityIntelCsme11Suite) TestCheckHostSecurityIntelBootGuardCSME11ErrInvalidProfile(c *C) {
-	err := CheckHostSecurityIntelBootGuardCSME11(HfstsRegistersCsme11{Hfsts6: 0xC7E0024A})
+	err := CheckHostSecurityIntelBootGuardCSME11(MeVersion{Major: 16}, HfstsRegistersCsme11{
+		Hfsts1: 0x94000245,
+		Hfsts6: 0xC7E0024A,
+	})
 	c.Check(err, ErrorMatches, `no hardware root-of-trust properly configured: cannot determine BootGuard profile: invalid profile`)
 	c.Check(err, FitsTypeOf, &NoHardwareRootOfTrustError{})
 }
 
 func (*hostSecurityIntelCsme11Suite) TestCheckHostSecurityIntelBootGuardCSME11ErrUnsupportedNoFVMEProfile(c *C) {
-	err := CheckHostSecurityIntelBootGuardCSME11(HfstsRegistersCsme11{Hfsts6: 0xC7E00002})
+	err := CheckHostSecurityIntelBootGuardCSME11(MeVersion{Major: 16}, HfstsRegistersCsme11{
+		Hfsts1: 0x94000245,
+		Hfsts6: 0xC7E00002,
+	})
 	c.Check(err, ErrorMatches, `no hardware root-of-trust properly configured: unsupported BootGuard profile`)
 	c.Check(err, FitsTypeOf, &NoHardwareRootOfTrustError{})
 }
 
 func (*hostSecurityIntelCsme11Suite) TestCheckHostSecurityIntelBootGuardCSME11ErrUnsupportedVMProfile(c *C) {
-	err := CheckHostSecurityIntelBootGuardCSME11(HfstsRegistersCsme11{Hfsts6: 0xC7E0030A})
+	err := CheckHostSecurityIntelBootGuardCSME11(MeVersion{Major: 16}, HfstsRegistersCsme11{
+		Hfsts1: 0x94000245,
+		Hfsts6: 0xC7E0030A,
+	})
 	c.Check(err, ErrorMatches, `no hardware root-of-trust properly configured: unsupported BootGuard profile`)
 	c.Check(err, FitsTypeOf, &NoHardwareRootOfTrustError{})
 }

efi/preinstall/check_host_security_intel_csme18.go

@@ -46,8 +46,13 @@ func (reg hfsts5Csme18) btgProfile() btgProfile {
 }
 
 const (
+	hfsts1Csme18SPIProtectionMode hfsts1Csme18 = 1 << 4
+
 	hfsts5Csme18BtgProfileValid hfsts5Csme18 = 1 << 1
 
+	hfsts6Csme18MfgLock    hfsts6Csme18 = 1 << 21
+	hfsts6Csme18FPFSOCLock hfsts6Csme18 = 1 << 30
+
 	// hfsts5Csme18BtgProfile is the bitmask for the BootGuard profile.
 	// fwupd defines this as 0xe0000, but I think this is off by one bit.
 	// I see a profile value of 5 on my XPS16 (which is what I expect) if
@@ -68,10 +73,30 @@ func toHfstsRegistersCsme18(regs hfstsRegisters) hfstsRegistersCsme18 {
 	}
 }
 
+func isInManufacturingModeCSME18(regs hfstsRegistersCsme18) bool {
+	// This is based on the checks from
+	// https://github.com/coreboot/coreboot/blob/eb5bdf06b92534b6f66f612297a4ccb69008b4ac/src/soc/intel/common/block/cse/cse_spec.c#L15
+	if regs.Hfsts1&hfsts1Csme18SPIProtectionMode > 0 {
+		return true
+	}
+	if regs.Hfsts6&hfsts6Csme18MfgLock == 0 {
+		return true
+	}
+	if regs.Hfsts6&hfsts6Csme18FPFSOCLock == 0 {
+		return true
+	}
+	return false
+}
+
 func checkHostSecurityIntelBootGuardCSME18(regs hfstsRegistersCsme18) error {
 	// These checks are based on the HSI checks performed in the pci-mei
 	// plugin in fwupd.
 
+	// Make sure that the system is not in manufacturing mode.
+	if isInManufacturingModeCSME18(regs) {
+		return &NoHardwareRootOfTrustError{errors.New("system is in manufacturing mode")}
+	}
+
 	// Check that the BootGuard profile is valid. I think that's what this
 	// is checking - fwupd's definition of this bit is just called "valid".
 	// This bit does exist for CSME #11, but the definition in slimbootloader

efi/preinstall/check_host_security_intel_csme18_test.go

@@ -32,29 +32,79 @@ type hostSecurityIntelCsme18Suite struct{}
 var _ = Suite(&hostSecurityIntelCsme18Suite{})
 
 func (*hostSecurityIntelCsme18Suite) TestCheckHostSecurityIntelBootGuardCSME18GoodFVMEProfile(c *C) {
-	err := CheckHostSecurityIntelBootGuardCSME18(HfstsRegistersCsme18{Hfsts5: 0x02F61F03})
+	err := CheckHostSecurityIntelBootGuardCSME18(HfstsRegistersCsme18{
+		Hfsts1: 0x94000245,
+		Hfsts5: 0x02F61F03,
+		Hfsts6: 0x40200000,
+	})
 	c.Check(err, IsNil)
 }
 
 func (*hostSecurityIntelCsme18Suite) TestCheckHostSecurityIntelBootGuardCSME18GoodFVEProfile(c *C) {
-	err := CheckHostSecurityIntelBootGuardCSME18(HfstsRegistersCsme18{Hfsts5: 0x02F21F03})
+	err := CheckHostSecurityIntelBootGuardCSME18(HfstsRegistersCsme18{
+		Hfsts1: 0x94000245,
+		Hfsts5: 0x02F21F03,
+		Hfsts6: 0x40200000,
+	})
 	c.Check(err, IsNil)
 }
 
+func (*hostSecurityIntelCsme18Suite) TestCheckHostSecurityIntelBootGuardCSME18ErrNoSPIProtection(c *C) {
+	err := CheckHostSecurityIntelBootGuardCSME18(HfstsRegistersCsme18{
+		Hfsts1: 0x94000255,
+		Hfsts5: 0x02F61F03,
+		Hfsts6: 0x40200000,
+	})
+	c.Check(err, ErrorMatches, `no hardware root-of-trust properly configured: system is in manufacturing mode`)
+	c.Check(err, FitsTypeOf, &NoHardwareRootOfTrustError{})
+}
+
+func (*hostSecurityIntelCsme18Suite) TestCheckHostSecurityIntelBootGuardCSME18ErrFPFsNotLocked(c *C) {
+	err := CheckHostSecurityIntelBootGuardCSME18(HfstsRegistersCsme18{
+		Hfsts1: 0x94000245,
+		Hfsts5: 0x02F61F03,
+		Hfsts6: 0x00200000,
+	})
+	c.Check(err, ErrorMatches, `no hardware root-of-trust properly configured: system is in manufacturing mode`)
+	c.Check(err, FitsTypeOf, &NoHardwareRootOfTrustError{})
+}
+
+func (*hostSecurityIntelCsme18Suite) TestCheckHostSecurityIntelBootGuardCSME18ErrNoManufLock(c *C) {
+	err := CheckHostSecurityIntelBootGuardCSME18(HfstsRegistersCsme18{
+		Hfsts1: 0x94000245,
+		Hfsts5: 0x02F61F03,
+		Hfsts6: 0x40000000,
+	})
+	c.Check(err, ErrorMatches, `no hardware root-of-trust properly configured: system is in manufacturing mode`)
+	c.Check(err, FitsTypeOf, &NoHardwareRootOfTrustError{})
+}
+
 func (*hostSecurityIntelCsme18Suite) TestCheckHostSecurityIntelBootGuardCSME18ErrInvalidProfile(c *C) {
-	err := CheckHostSecurityIntelBootGuardCSME18(HfstsRegistersCsme18{Hfsts5: 0x02F61F01})
+	err := CheckHostSecurityIntelBootGuardCSME18(HfstsRegistersCsme18{
+		Hfsts1: 0x94000245,
+		Hfsts5: 0x02F61F01,
+		Hfsts6: 0x40200000,
+	})
 	c.Check(err, ErrorMatches, `no hardware root-of-trust properly configured: invalid BootGuard profile`)
 	c.Check(err, FitsTypeOf, &NoHardwareRootOfTrustError{})
 }
 
 func (*hostSecurityIntelCsme18Suite) TestCheckHostSecurityIntelBootGuardCSME18ErrUnsupportedNoFVMEProfile(c *C) {
-	err := CheckHostSecurityIntelBootGuardCSME18(HfstsRegistersCsme18{Hfsts5: 0x02E21F03})
+	err := CheckHostSecurityIntelBootGuardCSME18(HfstsRegistersCsme18{
+		Hfsts1: 0x94000245,
+		Hfsts5: 0x02E21F03,
+		Hfsts6: 0x40200000,
+	})
 	c.Check(err, ErrorMatches, `no hardware root-of-trust properly configured: unsupported BootGuard profile`)
 	c.Check(err, FitsTypeOf, &NoHardwareRootOfTrustError{})
 }
 
 func (*hostSecurityIntelCsme18Suite) TestCheckHostSecurityIntelBootGuardCSME18ErrUnsupportedVMProfile(c *C) {
-	err := CheckHostSecurityIntelBootGuardCSME18(HfstsRegistersCsme18{Hfsts5: 0x02EE1F03})
+	err := CheckHostSecurityIntelBootGuardCSME18(HfstsRegistersCsme18{
+		Hfsts1: 0x94000245,
+		Hfsts5: 0x02EE1F03,
+		Hfsts6: 0x40200000,
+	})
 	c.Check(err, ErrorMatches, `no hardware root-of-trust properly configured: unsupported BootGuard profile`)
 	c.Check(err, FitsTypeOf, &NoHardwareRootOfTrustError{})
 }