Backport 26987 ([ownership] Test the key algorithm with pubkey for key equivalence)

sw/device/silicon_creator/lib/ownership/owner_block.c

@@ -43,6 +43,18 @@ enum {
   kProtectSlots = 8,
 };
 
+hardened_bool_t owner_block_owner_key_equal(void) {
+  hardened_bool_t result =
+      hardened_memeq(owner_page[0].owner_key.raw, owner_page[1].owner_key.raw,
+                     ARRAYSIZE(owner_page[0].owner_key.raw));
+  result ^= owner_page[0].ownership_key_alg;
+  result ^= owner_page[1].ownership_key_alg;
+  if (launder32(result) != kHardenedBoolTrue) {
+    return kHardenedBoolFalse;
+  }
+  return result;
+}
+
 hardened_bool_t owner_block_newversion_mode(void) {
   if (owner_page_valid[0] == kOwnerPageStatusSealed &&
       (owner_page[0].update_mode == kOwnershipUpdateModeNewVersion ||
@@ -66,15 +78,15 @@ hardened_bool_t owner_block_page1_valid_for_transfer(boot_data_t *bootdata) {
       case kOwnershipStateUnlockedSelf:
         // In UnlockedSelf, the owner key must be the same.  If not,
         // skip parsing of Owner Page 1.
-        if (hardened_memeq(
-                owner_page[0].owner_key.raw, owner_page[1].owner_key.raw,
-                ARRAYSIZE(owner_page[0].owner_key.raw)) == kHardenedBoolTrue) {
+        if (owner_block_owner_key_equal() == kHardenedBoolTrue) {
           return kHardenedBoolTrue;
         }
         break;
       case kOwnershipStateUnlockedEndorsed:
         // In UnlockedEndorsed, the owner key must match the key endorsed by the
         // next_owner field in bootdata.  If not, skip parsing owner page 1.
+        //
+        // FIXME: Mix in the key algorithm identifier.
         hmac_sha256(owner_page[1].owner_key.raw,
                     sizeof(owner_page[1].owner_key.raw), &digest);
         if (hardened_memeq(bootdata->next_owner, digest.digest,

sw/device/silicon_creator/lib/ownership/owner_block.h

@@ -62,6 +62,13 @@ typedef struct owner_application_keyring {
   const owner_application_key_t *key[16];
 } owner_application_keyring_t;
 
+/**
+ * Determine if the owner keys are equal between owner page 0 and 1.
+ *
+ * @return kHardenedBoolTrue if the owner key are the same between both pages.
+ */
+hardened_bool_t owner_block_owner_key_equal(void);
+
 /**
  * Determine if the ownership update mode is one of the "newversion" modes.
  *

sw/device/silicon_creator/lib/ownership/ownership.c

@@ -71,9 +71,7 @@ static rom_error_t locked_owner_init(boot_data_t *bootdata,
       owner_page_valid[1] == kOwnerPageStatusSigned &&
       owner_block_newversion_mode() == kHardenedBoolTrue &&
       owner_page[1].config_version > owner_page[0].config_version &&
-      hardened_memeq(owner_page[0].owner_key.raw, owner_page[1].owner_key.raw,
-                     ARRAYSIZE(owner_page[0].owner_key.raw)) ==
-          kHardenedBoolTrue) {
+      owner_block_owner_key_equal() == kHardenedBoolTrue) {
     rom_error_t error =
         ownership_activate(bootdata, /*write_both_pages=*/kHardenedBoolFalse);
     if (error == kErrorOk) {

sw/device/silicon_creator/rom_ext/defs.bzl

@@ -32,6 +32,14 @@ TEST_OWNER_CONFIGS = {
         "owner_defines": ["TEST_OWNER_KEY_ALG_HYBRID_SPX_PURE=1"],
         "rescue_module": ["//sw/device/silicon_creator/lib/rescue:rescue_xmodem"],
     },
+    "hybrid_owner_update_newversion": {
+        # Enable hybrid ECDSA/SPX+ ownership.
+        "owner_defines": [
+            "TEST_OWNER_UPDATE_MODE=kOwnershipUpdateModeNewVersion",
+            "TEST_OWNER_KEY_ALG_HYBRID_SPX_PREHASH=1",
+        ],
+        "rescue_module": ["//sw/device/silicon_creator/lib/rescue:rescue_xmodem"],
+    },
     "owner_update_newversion": {
         # Enable the NewVersion update mode of ownership.
         "owner_defines": ["TEST_OWNER_UPDATE_MODE=kOwnershipUpdateModeNewVersion"],

sw/device/silicon_creator/rom_ext/e2e/ownership/BUILD

@@ -754,3 +754,56 @@ ownership_transfer_test(
         "//sw/device/silicon_creator/lib/drivers:retention_sram",
     ],
 )
+
+ownership_transfer_test(
+    name = "newversion_pq_to_pq_update_test",
+    ecdsa_key = {
+        "//sw/device/silicon_creator/lib/ownership/keys/fake:app_prod_ecdsa": "app_prod",
+    },
+    fpga = fpga_params(
+        changes_otp = True,
+        rom_ext = "//sw/device/silicon_creator/rom_ext:rom_ext_hybrid_owner_update_newversion",
+        test_cmd = """
+            --clear-bitstream
+            --bootstrap={firmware}
+            --next-owner-key=$(location //sw/device/silicon_creator/lib/ownership/keys/fake:owner_key)
+            --next-unlock-key=$(location //sw/device/silicon_creator/lib/ownership/keys/fake:unlock_key)
+            --next-activate-key=$(location //sw/device/silicon_creator/lib/ownership/keys/fake:activate_key)
+            --next-application-key=$(location //sw/device/silicon_creator/lib/ownership/keys/fake:app_prod_ecdsa_pub)
+            --next-key-alg=HybridSpxPrehash
+            --next-owner-key-spx=$(location //sw/device/silicon_creator/lib/ownership/keys/fake:owner_key_spx)
+            --next-unlock-key-spx=$(location //sw/device/silicon_creator/lib/ownership/keys/fake:unlock_key_spx)
+            --next-activate-key-spx=$(location //sw/device/silicon_creator/lib/ownership/keys/fake:activate_key_spx)
+            --config-version=2
+            # We expect to see the version updated and the owner key the same.
+            --expect "config_version = 2"
+            --expect "owner_key = 8e3dcb50"
+        """,
+        test_harness = "//sw/host/tests/ownership:newversion_test",
+    ),
+)
+
+ownership_transfer_test(
+    name = "newversion_pq_downgrade_test",
+    ecdsa_key = {
+        "//sw/device/silicon_creator/lib/ownership/keys/fake:app_prod_ecdsa": "app_prod",
+    },
+    fpga = fpga_params(
+        changes_otp = True,
+        rom_ext = "//sw/device/silicon_creator/rom_ext:rom_ext_hybrid_owner_update_newversion",
+        test_cmd = """
+            --clear-bitstream
+            --bootstrap={firmware}
+            --next-owner-key=$(location //sw/device/silicon_creator/lib/ownership/keys/fake:owner_key)
+            --next-unlock-key=$(location //sw/device/silicon_creator/lib/ownership/keys/fake:unlock_key)
+            --next-activate-key=$(location //sw/device/silicon_creator/lib/ownership/keys/fake:activate_key)
+            --next-application-key=$(location //sw/device/silicon_creator/lib/ownership/keys/fake:app_prod_ecdsa_pub)
+            --next-owner-key-spx=$(location //sw/device/silicon_creator/lib/ownership/keys/fake:owner_key_spx)
+            --config-version=2
+            # We expect to see the version unchanged.
+            --expect "config_version = 1"
+            --expect "owner_key = 8e3dcb50"
+        """,
+        test_harness = "//sw/host/tests/ownership:newversion_test",
+    ),
+)

sw/host/tests/ownership/newversion_test.rs

@@ -26,6 +26,8 @@ struct Opts {
     /// Console receive timeout.
     #[arg(long, value_parser = humantime::parse_duration, default_value = "10s")]
     timeout: Duration,
+    #[arg(long, default_value_t = OwnershipKeyAlg::EcdsaP256, help = "Current Owner key algorithm")]
+    next_key_alg: OwnershipKeyAlg,
     #[arg(long, help = "Next Owner private key (ECDSA P256)")]
     next_owner_key: Option<PathBuf>,
     #[arg(long, help = "Next Owner public key (ECDSA P256)")]
@@ -36,6 +38,16 @@ struct Opts {
     next_unlock_key: Option<PathBuf>,
     #[arg(long, help = "Next Owner's application public key (ECDSA P256)")]
     next_application_key: PathBuf,
+
+    #[arg(long, help = "Next Owner private key (SPX)")]
+    next_owner_key_spx: Option<PathBuf>,
+    #[arg(long, help = "Next Owner public key (SPX)")]
+    next_owner_key_spx_pub: Option<PathBuf>,
+    #[arg(long, help = "Next Owner activate private key (SPX)")]
+    next_activate_key_spx: Option<PathBuf>,
+    #[arg(long, help = "Next Owner unlock private key (SPX)")]
+    next_unlock_key_spx: Option<PathBuf>,
+
     #[arg(
         long,
         default_value_t = transfer_lib::TEST_OWNER_CONFIG_VERSION,
@@ -70,17 +82,26 @@ fn newversion_test(opts: &Opts, transport: &TransportWrapper) -> Result<()> {
 
     log::info!("###### Get Device Info ######");
     rescue.enter(transport, EntryMode::Reset)?;
-    let devid = rescue.get_device_id()?;
+    let (data, devid) = transfer_lib::get_device_info(transport, &rescue)?;
 
     log::info!("###### Upload Owner Block ######");
     transfer_lib::create_owner(
         transport,
         &rescue,
-        /*nonce=*/ 0,
-        OwnershipKeyAlg::EcdsaP256,
-        HybridPair::load(opts.next_owner_key.as_deref(), None)?,
-        HybridPair::load(opts.next_activate_key.as_deref(), None)?,
-        HybridPair::load(opts.next_unlock_key.as_deref(), None)?,
+        data.rom_ext_nonce,
+        opts.next_key_alg,
+        HybridPair::load(
+            opts.next_owner_key.as_deref(),
+            opts.next_owner_key_spx.as_deref(),
+        )?,
+        HybridPair::load(
+            opts.next_activate_key.as_deref(),
+            opts.next_activate_key_spx.as_deref(),
+        )?,
+        HybridPair::load(
+            opts.next_unlock_key.as_deref(),
+            opts.next_unlock_key_spx.as_deref(),
+        )?,
         &opts.next_application_key,
         opts.config_kind,
         /*customize=*/

sw/host/tests/ownership/transfer_lib.rs

@@ -235,11 +235,6 @@ impl HybridPair {
         if key_alg.is_spx() && self.spx.is_none() {
             return Err(anyhow!("{name} using {key_alg} requires an SPX key"));
         }
-        if !key_alg.is_spx() && self.spx.is_some() {
-            return Err(anyhow!(
-                "{name} using {key_alg} has an SPX key, but doesn't need one"
-            ));
-        }
         Ok(())
     }