tillitis / tillitis-key1

Board designs, FPGA verilog, firmware for TKey, the flexible and open USB security key 🔑
https://www.tillitis.se
382 stars 24 forks source link

Remove possibly redundant randomized RAM scrambling during randomised RAM fill #225

Closed secworks closed 3 weeks ago

secworks commented 1 month ago

(This issue is primarily to document an observation.)

The functionality in FW to fill the RAM with pseudo random data is done in scramble_ram(), see: https://github.com/tillitis/tillitis-key1/blob/0454e023cd5c05cf8c84df358c45a05f291e153e/hw/application_fpga/fw/tk1/main.c#L366

The function starts by setting the HW scrambling parameters in the memory system to random values. Then the RAM is filled with 'random data' with a PRNG in a loop. Finally the HW scrambling parameters are updated again, to ensure that the scrambling on device app memory accesses are not the same as the ones used to fill the RAM, possibly adding a second layer of obfuscation.

The possible issue with this is that setting of the HW scrambling parameters are done using the TRNG. The TRNG has low bitrate. Basically the effective bit generation speed is in order of one kbps. Since we need 32 bits for each call, the CPU will possibly have to wait thousands of cycles. Esp for the second and fourth request to the TRNG since they happen close in time to request one and three, not giving the TRNG time to generate enough bits.

Removing the first two calls, the one before the random fill loop would probably reduce the time for the FW to be ready to load a device app. Removing them would also reduce the size of the FW with a few bytes.

The big question is if the first scrambling makes the random fill better or not. At least address scrambling should make it harder to guess the PRNG state. But since the scrambling is quite simplistic, this potential gain in obfuscation may not be very big. In fact it may be so that it cause patterns to be inserted. (This is primarily in context of the data scrambling.)

The current PRNG that generates the random data written to RAM is very simplistic. There is a PR to replace this with something better, see: https://github.com/tillitis/tillitis-key1/pull/201

But replacing the PRNG with something better is probably separate from this discussion. There are also ideas to replace the RAM access scrambling in the memory system with something better. That is also outside of this discussion.

dehanj commented 1 month ago

Measured the time it takes to do one set:

// Set RAM address and data scrambling values
*ram_rand = rnd_word();
*ram_scramble = rnd_word();

it takes 30 ms, and from the code I would say that both the calls to the rnd_word() would be a maximal delay in form of waiting for the TRNG to be ready.