Repository: chipsalliance/caliptra-sw
Pull request #377 (2023-06-13 19:44:17)

ECDSA: Add range check for R and S is ECDSA verify
R and S must be in the range [1, n-1]. Add checks for this to ECDSA driver. Additionally, pull some special-case wycheproof test vectors to run.


Patch:
diff --git a/drivers/src/ecc384.rs b/drivers/src/ecc384.rs
index edb09218..0214f44b 100644
--- a/drivers/src/ecc384.rs
+++ b/drivers/src/ecc384.rs
@@ -17,6 +17,7 @@ use crate::{
     array_concat3, wait, Array4x12, CaliptraError, CaliptraResult, KeyReadArgs, KeyWriteArgs, Trng,
 };
 use caliptra_registers::ecc::EccReg;
+use core::cmp::Ordering;
 use zerocopy::{AsBytes, FromBytes};
 
 /// ECC-384 Coordinate
@@ -144,6 +145,35 @@ impl Ecc384 {
     pub fn new(ecc: EccReg) -> Self {
         Self { ecc }
     }
+
+    // Check that `scalar` is in the range [1, n-1] for the P-384 curve
+    fn scalar_range_check(scalar: &Ecc384Scalar) -> bool {
+        // n-1 for The NIST P-384 curve
+        const SECP384_ORDER_MIN1: &[u32] = &[
+            0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xc7634d81,
+            0xf4372ddf, 0x581a0db2, 0x48b0a77a, 0xecec196a, 0xccc52972,
+        ];
+
+        // Check scalar <= n-1
+        for (i, word) in SECP384_ORDER_MIN1.iter().enumerate() {
+            match scalar.0[i].cmp(word) {
+                Ordering::Greater => return false,
+                Ordering::Less => break,
+                Ordering::Equal => continue,
+            }
+        }
+
+        // If scalar is non-zero, return true
+        for word in scalar.0 {
+            if word != 0 {
+                return true;
+            }
+        }
+
+        // scalar is zero
+        false
+    }
+
     /// Generate ECC-384 Key Pair
     ///
     /// # Arguments
@@ -289,6 +319,11 @@ impl Ecc384 {
         digest: &Ecc384Scalar,
         signature: &Ecc384Signature,
     ) -> CaliptraResult<bool> {
+        // If R or S are not in the range [1, N-1], signature check must fail
+        if !Self::scalar_range_check(&signature.r) || !Self::scalar_range_check(&signature.s) {
+            return Ok(false);
+        }
+
         let ecc = self.ecc.regs_mut();
 
         // Wait for hardware ready
diff --git a/error/src/lib.rs b/error/src/lib.rs
index 65db9870..86370c30 100644
--- a/error/src/lib.rs
+++ b/error/src/lib.rs
@@ -265,7 +265,7 @@ impl CaliptraError {
     pub const RUNTIME_INTERNAL: CaliptraError = CaliptraError::new_const(0x000E0001);
     pub const RUNTIME_UNIMPLEMENTED_COMMAND: CaliptraError = CaliptraError::new_const(0x000E0002);
     pub const RUNTIME_INSUFFICIENT_MEMORY: CaliptraError = CaliptraError::new_const(0x000E0003);
-    pub const RUNTIME_ECDSA_VERIF_FAILED: CaliptraError = CaliptraError::new_const(0x000E0004);
+    pub const RUNTIME_ECDSA_VERIFY_FAILED: CaliptraError = CaliptraError::new_const(0x000E0004);
     pub const RUNTIME_INVALID_CHECKSUM: CaliptraError = CaliptraError::new_const(0x000E0005);
     pub const RUNTIME_FIPS_UNIMPLEMENTED: CaliptraError = CaliptraError::new_const(0x000E0006);
 
diff --git a/runtime/src/verify.rs b/runtime/src/verify.rs
index 3fb80457..adcee626 100644
--- a/runtime/src/verify.rs
+++ b/runtime/src/verify.rs
@@ -38,7 +38,7 @@ pub(crate) fn handle_ecdsa_verify(drivers: &mut Drivers, cmd_args: &[u8]) -> Cal
 
         let success = drivers.ecdsa.verify(&pubkey, &digest, &sig)?;
         if !success {
-            return Err(CaliptraError::RUNTIME_ECDSA_VERIF_FAILED);
+            return Err(CaliptraError::RUNTIME_ECDSA_VERIFY_FAILED);
         }
     } else {
         return Err(CaliptraError::RUNTIME_INSUFFICIENT_MEMORY);


Test patch:
diff --git a/runtime/tests/common.rs b/runtime/tests/common.rs
new file mode 100644
index 00000000..9076ddf2
--- /dev/null
+++ b/runtime/tests/common.rs
@@ -0,0 +1,65 @@
+// Licensed under the Apache-2.0 license
+
+use caliptra_builder::{FwId, ImageOptions, APP_WITH_UART, FMC_WITH_UART, ROM_WITH_UART};
+use caliptra_hw_model::{BootParams, DefaultHwModel, HwModel, InitParams};
+
+// Run test_bin as a ROM image. The is used for faster tests that can run
+// against verilator
+pub fn run_rom_test(test_bin_name: &'static str) -> DefaultHwModel {
+    static FEATURES: &[&str] = &["emu", "riscv"];
+
+    let runtime_fwid = FwId {
+        crate_name: "caliptra-runtime-test-bin",
+        bin_name: test_bin_name,
+        features: FEATURES,
+        ..Default::default()
+    };
+
+    let rom = caliptra_builder::build_firmware_rom(&runtime_fwid).unwrap();
+
+    caliptra_hw_model::new(BootParams {
+        init_params: InitParams {
+            rom: &rom,
+            ..Default::default()
+        },
+        ..Default::default()
+    })
+    .unwrap()
+}
+
+// Run a test which boots ROM -> FMC -> test_bin. If test_bin_name is None,
+// run the production runtime image.
+pub fn run_rt_test(test_bin_name: Option<&'static str>) -> DefaultHwModel {
+    let runtime_fwid = match test_bin_name {
+        Some(bin) => FwId {
+            crate_name: "caliptra-runtime-test-bin",
+            bin_name: bin,
+            features: &["emu", "riscv", "runtime"],
+            ..Default::default()
+        },
+        None => APP_WITH_UART,
+    };
+
+    let rom = caliptra_builder::build_firmware_rom(&ROM_WITH_UART).unwrap();
+
+    let image = caliptra_builder::build_and_sign_image(
+        &FMC_WITH_UART,
+        &runtime_fwid,
+        ImageOptions::default(),
+    )
+    .unwrap();
+
+    let mut model = caliptra_hw_model::new(BootParams {
+        init_params: InitParams {
+            rom: &rom,
+            ..Default::default()
+        },
+        fw_image: Some(&image.to_bytes().unwrap()),
+        ..Default::default()
+    })
+    .unwrap();
+
+    model.step_until(|m| m.soc_ifc().cptra_flow_status().read().ready_for_fw());
+
+    model
+}
diff --git a/runtime/tests/ecdsa.rs b/runtime/tests/ecdsa.rs
new file mode 100644
index 00000000..28127599
--- /dev/null
+++ b/runtime/tests/ecdsa.rs
@@ -0,0 +1,151 @@
+// Licensed under the Apache-2.0 license.
+pub mod common;
+
+use caliptra_drivers::CaliptraError;
+use caliptra_hw_model::{HwModel, ModelError, ShaAccMode};
+use caliptra_runtime::{CommandId, EcdsaVerifyCmd};
+use common::run_rom_test;
+use zerocopy::AsBytes;
+
+// This file includes some tests from Wycheproof to testing specific common
+// ECDSA problems.
+// In the long term, this file should just run the entire Wycheproof test
+// vector file wycheproof/testvectors_v1/ecdsa_secp384r1_sha384_test.json
+
+struct EcdsaTestCase<'a> {
+    msg: &'a [u8],
+    pub_x: [u8; 48],
+    pub_y: [u8; 48],
+    sig_r: [u8; 48],
+    sig_s: [u8; 48],
+}
+
+#[test]
+fn test_r_s_ranges() {
+    let mut model = run_rom_test("mbox");
+
+    model.step_until(|m| {
+        m.soc_mbox().status().read().mbox_fsm_ps().mbox_idle()
+            && m.soc_ifc().cptra_boot_status().read() == 1
+    });
+
+    let pub_key_x = [
+        0x2d, 0xa5, 0x7d, 0xda, 0x10, 0x89, 0x27, 0x6a, 0x54, 0x3f, 0x9f, 0xfd, 0xac, 0x0b, 0xff,
+        0x0d, 0x97, 0x6c, 0xad, 0x71, 0xeb, 0x72, 0x80, 0xe7, 0xd9, 0xbf, 0xd9, 0xfe, 0xe4, 0xbd,
+        0xb2, 0xf2, 0x0f, 0x47, 0xff, 0x88, 0x82, 0x74, 0x38, 0x97, 0x72, 0xd9, 0x8c, 0xc5, 0x75,
+        0x21, 0x38, 0xaa,
+    ];
+    let pub_key_y = [
+        0x4b, 0x6d, 0x05, 0x4d, 0x69, 0xdc, 0xf3, 0xe2, 0x5e, 0xc4, 0x9d, 0xf8, 0x70, 0x71, 0x5e,
+        0x34, 0x88, 0x3b, 0x18, 0x36, 0x19, 0x7d, 0x76, 0xf8, 0xad, 0x96, 0x2e, 0x78, 0xf6, 0x57,
+        0x1b, 0xbc, 0x74, 0x07, 0xb0, 0xd6, 0x09, 0x1f, 0x9e, 0x4d, 0x88, 0xf0, 0x14, 0x27, 0x44,
+        0x06, 0x17, 0x4f,
+    ];
+
+    let cases = [
+        // tcId: 168
+        // Signature with special case values r=0 and s=0
+        EcdsaTestCase {
+            msg: &[0x31, 0x32, 0x33, 0x34, 0x30, 0x30],
+            pub_x: pub_key_x,
+            pub_y: pub_key_y,
+            sig_r: [
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+            ],
+            sig_s: [
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+            ],
+        },
+        // tcId: 169
+        // Signature with special case values r=0 and s=1
+        EcdsaTestCase {
+            msg: &[0x31, 0x32, 0x33, 0x34, 0x30, 0x30],
+            pub_x: pub_key_x,
+            pub_y: pub_key_y,
+            sig_r: [
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+            ],
+            sig_s: [
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
+            ],
+        },
+        // tcId: 176
+        // Signature with special case values r=1 and s=0
+        EcdsaTestCase {
+            msg: &[0x31, 0x32, 0x33, 0x34, 0x30, 0x30],
+            pub_x: pub_key_x,
+            pub_y: pub_key_y,
+            sig_r: [
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
+            ],
+            sig_s: [
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+            ],
+        },
+        // tcId: 193
+        // Signature with special case values r=n and s=1
+        EcdsaTestCase {
+            msg: &[0x31, 0x32, 0x33, 0x34, 0x30, 0x30],
+            pub_x: pub_key_x,
+            pub_y: pub_key_y,
+            sig_r: [
+                0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+                0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xc7, 0x63, 0x4d, 0x81,
+                0xf4, 0x37, 0x2d, 0xdf, 0x58, 0x1a, 0x0d, 0xb2, 0x48, 0xb0, 0xa7, 0x7a, 0xec, 0xec,
+                0x19, 0x6a, 0xcc, 0xc5, 0x29, 0x73,
+            ],
+            sig_s: [
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
+            ],
+        },
+    ];
+
+    for c in cases {
+        // Stream to SHA ACC
+        model
+            .compute_sha512_acc_digest(c.msg, ShaAccMode::Sha384Stream)
+            .unwrap();
+
+        let mut cmd = EcdsaVerifyCmd {
+            chksum: 0,
+            pub_key_x: c.pub_x,
+            pub_key_y: c.pub_y,
+            signature_r: c.sig_r,
+            signature_s: c.sig_s,
+        };
+
+        cmd.chksum = caliptra_common::checksum::calc_checksum(
+            u32::from(CommandId::ECDSA384_VERIFY),
+            &cmd.as_bytes()[4..],
+        );
+
+        let resp = model.mailbox_execute(u32::from(CommandId::ECDSA384_VERIFY), cmd.as_bytes());
+        assert_eq!(
+            resp,
+            Err(ModelError::MailboxCmdFailed(
+                CaliptraError::RUNTIME_ECDSA_VERIFY_FAILED.into()
+            ))
+        );
+    }
+}
diff --git a/runtime/tests/integration_tests.rs b/runtime/tests/integration_tests.rs
index afb549a8..4bc42238 100644
--- a/runtime/tests/integration_tests.rs
+++ b/runtime/tests/integration_tests.rs
@@ -1,9 +1,10 @@
 // Licensed under the Apache-2.0 license.
+pub mod common;
 
-use caliptra_builder::{FwId, ImageOptions, APP_WITH_UART, FMC_WITH_UART, ROM_WITH_UART};
 use caliptra_drivers::Ecc384PubKey;
-use caliptra_hw_model::{BootParams, DefaultHwModel, HwModel, InitParams, ModelError, ShaAccMode};
+use caliptra_hw_model::{HwModel, ModelError, ShaAccMode};
 use caliptra_runtime::{CommandId, EcdsaVerifyCmd};
+use common::{run_rom_test, run_rt_test};
 use openssl::{
     bn::BigNum,
     ec::{EcGroup, EcKey},
@@ -13,67 +14,6 @@ use openssl::{
 };
 use zerocopy::{AsBytes, FromBytes};
 
-// Run test_bin as a ROM image. The is used for faster tests that can run
-// against verilator
-fn run_rom_test(test_bin_name: &'static str) -> DefaultHwModel {
-    static FEATURES: &[&str] = &["emu", "riscv"];
-
-    let runtime_fwid = FwId {
-        crate_name: "caliptra-runtime-test-bin",
-        bin_name: test_bin_name,
-        features: FEATURES,
-        ..Default::default()
-    };
-
-    let rom = caliptra_builder::build_firmware_rom(&runtime_fwid).unwrap();
-
-    caliptra_hw_model::new(BootParams {
-        init_params: InitParams {
-            rom: &rom,
-            ..Default::default()
-        },
-        ..Default::default()
-    })
-    .unwrap()
-}
-
-// Run a test which boots ROM -> FMC -> test_bin. If test_bin_name is None,
-// run the production runtime image.
-fn run_rt_test(test_bin_name: Option<&'static str>) -> DefaultHwModel {
-    let runtime_fwid = match test_bin_name {
-        Some(bin) => FwId {
-            crate_name: "caliptra-runtime-test-bin",
-            bin_name: bin,
-            features: &["emu", "riscv", "runtime"],
-            ..Default::default()
-        },
-        None => APP_WITH_UART,
-    };
-
-    let rom = caliptra_builder::build_firmware_rom(&ROM_WITH_UART).unwrap();
-
-    let image = caliptra_builder::build_and_sign_image(
-        &FMC_WITH_UART,
-        &runtime_fwid,
-        ImageOptions::default(),
-    )
-    .unwrap();
-
-    let mut model = caliptra_hw_model::new(BootParams {
-        init_params: InitParams {
-            rom: &rom,
-            ..Default::default()
-        },
-        fw_image: Some(&image.to_bytes().unwrap()),
-        ..Default::default()
-    })
-    .unwrap();
-
-    model.step_until(|m| m.soc_ifc().cptra_flow_status().read().ready_for_fw());
-
-    model
-}
-
 #[test]
 fn test_standard() {
     // Test that the normal runtime firmware boots.