* Re: [PATCH RFC v4 1/1] random: WARN on large getrandom() waits and introduce getrandom2()
From: Willy Tarreau @ 2019-09-21 3:05 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Linus Torvalds, Ahmed S. Darwish, Lennart Poettering,
Theodore Y. Ts'o, Eric W. Biederman, Alexander E. Patrakov,
Michael Kerrisk, Matthew Garrett, lkml, Ext4 Developers List,
Linux API, linux-man
In-Reply-To: <CALCETrWCjGHKnKikj+YVw22Ufpmnh1TCdGPjG2RL-qzsF=wisA@mail.gmail.com>
On Fri, Sep 20, 2019 at 04:30:20PM -0700, Andy Lutomirski wrote:
> So I think that just improving the
> getrandom()-is-blocking-on-x86-and-arm behavior, adding GRND_INSECURE
> and GRND_SECURE_BLOCKING, and adding the warning if 0 is passed is
> good enough.
I think so as well. Anyway, keep in mind that *with a sane API*,
userland can improve very quickly (faster than kernel deployments in
field). But userland developers need reliable and testable support for
features. If it's enough to do #ifndef GRND_xxx/#define GRND_xxx and
call getrandom() with these flags to detect support, it's basically 5
reliable lines of code to add to userland to make a warning disappear
and/or to allow a system that previously failed to boot to now boot. So
this gives strong incentive to userland to adopt the new API, provided
there's a way for the developer to understand what's happening (which
the warning does).
If we do it right, all we'll hear are userland developers complaining
that those stupid kernel developers have changed their API again and
really don't know what they want. That will be a good sign that the
warning flows back to them and that adoption is taking.
And if the change is small enough, maybe it could make sense to backport
it to stable versions to fix boot issues. With a testable feature it
does make sense.
Willy
^ permalink raw reply
* Re: [PATCH RFC v4 1/1] random: WARN on large getrandom() waits and introduce getrandom2()
From: Andy Lutomirski @ 2019-09-20 23:30 UTC (permalink / raw)
To: Linus Torvalds
Cc: Andy Lutomirski, Ahmed S. Darwish, Lennart Poettering,
Theodore Y. Ts'o, Eric W. Biederman, Alexander E. Patrakov,
Michael Kerrisk, Willy Tarreau, Matthew Garrett, lkml,
Ext4 Developers List, Linux API, linux-man
In-Reply-To: <CAHk-=whz7Okts01ygAP6GZWBvCV7s==CKjghmOp+r+LWketBYQ@mail.gmail.com>
On Fri, Sep 20, 2019 at 3:44 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Fri, Sep 20, 2019 at 1:51 PM Andy Lutomirski <luto@kernel.org> wrote:
> >
> > To be clear, when I say "blocking", I mean "blocks until we're ready,
> > but we make sure we're ready in a moderately timely manner".
>
> .. an I want a pony.
>
> The problem is that you start from an assumption that we simply can't
> seem to do.
Eh, fair enough, I wasn't thinking about platforms without fast clocks.
I'm very nervous about allowing getrandom(..., 0) to fail with
-EAGAIN, though. On a very, very brief search, I didn't find any
programs that would incorrectly assume it worked, but I can easily
imagine programs crashing, and that might be bad, too. At the end of
the day, most user programmers who call getrandom() really did notice
that we flubbed the ABI, and either they were too lazy to fall back to
/dev/urandom, or they didn't want to for some reason, or they
genuinely want the blocking behavior. And people who work with little
embedded systems without good clocks that basically can't generate
random numbers already know this, and they have little scripts to help
out.
So I think that just improving the
getrandom()-is-blocking-on-x86-and-arm behavior, adding GRND_INSECURE
and GRND_SECURE_BLOCKING, and adding the warning if 0 is passed is
good enough. I suppose we could also have separate
GRND_SECURE_BLOCKING and GRND_SECURE_BLOCK_FOREVER. We could also say
that, if you want to block forever, you should poll() on /dev/random
(with my patches applied, where this actually does what users would
want).
--Andy
^ permalink raw reply
* Re: [PATCH RFC v4 1/1] random: WARN on large getrandom() waits and introduce getrandom2()
From: Linus Torvalds @ 2019-09-20 22:44 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Ahmed S. Darwish, Lennart Poettering, Theodore Y. Ts'o,
Eric W. Biederman, Alexander E. Patrakov, Michael Kerrisk,
Willy Tarreau, Matthew Garrett, lkml, Ext4 Developers List,
Linux API, linux-man
In-Reply-To: <CALCETrXMp3dJaKDm+RQijQEUuPNPmpKWr8Ljf+RqycXChGnKrw@mail.gmail.com>
On Fri, Sep 20, 2019 at 1:51 PM Andy Lutomirski <luto@kernel.org> wrote:
>
> To be clear, when I say "blocking", I mean "blocks until we're ready,
> but we make sure we're ready in a moderately timely manner".
.. an I want a pony.
The problem is that you start from an assumption that we simply can't
seem to do.
> In other words, I want GRND_SECURE_BLOCKING and /dev/random reads to
> genuinely always work and to genuinely never take much longer than 5s.
> I don't want a special case where they fail.
Honestly, if that's the case and we _had_ such a methoc of
initializing the rng, then I suspect we could just ignore the flags
entirely, with the possible exception of GRND_NONBLOCK. And even that
is "possible exception", because once your worst-case is a one-time
delay of 5s at boot time thing, you might as well consider it
nonblocking in general.
Yes, there are some in-kernel users that really can't afford to do
even that 5s delay (not just may they be atomic, but more likely it's
just that we don't want to delay _everything_ by 5s), but they don't
use the getrandom() system call anyway.
> The exposed user APIs are, subject to bikeshedding that can happen
> later over the actual values, etc:
So the thing is, you start from the impossible assumption, and _if_
you hold that assumption then we might as well just keep the existing
"zero means blocking", because nobody mind.
I'd love to say "yes, we can guarantee good enough entropy for
everybody in 5s and we don't even need to warn about it, because
everybody will be comfortable with the state of our entropy at that
point".
It sounds like a _lovely_ model.
But honestly, it simply sounds unlikely.
Now, there are different kinds of unlikely.
In particular, if you actually have a CPU cycle counter that actually
runs at least on the same order of magnitude as the CPU frequency -
then I believe in the jitter entropy more than in many other cases.
Sadly, many platforms don't have that kind of cycle counter.
I've also not seen a hugely believable "yes, the jitter entropy is
real" paper. Alexander points to the existing jitterentropy crypto
code, and claims it can fill all our entropy needs in two seconds, but
there are big caveats:
(a) that code uses get_random_entropy(), which on a PC is that nice
fast TSC that we want. On other platforms (or on really old PC's - we
technically support CPU's still that don't have rdtsc)? It might be
zero. Every time.
(b) How was it tested? There are lots of randomness tests, but most
of them can be fooled with a simple counter through a cryptographic
hash - which you basically need to do anyway on whatever entropy
source you have in order to "whiten" it. It's simply _really_ hard to
decide on entropy.
So it's really easy to make the randomness of some input look really
good, without any real idea how good it truly is. And maybe it really
is very very good on one particular machine, and then on another one
(with either a simpler in-order core or a lower-frequency timestamp
counter) it might be horrendously bad, and you'll never know,
So I'd love to believe in your simple model. Really. I just don't see
how to get there reliably.
Matthew Garrettpointed to one analysis on jitterentropy, and that one
wasn't all that optimistic.
I do think jitterentropy would likely be good enough in practice - at
least on PC's with a TSC - for the fairly small window at boot and
getrandom(0). As I mentioned, I don't think it will make anybody
_happy_, but it might be one of those things where it's a compromise
that at least works for people, with the key generation people who are
really unhappy with it having a new option for their case.
And maybe Alexander can convince people that when you run the
jitterentropy code a hundred billion times, the end result (not the
random stream from it, but the jitter bits themselves - but I'm not
even sure how to boil it down) - really is random.
Linus
^ permalink raw reply
* Re: [PATCH RFC v4 1/1] random: WARN on large getrandom() waits and introduce getrandom2()
From: Andy Lutomirski @ 2019-09-20 20:51 UTC (permalink / raw)
To: Linus Torvalds
Cc: Andy Lutomirski, Ahmed S. Darwish, Lennart Poettering,
Theodore Y. Ts'o, Eric W. Biederman, Alexander E. Patrakov,
Michael Kerrisk, Willy Tarreau, Matthew Garrett, lkml,
Ext4 Developers List, Linux API, linux-man
In-Reply-To: <CAHk-=whJ3kmcZp=Ws+uXnRB9KokG6nXSQCSuBnerG--jkAfP5w@mail.gmail.com>
On Fri, Sep 20, 2019 at 12:51 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> > And the only real question is how to map existing users to these
> > semantics. I see two sensible choices:
> >
> > 1. 0 means "secure, blocking". I think this is not what we'd do if we
> > could go back in time and chage the ABI from day 1, but I think it's
> > actually good enough. As long as this mode won't deadlock, it's not
> > *that* bad if programs are using it when they wanted "insecure".
>
> It's exactly that "as long as it won't deadlock" that is our current problem.
>
> It *does* deadlock.
>
> So it can't mean "blocking" in any long-term meaning.
>
> It can mean "blocks for up to 15 seconds" or something like that. I'd
> honestly prefer a smaller number, but I think 15 seconds is an
> acceptable "your user space is buggy, but we won't make you think the
> machine hung".
To be clear, when I say "blocking", I mean "blocks until we're ready,
but we make sure we're ready in a moderately timely manner".
Rather than answering everything point by point, here's a updated
mini-proposal and some thoughts. There are two families of security
people that I think we care about. One is the FIPS or CC or PCI
crowd, and they might, quite reasonably, demand actual hardware RNGs.
We should make the hwrng API stop sucking and they should be happy.
(This means expose an hwrng device node per physical device, IMO.)
The other is the one who wants getrandom(), etc to be convincingly
secure and is willing to do some actual analysis. And I think we can
make them quite happy like this:
In the kernel, we have two types of requests for random numbers: a
request for "secure" bytes and a request for "insecure" bytes.
Requests for "secure" bytes can block or return -EAGAIN. Requests for
"insecure" bytes succeed without waiting. In addition, we have a
jitter entropy mechanism (maybe the one mjg59 referenced, maybe
Alexander's -- doesn't really matter) and we *guarantee* that jitter
entropy, by itself, is enough to get the "secure" generator working
after, say, 5s of effort. By this, I mean that, on an idle system, it
finishes in 5s and, on a fully loaded system, it's allowed to take a
little while longer but not too much longer.
In other words, I want GRND_SECURE_BLOCKING and /dev/random reads to
genuinely always work and to genuinely never take much longer than 5s.
I don't want a special case where they fail.
The exposed user APIs are, subject to bikeshedding that can happen
later over the actual values, etc:
GRND_SECURE_BLOCKING: returns "secure" output and blocks until it's
ready. This never fails, but it also never blocks forever.
GRND_SECURE_NONBLOCKING: same but returns -EAGAIN instead of blocking.
GRND_INSECURE: returns "insecure" output immediately. I think we do
need this -- the "secure" mode may take a little while at early boot,
and libraries that initialize themselves with some randomness really
do want a way to get some numbers without any delay whatsoever.
0: either the same as GRND_SECURE_BLOCKING plus a warning or the
"accelerated" version. The "accelerated" version means wait up to 2s
for secure numbers and, if there still aren't any, fall back to
"insecure".
GRND_RANDOM: either the same as 0 or the same as GRND_SECURE_BLOCKING
but with a warning. I don't particularly care either way.
I'm okay with a well-defined semantic like I proposed for an
accelerated mode. I don't really want to try to define what a
secure-but-not-as-secure mode means as a separate complication that
the underlying RNG needs to support forever. I don't think the
security folks would like that either.
How does this sound?
^ permalink raw reply
* Re: [PATCH RFC v4 1/1] random: WARN on large getrandom() waits and introduce getrandom2()
From: Matthew Garrett @ 2019-09-20 20:17 UTC (permalink / raw)
To: Linus Torvalds
Cc: Andy Lutomirski, Ahmed S. Darwish, Lennart Poettering,
Theodore Y. Ts'o, Eric W. Biederman, Alexander E. Patrakov,
Michael Kerrisk, Willy Tarreau, lkml, Ext4 Developers List,
Linux API, linux-man
In-Reply-To: <CAHk-=whJ3kmcZp=Ws+uXnRB9KokG6nXSQCSuBnerG--jkAfP5w@mail.gmail.com>
On Fri, Sep 20, 2019 at 12:51:12PM -0700, Linus Torvalds wrote:
> So we absolutely _will_ come up with some way 0 ends the wait. Whether
> it's _just_ a timeout, or whether it's jitter-entropy or whatever, it
> will happen.
FWIW, Zircon uses the jitter entropy generator to seed the CRNG and
documented their findings in
https://fuchsia.dev/fuchsia-src/zircon/jitterentropy/config-basic .
--
Matthew Garrett | mjg59@srcf.ucam.org
^ permalink raw reply
* Re: [PATCH RFC v4 1/1] random: WARN on large getrandom() waits and introduce getrandom2()
From: Alexander E. Patrakov @ 2019-09-20 20:11 UTC (permalink / raw)
To: Linus Torvalds, Andy Lutomirski
Cc: Ahmed S. Darwish, Lennart Poettering, Theodore Y. Ts'o,
Eric W. Biederman, Michael Kerrisk, Willy Tarreau,
Matthew Garrett, lkml, Ext4 Developers List, Linux API, linux-man
In-Reply-To: <CAHk-=whJ3kmcZp=Ws+uXnRB9KokG6nXSQCSuBnerG--jkAfP5w@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1459 bytes --]
21.09.2019 00:51, Linus Torvalds пишет:
> And we'll also have to make getrandom(0) be really _timely_. Security
> people would likely rather wait for minutes before they are happy with
> it. But because it's a boot constraint as things are now, it will not
> just be jitter-entropy, it will be _accelerated_ jitter-entropy in 15
> seconds or whatever, and since it can't use up all of CPU time, it's
> realistically more like "15 second timeout, but less of actual CPU
> time for jitter".
I don't think that "accelerated jitter" makes sense. The jitterentropy
hwrng that I sent earlier fills the entropy buffer in less than 2
seconds, even with quality=4, so there is no need to accelerate it even
more.
> That said, if we can all convince everybody (hah!) that jitter entropy
> in the kernel would be sufficient, then we can make the whole point
> entirely moot, and just say "we'll just change crng_wait() to do
> jitter entropy instead and be done with it. Then any getrandom() user
> will just basically wait for a (very limited) time and the system will
> be happy.
>
> If that is the case we wouldn't need new flags at all. But I don't
> think you can make everybody agree to that, which is why I suspect
> we'll need the new flag, and I'll just take the heat for saying "0 is
> now off limits, because it does this thing that a lot of people
> dislike".
I 100% agree with that.
--
Alexander E. Patrakov
[-- Attachment #2: Криптографическая подпись S/MIME --]
[-- Type: application/pkcs7-signature, Size: 4052 bytes --]
^ permalink raw reply
* Re: [PATCH RFC v4 1/1] random: WARN on large getrandom() waits and introduce getrandom2()
From: Linus Torvalds @ 2019-09-20 20:02 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Willy Tarreau, Ahmed S. Darwish, Lennart Poettering,
Theodore Y. Ts'o, Eric W. Biederman, Alexander E. Patrakov,
Michael Kerrisk, Matthew Garrett, lkml, Ext4 Developers List,
Linux API, linux-man
In-Reply-To: <CALCETrW_mw0qOR2oqYC0+T6V65c+t+Vdxk5Jb6S+sPTqN6SXfw@mail.gmail.com>
On Fri, Sep 20, 2019 at 12:22 PM Andy Lutomirski <luto@kernel.org> wrote:
>
> Here are some possible approaches that come to mind:
>
> int count;
> while (crng isn't inited) {
> msleep(1);
> }
>
> and modify add_timer_randomness() to at least credit a tiny bit to
> crng_init_cnt.
I'd love that, but we don't actually call add_timer_randomness() for timers.
Yeah, the name is misleading.
What the "timer" in add_timer_randomness() means is that we look at
the timing between calls. And we may actually have (long ago) called
it for timer interrupts. But we don't any more.
The only actual users of add_timer_randomness() is
add_input_randomness() and add_disk_randomness(). And it turns out
that even disk IO doesn't really call add_disk_randomness(), so the
only _real_ user is that keyboard input thing.
Which means that unless you sit at the machine and type things in,
add_timer_randomness() _never_ gets called.
No, the real source of entropy right now is
add_interrupt_randomness(), which is called for all device interrupts.
But note the "device interrupts" part. Not the timer interrupt. That's
special, and has its own low-level architecture rules. So only the
normal IO interrupts (like disk/network/etc).
So timers right now do not add _anything_ to the randomness pool. Not
noise, not entropy.
But yes, what you can do is a jitter entropy thing, which basically
does what you suggest, except instead of "msleep(1)" it does something
like
while (crng isn't inited) {
sched_yield();
do_a_round_of_memory_accesses_etc();
add_cycle_counter_entropy();
}
and with a lot of handwaving you'll convince a certain amount of
people that yes, the timing of the above is unpredictable enough that
the entropy you add is real.
Linus
^ permalink raw reply
* Re: [PATCH RFC v4 1/1] random: WARN on large getrandom() waits and introduce getrandom2()
From: Andy Lutomirski @ 2019-09-20 19:52 UTC (permalink / raw)
To: Willy Tarreau
Cc: Andy Lutomirski, Linus Torvalds, Ahmed S. Darwish,
Lennart Poettering, Theodore Y. Ts'o, Eric W. Biederman,
Alexander E. Patrakov, Michael Kerrisk, Matthew Garrett, lkml,
Ext4 Developers List, Linux API, linux-man
In-Reply-To: <20190920193740.GD1889@1wt.eu>
> On Sep 20, 2019, at 12:37 PM, Willy Tarreau <w@1wt.eu> wrote:
>
> On Fri, Sep 20, 2019 at 12:22:17PM -0700, Andy Lutomirski wrote:
>> Perhaps userland could register a helper that takes over and does
>> something better?
>
> If userland sees the failure it can do whatever the developer/distro
> packager thought suitable for the system facing this condition.
>
>> But I think the kernel really should do something
>> vaguely reasonable all by itself.
>
> Definitely, that's what Linus' proposal was doing. Sleeping for some time
> is what I call "vaguely reasonable".
I don’t buy it. We have existing programs that can deadlock on boot. Just throwing -EAGAIN at them in a syscall that didn’t previously block does not strike me as reasonable.
>
>> If nothing else, we want the ext4
>> patch that provoked this whole discussion to be applied,
>
> Oh absolutely!
>
>> which means
>> that we need to unbreak userspace somehow, and returning garbage it to
>> is not a good choice.
>
> It depends how it's used. I'd claim that we certainly use randoms for
> other things (such as ASLR/hashtables) *before* using them to generate
> long lived keys thus we can have a bit more time to get some more
> entropy before reaching the point of producing these keys.
The problem is that we don’t know what userspace is doing with the output from getrandom(..., 0), so I think we have to be conservative. New kernels need to work with old user code. It’s okay if they’re slower to boot than they could be.
>
>> Here are some possible approaches that come to mind:
>>
>> int count;
>> while (crng isn't inited) {
>> msleep(1);
>> }
>>
>> and modify add_timer_randomness() to at least credit a tiny bit to
>> crng_init_cnt.
>
> Without a timeout it's sure we'll still face some situations where
> it blocks forever, which is the current problem.
The point is that we keep the timer running by looping like this, which should cause add_timer_randomness() to get called continuously, which should prevent the deadlock. I assume the deadlock is because we go into nohz-idle and we sit there with nothing happening at all.
>
>> Or we do something like intentionally triggering readahead on some
>> offset on the root block device.
>
> You don't necessarily have such a device, especially when you're
> in an initramfs. It's precisely where userland can be smarter. When
> the caller is sfdisk for example, it does have more chances to try
> to perform I/O than when it's a tiny http server starting to present
> a configuration page.
What I mean is: allow user code to register a usermode helper that helps get entropy. Or just convince distros to bundle some useful daemon that starts at early boot and lives in the initramfs.
^ permalink raw reply
* Re: [PATCH RFC v4 1/1] random: WARN on large getrandom() waits and introduce getrandom2()
From: Linus Torvalds @ 2019-09-20 19:51 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Ahmed S. Darwish, Lennart Poettering, Theodore Y. Ts'o,
Eric W. Biederman, Alexander E. Patrakov, Michael Kerrisk,
Willy Tarreau, Matthew Garrett, lkml, Ext4 Developers List,
Linux API, linux-man
In-Reply-To: <CALCETrUEqjFmPvpcJQwJe3dNbz8eaJ4k3_AV2u0v96MffjLn+g@mail.gmail.com>
On Fri, Sep 20, 2019 at 12:12 PM Andy Lutomirski <luto@kernel.org> wrote:
>
> The problem is that new programs will have to try the new flag value
> and, if it returns -EINVAL, fall back to 0. This isn't so great.
Don't be silly.
Of course they will do that, but so what? With a new kernel, they'll
get the behavior they expect. And with an old kernel, they'll get the
behavior they expect.
They'd never fall back to to "0 means something I didn't want",
exactly because we'd make this new flag be the first change.
> Wait, are you suggesting that 0 means invoke jitter-entropy or
> whatever and GRND_SECURE_BLOCKING means not wait forever and deadlock?
> That's no good -- people will want to continue using 0 because the
> behavior is better.
I assume that "not wait forever" was meant to be "wait forever".
So the one thing we have to do is break the "0 waits forever". I
guarantee that will happen. I will override Ted if he just NAk's it,
because we simply _cannot_ continue with it.
So we absolutely _will_ come up with some way 0 ends the wait. Whether
it's _just_ a timeout, or whether it's jitter-entropy or whatever, it
will happen.
But we'll also make getrandom(0) do the annoying warning, because it's
just ambiguous. And I suspect you'll find that a lot of security
people don't really like jitter-entropy, at least not in whatever
cut-down format we'll likely have to use in the kernel.
And we'll also have to make getrandom(0) be really _timely_. Security
people would likely rather wait for minutes before they are happy with
it. But because it's a boot constraint as things are now, it will not
just be jitter-entropy, it will be _accelerated_ jitter-entropy in 15
seconds or whatever, and since it can't use up all of CPU time, it's
realistically more like "15 second timeout, but less of actual CPU
time for jitter".
We can try to be clever with a background thread and a lot of
yielding(), so that if the CPU is actually idle we'll get most of that
15 seconds for whatever jitter, but end result is that it's still
accelerated.
Do I believe we can do a good job in that kind of timeframe?
Absolutely. The whole point should be that it's still "good enough",
and as has been pointed out, that same jitter entropy that people are
worried about is just done in user space right now instead.
But do I believe that security people would prefer a non-accelerated
GRND_SECURE_BLOCKING? Yes I do. That doesn't mean that
GRND_SECURE_BLOCKING shouldn't use jitter entropy too, but it doesn't
need the same kind of "let's hurry this up because it might be during
early boot and block things".
That said, if we can all convince everybody (hah!) that jitter entropy
in the kernel would be sufficient, then we can make the whole point
entirely moot, and just say "we'll just change crng_wait() to do
jitter entropy instead and be done with it. Then any getrandom() user
will just basically wait for a (very limited) time and the system will
be happy.
If that is the case we wouldn't need new flags at all. But I don't
think you can make everybody agree to that, which is why I suspect
we'll need the new flag, and I'll just take the heat for saying "0 is
now off limits, because it does this thing that a lot of people
dislike".
> IMO this is confusing. The GRND_RANDOM flag was IMO a mistake and
> should just be retired. Let's enumerate useful cases and then give
> them sane values.
That's basically what I'm doing. I enumerate the new values.
But the enumerations have hidden meaning, because the actual bits do
matter. The GRND_EXPLICIT bit isn't supposed to be used by any user,
but it has the value it has because it makes old kernels return
-EINVAL.
But if people hate the bit names, we can just do an enum and be done with it:
enum grnd_flags {
GRND_NONBLOCK = 1,
GRND_RANDOM, // Don't use!
GRND_RANDOM_NONBLOCK, // Don't use
GRND_UNUSED,
GRND_INSECURE,
GRND_SECURE_BLOCKING,
GRND_SECURE_NONBLOCKING,
};
but the values now have a _hidden_ pattern (because we currently have
that "| GRND_NONBLOCK" pattern that I want to make sure still
continues to work, rather than give unexpected behavior in case
somebody continues to use it).
So the _only_ difference between the above and what I suggested is
that I made the bit pattern explicit rather than hidden in the value.
> And the only real question is how to map existing users to these
> semantics. I see two sensible choices:
>
> 1. 0 means "secure, blocking". I think this is not what we'd do if we
> could go back in time and chage the ABI from day 1, but I think it's
> actually good enough. As long as this mode won't deadlock, it's not
> *that* bad if programs are using it when they wanted "insecure".
It's exactly that "as long as it won't deadlock" that is our current problem.
It *does* deadlock.
So it can't mean "blocking" in any long-term meaning.
It can mean "blocks for up to 15 seconds" or something like that. I'd
honestly prefer a smaller number, but I think 15 seconds is an
acceptable "your user space is buggy, but we won't make you think the
machine hung".
> 2. 0 means "secure, blocking, but warn". Some new value means
> "secure, blocking, don't warn". The problem is that new applications
> will have to fall back to 0 to continue supporting old kernels.
The same comment about blocking.
Maybe you came in in the middle, and didn't see the whole "reduced IO
patterns means that boot blocks forever" part of the original problem.
THAT is why 0 will absolutely change behaviour.
Linus
^ permalink raw reply
* Re: [PATCH RFC v4 1/1] random: WARN on large getrandom() waits and introduce getrandom2()
From: Willy Tarreau @ 2019-09-20 19:37 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Linus Torvalds, Ahmed S. Darwish, Lennart Poettering,
Theodore Y. Ts'o, Eric W. Biederman, Alexander E. Patrakov,
Michael Kerrisk, Matthew Garrett, lkml, Ext4 Developers List,
Linux API, linux-man
In-Reply-To: <CALCETrW_mw0qOR2oqYC0+T6V65c+t+Vdxk5Jb6S+sPTqN6SXfw@mail.gmail.com>
On Fri, Sep 20, 2019 at 12:22:17PM -0700, Andy Lutomirski wrote:
> Perhaps userland could register a helper that takes over and does
> something better?
If userland sees the failure it can do whatever the developer/distro
packager thought suitable for the system facing this condition.
> But I think the kernel really should do something
> vaguely reasonable all by itself.
Definitely, that's what Linus' proposal was doing. Sleeping for some time
is what I call "vaguely reasonable".
> If nothing else, we want the ext4
> patch that provoked this whole discussion to be applied,
Oh absolutely!
> which means
> that we need to unbreak userspace somehow, and returning garbage it to
> is not a good choice.
It depends how it's used. I'd claim that we certainly use randoms for
other things (such as ASLR/hashtables) *before* using them to generate
long lived keys thus we can have a bit more time to get some more
entropy before reaching the point of producing these keys.
> Here are some possible approaches that come to mind:
>
> int count;
> while (crng isn't inited) {
> msleep(1);
> }
>
> and modify add_timer_randomness() to at least credit a tiny bit to
> crng_init_cnt.
Without a timeout it's sure we'll still face some situations where
it blocks forever, which is the current problem.
> Or we do something like intentionally triggering readahead on some
> offset on the root block device.
You don't necessarily have such a device, especially when you're
in an initramfs. It's precisely where userland can be smarter. When
the caller is sfdisk for example, it does have more chances to try
to perform I/O than when it's a tiny http server starting to present
a configuration page.
> We should definitely not trigger *blocking* IO.
I think I agree.
> Also, I wonder if the real problem preventing the RNG from staring up
> is that the crng_init_cnt threshold is too high. We have a rather
> baroque accounting system, and it seems like we can accumulate and
> credit entropy for a very long time indeed without actually
> considering ourselves done.
I have no opinion on this, lacking the skills to evaluate the situation.
What I can say for sure is that I've faced the non-booting issue quite a
number of times on headless systems, and conversely in the 2.4 era, my
front reverse-proxy by then had the same SSH key as 89 other machines on
the net. So there's surely a sweet spot to find between those two extremes.
I tend to think that waiting *a little bit* for the *first* random is
acceptable, even 10-15s, by the time the user starts to think about
pressing the reset button the system might finish to boot. Hashing some
RAM locations and the RTC when present can also help a little bit. If
at least my machine by then had combined the RTC's date and time with
the hash, chances for a key collision would have gone down to one over
many thousands.
Willy
^ permalink raw reply
* Re: [PATCH RFC v4 1/1] random: WARN on large getrandom() waits and introduce getrandom2()
From: Andy Lutomirski @ 2019-09-20 19:22 UTC (permalink / raw)
To: Willy Tarreau
Cc: Andy Lutomirski, Linus Torvalds, Ahmed S. Darwish,
Lennart Poettering, Theodore Y. Ts'o, Eric W. Biederman,
Alexander E. Patrakov, Michael Kerrisk, Matthew Garrett, lkml,
Ext4 Developers List, Linux API, linux-man
In-Reply-To: <20190920181216.GA1889@1wt.eu>
On Fri, Sep 20, 2019 at 11:12 AM Willy Tarreau <w@1wt.eu> wrote:
>
> Hi Andy,
>
> On Fri, Sep 20, 2019 at 10:52:30AM -0700, Andy Lutomirski wrote:
> > 2. Fix what is arguably a straight up kernel bug, not even an ABI
> > issue: when a user program is blocking in getrandom(..., 0), the
> > kernel happily sits there doing absolutely nothing and deadlocks the
> > system as a result. This IMO isn't an ABI issue -- it's an
> > implementation problem. How about we make getrandom() (probably
> > actually wait_for_random_bytes()) do something useful to try to seed
> > the RNG if the system is otherwise not doing IO.
>
> I thought about it as well with my old MSDOS reflexes, but here I
> doubt we can do a lot. It seems fishy to me to start to fiddle with
> various drivers from within a getrandom() syscall, we could sometimes
> even end up waiting even longer because one device is already locked,
> and when we have access there there's not much we can do without
> risking to cause some harm. On desktop systems you have a bit more
> choice than on headless systems (blink keyboard leds and time the
> interrupts, run some disk accesses when there's still a disk, get a
> copy of the last buffer of the audio input and/or output, turn on
> the microphone and/or webcam, and collect some data). Many of them
> cannot always be used. We could do some more portable stuff like scan
> and hash the totality of the RAM. But that's all quite bad and
> unreliable and at this point it's better to tell userland "here's
> what I could get for you, if you want better, do it yourself" and the
> userland can then ask the user "dear user, I really need valid entropy
> this time to generate your GPG key, please type frantically on this
> keyboard". And it will be more reliable this way in my opinion.
Perhaps userland could register a helper that takes over and does
something better? But I think the kernel really should do something
vaguely reasonable all by itself. If nothing else, we want the ext4
patch that provoked this whole discussion to be applied, which means
that we need to unbreak userspace somehow, and returning garbage it to
is not a good choice.
Here are some possible approaches that come to mind:
int count;
while (crng isn't inited) {
msleep(1);
}
and modify add_timer_randomness() to at least credit a tiny bit to
crng_init_cnt.
Or we do something like intentionally triggering readahead on some
offset on the root block device. We should definitely not trigger
*blocking* IO.
Also, I wonder if the real problem preventing the RNG from staring up
is that the crng_init_cnt threshold is too high. We have a rather
baroque accounting system, and it seems like we can accumulate and
credit entropy for a very long time indeed without actually
considering ourselves done.
--Andy
^ permalink raw reply
* Re: [PATCH RFC v4 1/1] random: WARN on large getrandom() waits and introduce getrandom2()
From: Andy Lutomirski @ 2019-09-20 19:12 UTC (permalink / raw)
To: Linus Torvalds
Cc: Andy Lutomirski, Ahmed S. Darwish, Lennart Poettering,
Theodore Y. Ts'o, Eric W. Biederman, Alexander E. Patrakov,
Michael Kerrisk, Willy Tarreau, Matthew Garrett, lkml,
Ext4 Developers List, Linux API, linux-man
In-Reply-To: <CAHk-=wjpTWgpo6d24pTv+ubfea_uEomX-sHjjOkdACfV-8Nmkg@mail.gmail.com>
> On Sep 20, 2019, at 11:10 AM, Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
> On Fri, Sep 20, 2019 at 10:52 AM Andy Lutomirski <luto@kernel.org> wrote:
>>
>> IMO, from the beginning, we should have done this:
>>
>> GRND_INSECURE: insecure. always works.
>>
>> GRND_SECURE_BLOCKING: does exactly what it says.
>>
>> 0: -EINVAL.
>
> Violently agreed. And that's kind of what the GRND_EXPLICIT is really
> aiming for.
>
> However, it's worth noting that nobody should ever use GRND_EXPLICIT
> directly. That's just the name for the bit. The actual users would use
> GRND_INSECURE or GRND_SECURE.
>
> And yes, maybe it's worth making the name be GRND_SECURE_BLOCKING just
> to make people see what the big deal is.
>
> In the meantime, we need that new bit just to be able to create the
> new semantics eventually. With a warning to nudge people in the right
> direction.
>
> We may never be able to return -EINVAL, but we can add the pr_notice()
> to discourage people from using it.
>
The problem is that new programs will have to try the new flag value
and, if it returns -EINVAL, fall back to 0. This isn't so great.
> And yes, we'll have to block - at least for a time - to get some
> entropy. But at some point we either start making entropy up, or we
> say "0 means jitter-entropy for ten seconds".
>
> That will _work_, but it will also make the security-people nervous,
> which is just one more hint that they should move to
> GRND_SECURE[_BLOCKING].
Wait, are you suggesting that 0 means invoke jitter-entropy or
whatever and GRND_SECURE_BLOCKING means not wait forever and deadlock?
That's no good -- people will want to continue using 0 because the
behavior is better. My point here is that asking for secure random
numbers isn’t some legacy oddity — it’s genuinely necessary. The
kernel should do whatever it needs to in order to make it work. We
really don’t want a situation where 0 means get me secure random
numbers reliably but spam the logs and GRND_SECURE_BLOCKING means
don’t spam the logs but risk deadlocking. This will encourage people
to pass 0 to get the improved behavior.
> So GRND_EXPLICIT is a bit that basically means "I am explicit about
> what behavior I want". But part of that is that you need to _state_
> the behavior too.
>
> So:
>
> - GRND_INSECURE is (GRND_EXPLICIT | GRND_NONBLOCK)
>
> As in "I explicitly ask you not to just not ever block": urandom
IMO this is confusing. The GRND_RANDOM flag was IMO a mistake and
should just be retired. Let's enumerate useful cases and then give
them sane values.
>
> - GRND_SECURE_BLOCKING is (GRND_EXPLICIT | GRND_RANDOM)
>
> As in "I explicitly ask you for those secure random numbers"
>
> - GRND_SECURE_NONBLOCKING is (GRND_EXPLICIT | GRND_RANDOM | GRND_NONBLOCK)
>
> As in "I want explicitly secure random numbers, but return -EAGAIN
> if that would block".
>
> Which are the three sane behaviors (that last one is useful for the "I
> can try to generate entropy if you don't have any" case. I'm not sure
> anybody will do it, but it definitely conceptually makes sense).
>
> And I agree that your naming is better.
I think this is the complete list of "good" behaviors for new programs:
"insecure": always works, never warns.
"secure, blocking": always returns *eventually* with secure output,
i.e., does something to avoid deadlocks
"secure, nonblocking" returns secure output immediately or returns -EAGAIN.
And the only real question is how to map existing users to these
semantics. I see two sensible choices:
1. 0 means "secure, blocking". I think this is not what we'd do if we
could go back in time and chage the ABI from day 1, but I think it's
actually good enough. As long as this mode won't deadlock, it's not
*that* bad if programs are using it when they wanted "insecure".
2. 0 means "secure, blocking, but warn". Some new value means
"secure, blocking, don't warn". The problem is that new applications
will have to fall back to 0 to continue supporting old kernels.
I briefly thought that maybe GRND_RANDOM would be a reasonable choice
for "secure, blocking, don't warn", but the effect on new programs on
old kernels will be unfortunate.
I'm willing to go along with #2 if you like it better than #1, and
I'll update my patches accordingly, but I prefer #1.
I do think we should make all the ABI changes that we want to make all
in one release. Let's not make programs think about their behavior on
more versions than necessary. So I'd like to get rid of the current
/dev/random semantics, add "insecure" mode, and do whatever deadlock
avoidance scheme we settle on in a single release.
--Andy
^ permalink raw reply
* Re: [PATCH RFC v4 1/1] random: WARN on large getrandom() waits and introduce getrandom2()
From: Andy Lutomirski @ 2019-09-20 18:29 UTC (permalink / raw)
To: Alexander E. Patrakov
Cc: Andy Lutomirski, Linus Torvalds, Ahmed S. Darwish,
Lennart Poettering, Theodore Y. Ts'o, Eric W. Biederman,
Michael Kerrisk, Willy Tarreau, Matthew Garrett, lkml,
Ext4 Developers List, Linux API, linux-man
In-Reply-To: <78a4b774-ef6b-62cb-57db-8e1ff8d29f72@gmail.com>
> On Sep 20, 2019, at 11:15 AM, Alexander E. Patrakov <patrakov@gmail.com> wrote:
>
> 20.09.2019 22:52, Andy Lutomirski пишет:
>> I think that, given existing software, we should make two or three
>> changes to fix the basic problems here:
>> 1. Add GRND_INSECURE: at least let new applications do the right thing
>> going forward.
>> 2. Fix what is arguably a straight up kernel bug, not even an ABI
>> issue: when a user program is blocking in getrandom(..., 0), the
>> kernel happily sits there doing absolutely nothing and deadlocks the
>> system as a result. This IMO isn't an ABI issue -- it's an
>> implementation problem. How about we make getrandom() (probably
>> actually wait_for_random_bytes()) do something useful to try to seed
>> the RNG if the system is otherwise not doing IO.
>> 3. Optionally, entirely in user code: Get glibc to add new *library*
>> functions: getentropy_secure_blocking() and getentropy_insecure() or
>> whatever they want to call them. Deprecate getentropy().
>> I think #2 is critical. Right now, suppose someone has a system that
>> neets to do a secure network request (a la Red Hat's Clevis). I have
>> no idea what Clevis actually does, but it wouldn't be particularly
>> crazy to do a DH exchange or sign with an EC key to ask some network
>> server to help unlock a dm-crypt volume. If the system does this at
>> boot, it needs to use getrandom(..., 0), GRND_EXPLICIT, or whatever,
>> because it NEEDS a secure random number. No about of ABI fiddling
>> will change this. The kernel should *work* in this case rather than
>> deadlocking.
>
> Let me express a little bit of disagreement with the logic here.
>
> I do agree that #2 is critical, and the Clevis use case is a perfect example why it is important. I doubt that it is solvable without trusting jitter entropy, or without provoking a dummy read on a random block device, just for timings, or maybe some other interaction with the external world - but Willy already said "it seems fishy". However, _if_ it is solved, then we don't need GRND_INSECURE, because solving #2 is equivalent to magically making secure random numbers always available.
>
>
I beg to differ. There is a big difference between “do your best *right now*” and “give me a real secure result in a vaguely timely manner”.
For example, the former is useful for ASLR or hash table randomization. The latter is not.
^ permalink raw reply
* Re: [PATCH RFC v4 1/1] random: WARN on large getrandom() waits and introduce getrandom2()
From: Willy Tarreau @ 2019-09-20 18:16 UTC (permalink / raw)
To: Linus Torvalds
Cc: Andy Lutomirski, Ahmed S. Darwish, Lennart Poettering,
Theodore Y. Ts'o, Eric W. Biederman, Alexander E. Patrakov,
Michael Kerrisk, Matthew Garrett, lkml, Ext4 Developers List,
Linux API, linux-man
In-Reply-To: <CAHk-=wjpTWgpo6d24pTv+ubfea_uEomX-sHjjOkdACfV-8Nmkg@mail.gmail.com>
On Fri, Sep 20, 2019 at 11:09:53AM -0700, Linus Torvalds wrote:
(...)
> So:
>
> - GRND_INSECURE is (GRND_EXPLICIT | GRND_NONBLOCK)
>
> As in "I explicitly ask you not to just not ever block": urandom
>
> - GRND_SECURE_BLOCKING is (GRND_EXPLICIT | GRND_RANDOM)
>
> As in "I explicitly ask you for those secure random numbers"
>
> - GRND_SECURE_NONBLOCKING is (GRND_EXPLICIT | GRND_RANDOM | GRND_NONBLOCK)
>
> As in "I want explicitly secure random numbers, but return -EAGAIN
> if that would block".
>
> Which are the three sane behaviors (that last one is useful for the "I
> can try to generate entropy if you don't have any" case. I'm not sure
> anybody will do it, but it definitely conceptually makes sense).
>
> And I agree that your naming is better.
>
> I had it as just "GRND_SECURE" for the blocking version, and
> "GRND_SECURE | GRND_NONBLOCK" for the "secure but return EAGAIN if you
> would need to block for entropy" version.
>
> But explicitly stating the blockingness in the name makes it clearer
> to the people who just want GRND_INSECURE, and makes them realize that
> they don't want the blocking version.
I really like it this way. Explicit and full control for the application
plus reasonable backwards compatibility, it sounds pretty good.
Willy
^ permalink raw reply
* Re: [PATCH RFC v4 1/1] random: WARN on large getrandom() waits and introduce getrandom2()
From: Alexander E. Patrakov @ 2019-09-20 18:15 UTC (permalink / raw)
To: Andy Lutomirski, Linus Torvalds
Cc: Ahmed S. Darwish, Lennart Poettering, Theodore Y. Ts'o,
Eric W. Biederman, Michael Kerrisk, Willy Tarreau,
Matthew Garrett, lkml, Ext4 Developers List, Linux API, linux-man
In-Reply-To: <CALCETrV=4TX2a4uV5t2xOFzv+zM_jnOtMLJna8Vb7uXz6S=wSw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2133 bytes --]
20.09.2019 22:52, Andy Lutomirski пишет:
> I think that, given existing software, we should make two or three
> changes to fix the basic problems here:
>
> 1. Add GRND_INSECURE: at least let new applications do the right thing
> going forward.
>
> 2. Fix what is arguably a straight up kernel bug, not even an ABI
> issue: when a user program is blocking in getrandom(..., 0), the
> kernel happily sits there doing absolutely nothing and deadlocks the
> system as a result. This IMO isn't an ABI issue -- it's an
> implementation problem. How about we make getrandom() (probably
> actually wait_for_random_bytes()) do something useful to try to seed
> the RNG if the system is otherwise not doing IO.
>
> 3. Optionally, entirely in user code: Get glibc to add new *library*
> functions: getentropy_secure_blocking() and getentropy_insecure() or
> whatever they want to call them. Deprecate getentropy().
>
> I think #2 is critical. Right now, suppose someone has a system that
> neets to do a secure network request (a la Red Hat's Clevis). I have
> no idea what Clevis actually does, but it wouldn't be particularly
> crazy to do a DH exchange or sign with an EC key to ask some network
> server to help unlock a dm-crypt volume. If the system does this at
> boot, it needs to use getrandom(..., 0), GRND_EXPLICIT, or whatever,
> because it NEEDS a secure random number. No about of ABI fiddling
> will change this. The kernel should *work* in this case rather than
> deadlocking.
Let me express a little bit of disagreement with the logic here.
I do agree that #2 is critical, and the Clevis use case is a perfect
example why it is important. I doubt that it is solvable without
trusting jitter entropy, or without provoking a dummy read on a random
block device, just for timings, or maybe some other interaction with the
external world - but Willy already said "it seems fishy". However, _if_
it is solved, then we don't need GRND_INSECURE, because solving #2 is
equivalent to magically making secure random numbers always available.
--
Alexander E. Patrakov
[-- Attachment #2: Криптографическая подпись S/MIME --]
[-- Type: application/pkcs7-signature, Size: 4052 bytes --]
^ permalink raw reply
* Re: [PATCH RFC v4 1/1] random: WARN on large getrandom() waits and introduce getrandom2()
From: Willy Tarreau @ 2019-09-20 18:12 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Linus Torvalds, Ahmed S. Darwish, Lennart Poettering,
Theodore Y. Ts'o, Eric W. Biederman, Alexander E. Patrakov,
Michael Kerrisk, Matthew Garrett, lkml, Ext4 Developers List,
Linux API, linux-man
In-Reply-To: <CALCETrV=4TX2a4uV5t2xOFzv+zM_jnOtMLJna8Vb7uXz6S=wSw@mail.gmail.com>
Hi Andy,
On Fri, Sep 20, 2019 at 10:52:30AM -0700, Andy Lutomirski wrote:
> 2. Fix what is arguably a straight up kernel bug, not even an ABI
> issue: when a user program is blocking in getrandom(..., 0), the
> kernel happily sits there doing absolutely nothing and deadlocks the
> system as a result. This IMO isn't an ABI issue -- it's an
> implementation problem. How about we make getrandom() (probably
> actually wait_for_random_bytes()) do something useful to try to seed
> the RNG if the system is otherwise not doing IO.
I thought about it as well with my old MSDOS reflexes, but here I
doubt we can do a lot. It seems fishy to me to start to fiddle with
various drivers from within a getrandom() syscall, we could sometimes
even end up waiting even longer because one device is already locked,
and when we have access there there's not much we can do without
risking to cause some harm. On desktop systems you have a bit more
choice than on headless systems (blink keyboard leds and time the
interrupts, run some disk accesses when there's still a disk, get a
copy of the last buffer of the audio input and/or output, turn on
the microphone and/or webcam, and collect some data). Many of them
cannot always be used. We could do some more portable stuff like scan
and hash the totality of the RAM. But that's all quite bad and
unreliable and at this point it's better to tell userland "here's
what I could get for you, if you want better, do it yourself" and the
userland can then ask the user "dear user, I really need valid entropy
this time to generate your GPG key, please type frantically on this
keyboard". And it will be more reliable this way in my opinion.
My analysis of the problem precisely lies in the fact that we've
always considered that the kernel had to provide randoms for any
use case and had to cover the most difficult cases and imposed
their constraints on simplest ones. Better let the application
decide.
Willy
^ permalink raw reply
* Re: [PATCH RFC v4 1/1] random: WARN on large getrandom() waits and introduce getrandom2()
From: Linus Torvalds @ 2019-09-20 18:09 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Ahmed S. Darwish, Lennart Poettering, Theodore Y. Ts'o,
Eric W. Biederman, Alexander E. Patrakov, Michael Kerrisk,
Willy Tarreau, Matthew Garrett, lkml, Ext4 Developers List,
Linux API, linux-man
In-Reply-To: <CALCETrV=4TX2a4uV5t2xOFzv+zM_jnOtMLJna8Vb7uXz6S=wSw@mail.gmail.com>
On Fri, Sep 20, 2019 at 10:52 AM Andy Lutomirski <luto@kernel.org> wrote:
>
> IMO, from the beginning, we should have done this:
>
> GRND_INSECURE: insecure. always works.
>
> GRND_SECURE_BLOCKING: does exactly what it says.
>
> 0: -EINVAL.
Violently agreed. And that's kind of what the GRND_EXPLICIT is really
aiming for.
However, it's worth noting that nobody should ever use GRND_EXPLICIT
directly. That's just the name for the bit. The actual users would use
GRND_INSECURE or GRND_SECURE.
And yes, maybe it's worth making the name be GRND_SECURE_BLOCKING just
to make people see what the big deal is.
In the meantime, we need that new bit just to be able to create the
new semantics eventually. With a warning to nudge people in the right
direction.
We may never be able to return -EINVAL, but we can add the pr_notice()
to discourage people from using it.
And yes, we'll have to block - at least for a time - to get some
entropy. But at some point we either start making entropy up, or we
say "0 means jitter-entropy for ten seconds".
That will _work_, but it will also make the security-people nervous,
which is just one more hint that they should move to
GRND_SECURE[_BLOCKING].
> getrandom(..., GRND_EXPLICIT): just fscking give me a number. it
> seems to work and it shuts up the warning
>
> And we're back to square one.
Actually, you didn't read the GRND_INSECURE patch, did you.
getrandom(GRND_EXPLICIT) on its own returns -EINVAL.
Because yes, I thought about it, and yes, I agree that it's the same
as the old 0.
So GRND_EXPLICIT is a bit that basically means "I am explicit about
what behavior I want". But part of that is that you need to _state_
the behavior too.
So:
- GRND_INSECURE is (GRND_EXPLICIT | GRND_NONBLOCK)
As in "I explicitly ask you not to just not ever block": urandom
- GRND_SECURE_BLOCKING is (GRND_EXPLICIT | GRND_RANDOM)
As in "I explicitly ask you for those secure random numbers"
- GRND_SECURE_NONBLOCKING is (GRND_EXPLICIT | GRND_RANDOM | GRND_NONBLOCK)
As in "I want explicitly secure random numbers, but return -EAGAIN
if that would block".
Which are the three sane behaviors (that last one is useful for the "I
can try to generate entropy if you don't have any" case. I'm not sure
anybody will do it, but it definitely conceptually makes sense).
And I agree that your naming is better.
I had it as just "GRND_SECURE" for the blocking version, and
"GRND_SECURE | GRND_NONBLOCK" for the "secure but return EAGAIN if you
would need to block for entropy" version.
But explicitly stating the blockingness in the name makes it clearer
to the people who just want GRND_INSECURE, and makes them realize that
they don't want the blocking version.
Linus
^ permalink raw reply
* Re: [PATCH RFC v4 1/1] random: WARN on large getrandom() waits and introduce getrandom2()
From: Ahmed S. Darwish @ 2019-09-20 17:56 UTC (permalink / raw)
To: Willy Tarreau
Cc: Linus Torvalds, Lennart Poettering, Theodore Y. Ts'o,
Eric W. Biederman, Alexander E. Patrakov, Michael Kerrisk,
Matthew Garrett, lkml, linux-ext4, linux-api, linux-man
In-Reply-To: <20190920172609.GA1832@1wt.eu>
On Fri, Sep 20, 2019 at 07:26:09PM +0200, Willy Tarreau wrote:
> Hi Ahmed,
>
> On Fri, Sep 20, 2019 at 03:46:09PM +0200, Ahmed S. Darwish wrote:
> > Problem is, glibc is still *really* slow in adopting linux syscall
> > wrappers, so I'm not optimistic about that...
> >
> > I still see the new system call as the sanest path, even provided
> > the cost of a new syscall number..
>
> New syscalls are always a pain to deal with in userland, because when
> they are introduced, everyone wants them long before they're available
> in glibc. So userland has to define NR_xxx for each supported arch and
> to perform the call itself.
>
> With flags adoption is instantaneous. Just #ifndef/#define, check if
> the flag is supported and that's done. The only valid reason for a new
> syscall is when the API changes (e.g. one extra arg, a la accept4()),
> which doesn't seem to be the case here. Otherwise please by all means
> avoid this in general.
>
I see. Thanks a lot for the explanation above :)
--
Ahmed Darwish
^ permalink raw reply
* Re: [PATCH RFC v4 1/1] random: WARN on large getrandom() waits and introduce getrandom2()
From: Andy Lutomirski @ 2019-09-20 17:52 UTC (permalink / raw)
To: Linus Torvalds
Cc: Andy Lutomirski, Ahmed S. Darwish, Lennart Poettering,
Theodore Y. Ts'o, Eric W. Biederman, Alexander E. Patrakov,
Michael Kerrisk, Willy Tarreau, Matthew Garrett, lkml,
Ext4 Developers List, Linux API, linux-man
In-Reply-To: <CAHk-=wgW8rN2EVL_Rdn63V9vQO0GkZ=RQFeqqsYJM==8fujpPg@mail.gmail.com>
On Fri, Sep 20, 2019 at 9:30 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Fri, Sep 20, 2019 at 7:34 AM Andy Lutomirski <luto@kernel.org> wrote:
> >
> > What is this GRND_EXPLICIT thing?
>
> Your own email gives the explanation:
>
> > Linus, I disagree that blocking while waiting for randomness is an
> > error. Sometimes you want to generate a key
>
> That's *exactly* why GRND_EXPLICIT needs to be done regardless.
>
> The keyword there is "Sometimes".
>
> But people currently use "getrandom(0)" when they DO NOT want a key,
> they just want some miscellaneous random numbers for some totally
> non-security-related reason.
>
> And that will continue. Exactly because the people who do not want a
> key by definition aren't thinking about it very hard.
I fully agree that this is a problem. It's a problem we brought on
ourselves because we screwed up the ABI from the beginning. The
question is what to do about it that doesn't cause its own set of
nasty problems.
> So GRND_EXPLICIT is there very much to make sure people who want true
> secure keys will say so, and five years from now we will not have the
> confusion between "Oh, I wasn't thinking about bootup". Because at a
> minimum, in the near future getrandom(0) will warn about the
> ambiguity. Or it will use some questionable jitter entropy that some
> real key users will look at sideways and go "I don't want that".
There are programs that call getrandom(0) *today* that expect secure
output. openssl does a horrible dance in which it calls getentropy()
if available and falls back to syscall(__NR_getrandom, buf, buflen, 0)
otherwise. We can't break this use case. Changing the semantics of
getrandom(0) out from under them seems like the worst kind of ABI
break -- existing applications will *appear* to continue working but
will, in fact, become insecure.
IMO, from the beginning, we should have done this:
GRND_INSECURE: insecure. always works.
GRND_SECURE_BLOCKING: does exactly what it says.
0: -EINVAL.
Using it correctly would be obvious. Something like GRND_EXPLICIT
would be a head-scratcher: people would have to look at the man page
and actually think about it, and it's still easy to get wrong:
getrandom(..., GRND_EXPLICIT): just fscking give me a number. it
seems to work and it shuts up the warning
And we're back to square one.
I think that, given existing software, we should make two or three
changes to fix the basic problems here:
1. Add GRND_INSECURE: at least let new applications do the right thing
going forward.
2. Fix what is arguably a straight up kernel bug, not even an ABI
issue: when a user program is blocking in getrandom(..., 0), the
kernel happily sits there doing absolutely nothing and deadlocks the
system as a result. This IMO isn't an ABI issue -- it's an
implementation problem. How about we make getrandom() (probably
actually wait_for_random_bytes()) do something useful to try to seed
the RNG if the system is otherwise not doing IO.
3. Optionally, entirely in user code: Get glibc to add new *library*
functions: getentropy_secure_blocking() and getentropy_insecure() or
whatever they want to call them. Deprecate getentropy().
I think #2 is critical. Right now, suppose someone has a system that
neets to do a secure network request (a la Red Hat's Clevis). I have
no idea what Clevis actually does, but it wouldn't be particularly
crazy to do a DH exchange or sign with an EC key to ask some network
server to help unlock a dm-crypt volume. If the system does this at
boot, it needs to use getrandom(..., 0), GRND_EXPLICIT, or whatever,
because it NEEDS a secure random number. No about of ABI fiddling
will change this. The kernel should *work* in this case rather than
deadlocking.
--Andy
^ permalink raw reply
* Re: [PATCH RFC v4 1/1] random: WARN on large getrandom() waits and introduce getrandom2()
From: Willy Tarreau @ 2019-09-20 17:26 UTC (permalink / raw)
To: Ahmed S. Darwish
Cc: Linus Torvalds, Lennart Poettering, Theodore Y. Ts'o,
Eric W. Biederman, Alexander E. Patrakov, Michael Kerrisk,
Matthew Garrett, lkml, linux-ext4, linux-api, linux-man
In-Reply-To: <20190920134609.GA2113@pc>
Hi Ahmed,
On Fri, Sep 20, 2019 at 03:46:09PM +0200, Ahmed S. Darwish wrote:
> Problem is, glibc is still *really* slow in adopting linux syscall
> wrappers, so I'm not optimistic about that...
>
> I still see the new system call as the sanest path, even provided
> the cost of a new syscall number..
New syscalls are always a pain to deal with in userland, because when
they are introduced, everyone wants them long before they're available
in glibc. So userland has to define NR_xxx for each supported arch and
to perform the call itself.
With flags adoption is instantaneous. Just #ifndef/#define, check if
the flag is supported and that's done. The only valid reason for a new
syscall is when the API changes (e.g. one extra arg, a la accept4()),
which doesn't seem to be the case here. Otherwise please by all means
avoid this in general.
Thanks,
Willy
^ permalink raw reply
* Re: [PATCH RFC v4 1/1] random: WARN on large getrandom() waits and introduce getrandom2()
From: Linus Torvalds @ 2019-09-20 16:29 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Ahmed S. Darwish, Lennart Poettering, Theodore Y. Ts'o,
Eric W. Biederman, Alexander E. Patrakov, Michael Kerrisk,
Willy Tarreau, Matthew Garrett, lkml, Ext4 Developers List,
Linux API, linux-man
In-Reply-To: <CALCETrWvE5es3i+to33y6jw=Yf0Tw6ZfV-6QWjZT5v0fo76tWw@mail.gmail.com>
On Fri, Sep 20, 2019 at 7:34 AM Andy Lutomirski <luto@kernel.org> wrote:
>
> What is this GRND_EXPLICIT thing?
Your own email gives the explanation:
> Linus, I disagree that blocking while waiting for randomness is an
> error. Sometimes you want to generate a key
That's *exactly* why GRND_EXPLICIT needs to be done regardless.
The keyword there is "Sometimes".
But people currently use "getrandom(0)" when they DO NOT want a key,
they just want some miscellaneous random numbers for some totally
non-security-related reason.
And that will continue. Exactly because the people who do not want a
key by definition aren't thinking about it very hard.
So the interface was very much mis-designed from the get-go. It was
designed purely for key people, even though generating keys is by no
means the most common reason for wanting a block of "random" numbers.
So GRND_EXPLICIT is there very much to make sure people who want true
secure keys will say so, and five years from now we will not have the
confusion between "Oh, I wasn't thinking about bootup". Because at a
minimum, in the near future getrandom(0) will warn about the
ambiguity. Or it will use some questionable jitter entropy that some
real key users will look at sideways and go "I don't want that".
This is an ABI design issue. The old ABI was fundamentally misdesigned
and actively encouraged the current situation of mixing secure and
insecure callers for that getrandom(0).
And it's entirely orthogonal to _any_ actual technical change we will
do (like removing the old GRND_RANDOM behavior entirely, which is
insane for other reasons and nobody ever wanted or likely used).
Linus
^ permalink raw reply
* Re: [RFC PATCH 2/3] fs: add RWF_ENCODED for writing compressed data
From: Jens Axboe @ 2019-09-20 16:25 UTC (permalink / raw)
To: Jann Horn, Omar Sandoval
Cc: linux-fsdevel, linux-btrfs, Dave Chinner, Linux API, Kernel Team,
Andy Lutomirski
In-Reply-To: <CAG48ez2GKv15Uj6Wzv0sG5v2bXyrSaCtRTw5Ok_ovja_CiO_fQ@mail.gmail.com>
On 9/19/19 9:44 AM, Jann Horn wrote:
> On Thu, Sep 19, 2019 at 8:54 AM Omar Sandoval <osandov@osandov.com> wrote:
>> Btrfs can transparently compress data written by the user. However, we'd
>> like to add an interface to write pre-compressed data directly to the
>> filesystem. This adds support for so-called "encoded writes" via
>> pwritev2().
>>
>> A new RWF_ENCODED flags indicates that a write is "encoded". If this
>> flag is set, iov[0].iov_base points to a struct encoded_iov which
>> contains metadata about the write: namely, the compression algorithm and
>> the unencoded (i.e., decompressed) length of the extent. iov[0].iov_len
>> must be set to sizeof(struct encoded_iov), which can be used to extend
>> the interface in the future. The remaining iovecs contain the encoded
>> extent.
>>
>> A similar interface for reading encoded data can be added to preadv2()
>> in the future.
>>
>> Filesystems must indicate that they support encoded writes by setting
>> FMODE_ENCODED_IO in ->file_open().
> [...]
>> +int import_encoded_write(struct kiocb *iocb, struct encoded_iov *encoded,
>> + struct iov_iter *from)
>> +{
>> + if (iov_iter_single_seg_count(from) != sizeof(*encoded))
>> + return -EINVAL;
>> + if (copy_from_iter(encoded, sizeof(*encoded), from) != sizeof(*encoded))
>> + return -EFAULT;
>> + if (encoded->compression == ENCODED_IOV_COMPRESSION_NONE &&
>> + encoded->encryption == ENCODED_IOV_ENCRYPTION_NONE) {
>> + iocb->ki_flags &= ~IOCB_ENCODED;
>> + return 0;
>> + }
>> + if (encoded->compression > ENCODED_IOV_COMPRESSION_TYPES ||
>> + encoded->encryption > ENCODED_IOV_ENCRYPTION_TYPES)
>> + return -EINVAL;
>> + if (!capable(CAP_SYS_ADMIN))
>> + return -EPERM;
>
> How does this capable() check interact with io_uring? Without having
> looked at this in detail, I suspect that when an encoded write is
> requested through io_uring, the capable() check might be executed on
> something like a workqueue worker thread, which is probably running
> with a full capability set.
If we can hit -EAGAIN before doing the import in io_uring, then yes,
this will probably bypass the check as it'll only happen from the
worker.
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH v2 0/7] Rework random blocking
From: Andy Lutomirski @ 2019-09-20 14:46 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Theodore Tso, LKML, Linux API, Kees Cook, Jason A. Donenfeld,
Ahmed S. Darwish, Lennart Poettering, Eric W. Biederman,
Alexander E. Patrakov, Michael Kerrisk, Willy Tarreau,
Matthew Garrett, Ext4 Developers List, linux-man
In-Reply-To: <cover.1568990048.git.luto@kernel.org>
On Fri, Sep 20, 2019 at 7:36 AM Andy Lutomirski <luto@kernel.org> wrote:
>
> This makes two major semantic changes to Linux's random APIs:
Hmm. I have at least two issues in here that need fixing:
- getrandom() can warn. Whoops.
- Repeatedly calling getentropy in GRND_INSECURE mode will interfere
will CRNG init.
v3 will fix these issues.
^ permalink raw reply
* Re: [RFC PATCH 1/3] fs: pass READ/WRITE to kiocb_set_rw_flags()
From: Jan Kara @ 2019-09-20 14:38 UTC (permalink / raw)
To: Omar Sandoval
Cc: linux-fsdevel, linux-btrfs, Dave Chinner, linux-api, kernel-team,
Jan Kara, Jens Axboe
In-Reply-To: <d23a40f0ad3fa0631fe6189b94811be473e7cc4a.1568875700.git.osandov@fb.com>
On Wed 18-09-19 23:53:44, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
>
> A following change will want to check whether an IO is a read or write
> in kiocb_set_rw_flags(). Additionally, aio and io_uring currently set
> the IOCB_WRITE flag on a kiocb right before calling call_write_iter(),
> but we can move that into the common code.
>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Jens Axboe <axboe@kernel.dk>
> Signed-off-by: Omar Sandoval <osandov@fb.com>
...
> index ffe35d97afcb..75c4b7680385 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -3351,8 +3351,11 @@ static inline int iocb_flags(struct file *file)
> return res;
> }
>
> -static inline int kiocb_set_rw_flags(struct kiocb *ki, rwf_t flags)
> +static inline int kiocb_set_rw_flags(int rw, struct kiocb *ki, rwf_t flags)
> {
> + if (rw == WRITE)
> + ki->ki_flags |= IOCB_WRITE;
> +
> if (unlikely(flags & ~RWF_SUPPORTED))
> return -EOPNOTSUPP;
I'd find it more natural if the destination argument (i.e., kiocb) stayed
to be the first argument of the function. Otherwise the patch looks good to
me.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* [PATCH v2 7/7] random: Remove kernel.random.read_wakeup_threshold
From: Andy Lutomirski @ 2019-09-20 14:36 UTC (permalink / raw)
To: Theodore Tso
Cc: LKML, Linux API, Kees Cook, Jason A. Donenfeld, Ahmed S. Darwish,
Lennart Poettering, Eric W. Biederman, Alexander E. Patrakov,
Michael Kerrisk, Willy Tarreau, Matthew Garrett,
Ext4 Developers List, linux-man, Andy Lutomirski
In-Reply-To: <cover.1568990048.git.luto@kernel.org>
It has no effect any more, so remove it. We can revert this if
there is some user code that expects to be able to set this sysctl.
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
drivers/char/random.c | 18 +-----------------
1 file changed, 1 insertion(+), 17 deletions(-)
diff --git a/drivers/char/random.c b/drivers/char/random.c
index 99fea5cc29a8..2a284f30cac4 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -369,12 +369,6 @@
#define ENTROPY_SHIFT 3
#define ENTROPY_BITS(r) ((r)->entropy_count >> ENTROPY_SHIFT)
-/*
- * The minimum number of bits of entropy before we wake up a read on
- * /dev/random. Should be enough to do a significant reseed.
- */
-static int random_read_wakeup_bits = 64;
-
/*
* If the entropy count falls under this number of bits, then we
* should wake up processes which are selecting or polling on write
@@ -1982,8 +1976,7 @@ SYSCALL_DEFINE3(getrandom, char __user *, buf, size_t, count,
#include <linux/sysctl.h>
-static int min_read_thresh = 8, min_write_thresh;
-static int max_read_thresh = OUTPUT_POOL_WORDS * 32;
+static int min_write_thresh;
static int max_write_thresh = INPUT_POOL_WORDS * 32;
static int random_min_urandom_seed = 60;
static char sysctl_bootid[16];
@@ -2058,15 +2051,6 @@ struct ctl_table random_table[] = {
.proc_handler = proc_do_entropy,
.data = &input_pool.entropy_count,
},
- {
- .procname = "read_wakeup_threshold",
- .data = &random_read_wakeup_bits,
- .maxlen = sizeof(int),
- .mode = 0644,
- .proc_handler = proc_dointvec_minmax,
- .extra1 = &min_read_thresh,
- .extra2 = &max_read_thresh,
- },
{
.procname = "write_wakeup_threshold",
.data = &random_write_wakeup_bits,
--
2.21.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox