juniskane commented on pull request #2488: URL: https://github.com/apache/incubator-nuttx/pull/2488#issuecomment-740798274
> But now you directly ignore the error at all, could you tell me how can the caller handle these error case securely? > > 1. nxsem_wait_uninterruptible return fail Implementation bug, corrected in https://github.com/apache/incubator-nuttx/pull/2497 > 2. user call getrandom too early and then no enough entropy is collected yet Not a problem in practise. We fill most of otherwise empty internal flash space with good quality random bytes during device production and then can activate flash read-out-protection. It is also easy to store gathered entropy across resets (internal flash, BBSRAM) and reseed back to entropy pool during board initialization phase. > If the entropy isn't collected enough, we should block and wait the enough entropy by default, or return -1 with errno set to EAGAIN if user pass GRND_NONBLOCK. But your implementation just log a warning: > > ``` > static void rng_buf_internal(FAR void *bytes, size_t nbytes) > { > if (!g_rng.output_initialized) > { > if (g_rng.rd_newentr < MIN_SEED_NEW_ENTROPY_WORDS) > { > cryptwarn("Entropy pool RNG initialized with very low entropy. " > " Consider implementing CONFIG_BOARD_INITRNGSEED!\n"); Better to do as the warning says :) You can always change this to PANIC() if want to be really sure we never run past this point with insufficient entropy. > But how you tell caller if there isn't enough entropy has been collected yet? or the external hardware RNG can't access because I2C/SPI/USB bus is broken. Can any real device do anything other than PANIC() if some of buses or other HW it really needs are broken? > It's arc4random_buf design problem: it doesn't return the error code. We should avoid to use this function, but it is wrong to drop getrandom return value. arc4random_buf() is *very carefully designed* by people who know how to design cryptographical software. Linux and Glibc maintainers botched this with their overly complex and error prone interface. arc4random functions were deliberately designed to be easy to use to avoid crypto failures due to misuse by their caller. Of course this places burden to implementator to get it right. Note that we have other APIs that can never fail, getpid(), getuid(), longjmp(), pthread_getspecific() etc. It seems Austin Group commenters got confused about this, interesting discussion though: https://www.austingroupbugs.net/view.php?id=859 ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
