On Tue, Nov 12, 2019 at 8:25 PM Stephan Müller <smueller@xxxxxxxxxx> wrote: > > Am Dienstag, 12. November 2019, 16:33:59 CET schrieb Andy Lutomirski: > > Hi Andy, > > > On Mon, Nov 11, 2019 at 11:13 AM Stephan Müller <smueller@xxxxxxxxxx> wrote: > > > The following patch set provides a different approach to /dev/random which > > > is called Linux Random Number Generator (LRNG) to collect entropy within > > > the Linux kernel. The main improvements compared to the existing > > > /dev/random is to provide sufficient entropy during boot time as well as > > > in virtual environments and when using SSDs. A secondary design goal is > > > to limit the impact of the entropy collection on massive parallel systems > > > and also allow the use accelerated cryptographic primitives. Also, all > > > steps of the entropic data processing are testable. > > > > This is very nice! > > > > > The LRNG patch set allows a user to select use of the existing /dev/random > > > or the LRNG during compile time. As the LRNG provides API and ABI > > > compatible interfaces to the existing /dev/random implementation, the > > > user can freely chose the RNG implementation without affecting kernel or > > > user space operations. > > > > > > This patch set provides early boot-time entropy which implies that no > > > additional flags to the getrandom(2) system call discussed recently on > > > the LKML is considered to be necessary. > > > > I'm uneasy about this. I fully believe that, *on x86*, this works. > > But on embedded systems with in-order CPUs, a single clock, and very > > lightweight boot processes, most or all of boot might be too > > deterministic for this to work. > > > > I have a somewhat competing patch set here: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/log/?h=random > > /kill-it > > > > (Ignore the "horrible test hack" and the debugfs part.) > > > > The basic summary is that I change /dev/random so that it becomes > > functionally identical to getrandom(..., 0) -- in other words, it > > blocks until the CRNG is initialized but is then identical to > > /dev/urandom. And I add getrandom(...., GRND_INSECURE) that is > > functionally identical to the existing /dev/urandom: it always returns > > *something* immediately, but it may or may not actually be > > cryptographically random or even random at all depending on system > > details. > > > > In other words, my series simplifies the ABI that we support. Right > > now, we have three ways to ask for random numbers with different > > semantics and we need to have to RNGs in the kernel at all time. With > > my changes, we have only two ways to ask for random numbers, and the > > /dev/random pool is entirely gone. > > > > Would you be amenable to merging this into your series (i.e. either > > merging the code or just the ideas)? This would let you get rid of > > things like the compile-time selection of the blocking TRNG, since the > > blocking TRNG would be entirely gone. > > I pulled your code and found the following based on my explanation that I > would suggest to keep the TRNG at least as an option. > > - 7d54ef8512b06baf396f12584f7f48a9558ecd0f does not seem applicable: Not surprising. It's just a cleanup to the existing code, and I doubt you inherited the oddity I'm fixing. > - 6a26a3146e5fb90878dca9fde8caa1ca4233156a: My handler for /dev/urandom and > getrandom(..., 0) are using one callback which issues a warning in both use > cases (see lrng_sdrng_read). So I think this patch may not be applicable as > the LRNG code implements warning about being unseeded. Probably true. What is the actual semantics of /dev/urandom with your series applied? Is there any situation in which it will block? > > - 3e8e159da49b44ae0bb08e68fa2be760722fa033: I am happy to take that code which > would almost directly apply. The last hunk however would be: > > if (!(flags & GRND_INSECURE) && unlikely(!lrng_state_operational())) { > > ==> Shall I apply it to my code base? If yes, how shall the changes to > random.h be handled? > This might be a question for Ted. Once the merge window opens, I'll resubmit it. > > - 920e97e7fc508e6f0da9c7dec94c8073fd63ab4d: I would pass on this patch due to > the following: it unconditionally starts removing the access to the TRNG (the > LRNG's logical equivalent to the blocking_pool). As patch 10/12 of the LRNG > patch series provides the TRNG that is a compile time option, your patch would > logically and functionally be equivalent when deselecting > CONFIG_LRNG_TRNG_SUPPORT in the LRNG without any further changes to the LRNG > code. Given your previous email about the TRNG, I'm wondering what the API for the TRNG should be. I am willing to grant that there are users who need a TRNG for various reasons, and that not all of them can use hwrng. (And the current hwrng API is pretty bad.) But I'm not convinced that /dev/random or getrandom(..., GRND_RANDOM) is a reasonable way to access it. A blocking_pool-style TRNG is a very limited resource, and I think it could make sense to require some sort of actual permission to use it. GRND_RANDOM has no access control at all, and everyone expects /dev/random to be world-readable. The most widespread user of /dev/random that I know of is gnupg, and gnupg really should not be using it. Would it make sense to have a /dev/true_random that is 0400 by default for users who actually need it? Then /dev/random and GRND_RANDOM could work as they do with my patch, and maybe it does the right thing for everyone. > > - 693b9ffdf0fdc93456b5ad293ac05edf240a531b: This patch is applicable to the > LRNG. In case CONFIG_LRNG_TRNG_SUPPORT is not set, the TRNG is not present. > Yet, the /dev/random and getrandom(GRND_RANDOM) would behave blocked until > fully initialized. I have now added the general blocking until the LRNG is > fully initialized to the common /dev/random and getrandom(GRND_RANDOM) > interface function of lrng_trng_read_common. With that, the LRNG would be > fully equivalent to this patch if CONFIG_LRNG_TRNG_SUPPORT is not set. Sounds reasonable. > By making the TRNG compile-time selectable, I was hoping to serve all users: I > wanted to cover the conclusions of the discussion to remove the blocking_pool. > On the other hand, however, I want to support requirements that need the > blocking behavior. I find it odd that /dev/random would be either a TRNG or not a TRNG depending on kernel configuration. For the small fraction of users that actually want a TRNG, wouldn't it be better to have an interface that fails outright if the TRNG is not enabled? --Andy
