[sw/sca] Optimize/Clean up code of batch modes in sca serial files

[wettermo]

Description

This issue is related to https://github.com/lowRISC/opentitan/pull/17416

@jadephilipoom made some good points on the batch mode implementation in the ecc256_keygen_serial.c file, which are also valid to each batch mode implementation in the other serial files. Hence, these suggested modifications should be applied to every serial file.

The points are:

• To fully comply with the style guide.
• To remove the global key/message/seed/etc. arrays, which store all values from each batch, and instead allocate enough stack memory locally for a single key/message/seed/etc. value. If we then directly process each single value this would allow us to remove the maximum number of traces per batch and also the second for-loop.
• To use memcpy(num_traces, data, data_len) instead of num_traces = read_32(data) . This is safer.

[wettermo]

I took a first look at this by optimizing/cleaning up the ecc256_keygen_serial.c batch mode code. I only implemented the first two points from above. So far, capturing runs through. The capture speed is very similar. But, as @vrozic already mentioned in https://github.com/lowRISC/opentitan/pull/17416, I'm observing different leakage than before:

Does anyone have an idea why the leakage could be different? @vrozic mentioned some "fake leakage" because of residual charge on one of the on-board capacitors, but I don't completely understand where this charge comes from. Does it simply come from constantly writing and reading every single seed or mask value shortly before capturing and triggering the OTBN app? And if there is unwanted leakage, does it then actually make sense to keep on working on this issue?

In case someone wants to take a closer look, the corresponding code is in my personal github repositories: https://github.com/wettermo/opentitan/commit/6300249a8ee0c5f94350f46543ef01552fac1ec3 (serial code) https://github.com/wettermo/ot-sca/commit/4a92bf20d2fce2c905a31a6ee1c57d5b8c6c07df (ot-sca containing the binary)

[wettermo]

We just discussed in the SCA WG meeting, that this shouldn't be further investigated, and that the code should remain as it is. The proposed code changes cause to much "fake leakage", which makes analysis too difficult. According to @vrozic this behavior was already observed for KAMC, which is the reason, why the code was changed to the current state in the first place. Therefore, I'll close this issue.

[jadephilipoom]

I agree it makes sense to not implement the second bullet point if it causes fake leakage. However, I'd still like to see the other two points addressed as a follow-up to #17416, primarily so that coding style is consistent across the repo :

• To fully comply with the style guide.
• To use memcpy(num_traces, data, data_len) instead of num_traces = read_32(data) . This is safer.

[wettermo]

However, I'd still like to see the other two points addressed as a follow-up to #17416, primarily so that coding style is consistent across the repo

Agree! Sorry, I overlooked that.