Hi Alex,
On Wed, Mar 9, 2022 at 3:10 AM Alexander Graf <graf@xxxxxxxxxx> wrote:
I'm having a hard time figuring out what's different between yourThe vmgenid driver basically works, though it is racy, because that ACPI
notification can arrive after the system is already running again. This
I believe enough people already pointed out that this assumption is
incorrect. The thing that is racy about VMGenID is the interrupt based
notification.
statement and mine. I said that the race is due to the notification.
You said that the race is due to the notification. What subtle thing
am I missing here that would lead you to say that my assumption is
incorrect? Or did you just misread?
The actual identifier is updated before the VM resumesRight. But more than just transactions, it's useful to preventing key
from its clone operation, so if you match on that you will know whether
you are in a new or old world. And that is enough to create
transactions: Save the identifier before a "crypto transaction",
validate before you finish, if they don't match, abort, reseed and replay.
reuse vulnerabilities, in which case, you store the current identifier
just before an ephemeral key is generated, and then subsequently check
to see that the identifier hasn't changed before transmitting anything
related to that key.
If you follow the logic at the beginning of the mail, you can createYes, as mentioned and discussed in depth before. However, your use of
something race free if you consume the hardware VMGenID counter. You can
not make it race free if you rely on the interrupt mechanism.
the word "counter" is problematic. Vmgenid is not a counter. It's a
unique identifier. That means you can't compare it with a single word
comparison but have to compare all of the 16 bytes. That seems
potentially expensive. It's for that reason that I suggested
augmenting the vmgenid spec with an additional word-sized _counter_
that could be mapped into the kernels and into userspaces.
So following that train of thought, if you expose the hardware VMGenIDRight.
to user space, you could allow user space to act race free based on
VMGenID. That means consumers of user space RNGs could validate whether
the ID is identical between the beginning of the crypto operation and
the end.
However, there are more complicated cases as well. What do you do withWere it a single word-sized integer, mapped into memory, that wouldn't
Samba for example? It needs to generate a new SID after the clone.
That's a super heavy operation. Do you want to have smbd constantly poll
on the VMGenID just to see whether it needs to kick off some
administrative actions?
be much of a problem at all. It could constantly read this before and
after every operation. The problem is that it's 16 bytes and
understandably applications don't want to deal with that clunkiness.
In that case, all we would need from the kernel is an easily readableActually, no, you need even less than that. All that's required is a
GenID that changes
sysfs/procfs file that can be poll()'d on. It doesn't need to have any
content. When poll() returns readable, the VM has been forked. Then
userspace rngs and other things like that can call getrandom() to
receive a fresh value to mix into whatever their operation is. Since
all we're talking about here is _event notification_, all we need is
that event, which is what poll() provides.
I'm also not a super big fan of putting all that logic into systemd. ItYes. poll() can do this. For the purposes of discussion, I've posted
means applications need to create their own notification mechanisms to
pass that cloning notification into actual processes. Don't we have any
mechanism that applications and libraries could use to natively get an
event when the GenID changes?
an implementation of this idea here:
https://lore.kernel.org/lkml/20220309215907.77526-1-Jason@xxxxxxxxx/
What I'm sort of leaning toward is doing something like that patch,
and then later if vmgenid ever grows an additional word-sized counter,
moving to explore the race-free approach. Given the amount of
programming required to actually implement the race-free approach
(transactions and careful study of each case), the poll() file
approach might be a medium-grade compromise for the time being.
Evidently that's what Microsoft decided too.