[otbn] Check seed_valid bit of OTP scrambling interface

[vogelpi]

@ctopal noted that inside OTBN, the seed_valid bit of the OTP scrambling interface is currently not used. We register the bit received from OTP but then don't do anything with it.

If that bit isn't set, it means OTP has used an all-zero constant to derive the scrambling key. Meaning the key is not really safe to use. My understanding is that the first OTBN secure wipe requesting new scrambling keys is anyway initialized after setting up the entropy complex, so this bit should always be set unless something fishy is going on. I suggest to change the OTBN design such that a recoverable error is triggered when receiving a scrambling key with seed_valid low.

Any thoughts @mundaym , @GregAC , @msfschaffner ?

[tjaychen]

I thought the seed valid was more related to whether a particular seed was burned and locked in the otp.
If that's the case, I don't necessarily think it has to be an error, because maybe during testing we have yet to setup otps yet.

it would make sense to indicate this as a status though, and perhaps instead of the default 0, some kind of random fixed constant that can be parameterized.

[GregAC]

I think what @tjaychen suggests is most sensible. We may want to trigger the dmem/imem secure wipe when the bit is 0 for some testing/bring-up reason. Also all recoverable errors currently only occur during software execution. So a recoverable error during execution adds new error behaviour which adds more DV work which I'd rather avoid if at all possible.

[msfschaffner]

So this bit was intended to provide some more info on provisioning status.
If the bit is zero, the key is still ephemeral and thus ok to use - but the seed to parameterize the KDF itself has not been provisioned yet and is thus set to zero.

I don't think we want to error on that, since we may (as @tjaychen pointed out) want to run some OTBN tests before the key seeds are provisioned.

It would however be good to expose this to SW via a CSR in a similar way as this has been done in the SRAM_CTRL so that SW could double check this:

opentitan/hw/ip/sram_ctrl/data/sram_ctrl.hjson

Lines 218 to 228 in 7dfc681

	{ bits: "4"
	name: "SCR_KEY_SEED_VALID"
	desc: '''
	Set to 1 if the scrambling key has been derived from a valid key seed in OTP.
	If !!STATUS.SCR_KEY_VALID is set to 1, !!STATUS.SCR_KEY_SEED_VALID should be 1
	except for cases where the scrambling key seeds have not yet been provisioned to
	OTP. In such a case, the scrambling key is still ephemeral (i.e., it is derived
	using entropy from CSRNG), but a default all-zero value is used as the key seed.
	''',
	resval: 0,
	}

[msfschaffner]

@tjaychen I think the scrambling key is not set to zero inside OTBN in that case.
If we wanted to set the key seed to a random netlist constant, we would have to change that inside the OTP_CTRL KDI...

[GregAC]

I think the conclusion here it's there's no major security concern though exposing the seed valid to SW for OTBN could be useful, though I think we should only consider this for a future release @vogelpi any thoughts?

[johannheyszl]

@jadephilipoom PTAL: The current understanding is, that there is no security concern because either an OTP read or ephemeral seed is used for scrambling, but it might be useful for SW to understand if an ephemeral or OTP based seed is used. IMO this probably does not help much and we could close as not planned.

[jadephilipoom]

As currently written, OTBN drivers (for both silicon creator and cryptolib) always wipe IMEM/DMEM before loading any application to OTBN, so they would refresh the scrambling key before use anyway. I also don't see a security issue here.

[vogelpi]

Okay, thanks for your input everybody (and sorry for my ultra late response). I agree that there are no security concerns then. I agree that it's not necessary to change the RTL of either OTBN or OTP. In both cases it wouldn't add much.

But I do think we should add some notes to the OTP documentation on this (similar to what we have in the SRAM controller, namely that keys are still ephemeral). I'll take care of this.

[vogelpi]

I just filed a PR to update the otp_ctrl documentation with the outcome of this discussion. See #20688.