[sw,cryptolib] Fix trigger_fault_if_fg-1 functions

The trigger_fault_if_fg0 functions had a bug where we never
trigger a fault. This new implementation should now correctly
trigger the fault.

For the case where no fault is triggered, we load address 0 into
w31. For the error case we try to load address 0 into
w39 (which doesn't exist), which triggers a ILLEGAL_INSN error.

Signed-off-by: Hakim Filali <[email protected]>

Author: h-filali

sw/device/lib/crypto/impl/ecc/p256.c

@@ -82,12 +82,12 @@ enum {
   /*
    * The expected instruction counts for constant time functions.
    */
-  kModeKeygenInsCnt = 574237,
-  kModeKeygenSideloadInsCnt = 574122,
-  kModeEcdhInsCnt = 581920,
-  kModeEcdhSideloadInsCnt = 581980,
-  kModeEcdsaSignInsCnt = 607411,
-  kModeEcdsaSignSideloadInsCnt = 607471,
+  kModeKeygenInsCnt = 573915,
+  kModeKeygenSideloadInsCnt = 573800,
+  kModeEcdhInsCnt = 581598,
+  kModeEcdhSideloadInsCnt = 581658,
+  kModeEcdsaSignInsCnt = 607087,
+  kModeEcdsaSignSideloadInsCnt = 607147,
 };
 
 static status_t p256_masked_scalar_write(p256_masked_scalar_t *src,

sw/otbn/crypto/boot.s

@@ -168,7 +168,7 @@ attestation_keygen:
      The check fails if both sides are not equal.
      FG0.Z <= (y^2) mod p == (x^2 + ax + b) mod p */
   bn.cmp   w18, w19
-  jal      x1, trigger_fault_if_fg0_not_z
+  jal      x1, trigger_fault_if_fg0_z
 
   ecall
 

sw/otbn/crypto/p256_base.s

@@ -43,33 +43,27 @@
  * sensitive; since aborting the program will be quicker than completing it,
  * the flag's value is likely clearly visible to an attacker through timing.
  *
- * @param[in]    w31: all-zero
- * @param[in]  FG0.Z: boolean indicating fault condition
+ * @param[in]  FG0.Z: boolean indicating fault condition when 1
  *
- * clobbered registers: x2
+ * clobbered registers: x2, w31
  * clobbered flag groups: none
  */
-trigger_fault_if_fg0_z:
+trigger_fault_if_fg0_not_z:
   /* Read the FG0.Z flag (position 3).
        x2 <= FG0.Z */
   csrrw     x2, FG0, x0
   andi      x2, x2, 8
-  srli      x2, x2, 3
+  addi      x2, x2, 31
 
-  /* Subtract FG0.Z from 0.
-       x2 <= 0 - x2 = FG0.Z ? 2^32 - 1 : 0 */
-  sub       x2, x0, x2
-
-  /* The `bn.lid` instruction causes an `BAD_DATA_ADDR` error if the
-     memory address is out of bounds. Therefore, if FG0.Z is 1, this
-     instruction causes an error, but if FG0.Z is 0 it simply loads the word at
-     address 0 into w31. */
-  li         x3, 31
-  bn.lid     x3, 0(x2)
+  /* The `bn.lid` instruction causes an `ILLEGAL_INSN` error if the index of the
+     bignum register (stored in x2 in this case) is invalid. Therefore, if FG0.Z
+     is 1, this instruction causes an error, but if FG0.Z is 0 it simply loads
+     the word at address 0 into w31. */
+  bn.lid    x2, 0(x0)
 
   /* If we get here, the flag must have been 0. Restore w31 to zero and return.
        w31 <= 0 */
-  bn.xor     w31, w31, w31
+  bn.xor    w31, w31, w31
 
   ret
 
@@ -84,29 +78,28 @@ trigger_fault_if_fg0_z:
  * sensitive; since aborting the program will be quicker than completing it,
  * the flag's value is likely clearly visible to an attacker through timing.
  *
- * @param[in]    w31: all-zero
- * @param[in]  FG0.Z: boolean indicating fault condition
+ * @param[in]  FG0.Z: boolean indicating fault condition when 0
  *
- * clobbered registers: x2
+ * clobbered registers: x2, w31
  * clobbered flag groups: none
  */
-trigger_fault_if_fg0_not_z:
+trigger_fault_if_fg0_z:
   /* Read the FG0.Z flag (position 3).
        x2 <= FG0.Z */
   csrrw     x2, FG0, x0
   andi      x2, x2, 8
-  slli      x2, x2, 3
+  xori      x2, x2, 8
+  addi      x2, x2, 31
 
-  /* The `bn.lid` instruction causes an `BAD_DATA_ADDR` error if the
-     memory address is out of bounds. Therefore, if FG0.Z is 1, this
-     instruction causes an error, but if FG0.Z is 0 it simply loads the word at
-     address 0 into w31. */
-  li         x3, 31
-  bn.lid     x3, 0(x2)
+  /* The `bn.lid` instruction causes an `ILLEGAL_INSN` error if the index of the
+     bignum register (stored in x2 in this case) is invalid. Therefore, if FG0.Z
+     is 0, this instruction causes an error, but if FG0.Z is 1 it simply loads
+     the word at address 0 into w31. */
+  bn.lid    x2, 0(x0)
 
   /* If we get here, the flag must have been 1. Restore w31 to zero and return.
        w31 <= 0 */
-  bn.xor     w31, w31, w31
+  bn.xor    w31, w31, w31
 
   ret
 
@@ -1358,7 +1351,7 @@ scalar_mult_int:
   bn.rshi   w2, w2, w20 >> 65
 
   /* double-and-add loop with decreasing index */
-  loopi     321, 64
+  loopi     321, 63
 
     /* double point Q
        Q = (w8, w9, w10) <= 2*(w8, w9, w10) = 2*Q */
@@ -1517,7 +1510,7 @@ scalar_mult_int:
 
      FG0.Z <= if (w10 == 0) then 1 else 0 */
   bn.cmp    w10, w31
-  jal       x0, trigger_fault_if_fg0_z
+  jal       x0, trigger_fault_if_fg0_not_z
 
 
 /**
@@ -1608,7 +1601,7 @@ p256_base_mult:
      The check fails if both sides are not equal.
      FG0.Z <= (y^2) mod p == (x^2 + ax + b) mod p */
   bn.cmp   w18, w19
-  jal      x1, trigger_fault_if_fg0_not_z
+  jal      x1, trigger_fault_if_fg0_z
 
   ret
 

sw/otbn/crypto/p256_shared_key.s

@@ -89,7 +89,7 @@ p256_shared_key:
      The check fails if both sides are not equal.
      FG0.Z <= (y^2) mod p == (x^2 + ax + b) mod p */
   bn.cmp   w18, w19
-  jal      x1, trigger_fault_if_fg0_not_z
+  jal      x1, trigger_fault_if_fg0_z
 
   /* Arithmetic masking:
    1. Generate a random mask

sw/otbn/crypto/p256_sign.s

@@ -126,7 +126,7 @@ p256_sign:
      The check fails if both sides are not equal.
      FG0.Z <= (y^2) mod p == (x^2 + ax + b) mod p */
   bn.cmp   w18, w19
-  jal      x1, trigger_fault_if_fg0_not_z
+  jal      x1, trigger_fault_if_fg0_z
 
   /* setup modulus n (curve order) and Barrett constant
      MOD <= w29 <= n = dmem[p256_n]; w28 <= u_n = dmem[p256_u_n]  */
@@ -288,7 +288,7 @@ p256_sign:
      which violates ECDSA private key requirements. This could technically be
      triggered by an unlucky key manager seed, but the probability is so low (~1/n)
      that it more likely indicates a fault attack. */
-  jal       x1, trigger_fault_if_fg0_z
+  jal       x1, trigger_fault_if_fg0_not_z
 
   /* w24 = r <= w11  mod n */
   bn.addm   w24, w11, w31

sw/otbn/crypto/p384_isoncurve.s

@@ -27,30 +27,29 @@
  * sensitive; since aborting the program will be quicker than completing it,
  * the flag's value is likely clearly visible to an attacker through timing.
  *
- * @param[in]    w31: all-zero
- * @param[in]  FG0.Z: boolean indicating fault condition
+ * @param[in]  FG0.Z: boolean indicating fault condition when 0
  *
- * clobbered registers: x2, x3
+ * clobbered registers: x2, w31
  * clobbered flag groups: none
  */
-.globl trigger_fault_if_fg0_not_z
-trigger_fault_if_fg0_not_z:
+.globl trigger_fault_if_fg0_z
+trigger_fault_if_fg0_z:
   /* Read the FG0.Z flag (position 3).
        x2 <= FG0.Z */
   csrrw     x2, FG0, x0
   andi      x2, x2, 8
-  slli      x2, x2, 3
+  xori      x2, x2, 8
+  addi      x2, x2, 31
 
-  /* The `bn.lid` instruction causes an `BAD_DATA_ADDR` error if the
-     memory address is out of bounds. Therefore, if FG0.Z is 1, this
-     instruction causes an error, but if FG0.Z is 0 it simply loads the word at
-     address 0 into w31. */
-  li         x3, 31
-  bn.lid     x3, 0(x2)
+  /* The `bn.lid` instruction causes an `ILLEGAL_INSN` error if the index of the
+     bignum register (stored in x2 in this case) is invalid. Therefore, if FG0.Z
+     is 1, this instruction causes an error, but if FG0.Z is 0 it simply loads
+     the word at address 0 into w31. */
+  bn.lid    x2, 0(x0)
 
   /* If we get here, the flag must have been 1. Restore w31 to zero and return.
        w31 <= 0 */
-  bn.xor     w31, w31, w31
+  bn.xor    w31, w31, w31
 
   ret
 
@@ -214,11 +213,11 @@ p384_isoncurve_check:
        FG0.Z <= (y^2) mod p == (x^2 + ax + b) mod p */
   bn.cmp    w4, w6
   /* Fail if FG0.Z is false. */
-  jal       x1, trigger_fault_if_fg0_not_z
+  jal       x1, trigger_fault_if_fg0_z
 
   bn.cmp    w5, w7
   /* Fail if FG0.Z is false. */
-  jal       x1, trigger_fault_if_fg0_not_z
+  jal       x1, trigger_fault_if_fg0_z
 
   ret
 

sw/otbn/crypto/p384_isoncurve_proj.s

@@ -60,11 +60,11 @@ p384_isoncurve_proj_check:
        FG0.Z <= (y^2) mod p == (x^2 + ax + b) mod p */
   bn.cmp    w4, w2
   /* Fail if FG0.Z is false. */
-  jal       x1, trigger_fault_if_fg0_not_z
+  jal       x1, trigger_fault_if_fg0_z
 
   bn.cmp    w5, w3
   /* Fail if FG0.Z is false. */
-  jal       x1, trigger_fault_if_fg0_not_z
+  jal       x1, trigger_fault_if_fg0_z
 
   ret
 

sw/otbn/crypto/tests/BUILD

@@ -207,6 +207,10 @@ otbn_sim_test(
 
 otbn_consttime_test(
     name = "p256_base_mult_consttime",
+    ignore = [
+        "trigger_fault_if_fg0_z",
+        "trigger_fault_if_fg0_not_z",
+    ],
     subroutine = "p256_base_mult",
     deps = [
         "//sw/otbn/crypto:run_p256",
@@ -275,6 +279,10 @@ otbn_sim_test(
 
 otbn_consttime_test(
     name = "p256_shared_key_consttime",
+    ignore = [
+        "trigger_fault_if_fg0_z",
+        "trigger_fault_if_fg0_not_z",
+    ],
     subroutine = "p256_shared_key",
     deps = [
         "//sw/otbn/crypto:run_p256",
@@ -674,6 +682,7 @@ otbn_consttime_test(
 otbn_consttime_test(
     name = "p384_scalar_mult_consttime",
     ignore = [
+        "trigger_fault_if_fg0_z",
         "trigger_fault_if_fg0_not_z",
     ],
     subroutine = "p384_scalar_mult",