Add optional EC security state and documentation

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.

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*)
 

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 <stdbool.h>
+#include <common/command.h>
+
+enum SecurityState security_get(void);
+bool security_set(enum SecurityState state);
+bool security_power(void);
+
+#endif // _BOARD_SECURITY_H

src/board/system76/common/power.c

@@ -22,6 +22,10 @@
 #include <board/espi.h>
 #endif
 
+#if CONFIG_SECURITY
+#include <board/security.h>
+#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
         }
     }
 

src/board/system76/common/security.c

@@ -0,0 +1,41 @@
+// SPDX-License-Identifier: GPL-3.0-only
+
+#include <board/gpio.h>
+#include <board/security.h>
+
+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;
+    }
+}

src/board/system76/common/smfi.c

@@ -17,11 +17,16 @@
 #include <stdio.h>
 #include <string.h>
 
-#ifndef __SCRATCH__
+#if !defined(__SCRATCH__)
 #include <board/scratch.h>
 #include <board/kbled.h>
 #include <board/kbscan.h>
-#endif
+
+#if CONFIG_SECURITY
+#include <board/security.h>
+#endif // CONFIG_SECURITY
+
+#endif // !defined(__SCRATCH__)
 #include <board/smfi.h>
 #include <common/command.h>
 #include <common/macro.h>
@@ -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();

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

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<u8> for SecurityState {
+    type Error = Error;
+
+    fn try_from(value: u8) -> Result<Self, Self::Error> {
+        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<A: Access> {
     access: A,
@@ -284,6 +314,19 @@ impl<A: Access> Ec<A> {
         self.command(Cmd::SetNoInput, &mut [no_input as u8])
     }
 
+    /// Get security state
+    pub unsafe fn security_get(&mut self) -> Result<SecurityState, Error> {
+       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<Box<dyn Access>>
     where A: 'static {
         Ec {

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;

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<Box<dyn Access>>, layer: u8, output: u8, input:
     ec.keymap_set(layer, output, input, value)
 }
 
+unsafe fn security_get(ec: &mut Ec<Box<dyn Access>>) -> Result<(), Error> {
+    println!("{:?}", ec.security_get()?);
+
+    Ok(())
+}
+
+unsafe fn security_set(ec: &mut Ec<Box<dyn Access>>, 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!()
     }
 }