Message-ID: <7a1cfd1c-9f0e-f134-e544-83ee6d3cd9c9@amazon.com>
Date:   Mon, 2 May 2022 19:59:08 +0200
From:   Alexander Graf <graf@...zon.com>
To:     Lennart Poettering <mzxreary@...inter.de>,
        "Jason A. Donenfeld" <Jason@...c4.com>
CC:     <linux-kernel@...r.kernel.org>, <linux-crypto@...r.kernel.org>,
        "Dominik Brodowski" <linux@...inikbrodowski.net>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Theodore Ts'o <tytso@....edu>,
        "Colm MacCarthaigh" <colmmacc@...zon.com>,
        Torben Hansen <htorben@...zon.co.uk>,
        Jann Horn <jannh@...gle.com>
Subject: Re: [PATCH 2/2] random: add fork_event sysctl for polling VM forks


On 02.05.22 18:51, Lennart Poettering wrote:
> On Mo, 02.05.22 18:12, Jason A. Donenfeld (Jason@...c4.com) wrote:
>
>>>> In order to inform userspace of virtual machine forks, this commit adds
>>>> a "fork_event" sysctl, which does not return any data, but allows
>>>> userspace processes to poll() on it for notification of VM forks.
>>>>
>>>> It avoids exposing the actual vmgenid from the hypervisor to userspace,
>>>> in case there is any randomness value in keeping it secret. Rather,
>>>> userspace is expected to simply use getrandom() if it wants a fresh
>>>> value.
>>> Wouldn't it make sense to expose a monotonic 64bit counter of detected
>>> VM forks since boot through read()? It might be interesting to know
>>> for userspace how many forks it missed the fork events for. Moreover it
>>> might be interesting to userspace to know if any fork happened so far
>>> *at* *all*, by checking if the counter is non-zero.
>> "Might be interesting" is different from "definitely useful". I'm not
>> going to add this without a clear use case. This feature is pretty
>> narrowly scoped in its objectives right now, and I intend to keep it
>> that way if possible.
> Sure, whatever. I mean, if you think it's preferable to have 3 API
> abstractions for the same concept each for it's special usecase, then
> that's certainly one way to do things. I personally would try to
> figure out a modicum of generalization for things like this. But maybe
> that' just me…
>
> I can just tell you, that in systemd we'd have a usecase for consuming
> such a generation counter: we try to provide stable MAC addresses for
> synthetic network interfaces managed by networkd, so we hash them from
> /etc/machine-id, but otoh people also want them to change when they
> clone their VMs. We could very nicely solve this if we had a
> generation counter easily accessible from userspace, that starts at 0
> initially. Because then we can hash as we always did when the counter
> is zero, but otherwise use something else, possibly hashed from the
> generation counter.
>
> But anyway, I understand you are not interested in
> generalization/other usecases, so I'll shut up.


Let's not turn this into a pit fight please :). I think it's a good idea 
to collect the use cases we all have and evaluate whether this patch is 
a good stepping stone towards the final solution.

At the end of the road, what I would like to see is

1) A way for libraries such as s2n to identify that a clone occurred. 
Because it's a deep-down library with no access to its own thread or the 
main loop, it can not rely on poll/select. Mmap of a file however would 
work great, as you can create transactions on top of a 64bit mmap'ed 
value for example.

We can have systemd generate and then provide such a file as long as we 
have an event based notification mechanism.

2) A way to notify larger applications (think Java here) that a system 
is going to be suspended soon so it can wipe PII before it gets cloned 
for example.

3) Notifications after clone so applications know they can regenerate VM 
unique data based on randomness.

For 2 and 3, applications should have native support for these events 
(think of poll() on the fork file) as well as external script based 
support (think "invoke systemctl restart smbd.service on clone").



Lennart, looking at the current sysctl proposal, systemd could poll() on 
the fork file. It would then be able to generate a /run/fork-id file 
which it can use for the flow above, right?

The sysctl proposal also gives us 3, if we implement the inhibitor 
proposal [1] in systemd.

That leaves 2, which we don't have a hardware interface for yet. We can 
still get to at least script level automation with inhibitors and 
manually triggering a "hey, you'll be suspended soon" event via systemd.

Overall, it sounds to me like the sysctl poll based kernel interface in 
this patch in combination with systemd inhibitors gives us an answer to 
most of the flows above.

I can see attractiveness in providing the /run/fork-id directly from the 
kernel though, to remove the dependency on systemd for poll-less 
notification of libraries.

Jason, how much complexity would it add to provide an mmap() and read() 
interface to a fork counter value to the sysctl? Read sounds like a 
trivial change on top of what you have already, mmap a bit more heavy 
lift. If we had both, it would allow us to implement a Linux standard 
fork detect path in libraries that does not rely on systemd.



Alex


[1] https://github.com/systemd/systemd/issues/20222




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879

Powered by blists - more mailing lists