Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

STM32H7 RNG driver should be more robust #1944

Closed
mkeeter opened this issue Dec 12, 2024 · 3 comments
Closed

STM32H7 RNG driver should be more robust #1944

mkeeter opened this issue Dec 12, 2024 · 3 comments

Comments

@mkeeter
Copy link
Collaborator

mkeeter commented Dec 12, 2024

On a recent manufacturing build, we tried to unlock a tech port and encountered

Dec 12 20:19:09.462 INFO creating SP handle on interface port5, component: faux-mgs
Dec 12 20:19:09.463 INFO initial discovery complete, addr: [fe80::aa40:25ff:fe05:3b00%4]:11111, interface: port5, component: faux-mgs
Error: Error response from SP: monorail: could not create challenge

This indicates that we were unable to read a nonce from the RNG:

fn get_ecdsa_challenge() -> Result<EcdsaSha2Nistp256Challenge, SpError> {
    // Get a nonce from our RNG driver
    let rng = drv_rng_api::Rng::from(RNG.get_task_id());
    let mut nonce = [0u8; 32];
    rng.fill(&mut nonce).map_err(|e| {
        ringbuf_entry!(Trace::RngFillFailed(e));
        SpError::Monorail(GwMonorailError::GetChallengeFailed)
    })?;

Looking at the ringbuf, the specific error is RngError::SeedError.

Doing a soft reset of the SP caused the error to resolve itself. However, that required connecting through the second tech port; if both tech ports were down, this could be a problem!

Looking at the stm32h7-rng driver, this error is returned this if RNG_SR.SEIS is set. However, we make no attempts at recovering.

The datasheet specifies a recovery procedure, which we should implement:

Image

@cbiffle
Copy link
Collaborator

cbiffle commented Dec 12, 2024

Concretely, what I would propose is:

  • Implement the recovery logic described in the manual.
  • Do that if we go to get data and SEIS is set.
  • Continue filling out the client buffer as long as the RNG makes forward progress between SEIS events. So if you can get at least 64 bits or whatever, great, keep doing the recovery.
  • Return a SeedError only if we get two successive SEIS events with no forward progress.

In the case of a client who sends e.g. a 128-byte buffer to fill out, this ensures that we'll fill the whole buffer if possible, even if we're seeing errors every 64 bytes or whatever. The alternative would be to extend the error type to indicate how many bytes were filled in before the RNG failed, but then clients would have to call back; better to do that in the driver, IMO.

@cbiffle
Copy link
Collaborator

cbiffle commented Dec 12, 2024

Note that once ereport facilities are available, the SEIS event should probably produce an ereport.

@cbiffle
Copy link
Collaborator

cbiffle commented Dec 13, 2024

We believe this to have been fixed by #1945 .

@cbiffle cbiffle closed this as completed Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants