Repository: chipsalliance/caliptra-sw
Pull request #567 (2023-07-31 21:36:24)

ecc driver: always verify as part of sign.
This is necessary to prevent leaking secret key material if the ecc hardware is glitched while doing the computation.


Patch:
diff --git a/drivers/src/ecc384.rs b/drivers/src/ecc384.rs
index eea03901..8f6bcb28 100644
--- a/drivers/src/ecc384.rs
+++ b/drivers/src/ecc384.rs
@@ -14,7 +14,8 @@ Abstract:
 
 use crate::kv_access::{KvAccess, KvAccessErr};
 use crate::{
-    array_concat3, wait, Array4x12, CaliptraError, CaliptraResult, KeyReadArgs, KeyWriteArgs, Trng,
+    array_concat3, okmutref, wait, Array4x12, CaliptraError, CaliptraResult, KeyReadArgs,
+    KeyWriteArgs, Trng,
 };
 use caliptra_registers::ecc::EccReg;
 use core::cmp::Ordering;
@@ -266,30 +267,20 @@ impl Ecc384 {
         // Pairwise consistency check.
         if perform_pct {
             let digest = Array4x12::new([0u32; 12]);
-            let mut sig = self.sign(&priv_key.into(), &digest, trng)?;
-            let result = self.verify(&pub_key, &digest, &sig)?;
-            if result != Ecc384Result::Success {
-                return Err(CaliptraError::DRIVER_ECC384_KEYGEN_PAIRWISE_CONSISTENCY_FAILURE);
+            match self.sign(&priv_key.into(), &pub_key, &digest, trng) {
+                Ok(mut sig) => sig.zeroize(),
+                Err(CaliptraError::DRIVER_ECC384_SIGN_VALIDATION_FAILED) => {
+                    return Err(CaliptraError::DRIVER_ECC384_KEYGEN_PAIRWISE_CONSISTENCY_FAILURE)
+                }
+                Err(err) => return Err(err),
             }
-            sig.zeroize();
         }
         self.zeroize_internal();
 
         Ok(pub_key)
     }
 
-    /// Sign the digest with specified private key
-    ///
-    /// # Arguments
-    ///
-    /// * `priv_key` - Private key
-    /// * `data` - Digest to sign
-    /// * `trng` - TRNG driver instance
-    ///
-    /// # Returns
-    ///
-    /// * `Ecc384Signature` - Generate signature
-    pub fn sign(
+    fn sign_internal(
         &mut self,
         priv_key: &Ecc384PrivKeyIn,
         data: &Ecc384Scalar,
@@ -333,6 +324,39 @@ impl Ecc384 {
         Ok(signature)
     }
 
+    /// Sign the digest with specified private key. To defend against glitching
+    /// attacks that could expose the private key, this function also verifies
+    /// the generated signature.
+    ///
+    /// # Arguments
+    ///
+    /// * `priv_key` - Private key
+    /// * `pub_key` - Public key to verify with
+    /// * `data` - Digest to sign
+    /// * `trng` - TRNG driver instance
+    ///
+    /// # Returns
+    ///
+    /// * `Ecc384Signature` - Generate signature
+    pub fn sign(
+        &mut self,
+        priv_key: &Ecc384PrivKeyIn,
+        pub_key: &Ecc384PubKey,
+        data: &Ecc384Scalar,
+        trng: &mut Trng,
+    ) -> CaliptraResult<Ecc384Signature> {
+        let mut sig_result = self.sign_internal(priv_key, data, trng);
+        let sig = okmutref(&mut sig_result)?;
+        match self.verify(pub_key, data, sig)? {
+            Ecc384Result::SigVerifyFailed => {
+                sig.zeroize();
+                return Err(CaliptraError::DRIVER_ECC384_SIGN_VALIDATION_FAILED);
+            }
+            Ecc384Result::Success => {}
+        };
+        sig_result
+    }
+
     /// Verify signature with specified public key and digest
     ///
     /// # Arguments
diff --git a/error/src/lib.rs b/error/src/lib.rs
index 7e5a9e36..3037de07 100644
--- a/error/src/lib.rs
+++ b/error/src/lib.rs
@@ -97,6 +97,8 @@ impl CaliptraError {
         CaliptraError::new_const(0x0005000c);
     pub const DRIVER_ECC384_KEYGEN_PAIRWISE_CONSISTENCY_FAILURE: CaliptraError =
         CaliptraError::new_const(0x0005000d);
+    pub const DRIVER_ECC384_SIGN_VALIDATION_FAILED: CaliptraError =
+        CaliptraError::new_const(0x0005000e);
 
     pub const DRIVER_KV_ERASE_USE_LOCK_SET_FAILURE: CaliptraError =
         CaliptraError::new_const(0x00060001);
diff --git a/fmc/src/flow/crypto.rs b/fmc/src/flow/crypto.rs
index 9c29b3c9..c966922b 100644
--- a/fmc/src/flow/crypto.rs
+++ b/fmc/src/flow/crypto.rs
@@ -146,13 +146,14 @@ impl Crypto {
     pub fn ecdsa384_sign(
         env: &mut FmcEnv,
         priv_key: KeyId,
+        pub_key: &Ecc384PubKey,
         data: &[u8],
     ) -> CaliptraResult<Ecc384Signature> {
         let digest = Self::sha384_digest(env, data);
         let digest = okref(&digest)?;
         let priv_key_args = KeyReadArgs::new(priv_key);
         let priv_key = Ecc384PrivKeyIn::Key(priv_key_args);
-        env.ecc384.sign(&priv_key, digest, &mut env.trng)
+        env.ecc384.sign(&priv_key, pub_key, digest, &mut env.trng)
     }
 
     /// Verify the ECC Signature
diff --git a/fmc/src/flow/rt_alias.rs b/fmc/src/flow/rt_alias.rs
index 126f9de0..3c106106 100644
--- a/fmc/src/flow/rt_alias.rs
+++ b/fmc/src/flow/rt_alias.rs
@@ -270,7 +270,7 @@ impl RtAliasLayer {
         // Sign the AliasRt To Be Signed DER Blob with AliasFMC Private Key in Key Vault Slot 7
         // AliasRtTbsDigest = sha384_digest(AliasRtTbs) AliaRtTbsCertSig = ecc384_sign(KvSlot5, AliasFmcTbsDigest)
 
-        let sig = Crypto::ecdsa384_sign(env, auth_priv_key, tbs.tbs());
+        let sig = Crypto::ecdsa384_sign(env, auth_priv_key, auth_pub_key, tbs.tbs());
         let sig = okref(&sig)?;
         // Clear the authority private key
         cprintln!(
diff --git a/kat/src/ecc384_kat.rs b/kat/src/ecc384_kat.rs
index 11470808..a5493c1b 100644
--- a/kat/src/ecc384_kat.rs
+++ b/kat/src/ecc384_kat.rs
@@ -68,13 +68,12 @@ impl Ecc384Kat {
     fn kat_signature_generate(&self, ecc: &mut Ecc384, trng: &mut Trng) -> CaliptraResult<()> {
         let digest = Array4x12::new([0u32; 12]);
         let signature = ecc
-            .sign(&Ecc384PrivKeyIn::from(&PRIV_KEY), &digest, trng)
+            .sign(&Ecc384PrivKeyIn::from(&PRIV_KEY), &PUB_KEY, &digest, trng)
             .map_err(|_| CaliptraError::ROM_KAT_ECC384_SIGNATURE_GENERATE_FAILURE)?;
 
         if signature != SIGNATURE {
             Err(CaliptraError::ROM_KAT_ECC384_SIGNATURE_GENERATE_FAILURE)?;
         }
-
         Ok(())
     }
 
diff --git a/rom/dev/src/flow/cold_reset/crypto.rs b/rom/dev/src/flow/cold_reset/crypto.rs
index 55198f6d..4363d95e 100644
--- a/rom/dev/src/flow/cold_reset/crypto.rs
+++ b/rom/dev/src/flow/cold_reset/crypto.rs
@@ -211,13 +211,14 @@ impl Crypto {
     pub fn ecdsa384_sign(
         env: &mut RomEnv,
         priv_key: KeyId,
+        pub_key: &Ecc384PubKey,
         data: &[u8],
     ) -> CaliptraResult<Ecc384Signature> {
         let mut digest = Self::sha384_digest(env, data);
         let digest = okmutref(&mut digest)?;
         let priv_key_args = KeyReadArgs::new(priv_key);
         let priv_key = Ecc384PrivKeyIn::Key(priv_key_args);
-        let result = env.ecc384.sign(&priv_key, digest, &mut env.trng);
+        let result = env.ecc384.sign(&priv_key, pub_key, digest, &mut env.trng);
         digest.0.fill(0);
         result
     }
diff --git a/rom/dev/src/flow/cold_reset/fmc_alias.rs b/rom/dev/src/flow/cold_reset/fmc_alias.rs
index b9873863..4ec6f5a0 100644
--- a/rom/dev/src/flow/cold_reset/fmc_alias.rs
+++ b/rom/dev/src/flow/cold_reset/fmc_alias.rs
@@ -170,7 +170,7 @@ impl FmcAliasLayer {
             "[afmc] Signing Cert with AUTHORITY.KEYID = {}",
             auth_priv_key as u8
         );
-        let mut sig = Crypto::ecdsa384_sign(env, auth_priv_key, tbs.tbs());
+        let mut sig = Crypto::ecdsa384_sign(env, auth_priv_key, auth_pub_key, tbs.tbs());
         let sig = okmutref(&mut sig)?;
 
         // Clear the authority private key
diff --git a/rom/dev/src/flow/cold_reset/idev_id.rs b/rom/dev/src/flow/cold_reset/idev_id.rs
index ca965a18..7c1ed5ae 100644
--- a/rom/dev/src/flow/cold_reset/idev_id.rs
+++ b/rom/dev/src/flow/cold_reset/idev_id.rs
@@ -229,7 +229,7 @@ impl InitDevIdLayer {
         );
 
         // Sign the the `To Be Signed` portion
-        let mut sig = Crypto::ecdsa384_sign(env, key_pair.priv_key, tbs.tbs());
+        let mut sig = Crypto::ecdsa384_sign(env, key_pair.priv_key, &key_pair.pub_key, tbs.tbs());
         let sig = okmutref(&mut sig)?;
 
         // Verify the signature of the `To Be Signed` portion
diff --git a/rom/dev/src/flow/cold_reset/ldev_id.rs b/rom/dev/src/flow/cold_reset/ldev_id.rs
index 921127ac..f5a14d62 100644
--- a/rom/dev/src/flow/cold_reset/ldev_id.rs
+++ b/rom/dev/src/flow/cold_reset/ldev_id.rs
@@ -165,7 +165,7 @@ impl LocalDevIdLayer {
             "[ldev] Signing Cert with AUTHORITY.KEYID = {}",
             auth_priv_key as u8
         );
-        let mut sig = Crypto::ecdsa384_sign(env, auth_priv_key, tbs.tbs());
+        let mut sig = Crypto::ecdsa384_sign(env, auth_priv_key, auth_pub_key, tbs.tbs());
         let sig = okmutref(&mut sig)?;
 
         // Clear the authority private key


Test patch:
diff --git a/drivers/test-fw/src/bin/ecc384_tests.rs b/drivers/test-fw/src/bin/ecc384_tests.rs
index 027383fa..9c876831 100644
--- a/drivers/test-fw/src/bin/ecc384_tests.rs
+++ b/drivers/test-fw/src/bin/ecc384_tests.rs
@@ -19,6 +19,7 @@ use caliptra_drivers::{
     Array4x12, Ecc384, Ecc384PrivKeyIn, Ecc384PrivKeyOut, Ecc384PubKey, Ecc384Result, Ecc384Scalar,
     Ecc384Seed, KeyId, KeyReadArgs, KeyUsage, KeyWriteArgs, Trng,
 };
+use caliptra_error::CaliptraError;
 use caliptra_kat::Ecc384Kat;
 use caliptra_registers::csrng::CsrngReg;
 use caliptra_registers::ecc::EccReg;
@@ -161,6 +162,10 @@ fn test_sign() {
     let digest = Array4x12::new([0u32; 12]);
     let result = ecc.sign(
         &Ecc384PrivKeyIn::from(&Array4x12::from(PRIV_KEY)),
+        &Ecc384PubKey {
+            x: Ecc384Scalar::from(PUB_KEY_X),
+            y: Ecc384Scalar::from(PUB_KEY_Y),
+        },
         &digest,
         &mut trng,
     );
@@ -170,6 +175,45 @@ fn test_sign() {
     assert_eq!(signature.s, Ecc384Scalar::from(SIGNATURE_S));
 }
 
+fn test_sign_validation_failure() {
+    let mut ecc = unsafe { Ecc384::new(EccReg::new()) };
+    let mut trng = unsafe {
+        Trng::new(
+            CsrngReg::new(),
+            EntropySrcReg::new(),
+            SocIfcTrngReg::new(),
+            &SocIfcReg::new(),
+        )
+        .unwrap()
+    };
+    let wrong_pub_key = Ecc384PubKey {
+        x: Ecc384Scalar::from([
+            0xD7, 0x9C, 0x6D, 0x97, 0x2B, 0x34, 0xA1, 0xDF, 0xC9, 0x16, 0xA7, 0xB6, 0xE0, 0xA9,
+            0x9B, 0x6B, 0x53, 0x87, 0xB3, 0x4D, 0xA2, 0x18, 0x76, 0x07, 0xC1, 0xAD, 0x0A, 0x4D,
+            0x1A, 0x8C, 0x2E, 0x41, 0x72, 0xAB, 0x5F, 0xA5, 0xD9, 0xAB, 0x58, 0xFE, 0x45, 0xE4,
+            0x3F, 0x56, 0xBB, 0xB6, 0x6B, 0xA4,
+        ]),
+        y: Ecc384Scalar::from([
+            0x5A, 0x73, 0x63, 0x93, 0x2B, 0x06, 0xB4, 0xF2, 0x23, 0xBE, 0xF0, 0xB6, 0x0A, 0x63,
+            0x90, 0x26, 0x51, 0x12, 0xDB, 0xBD, 0x0A, 0xAE, 0x67, 0xFE, 0xF2, 0x6B, 0x46, 0x5B,
+            0xE9, 0x35, 0xB4, 0x8E, 0x45, 0x1E, 0x68, 0xD1, 0x6F, 0x11, 0x18, 0xF2, 0xB3, 0x2B,
+            0x4C, 0x28, 0x60, 0x87, 0x49, 0xED,
+        ]),
+    };
+
+    let digest = Array4x12::new([0u32; 12]);
+    let result = ecc.sign(
+        &Ecc384PrivKeyIn::from(&Array4x12::from(PRIV_KEY)),
+        &wrong_pub_key,
+        &digest,
+        &mut trng,
+    );
+    assert_eq!(
+        result,
+        Err(CaliptraError::DRIVER_ECC384_SIGN_VALIDATION_FAILED)
+    );
+}
+
 fn test_verify() {
     let mut ecc = unsafe { Ecc384::new(EccReg::new()) };
     let mut trng = unsafe {
@@ -184,6 +228,10 @@ fn test_verify() {
     let digest = Array4x12::new([0u32; 12]);
     let result = ecc.sign(
         &Ecc384PrivKeyIn::from(&Array4x12::from(PRIV_KEY)),
+        &Ecc384PubKey {
+            x: Ecc384Scalar::from(PUB_KEY_X),
+            y: Ecc384Scalar::from(PUB_KEY_Y),
+        },
         &digest,
         &mut trng,
     );
@@ -212,6 +260,10 @@ fn test_verify_failure() {
     let digest = Array4x12::new([0u32; 12]);
     let result = ecc.sign(
         &Ecc384PrivKeyIn::from(&Array4x12::from(PRIV_KEY)),
+        &Ecc384PubKey {
+            x: Ecc384Scalar::from(PUB_KEY_X),
+            y: Ecc384Scalar::from(PUB_KEY_Y),
+        },
         &digest,
         &mut trng,
     );
@@ -263,7 +315,7 @@ fn test_kv_seed_from_input_msg_from_input() {
     let digest = Array4x12::new([0u32; 12]);
     let key_in_1 = KeyReadArgs::new(KeyId::KeyId2);
 
-    let result = ecc.sign(&key_in_1.into(), &digest, &mut trng);
+    let result = ecc.sign(&key_in_1.into(), &pub_key, &digest, &mut trng);
     assert!(result.is_ok());
     let signature = result.unwrap();
     assert_eq!(signature.r, Ecc384Scalar::from(SIGNATURE_R));
@@ -383,7 +435,12 @@ fn test_kv_seed_from_kv_msg_from_input() {
         0xe8, 0x8c, 0x10,
     ];
     let key_in_priv_key = KeyReadArgs::new(KeyId::KeyId1);
-    let result = ecc.sign(&key_in_priv_key.into(), &Array4x12::from(msg), &mut trng);
+    let result = ecc.sign(
+        &key_in_priv_key.into(),
+        &pub_key,
+        &Array4x12::from(msg),
+        &mut trng,
+    );
     assert!(result.is_ok());
     let signature = result.unwrap();
     assert_eq!(signature.r, Ecc384Scalar::from(sig_r));
@@ -422,6 +479,7 @@ test_suite! {
     test_gen_key_pair,
     test_gen_key_pair_with_iv,
     test_sign,
+    test_sign_validation_failure,
     test_verify,
     test_verify_failure,
     test_kv_seed_from_input_msg_from_input,