Port the network stack to use the randombytes RTOS API rather than having each compartment re-implement rand(). - #121
Conversation
e8ff670 to
e9d6eef
Compare
davidchisnall
left a comment
davidchisnall
left a comment
There was a problem hiding this comment.
I left one comment but it applies across the rest.
I’m not sure when we can merge this. We try hard to avoid the requirement to version bump the RTOS and network stack at the same time, but these all use the helper that was added to the RTOS yesterday, so merging this means anyone using this needs to have yesterday’s RTOS or newer. The helper means that we can’t just gate this on the existence of the randombytes.h header and fall back to the old version.
Once we merge this, we’ll have a flag day. We should delete all of the old compatibility code in other places at the same time.
| // Prepare the query answer buffer and ID for the new query. | ||
| memset(&queryResult, 0, sizeof(NetworkAddress)); | ||
| queryID = rand(); | ||
| if (!randombytes(queryID)) |
There was a problem hiding this comment.
Our style recommends using explicit != 0, rather than relying on implicit conversion to bool, to make it clear that the return is not Boolean.
It might be better to return the error code that randombytes returns, rather than -EAGAIN. For example, if this returns a not-enough-stack / trusted stack error then the caller can’t just retry and expect success in the future.
Actually, I came up with some concepts trickery to allow this network stack to re-define the wrappers if they're not available from the RTOS. That should let us avoid a flag day. |
…ving each compartment re-implement rand().
2 participants
No description provided.