[GitHub] [incubator-nuttx] juniskane edited a comment on pull request #2497: crypto/random_pool.c: fix getrandom() when thread calling it gets can…

[GitBox <gi...@apache.org>]

juniskane edited a comment on pull request #2497:
URL: https://github.com/apache/incubator-nuttx/pull/2497#issuecomment-742654501


   > at least, three libc(glibc, musl and bionic) implement getrandom:
   > https://github.com/bminor/musl/blob/master/include/sys/random.h#L15
   > https://github.com/aosp-mirror/platform_bionic/blob/master/libc/include/sys/random.h#L65
   
   And other operating systems implement various alternatives to getrandom(). Do we add FreeBSD's read_random(2), OpenBSD's old sysctl(3) KERN_ARND, MS Windows CryptGenRandom(), and Apple iOS's SecRandom() to NuttX as well?
   
   Solaris 11.3 has getrandom() with this prototype
   
   `int getrandom(void *buf, size_t buflen, unsigned int flags);`
   
   while Linux and FreeBSD use ssize_t as return value. Solaris versions semantics are also slightly different, buflen is limited to 1024 and no EINTR is specified and maybe there are more subtle differences as well between Linux, FreeBSD, Solaris and the various libc wrappers? Solaris man page: https://docs.oracle.com/cd/E86824_01/html/E54765/getrandom-2.html
   
   It seems Linux kernel folks have had serious problems with getrandom(), please read
   https://lwn.net/Articles/800509/
   So there is more to this than just the function prototype. What should getrandom(..., 0) do for instance on NuttX? Should the default be blocking or non-blocking? It seems this has varied in Linux between kernel versions. I think non-blocking is a better choice and Linux people got that wrong.
   
   I don't want to have bugs in embedded systems, where system hangs waiting for entropy that will never come, this condition perhaps triggered by unrelated commit that optimizes away some interrrupt activity from some other part of the system. Only reliable defence against that is to provide enough initial entropy so that the random pool never blocks. The exact mechanism for initial entropy has to be left to system designer as it varies depending on HW, factory setup, threat model etc.
   
   > Does INVIOLABLES allow you to define a new API doesn't come from any POSIX like OS?
   
   getrandom() was added by Jussi Kivilinna in 2017 (commit dffb8a67) Before that it existed at least from 2015 in our (in-house, but publicly available) NuttX fork. That was few years before INVIOLABLES even existed. Linux added getrandom(2) only about year earlier in 2014. As many current contributors were not active in this project back then, it is not very good idea to impute current criteria back in time for that far.
   
   >     1. Fix the prototype of getrandom to confirm man's defintion
   > 
   >     2. Implement arc4random_buf and remove getrandom
   > 
   > If you don't like both approach, the best thing is to remove getrandom from the code base.
   
   getrandom() has been used for years in multiple NuttX products shipped to real users. We don't have a habit of removing existing OS features, especially ones that have been in use for years, just because someone does not like every little detail about them. We keep things like netlink sockets or 6LowPAN even while they are not in POSIX
   
   Changing the prototype affects existing callers, so if you insist having three argument getrandom() then I propose that the same commit renames current function as arc4random_buf() as it really is intended to be a drop-in placement for that OpenBSD derived API. This way existing callers can do a simple rename, without having to figure what flags to pass to getrandom() (I guess none of the flags will be implemented, but that may change in future).


----------------------------------------------------------------
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:
users@infra.apache.org