[sival] Add test for the entropy source's firmware extract and insert mode

[jwnrt]

Summary

This PR extends the entropy source firmware override observe fifo test to enable "extract and insert" mode.

1. Adds an interrupt handler for the es_observe_fifo_ready interrupt which reads 32 words (the FIFO threshold value) from the observe FIFO and writes it back again.
2. Allows configuring the entropy source with extract and insert mode, enabling that interrupt handler.
3. For each of the "single bit" configurations, checks the entropy source is working by generating a set of random numbers and running AES, KMAC, and HMAC.

[h-filali]

Looks pretty darn good already. Thanks for working on this!
What do you think about adding some more HW entropy consumers?

[jwnrt]

What do you think about adding some more HW entropy consumers?

Yeah, that would be good. Do you have any suggestions? I'm actually not very familiar with which blocks require entropy 😅

[h-filali]

What do you think about adding some more HW entropy consumers?

Yeah, that would be good. Do you have any suggestions? I'm actually not very familiar with which blocks require entropy 😅

You may use any of the endpoints of the EDN I believe there are 8. Of the top of my head there is AES, KMAC, alert_handler, AST, OTBN urand, OTBN rand, IBEX. You may also draw inspiration from the powervirus test or from some of my edn tests.

[vogelpi]

Thanks @jwnrt for working on this!

I think this is going in the right direction but a couple of things should be changed before we merge this test. I am giving some more detailed feedback and explanations inline below.

1. Adds an interrupt handler for the es_observe_fifo_ready interrupt which reads 32 words (the FIFO threshold value) from the observe FIFO and writes it back again.

I think this is perfectly fine for a proof of concept but we will want to read at least 1024 consecutive samples, i.e., the window (can be 4-bit samples, i.e., 4 kByte of data or 1-bit samples in single-bit mode). The reason is that firmware will need to read a consecutive chunk of entropy for the health testing (done by firmware in this mode but obviously not part of this test). Similar to the first test @pamaury wrote, you'll need to:

• test that the FIFO doesn't overflow while reading the 1024 samples
• when it overflows, the FIFO is emptied before starting collecting entropy for the next window
• do a couple of iterations/windows without restarting the entropy source in between, to stress test the whole entropy complex. For this, you can configure the EDNs in auto mode and request a lot of entropy (@h-filali knows how to achieve this).

TODO and questions

FIPS mode

Currently, the test has to disable FIPS mode when running with extract and insert. With FIPS enabled, CSRNG never instantiates. This is likely due to software not being able to set the FIPS bit which CSRNG probably expects, fixed by #21369.

I think this is unrelated. The problem with FIPS mode is that the data you write into the FW_OV_WR_DATA register will be injected into the conditioner. Firmware also has to kick off the conditioner. If this is not done, ENTROPY_SRC never generates a seed and that's why CSRNG fails to instantiate: it doesn't get the seed for the instantiation. How to operate the conditioner in firmware override: extract and insert mode is described here: https://opentitan.org/book/hw/ip/entropy_src/doc/programmers_guide.html#firmware-override---extract--insert

The second challenge here is that whether a seed is / can be produced depends on the amount of data you insert: without conditioning it's 384 bits or 12 32-bit words, with conditioning it may be more. When fully relying on the hardware pipeline, we use 2048 bits per default for one 384-bit seed. So that would be the content of the full observe FIFO. I think right now, you're at most writing 1024 bits.

In general the amount of data the test extracts and then inserts doesn't need to match. It's fine to extract more than what you insert (the other way round would be less meaningful). Firmware would most likely do this as well.

Checking the output of the entropy source

Is generating random numbers the best way to check the output of the entropy source? Is there something else that would be better to run? I've checked that when the interrupt handler is not writing data back into the FIFO the random number generators time out. Should I try incorporate that into the test somehow?

Yes, it's intended to check various hardware entropy consumers are run and indeed finish their operation as @h-filali mentioned. One source of inspiration is the power virus test. IIRC, Hakim also did something similar but simpler for one of the EDN SiVal tests.

Ideally, you can also extend this test to check the DEBUG_STATUS before injecting a seed. This has a field ENTROPY_FIFO_DEPTH indicating if the final FIFO in the pipeline is empty (CSRNG stalls) or full (any seed injected by firmware gets dropped).

Sorry for the long message. I'll be OoO tomorrow and wanted to be sure you're not blocked on my input :-)

[jwnrt]

I've changed the ISR implementation to collect the whole 1024 buffer and dump it if there were any overflows to ensure contiguous samples, but it's slowed the test way down. Probably an issue with my implementation.

I haven't added the extra entropy consumers yet, but will get to it next

[pamaury]

For what it is worth, I don't think it is necessary to read 1024 samples in the ISR. It seems perfectly fine to me to read, for example, 512 samples the first time it fires and then 512 the second time. The ISR can check if the FIFO overflow in the mean time so it can still determine if it got contiguous samples or not.
In fact, waiting until we have 1024 samples seem very inefficient because now we won't start the conditioner until after collection instead of slowly pushing the data through the conditioner as it becomes available.

