diff --git a/doc/security.md b/doc/security.md new file mode 100644 index 0000000..7fb686b --- /dev/null +++ b/doc/security.md @@ -0,0 +1,13 @@ +# Firmware security + +The firmware security feature can be configured by setting `CONFIG_SECURITY=1` +in the `src/board/system76/[board]/board.mk` file. This feature prevents +programming the EC firmware at runtime, unless the EC is unlocked with the +`system76-ectool security unlock` command. After this, on the next reboot, the +EC will respond to the SPI and reset commands. On boards where the `ME_WE` GPIO +exists, it will be set high when the EC security state is unlocked. + +Other firmware components can use this state to perform their own locking and +unlocking primitives. For example, in `coreboot`, flash regions may be locked +when the EC security state is locked. In `EDK2`, a physical presence dialog may +be shown when the EC security state is unlocked. diff --git a/src/board/system76/common/common.mk b/src/board/system76/common/common.mk index 8707136..bbb55b0 100644 --- a/src/board/system76/common/common.mk +++ b/src/board/system76/common/common.mk @@ -22,6 +22,7 @@ board-common-y += power.c board-common-y += ps2.c board-common-y += pwm.c board-common-y += scratch.c +board-common-$(CONFIG_SECURITY) += security.c board-common-y += smbus.c board-common-y += smfi.c board-common-y += stdio.c @@ -42,6 +43,10 @@ CFLAGS+=-DLEVEL=4 # Uncomment to enable I2C debug on 0x76 #CFLAGS+=-DI2C_DEBUGGER=0x76 +ifeq ($(CONFIG_SECURITY),y) +CFLAGS+=-DCONFIG_SECURITY=1 +endif + # Set external programmer PROGRAMMER=$(wildcard /dev/serial/by-id/usb-Arduino*) diff --git a/src/board/system76/common/include/board/security.h b/src/board/system76/common/include/board/security.h new file mode 100644 index 0000000..aecca07 --- /dev/null +++ b/src/board/system76/common/include/board/security.h @@ -0,0 +1,13 @@ +// SPDX-License-Identifier: GPL-3.0-only + +#ifndef _BOARD_SECURITY_H +#define _BOARD_SECURITY_H + +#include +#include + +enum SecurityState security_get(void); +bool security_set(enum SecurityState state); +bool security_power(void); + +#endif // _BOARD_SECURITY_H diff --git a/src/board/system76/common/power.c b/src/board/system76/common/power.c index 8533796..8f3ce70 100644 --- a/src/board/system76/common/power.c +++ b/src/board/system76/common/power.c @@ -22,6 +22,10 @@ #include #endif +#if CONFIG_SECURITY +#include +#endif // CONFIG_SECURITY + #define GPIO_SET_DEBUG(G, V) \ { \ DEBUG("%s = %s\n", #G, V ? "true" : "false"); \ @@ -532,6 +536,13 @@ void power_event(void) { // Disable S5 power plane if not needed if (power_state == POWER_STATE_S5) { power_off(); + +#if CONFIG_SECURITY + // Handle security state changes if necessary + if (security_power()) { + power_on(); + } +#endif // CONFIG_SECURITY } } diff --git a/src/board/system76/common/security.c b/src/board/system76/common/security.c new file mode 100644 index 0000000..d585c56 --- /dev/null +++ b/src/board/system76/common/security.c @@ -0,0 +1,41 @@ +// SPDX-License-Identifier: GPL-3.0-only + +#include +#include + +static enum SecurityState security_state = SECURITY_STATE_LOCK; + +enum SecurityState security_get(void) { + return security_state; +} + +bool security_set(enum SecurityState state) { + switch (state) { + // Allow perpare states to be set + case SECURITY_STATE_PREPARE_LOCK: + case SECURITY_STATE_PREPARE_UNLOCK: + security_state = state; + return true; + // Any other states will be ignored + default: + return false; + } +} + +bool security_power(void) { + switch (security_state) { + // Apply lock state and power on + case SECURITY_STATE_PREPARE_LOCK: + gpio_set(&ME_WE, false); + security_state = SECURITY_STATE_LOCK; + return true; + // Apply unlock state and power on + case SECURITY_STATE_PREPARE_UNLOCK: + gpio_set(&ME_WE, true); + security_state = SECURITY_STATE_UNLOCK; + return true; + // Any other states will be ignored + default: + return false; + } +} diff --git a/src/board/system76/common/smfi.c b/src/board/system76/common/smfi.c index 8e293c6..4b41fcc 100644 --- a/src/board/system76/common/smfi.c +++ b/src/board/system76/common/smfi.c @@ -17,11 +17,16 @@ #include #include -#ifndef __SCRATCH__ +#if !defined(__SCRATCH__) #include #include #include -#endif + +#if CONFIG_SECURITY +#include +#endif // CONFIG_SECURITY + +#endif // !defined(__SCRATCH__) #include #include #include @@ -242,6 +247,23 @@ static enum Result cmd_matrix_get(void) { } return RES_OK; } + +#if CONFIG_SECURITY +static enum Result cmd_security_get(void) { + smfi_cmd[SMFI_CMD_DATA] = security_get(); + return RES_OK; +} + +static enum Result cmd_security_set(void) { + enum SecurityState state = smfi_cmd[SMFI_CMD_DATA]; + if (security_set(state)) { + return RES_OK; + } else { + return RES_ERR; + } +} +#endif // CONFIG_SECURITY + #endif // !defined(__SCRATCH__) #if defined(__SCRATCH__) @@ -286,6 +308,14 @@ static enum Result cmd_spi(void) { #if defined(__SCRATCH__) return cmd_spi_scratch(); #else // defined(__SCRATCH__) + +#if CONFIG_SECURITY + if (security_get() != SECURITY_STATE_UNLOCK) { + // EC must be unlocked to allow flashing + return RES_ERR; + } +#endif // CONFIG_SECURITY + if (smfi_cmd[SMFI_CMD_DATA] & CMD_SPI_FLAG_SCRATCH) { scratch_trampoline(); } @@ -296,6 +326,17 @@ static enum Result cmd_spi(void) { } static enum Result cmd_reset(void) { +#if !defined(__SCRATCH__) + +#if CONFIG_SECURITY + if (security_get() != SECURITY_STATE_UNLOCK) { + // EC must be unlocked to allow watchdog reset + return RES_ERR; + } +#endif // CONFIG_SECURITY + +#endif // !defined(__SCRATCH__) + // Attempt to trigger watchdog reset ETWCFG |= BIT(5); EWDKEYR = 0; @@ -370,6 +411,16 @@ void smfi_event(void) { case CMD_MATRIX_GET: smfi_cmd[SMFI_CMD_RES] = cmd_matrix_get(); break; + +#if CONFIG_SECURITY + case CMD_SECURITY_GET: + smfi_cmd[SMFI_CMD_RES] = cmd_security_get(); + break; + case CMD_SECURITY_SET: + smfi_cmd[SMFI_CMD_RES] = cmd_security_set(); + break; +#endif // CONFIG_SECURITY + #endif // !defined(__SCRATCH__) case CMD_SPI: smfi_cmd[SMFI_CMD_RES] = cmd_spi(); diff --git a/src/common/include/common/command.h b/src/common/include/common/command.h index 1c172dd..15c1ad3 100644 --- a/src/common/include/common/command.h +++ b/src/common/include/common/command.h @@ -46,6 +46,10 @@ enum Command { CMD_LED_SAVE = 18, // Enable/disable no input mode CMD_SET_NO_INPUT = 19, + // Get security state + CMD_SECURITY_GET = 20, + // Set security state + CMD_SECURITY_SET = 21, //TODO }; @@ -70,4 +74,15 @@ enum CommandSpiFlag { #define CMD_LED_INDEX_ALL 0xFF +enum SecurityState { + // Default value, flashing is prevented, cannot be set with CMD_SECURITY_SET + SECURITY_STATE_LOCK = 0, + // Flashing is allowed, cannot be set with CMD_SECURITY_SET + SECURITY_STATE_UNLOCK = 1, + // Flashing will be prevented on the next reboot + SECURITY_STATE_PREPARE_LOCK = 2, + // Flashing will be allowed on the next reboot + SECURITY_STATE_PREPARE_UNLOCK = 3, +}; + #endif // _COMMON_COMMAND_H diff --git a/tool/src/ec.rs b/tool/src/ec.rs index b3a90d1..8689717 100644 --- a/tool/src/ec.rs +++ b/tool/src/ec.rs @@ -5,6 +5,7 @@ use alloc::{ boxed::Box, vec, }; +use core::convert::TryFrom; use crate::{ Access, @@ -36,6 +37,8 @@ enum Cmd { MatrixGet = 17, LedSave = 18, SetNoInput = 19, + SecurityGet = 20, + SecuritySet = 21, } const CMD_SPI_FLAG_READ: u8 = 1 << 0; @@ -43,6 +46,33 @@ const CMD_SPI_FLAG_DISABLE: u8 = 1 << 1; const CMD_SPI_FLAG_SCRATCH: u8 = 1 << 2; const CMD_SPI_FLAG_BACKUP: u8 = 1 << 3; +#[derive(Clone, Copy, Debug)] +#[repr(u8)] +pub enum SecurityState { + // Default value, flashing is prevented, cannot be set with security_set + Lock = 0, + // Flashing is allowed, cannot be set with security_set + Unlock = 1, + // Flashing will be prevented on the next reboot + PrepareLock = 2, + // Flashing will be allowed on the next reboot + PrepareUnlock = 3, +} + +impl TryFrom for SecurityState { + type Error = Error; + + fn try_from(value: u8) -> Result { + match value { + 0 => Ok(Self::Lock), + 1 => Ok(Self::Unlock), + 2 => Ok(Self::PrepareLock), + 3 => Ok(Self::PrepareUnlock), + _ => Err(Error::Verify), + } + } +} + /// Run EC commands using a provided access method pub struct Ec { access: A, @@ -284,6 +314,19 @@ impl Ec { self.command(Cmd::SetNoInput, &mut [no_input as u8]) } + /// Get security state + pub unsafe fn security_get(&mut self) -> Result { + let mut data = [0]; + self.command(Cmd::SecurityGet, &mut data)?; + SecurityState::try_from(data[0]) + } + + /// Set security state + pub unsafe fn security_set(&mut self, state: SecurityState) -> Result<(), Error> { + let mut data = [state as u8]; + self.command(Cmd::SecuritySet, &mut data) + } + pub fn into_dyn(self) -> Ec> where A: 'static { Ec { diff --git a/tool/src/lib.rs b/tool/src/lib.rs index a5650bb..5afcac4 100644 --- a/tool/src/lib.rs +++ b/tool/src/lib.rs @@ -25,7 +25,7 @@ extern crate alloc; pub use self::access::*; mod access; -pub use self::ec::Ec; +pub use self::ec::{Ec, SecurityState}; mod ec; pub use self::error::Error; diff --git a/tool/src/main.rs b/tool/src/main.rs index 1d6740a..eef1735 100644 --- a/tool/src/main.rs +++ b/tool/src/main.rs @@ -9,6 +9,7 @@ use ectool::{ Ec, Error, Firmware, + SecurityState, StdTimeout, Spi, SpiRom, @@ -276,6 +277,19 @@ unsafe fn keymap_set(ec: &mut Ec>, layer: u8, output: u8, input: ec.keymap_set(layer, output, input, value) } +unsafe fn security_get(ec: &mut Ec>) -> Result<(), Error> { + println!("{:?}", ec.security_get()?); + + Ok(()) +} + +unsafe fn security_set(ec: &mut Ec>, state: SecurityState) -> Result<(), Error> { + ec.security_set(state)?; + println!("Shut down the system for the security state to take effect"); + + Ok(()) +} + fn parse_color(s: &str) -> Result<(u8, u8, u8), String> { let r = u8::from_str_radix(&s[0..2], 16); let g = u8::from_str_radix(&s[2..4], 16); @@ -375,6 +389,11 @@ fn main() { .required(true) ) ) + .subcommand(SubCommand::with_name("security") + .arg(Arg::with_name("state") + .possible_values(&["lock", "unlock"]) + ) + ) .get_matches(); let get_ec = || -> Result<_, Error> { @@ -609,7 +628,35 @@ fn main() { process::exit(1); } } - } + }, + Some(("security", sub_m)) => { + match sub_m.value_of("state") { + Some(value) => { + let state = match value { + "lock" => SecurityState::PrepareLock, + "unlock" => SecurityState::PrepareUnlock, + _ => { + eprintln!("invalid security state '{}': must be 'lock' or 'unlock'", value); + process::exit(1); + } + }; + match unsafe { security_set(&mut ec, state) } { + Ok(()) => (), + Err(err) => { + eprintln!("failed to set security state to '{}': {:X?}", value, err); + process::exit(1); + }, + } + }, + None => match unsafe { security_get(&mut ec) } { + Ok(()) => (), + Err(err) => { + eprintln!("failed to get security state: {:X?}", err); + process::exit(1); + }, + }, + } + }, _ => unreachable!() } }