diff --git a/sw/device/lib/crypto/impl/ecc/p256.c b/sw/device/lib/crypto/impl/ecc/p256.c index 1d32b60d2d409..2bc141e54c300 100644 --- a/sw/device/lib/crypto/impl/ecc/p256.c +++ b/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, diff --git a/sw/otbn/crypto/boot.s b/sw/otbn/crypto/boot.s index 3e0261657184a..cf4374e642422 100644 --- a/sw/otbn/crypto/boot.s +++ b/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 diff --git a/sw/otbn/crypto/p256_base.s b/sw/otbn/crypto/p256_base.s index 71fca5fb58aac..b7f430e6723fc 100644 --- a/sw/otbn/crypto/p256_base.s +++ b/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 diff --git a/sw/otbn/crypto/p256_shared_key.s b/sw/otbn/crypto/p256_shared_key.s index ddf13a95fad30..b9daf2797eb25 100644 --- a/sw/otbn/crypto/p256_shared_key.s +++ b/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 diff --git a/sw/otbn/crypto/p256_sign.s b/sw/otbn/crypto/p256_sign.s index 66a186b1fd5d8..487b7a675143a 100644 --- a/sw/otbn/crypto/p256_sign.s +++ b/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 diff --git a/sw/otbn/crypto/p384_isoncurve.s b/sw/otbn/crypto/p384_isoncurve.s index 37b0497f06ec1..80b291541e193 100644 --- a/sw/otbn/crypto/p384_isoncurve.s +++ b/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 diff --git a/sw/otbn/crypto/p384_isoncurve_proj.s b/sw/otbn/crypto/p384_isoncurve_proj.s index c77109f18e8f3..a3a3aacfdbd87 100644 --- a/sw/otbn/crypto/p384_isoncurve_proj.s +++ b/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 diff --git a/sw/otbn/crypto/tests/BUILD b/sw/otbn/crypto/tests/BUILD index 52dc6c3d74efa..eb33684e4009e 100644 --- a/sw/otbn/crypto/tests/BUILD +++ b/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",