[jwnrt]

I guess the problem is that if we push 512 bytes on the first interrupt and then get an overflow, we can't take back those 512 bytes that we already sent through (I don't think). That would mean that the next 512 bytes get added on top and there's no way to tell the downstream consumers that what they're seeing is two sets of 512 bytes and not one contiguous set of 1024. Is that correct?

[pamaury]

This is why the reading and writing paths need to be asynchronous in my opinion: there is one big 1024-sample buffer. The reading ISR regularly reads samples from the observe FIFO and writes them to the buffer. It needs to do a small amount of work each time to not block the rest of the system (so read way less than 512 samples). Concurrently, probably using a timer, another ISR pushes data from the buffer into the conditioner when there is space available; most importantly it does not block.
This is arguably a more complex setup but it is also more efficient and more representative of an actual workload in my opinion. Also, with this setup, it should be impossible for an observe FIFO overflow to occur: the reading ISR should be fast enough to read samples and the buffer is big enough to store all 1024 samples.

[jwnrt]

Just so I'm understanding, are you saying to have the read ISR push small amounts of data into an in-memory buffer so that the observe FIFO doesn't overflow, then have another ISR on a timer which takes chunks of 1024 bytes from that buffer and writes them through to the FIFO that does conditioning and the rest of the entropy source chain?

[pamaury]

Just so I'm understanding, are you saying to have the read ISR push small amounts of data into an in-memory buffer so that the observe FIFO doesn't overflow, then have another ISR on a timer which takes chunks of 1024 bytes from that buffer and writes them through to the FIFO that does conditioning and the rest of the entropy source chain?

Yes exactly.

[jwnrt]

I tried that with an ISR that blocked and wrote the whole buffer of 1024 to the conditioner in one go and it was too slow. I could try again with an ISR that writes small chunks at a time, but only starts once 1024 contiguous samples are there. The implementation might be a bit more complex, but I'll see if it's doable.

[vogelpi]

@pamaury 's approach sounds reasonable to me.

Regarding feeding the conditioner: It's SHA3-384, so the block size is 832 bits. This means after pushing the first 832 bits into the conditioner, it should run for the first time (this will take around 50 clock cycles). I am not 100% sure anymore, it may be that the conditioner will be triggered automatically or that software has to trigger it explicitly. Then, the remaining 1024 - 832 = 192 bits should be written and the conditioner should be triggered by software to produce the final seed.

[jwnrt]

I've updated the implementation to read and write 32 words at a time in each interrupt while still handling overflows. This runs a lot faster. It's triggered by the "entropy FIFO full" and "entropy requested" interrupts, but I can try and change it to a timer if needed (setting up a repeating timer ended up being more complicated than I expected).

I've also separated this test off from the observe test because it was starting to get quite complex.

I still need to add a bunch more entropy consumers.

[jwnrt]

Sorry for the delay, I've added some usage of AES, KMAC, and HMAC. Please let me know if my usage of them isn't sufficient. I didn't copy all of the things that the power virus test does because it's quite complex and a lot would need to be moved across. The results of these processes aren't checked, just the statuses.

[pamaury]

Really nice @jwnrt :) I left a couple of minor comments.

[vogelpi]

Very nice work @jwnrt ! Thanks for doing this.

Besides mostly minor comments, I have two things:

1. Ideally some more entropy can be injected into the conditioner but I see the issue you raised. So lets maybe not do it in this iteration.
2. HMAC doesn't use entropy from EDN but OTBN would use both EDNs. I suggest to execute the otbn_randomness_test.

[jwnrt]

I have moved some functions to testutils, fixed up some comments, changed some flags, and swapped HMAC for OTBN.

[vogelpi]

Awesome, thanks @jwnrt !

[vogelpi]

This resolves #20952.

[github-actions]

Backport failed for earlgrey_es_sival, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin earlgrey_es_sival
git worktree add -d .worktree/backport-21562-to-earlgrey_es_sival origin/earlgrey_es_sival
cd .worktree/backport-21562-to-earlgrey_es_sival
git switch --create backport-21562-to-earlgrey_es_sival
git cherry-pick -x 9d098ddaba218c63168005096d7259af9dd73cb9 7f914a33cfc39807d575f715d74fa68208b64527 9f56456f3a557d48898307c8005b4f84bc8a402c

[jwnrt]

I think the changes that will be needed for the backport are:

1. The workaround for the observe FIFO overflow will need to be added back.
2. OTBN required the FIPS bit to be set by software which isn't possible on earlgrey_es_sival.

If the OTBN issue cannot be worked around then we may have to depend on the other entropy consumers.

[vogelpi]

Your assessment is correct @jwnrt . We won't be able to use OTBN on earlgrey_es_sival for this test. Maybe we can do a couple of iteration of AES and KMAC instead.