[crypto] Add a counter to the main loop of ECDSA-P256 verify.

[jadephilipoom]

This is a simple, lightweight hardening measure against fault-injection attacks on the loop control flow in OTBN, just in case the hardware hardening measures don't hold up as well as we'd like.

The OTBN ECDSA-P256 verification program returns two results: an "OK" value that's either kHardenedBoolTrue or kHardenedBoolFalse, and the recovered "r" value that should be equal to "r" from the signature if the signature was valid. Ibex needs to check both values.

The new counter basically just piggybacks on the existing "OK" value; instead of directly writing kHardenedBoolTrue into the "OK" buffer, we load the constant kHardenedBoolTrue ^ 256 and then XOR it with a counter that gets incremented on every loop iteration. If the counter says 256 iterations, we get the right value, but if not we'll get something else and Ibex will detect that something is wrong in the code here:

opentitan/sw/device/silicon_creator/lib/otbn_boot_services.c

Lines 269 to 278 in 5655312

	// Check if the signature passed basic checks.
	uint32_t ok;
	HARDENED_RETURN_IF_ERROR(sc_otbn_dmem_read(1, kOtbnVarBootOk, &ok));
	if (launder32(ok) != kHardenedBoolTrue) {
	return kErrorSigverifyBadEcdsaSignature;
	}
	// Read the status value again as an extra hardening measure.
	HARDENED_RETURN_IF_ERROR(sc_otbn_dmem_read(1, kOtbnVarBootOk, &ok));
	HARDENED_CHECK_EQ(ok, kHardenedBoolTrue);

If we wanted to be even more strict, we could cause a more catastrophic error if ok != kHardenedBoolFalse. We could also consider starting the counter at 2^32-1 instead of 0, so that the expected value is 255, which would just give us more bits that the counter flips. Open to opinions on more advanced strategies, but for now I just went with the simplest version.

[johannheyszl]

Thanks @jadephilipoom this is great. I have reviewed the code - it looks effective and correct!

[vogelpi]

Thanks @jadephilipoom and @johannheyszl ! The change looks good to me.

IMO, we don't need to further harden the newly added counter: if an adversary managed to glitch the counter, they would still need to glitch other stuff to actually exploit this. But if there is a very easy way to further improve things I also wouldn't say no :-)

[jadephilipoom]

But if there is a very easy way to further improve things I also wouldn't say no :-)

Challenge accepted 🙂 After a little thought and taking a look at the code again, I added a new commit that expands on the idea just a little bit. Instead of using addi and a constant 1, it sets register x11 to 1 at the start of the loop and uses that to increment the counter. At various points in the control flow that directly follow branches on boolean values, I used and/srl with the x11 register so that it stays 1 if the control flow is followed correctly and gets flipped to 0 otherwise. That way, faults that cause the branches to get skipped would also cause the counter to not increment. I can remove this second commit if folks think the complexity isn't worth it. (The effect on performance is negligible.)

[johannheyszl]

Thank @jadephilipoom you this is very nice. I support keeping it.

[timothytrippel]

Thanks @jadephilipoom , this is great! Let's get this in once CI passes, and hopefully tonight's or tomorrow's nightly regression can run with the change.

[timothytrippel]

One thing: you will probably have to update the rom self hash test since it will fail on the fpga if the rom changes at all now that it is running in CI. See the instructions here: https://cs.opensource.google/opentitan/opentitan/+/master:sw/device/silicon_creator/rom/e2e/release/rom_e2e_self_hash_test.c;drc=9d2140b0ad00c7337454f85edb7772d611a5615b;l=31

[jadephilipoom]

One thing: you will probably have to update the rom self hash test since it will fail on the fpga if the rom changes at all now that it is running in CI. See the instructions here: https://cs.opensource.google/opentitan/opentitan/+/master:sw/device/silicon_creator/rom/e2e/release/rom_e2e_self_hash_test.c;drc=9d2140b0ad00c7337454f85edb7772d611a5615b;l=31

Thanks for the tip, should be updated now!