4 ms·

RDRAND is not guaranteed to always succeed (and never was). You’re supposed to retry on failure. (Although linked in that thread systemd code has fallback any

by lelf 7y ago

RDRAND is not guaranteed to always succeed (and never was). You’re supposed to retry on failure. (Although linked in that thread systemd code has fallback anyway, so I’m not sure how it fails at all). Edit: not to mention that it’s better just to not use it, ever. Quite sane and sensible thing.

baq 7y ago
i don't see any reason to not xor rdrand output with a prng if rdrand is available. please enlighten us why you think otherwise as i'm really interested in assumptions that lead to this conclusion.
- lvh 7y ago
  Do you mean if a PRNG is available? Because the answer is obvious if RDRAND is available: you don't do that because sometimes it fails, failure is almost always catastrophic but sublte, and CSRPNGs are not a bottleneck in almost all cases.
  - amluto 7y ago
    RDRAND has major benefits over any CSPRNG: there is no software-visible state that could possibly leak. With attacks like Spectre, there’s always a concern that your CSPRNG secrets could be leaked.
    - lvh 7y ago
      The scheme we’re discussing XORs the two at the end, so that flaw doesn’t apply. Also: empirically, “kernel leaks CSPRNG” state does not seem to be as much of a problem as “userspace is convinced it knows better than urandom/getrandom”.
zaroth 7y ago
Sounds like a bit of code which was hard to trigger the negative test and therefore the fallback failed to work properly. Not sure how the kernel devs generally go about testing the “this virtually never happens” code paths without adding debug switches to every unhappy path. Certainly I doubt they are using DI/IoC to wrap an interface to RDRAND which allows unit testing the failure modes. At least the result is a failure to generate a key, not a compromised key.
- SolarNet 7y ago
  Virtual machines perhaps.
- marshray 7y ago
  The 'edit-compile-run-debug' cycle is so expensive, chip design is all about simulation. Chip vendors, and the tools they use to design them, actually do a significant amount of work for error cases. This is important not just for correctness, but for production yield, reliability, temperature and radiation hardness, etc. As chips get larger and more dense this becomes more and more important. Single Event Upset (where one bit flips) is an example of the type of error. https://en.wikipedia.org/wiki/Single_event_upset https://en.wikipedia.org/wiki/Single_event_upset
- loeg 7y ago
  > Sounds like a bit of code which was hard to trigger the negative test and therefore the fallback failed to work properly. No; it turns out that's giving systemd too much credit (sadly). See [1]. The problem appears to be that RDRAND was signalling success, but producing a nonrandom value. This is bad and a violation of the specification. Can't speak to Linux kernel development, and in this particular case, that isn't the problem. The linked bug involves systemd using the world's worst random number generator. A security engineer goes into more detail on this twitter thread[1]: https://twitter.com/FiloSottile/status/1125840275346198529 https://twitter.com/FiloSottile/status/1125840275346198529 (or unrolled: https://threadreaderapp.com/thread/1125840275346198529.html?refreshed=yes https://threadreaderapp.com/thread/1125840275346198529.html?... ). > At least the result is a failure to generate a key, not a compromised key. In fact, the result is a compromised key -- the bug report is due to colliding "globally unique" identifies generated through a flawed random gathering process.
  - poettering 7y ago
    That "security engineer" appears to imply we use rand() to create crypto keys. I am not sure what he has been smoking. We don't. He appears to be just somebody who doesn't research very well... And no. We do no generate "keys" from either rand(), nor from RDRAND. We use RDRAND for uuid generation, and rand() for seeding hash tables if there's no better source of entropy. That's all. Jeezuz christ. I mean, if we seed hash tables, which options do we have? getrandom()? that either blocks (sometimes very long in virtualized envs) or fails during early boot (and systemd runs in earliest boot being the first process to launch), when the random pool is not initialized. /dev/urandom? That logs loudly about any use when the pool is not initialized. So yeah, we opted to first use RDRAND if available, then fall back to getrandom() with GRND_NONBLOCK, and finally to plain rand() if neither of that works. That has everything we need: doesn't delay boot forever. Doesn't cause log messages about us using a non-initialized pool. It potentially gives us crappy seeds for the hashtable initially, but given that we reseed when we see too many collisions and then getrandom() actually works correctly, we should be fine. Anyway, you can stick that "security engineer" somewhere, that smug, unresearched bitching on Twitter is just annoying. Lennart
kzrdude 7y ago
So - are they not checking for error in this case, and using a value anyway?
- Xylakant 7y ago
  They are checking for an error and there is none, but the value returned is bogus (always the same, causing collisions).
iforgotpassword 7y ago
To be fair from that thread I can't infer whether they check if the call succeeded. It might very well be it returns success but still yields -1.
JdeBP 7y ago
The instruction is signalling success, but returning FFFFFFFFFFFFFFFF as the random value. * https://github.com/systemd/systemd/issues/11810#issuecomment-490165413 https://github.com/systemd/systemd/issues/11810#issuecomment... H. Peter Anvin's educated guess was that some MSR flag, that has the effect of controlling this instruction, is not being saved and restored in the processor across a suspend, and the result is a processor state where it signals that the instruction is succeeding but it is not actually returning random values. * https://bugzilla.kernel.org/show_bug.cgi?id=85911#c4 https://bugzilla.kernel.org/show_bug.cgi?id=85911#c4
- lelf 7y ago
  > The instruction is signalling success Let’s wait for clarifications how that person has done the tests. However: the bug is about systemd failing to get entropy, not getting nonsense entropy.
  - deleted 7y ago
    [deleted]
  - JdeBP 7y ago
    How xe was doing this was in fact explained in the comment hyperlinked right at the start of this page, and H. Peter Anvin was thinking about this over 4 years ago. But for the doubters Vladislav has already reiterated the point. * https://github.com/systemd/systemd/issues/11810#issuecomment-490202021 https://github.com/systemd/systemd/issues/11810#issuecomment... There is no "however". This bug is about code that is, according to the AMD doco, using the instruction correctly; but that is, because the AMD processor has this possible state after a suspend+resume, getting all-ones as its random data, thereby causing ID collisions in a fairly wide range of possible things from freshly re-generated machine IDs to journal file header block IDs, and including unit invocation IDs. * https://github.com/systemd/systemd/blob/717e8eda77b93ac396dc7ed325d5bcc61dfe2fa8/src/core/machine-id-setup.c#L75 https://github.com/systemd/systemd/blob/717e8eda77b93ac396dc... * https://github.com/systemd/systemd/blob/717e8eda77b93ac396dc7ed325d5bcc61dfe2fa8/src/journal/journal-file.c#L425 https://github.com/systemd/systemd/blob/717e8eda77b93ac396dc... * https://github.com/systemd/systemd/blob/717e8eda77b93ac396dc7ed325d5bcc61dfe2fa8/src/core/unit.c#L5119 https://github.com/systemd/systemd/blob/717e8eda77b93ac396dc... This indicates that a "should be fine" in another comment is not in fact true. (-: * https://github.com/systemd/systemd/blob/717e8eda77b93ac396dc7ed325d5bcc61dfe2fa8/src/libsystemd/sd-id128/sd-id128.c#L272 https://github.com/systemd/systemd/blob/717e8eda77b93ac396dc...

by lelf 7y ago

baq 7y ago

i don't see any reason to not xor rdrand output with a prng if rdrand is available. please enlighten us why you think otherwise as i'm really interested in assumptions that lead to this conclusion.

lvh 7y ago
Do you mean if a PRNG is available? Because the answer is obvious if RDRAND is available: you don't do that because sometimes it fails, failure is almost always catastrophic but sublte, and CSRPNGs are not a bottleneck in almost all cases.
- amluto 7y ago
  RDRAND has major benefits over any CSPRNG: there is no software-visible state that could possibly leak. With attacks like Spectre, there’s always a concern that your CSPRNG secrets could be leaked.
  - lvh 7y ago
    The scheme we’re discussing XORs the two at the end, so that flaw doesn’t apply. Also: empirically, “kernel leaks CSPRNG” state does not seem to be as much of a problem as “userspace is convinced it knows better than urandom/getrandom”.

zaroth 7y ago

Sounds like a bit of code which was hard to trigger the negative test and therefore the fallback failed to work properly. Not sure how the kernel devs generally go about testing the “this virtually never happens” code paths without adding debug switches to every unhappy path. Certainly I doubt they are using DI/IoC to wrap an interface to RDRAND which allows unit testing the failure modes. At least the result is a failure to generate a key, not a compromised key.

SolarNet 7y ago
Virtual machines perhaps.
marshray 7y ago
The 'edit-compile-run-debug' cycle is so expensive, chip design is all about simulation. Chip vendors, and the tools they use to design them, actually do a significant amount of work for error cases. This is important not just for correctness, but for production yield, reliability, temperature and radiation hardness, etc. As chips get larger and more dense this becomes more and more important. Single Event Upset (where one bit flips) is an example of the type of error. https://en.wikipedia.org/wiki/Single_event_upset https://en.wikipedia.org/wiki/Single_event_upset
loeg 7y ago
> Sounds like a bit of code which was hard to trigger the negative test and therefore the fallback failed to work properly. No; it turns out that's giving systemd too much credit (sadly). See [1]. The problem appears to be that RDRAND was signalling success, but producing a nonrandom value. This is bad and a violation of the specification. Can't speak to Linux kernel development, and in this particular case, that isn't the problem. The linked bug involves systemd using the world's worst random number generator. A security engineer goes into more detail on this twitter thread[1]: https://twitter.com/FiloSottile/status/1125840275346198529 https://twitter.com/FiloSottile/status/1125840275346198529 (or unrolled: https://threadreaderapp.com/thread/1125840275346198529.html?refreshed=yes https://threadreaderapp.com/thread/1125840275346198529.html?... ). > At least the result is a failure to generate a key, not a compromised key. In fact, the result is a compromised key -- the bug report is due to colliding "globally unique" identifies generated through a flawed random gathering process.
- poettering 7y ago
  That "security engineer" appears to imply we use rand() to create crypto keys. I am not sure what he has been smoking. We don't. He appears to be just somebody who doesn't research very well... And no. We do no generate "keys" from either rand(), nor from RDRAND. We use RDRAND for uuid generation, and rand() for seeding hash tables if there's no better source of entropy. That's all. Jeezuz christ. I mean, if we seed hash tables, which options do we have? getrandom()? that either blocks (sometimes very long in virtualized envs) or fails during early boot (and systemd runs in earliest boot being the first process to launch), when the random pool is not initialized. /dev/urandom? That logs loudly about any use when the pool is not initialized. So yeah, we opted to first use RDRAND if available, then fall back to getrandom() with GRND_NONBLOCK, and finally to plain rand() if neither of that works. That has everything we need: doesn't delay boot forever. Doesn't cause log messages about us using a non-initialized pool. It potentially gives us crappy seeds for the hashtable initially, but given that we reseed when we see too many collisions and then getrandom() actually works correctly, we should be fine. Anyway, you can stick that "security engineer" somewhere, that smug, unresearched bitching on Twitter is just annoying. Lennart

kzrdude 7y ago

So - are they not checking for error in this case, and using a value anyway?

Xylakant 7y ago
They are checking for an error and there is none, but the value returned is bogus (always the same, causing collisions).

iforgotpassword 7y ago

To be fair from that thread I can't infer whether they check if the call succeeded. It might very well be it returns success but still yields -1.

JdeBP 7y ago

The instruction is signalling success, but returning FFFFFFFFFFFFFFFF as the random value. * https://github.com/systemd/systemd/issues/11810#issuecomment-490165413 https://github.com/systemd/systemd/issues/11810#issuecomment... H. Peter Anvin's educated guess was that some MSR flag, that has the effect of controlling this instruction, is not being saved and restored in the processor across a suspend, and the result is a processor state where it signals that the instruction is succeeding but it is not actually returning random values. * https://bugzilla.kernel.org/show_bug.cgi?id=85911#c4 https://bugzilla.kernel.org/show_bug.cgi?id=85911#c4

lelf 7y ago
> The instruction is signalling success Let’s wait for clarifications how that person has done the tests. However: the bug is about systemd failing to get entropy, not getting nonsense entropy.
- deleted 7y ago
  [deleted]
- JdeBP 7y ago
  How xe was doing this was in fact explained in the comment hyperlinked right at the start of this page, and H. Peter Anvin was thinking about this over 4 years ago. But for the doubters Vladislav has already reiterated the point. * https://github.com/systemd/systemd/issues/11810#issuecomment-490202021 https://github.com/systemd/systemd/issues/11810#issuecomment... There is no "however". This bug is about code that is, according to the AMD doco, using the instruction correctly; but that is, because the AMD processor has this possible state after a suspend+resume, getting all-ones as its random data, thereby causing ID collisions in a fairly wide range of possible things from freshly re-generated machine IDs to journal file header block IDs, and including unit invocation IDs. * https://github.com/systemd/systemd/blob/717e8eda77b93ac396dc7ed325d5bcc61dfe2fa8/src/core/machine-id-setup.c#L75 https://github.com/systemd/systemd/blob/717e8eda77b93ac396dc... * https://github.com/systemd/systemd/blob/717e8eda77b93ac396dc7ed325d5bcc61dfe2fa8/src/journal/journal-file.c#L425 https://github.com/systemd/systemd/blob/717e8eda77b93ac396dc... * https://github.com/systemd/systemd/blob/717e8eda77b93ac396dc7ed325d5bcc61dfe2fa8/src/core/unit.c#L5119 https://github.com/systemd/systemd/blob/717e8eda77b93ac396dc... This indicates that a "should be fine" in another comment is not in fact true. (-: * https://github.com/systemd/systemd/blob/717e8eda77b93ac396dc7ed325d5bcc61dfe2fa8/src/libsystemd/sd-id128/sd-id128.c#L272 https://github.com/systemd/systemd/blob/717e8eda77b93ac396dc...