cryptsetup: create /run/generator #18448
Conversation
https://bugzilla.redhat.com/show_bug.cgi?id=1897802: When booting from a luks-encrypted root fs systemd-cryptsetup issues the following warning: systemd-cryptsetup[385]: WARNING: Locking directory /run/cryptsetup is missing! Another option would be to create the directory from pid1, early on. Then individual tools could assume it always exists. But that seems somewhat nicer to only create from tools that will use it.
|
nit: typo in commit message: |
|
Good point. I don't think it makes sense to push a change for this, since that'd trigger the CI again. We can do squash-and-merge. |
|
Isnt this an implementation detail of libcryptsetup? I.e. shouldnt libcryptsetup create the dir itself when it sees it missing? |
It does IIRC - but it complains loudly while doing so |
|
My thinking (which I should have put in the commit message…), was that it is reasonable for the library to not create the dir on its own: after all, it's just a library and it shouldn't manage "global" state on its own. Whether the presence of the directory is such global state or just an implementation detail is up for debate. |
|
@mbroz what's your take on this?
Well the library wants to put stuff below the dir, hence it better put the dir in place, too, no? i.e. if it is willing to own the stuff below the dir, it should also be willing to own the dir itself... |
| static void mkdir_run_cryptsetup(void) { | ||
| /* libcryptsetup expects the locking directory to exists. Let's create it if missing, but only if we | ||
| * are root. */ | ||
| if (getuid() == 0) |
There was a problem hiding this comment.
the tool only works as root...
There was a problem hiding this comment.
For actiavation, yes, CAP_SYSADMIN is needed (strict requirement for device-mapper ioctl).
Anyway, if you cannot use temp files configuration here (too early?), I think this patch is ok.
There was a problem hiding this comment.
@mbroz the main question is though: why doesn't libcryptsetup create the dir on its own, when it notices it is missing and it needs to create a file in there? i.e. it could just do an mkdir() right before creating whatever it needs to create and ignore things on EEXIST
There was a problem hiding this comment.
There is already a fallback that creates this dir in libcryptsetup (and prints the warning)
https://gitlab.com/cryptsetup/cryptsetup/-/blob/master/lib/utils_device_locking.c#L112
So the question is why this is not executed...
There was a problem hiding this comment.
Hmm, can't the warning be dropped? i.e. is there really reason to warn about this? maybe downgrade to debug level at least?
There was a problem hiding this comment.
If the later transititon to tmpfiles is safe, then why not (move the message to debug). @oniko ?
There was a problem hiding this comment.
@mbroz yes, we can make it debug message, it's just:
are we sure we don't hide some ugly issue by this? I don't understand systemd early boot internals good enough. If /run/cryptsetup directory is created exactly only once, it should be safe. But if there's possibility of having two or more locking directory instances it's roadway to metadata corruption report...
There was a problem hiding this comment.
are we sure we don't hide some ugly issue by this? I don't understand systemd early boot internals good enough. If /run/cryptsetup directory is created exactly only once, it should be safe. But if there's possibility of having two or more locking directory instances it's roadway to metadata corruption report...
Should be safe. /run is mounted in the initrd first, without selinux. During the initrd→host transition the mount is moved to the new root, and thus survives in its entirety with all open files intact. During the transition after PID 1 loaded the selinux policy it will iterate through /run in its entirety and relabel things, so that they match the policy. Then, eventually systemd-tmpfiles will run, and create the dir if it is missing, and adjust perms/ownership if it already exists. And that's all.
So, the outcome whether tmpfiles creates it or libcryptsetup already created it first should be the same: the dir will exist, will be selinux handled, will have right perms access mode, and will per the very same dir since it was created at first regardless if that's in the initrd or the host, and all files and their fds will remain intact through the whole procedure.
TLDR: this should be safe. (and if there's a problem it would be up to systemd/dracut to fix it, not libcryptsetup)
There was a problem hiding this comment.
ok, see https://gitlab.com/cryptsetup/cryptsetup/-/commit/1e7521c0564936fdbf098ce448e91e0ba25bffae
(I think this makes this PR obsolete :)
There will be soon stable cryptsetup minor release fixing problems in dm-integrity, so I will backport it there.
https://bugzilla.redhat.com/show_bug.cgi?id=1897802:
When booting from a luks-encrypted root fs systemd-cryptsetup issues the following warning:
systemd-cryptsetup[385]: WARNING: Locking directory /run/cryptsetup is missing!
Another option would be to create the directory from pid1, early on. Then
individual tools could assume it always exists. But that seems somewhat nicer
to only create from tools that will use it.