Make user/group info reliable operation across chroot #2503
Conversation
|
What about users and groups that are not in IMO the only way to get everything right is to either reimplement glibc nsswitch (yuck) or to fork/exec a subprocess and have that chroot before it makes any user or group lookups. |
|
Ignoring users from network services is quite intentional. That's mentioned in the commit message but perhaps it deserves to be expanded. see the discussion in #882. |
|
I would be more happy if it does the local parsing only in the chroot case, like with my old patch. |
|
Doing one thing for a chroot and another for the host seems inconsistent to me. Assuming we accept that rpm needs to honor NSS then doing a local parse for the chroot is just different kind of buggy, because the chroot may just as well be configured for something else. I'm inclined to agree with this @keszybz 's comment: #882 (comment) - the only thing that's well defined for rpm purposes is the local files, and we'll be better off with consistent, clearly defined behavior. This goes for user/group creation too. |
pmatilai
force-pushed
the
ugid-pr
branch
2 times, most recently
from
831db8e to
1cd4bbe
Compare
|
One possibility is to (perhaps optionally) fall back to NSS if local lookup fails, for the non-chroot case. That said, I'd rather leave that fallback out initially and only add later if it turns out unavoidable. The thing is, NSS in a chroot is a lost cause because doing the first lookup inside the chroot means the we or the alleged helper would try to dlopen() stuff from the chroot, which could be anything at all, or be entirely missing. So if we accept NSS use for packaged files, we'll accept forever having undefined behavior for verify in chroot, which is basically the case we're trying to solve here. And in that case we might just as well not bother with this at all. |
|
Should also fix #1789 for real |
pmatilai
force-pushed
the
ugid-pr
branch
2 times, most recently
from
b086193 to
f2a24cc
Compare
|
Elaborated a bit on the intentional NSS exclusion in the commit message and docs as well. |
|
Apart from the inline comments I made, the change itself looks good! |
There's no telling what sort of caching getpwnam() and friends perform behind the scenes, and worse, there's no way to explicitly reset those caches. This can lead to chrooted operations using user/group data from the host, which is simply wrong. Do our own parsing of /etc/passwd and /etc/group to fix. Besides the chroot matter, we then only ever lookup local system users and groups and not something from eg network name services. We cannot reliably do NSS in a chroot because doing so would require relying on binaries in the chroot, which in itself is completely unreliable. NSS is also unreliable in various rescue mode situations. Bottom line, local files are the only ones that can be guaranteed for rpm operation, and this change basically enforces that. Technically we should track chroot status for each lookup and flush the cache if the state changed, but this is an internal API and rpm usages only ever call it from one side of the chroot for a given operation. Fixes: rpm-software-management#882, rpm-software-management#1789
The simple cache whose efficiency troubled ewt back in 1997 (see commit 97999ce) has proven more than adequate over the years. In a local testcase based on Fedora 33 server iso contents, an install of 1765 packages consisting of 201344 files did a whopping 27 user + groups combined. So a few more alloc+free is not going to make the damnest difference, don't bother with reallocing the cache buffer, just strdup() a new one when needed.
|
Typo + poor function name addressed in the last push, plus rebased. |
|
Okay no further comments from anybody... This needs to go into the beta to give us a chance to react if it turns out to be a disaster. |
There's no telling what sort of caching getpwnam() and friends perform behind the scenes, and worse, there's no way to explicitly reset those caches. This can lead to chrooted operations using user/group data from the host, which is simply wrong.
Do our own parsing of /etc/passwd and /etc/group to fix. Besides the chroot matter, we then only ever lookup local system users and groups and not something from eg network name services. Technically we should track chroot status for each lookup and flush the cache if the state changed, but this is an internal API and rpm usages only ever call it from one side of the chroot for a given operation.
Fixes: #882