unit: drop Before=sysinit.target from systemd-random-seed.service

[yuwata]

Follow-up for 26ded55.

The commit says,

Note that with this change sysinit.target (and thus early boot) is NOT
systematically delayed until the entropy pool is initialized,
#4271 (comment)
But the dependency was not dropped.

This was found by David Seifert (@SoapGentoo). See #4271 (comment)

Hopefully fixes #13252.

[mbiebl]

Hm, won't this change the semantices of sysinit.target and therefor DefaultDependencies=true which is used by hundreds if not thousand .service units out there.
Do we really expect that all those .service units are audited and checked if they use/need /dev/urandom or getrandom() and now need to specify an explicit dependency/ordering?

[SoapGentoo]

@mbiebl that was the intention of the initial commit in the first place? "[...] and would mean that regular services using /dev/urandom would have to be individually ordered against systemd-random-seed.service"

[mbiebl]

Urgh, then I'm not convinced this is a good idea as this will cause a lot of churn

[keszybz]

Sorry, but this needs a more comprehensive fix.
Before 26ded55, random-seed.service was "quick", and it could reasonably be ordered before sysinit.target. But now it can block. We have two distinct steps: one is feeding the seed to the kernel, which we want to do before sysinit.target as before, and the second step is to do blocking getrandom() and write the seed file.

My suggestion would be to turn the service into Type=notify, and send out READY=1 as soon as we have fed the kernel, and do the remaining step without blocking sysinit.target.

[boucman]

But how would a unit order itself after the second step in that case ? should services needing "real" entropy check what the kernel says themselves ?

[keszybz]

Services needing real entropy should just call getrandom() in a blocking way and wait for the reply ;) Yeah, my proposed change would undo the part about systemd-random-seed.service being a synchronization point. But I don't think we need such a point if we have a blocking syscall.

[poettering]

Thing is there are various uses that use /dev/urandom directly, most prominently cryptsetup when configured for swap with a key that is picked randomly during boot generally lists /dev/urandom as password file in /etc/crypttab. Our cryptsetup-generator already adds an ordering dependency towards this unit, assuming this helps, but so far it didn't.

I think this PR is actually correct, and is what I intended to do, but didn't by mistake: make the unit blocking, so that code can order itself behind it, if it needs an initialized pool. Specifically, the cryptsetup swap case is now finally safe.

I don't see any reason to delay sysinit.target at all for neither proper initialization nor fake initialization, there's not much gained if we do that. I mean, either you want the pool initialized or you don't care. But requiring that the disk random seed is added to the pool without also requiring the pool fully initialized is bit weird I think.

Moreover, if we really wanted to add two sync points we'd need to separate units: one unit that just writes the seed to the kernel, and then the other one that waits until the pool is initialized. It can't be the same unit waiting for both, that's semantcially not expressible in systemd. And given that I fail to see the usecase let's just not do it...

Hence, I think it's right to just merge this patch.

Does this make sense?

[keszybz]

Yeah... OK. Since we didn't credit entropy before, the service didn't really mean much. So by dropping it from sysinit.target we're not really changing any expectations.

[mbiebl]

I can't confirm the fix. See #13252 (comment)