Bug#1458: unattended Debian installations (using preseed) fail when x2gobroker-ssh is involved
package: x2gobroker-ssh version 0.0.4.1 severity: important
The postinst file for x2gobroker-ssh contains a check to see if the nscd binary is somewhere in the path. If it is, then nscd -i group is called, to purge the group cache (which makes sense, as a new group has been added in a previous step, so the cached information is outdated). If it is not, then it is assumed that nscd is not installed, thus, no group information is cached, and thus, no purge is needed.
This fails to take into account that nscd might be (freshly) installed, but not running. Which is the case during a preseeded Debian installation (and probably when running debootstrap as well).
The result is that "nscd -i group" is called even though nscd isn't active at the moment.
Which triggers a return code of "1".
Which in turn causes the postinst script to report "1" as its return code.
This, of course, gets passed back up to the debian-installer, which decides to abort the installation, as it seems that something went wrong during installation of the package.
The patch is rather simple: Instead of checking for the presence of the executable in the file system, check if it is currently running:
if which nscd 1>/dev/null; then
if ps -C nscd 1>/dev/null; then
As this is currently breaking all automated x2gobroker-ssh installs, I would suggest releasing a new version containing only this patch ASAP, if a full release with all changes since the last proper release of x2gobroker-ssh would bring too many changes at once for a timely release.
Kind Regards, Stefan Baur
-- BAUR-ITCS UG (haftungsbeschränkt) Geschäftsführer: Stefan Baur Eichenäckerweg 10, 89081 Ulm | Registergericht Ulm, HRB 724364 Fon/Fax 0731 40 34 66-36/-35 | USt-IdNr.: DE268653243
I think it is a totally wrong approach to fiddle with nscd. Creating a group using system tools should take of that already. If not it's a bug, I'd say.
Besides: what about sssd that can also cache groups?
On Fri, Apr 17, 2020 at 4:11 PM Stefan Baur <X2Go-ML-1@baur-itcs.de> wrote:
package: x2gobroker-ssh version 0.0.4.1 severity: important
The postinst file for x2gobroker-ssh contains a check to see if the nscd binary is somewhere in the path. If it is, then nscd -i group is called, to purge the group cache (which makes sense, as a new group has been added in a previous step, so the cached information is outdated). If it is not, then it is assumed that nscd is not installed, thus, no group information is cached, and thus, no purge is needed.
This fails to take into account that nscd might be (freshly) installed, but not running. Which is the case during a preseeded Debian installation (and probably when running debootstrap as well).
The result is that "nscd -i group" is called even though nscd isn't active at the moment.
Which triggers a return code of "1".
Which in turn causes the postinst script to report "1" as its return code.
This, of course, gets passed back up to the debian-installer, which decides to abort the installation, as it seems that something went wrong during installation of the package.
The patch is rather simple: Instead of checking for the presence of the executable in the file system, check if it is currently running:
if which nscd 1>/dev/null; then
if ps -C nscd 1>/dev/null; thenAs this is currently breaking all automated x2gobroker-ssh installs, I would suggest releasing a new version containing only this patch ASAP, if a full release with all changes since the last proper release of x2gobroker-ssh would bring too many changes at once for a timely release.
Kind Regards, Stefan Baur
-- BAUR-ITCS UG (haftungsbeschränkt) Geschäftsführer: Stefan Baur Eichenäckerweg 10, 89081 Ulm | Registergericht Ulm, HRB 724364 Fon/Fax 0731 40 34 66-36/-35 | USt-IdNr.: DE268653243
x2go-dev mailing list x2go-dev@lists.x2go.org https://lists.x2go.org/listinfo/x2go-dev
Am 17.04.20 um 16:24 schrieb Ulrich Sibiller:
I think it is a totally wrong approach to fiddle with nscd. Creating a group using system tools should take of that already. If not it's a bug, I'd say.
LOL. nscd caching the wrong(TM) things at the wrong(TM) time is an issue that's probably as old as Unix (or at least nscd) itself. If you take a look at the postinst script in question, you will see that it does, in fact, use the system tools to add the group. Still, it is neccesary to flush the cache or things have a tendency to go wrong.
Besides: what about sssd that can also cache groups?
That's a more interesting question, and we might have to add a check for it as well. But as of right now, sssd being installed in combination with x2gobroker-ssh during a preseeded installation won't break anything.
-Stefan
-- BAUR-ITCS UG (haftungsbeschränkt) Geschäftsführer: Stefan Baur Eichenäckerweg 10, 89081 Ulm | Registergericht Ulm, HRB 724364 Fon/Fax 0731 40 34 66-36/-35 | USt-IdNr.: DE268653243
On Fri, Apr 17, 2020 at 4:50 PM Stefan Baur <X2Go-ML-1@baur-itcs.de> wrote:
Am 17.04.20 um 16:24 schrieb Ulrich Sibiller:
I think it is a totally wrong approach to fiddle with nscd. Creating a group using system tools should take of that already. If not it's a bug, I'd say.
LOL. nscd caching the wrong(TM) things at the wrong(TM) time is an issue that's probably as old as Unix (or at least nscd) itself. If you take a look at the postinst script in question, you will see that it does, in fact, use the system tools to add the group. Still, it is neccesary to flush the cache or things have a tendency to go wrong.
Well, if you go that route there are more things to take into account:
- is nscd properly configured to cache groups at all?
- is there a distro-tool available for configuring/flushing/handling nscd
- are the multiple versions of nscd around? Which one to take?
- probably more
- what happens if a newer version of nscd is around that needs to be called otherwise
- waht happens if the nscd binary is something completely different and just happens to have the same name?
- same for ssd
- same for ANY other caching mechanism you might not even know
All these things tend to break sooner or later. That's the reason why you should not do this in an installation script but report a bug instead. This must be fixed at distro level.
Are you aware of any installation postscripts other than x2go that handle nscd problems?
Besides: what about sssd that can also cache groups?
That's a more interesting question, and we might have to add a check for it as well. But as of right now, sssd being installed in combination with x2gobroker-ssh during a preseeded installation won't break anything.
I have seen the weirdest problems with sssd (and nscd as well). An I still have one bug open at redhat for more than year which redhat has not fixed yet...
While you can work around such problems in local (site) scripts or as local administrator you should NOT include such workaround in release packages.
Uli
Uli,
let me make this clear, in case you missed the "severity: important" tag I gave this bug:
Currently, all automated Debian installs for x2gobroker-ssh are failing due to this faulty piece of code.
Even our own demo install scripts we have in the Wiki, for the newbies that want to try out X2GoBroker without having to figure out how to configure this hairy beast manually, are broken.
This is NOT GOOD.
At the same time, "when in doubt, dike it out" is not a feasible approach, as this code has been put there for a reason - and we can assume it was added after things broke while testing a manual install.
So if we remove it, we're likely to get another bug report tagged "severity: important", just coming from the other direction, begging us to add this code again.
That is why our only option right now is to deploy this patch into stable ASAP, to make the code do what it is supposed to do - flush the cache IF nscd is RUNNING, and ONLY THEN.
No one is claiming that this is a magic cure-all for every problem there may be with nscd or sssd caching.
But it WILL fix an actual issue we have RIGHT NOW and which is blocking users from deploying x2gobroker-ssh in an automated way. AND it will NOT make things WORSE for anyone else.
After the release, we can re-open this bug and downgrade its severity as a reminder that this caching issue should be investigated further. But we won't be under pressure to get something working again that worked before like we are right now.
-Stefan
Am 17.04.20 um 20:02 schrieb Ulrich Sibiller:
On Fri, Apr 17, 2020 at 4:50 PM Stefan Baur <X2Go-ML-1@baur-itcs.de> wrote:
Am 17.04.20 um 16:24 schrieb Ulrich Sibiller:
I think it is a totally wrong approach to fiddle with nscd. Creating a group using system tools should take of that already. If not it's a bug, I'd say.
LOL. nscd caching the wrong(TM) things at the wrong(TM) time is an issue that's probably as old as Unix (or at least nscd) itself. If you take a look at the postinst script in question, you will see that it does, in fact, use the system tools to add the group. Still, it is neccesary to flush the cache or things have a tendency to go wrong.
Well, if you go that route there are more things to take into account:
- is nscd properly configured to cache groups at all?
- is there a distro-tool available for configuring/flushing/handling nscd
- are the multiple versions of nscd around? Which one to take?
- probably more
- what happens if a newer version of nscd is around that needs to be called otherwise
- waht happens if the nscd binary is something completely different and just happens to have the same name?
- same for ssd
- same for ANY other caching mechanism you might not even know
All these things tend to break sooner or later. That's the reason why you should not do this in an installation script but report a bug instead. This must be fixed at distro level.
Are you aware of any installation postscripts other than x2go that handle nscd problems?
Besides: what about sssd that can also cache groups?
That's a more interesting question, and we might have to add a check for it as well. But as of right now, sssd being installed in combination with x2gobroker-ssh during a preseeded installation won't break anything.
I have seen the weirdest problems with sssd (and nscd as well). An I still have one bug open at redhat for more than year which redhat has not fixed yet...
While you can work around such problems in local (site) scripts or as local administrator you should NOT include such workaround in release packages.
Uli
-- BAUR-ITCS UG (haftungsbeschränkt) Geschäftsführer: Stefan Baur Eichenäckerweg 10, 89081 Ulm | Registergericht Ulm, HRB 724364 Fon/Fax 0731 40 34 66-36/-35 | USt-IdNr.: DE268653243
On Fri, Apr 17, 2020 at 8:36 PM Stefan Baur <X2Go-ML-1@baur-itcs.de> wrote:
let me make this clear, in case you missed the "severity: important" tag I gave this bug:
Yes, I did.
Currently, all automated Debian installs for x2gobroker-ssh are failing due to this faulty piece of code.
So, let's make this clear: Code, which should not be there at all ist now breaking installation. That's exactly my point... It should not be there.
I have just checked on my Debian 10. Here's a snipped from /usr/sbin/groupadd: ... printf (gtx("Adding group `%s' (GID %d) ...\n"),$new_name,$new_gid) if $verbose; &invalidate_nscd("group"); my $groupadd = &which('groupadd'); &systemcall($groupadd, '-g', $new_gid, $new_name); &invalidate_nscd("group"); print (gtx("Done.\n")) if $verbose; ...
So this is where this kind of code belongs to. Debian has everything in place.
So I agree that this should be fixed asap in x2gobroker-ssh, but it should be fixed by removing the nscd code altogether.
So if we remove it, we're likely to get another bug report tagged "severity: important", just coming from the other direction, begging us to add this code again.
No, those people begging for it can be instructed to check their distro. And to run the flush manually. Or even to simply reboot.
That is why our only option right now is to deploy this patch into stable ASAP, to make the code do what it is supposed to do - flush the cache IF nscd is RUNNING, and ONLY THEN.
No one is claiming that this is a magic cure-all for every problem there may be with nscd or sssd caching.
I see it - as this bug report proves - as an unneccessary source for problems.
Uli
Am 17.04.20 um 21:59 schrieb Ulrich Sibiller:
On Fri, Apr 17, 2020 at 8:36 PM Stefan Baur <X2Go-ML-1@baur-itcs.de> wrote:
Currently, all automated Debian installs for x2gobroker-ssh are failing due to this faulty piece of code.
So, let's make this clear: Code, which should not be there at all ist now breaking installation. That's exactly my point... It should not be there.
Code that has been put there to solve an actual issue, but which has a flaw in its detection routine as to when it should trigger.
I have just checked on my Debian 10. Here's a snipped from /usr/sbin/groupadd: ... printf (gtx("Adding group `%s' (GID %d) ...\n"),$new_name,$new_gid) if $verbose; &invalidate_nscd("group"); my $groupadd = &which('groupadd'); &systemcall($groupadd, '-g', $new_gid, $new_name); &invalidate_nscd("group"); print (gtx("Done.\n")) if $verbose; ...
So this is where this kind of code belongs to. Debian has everything in place.
And it seems to be broken, or this code in the postinst script wouldn't be needed. Or maybe it was only added to groupadd after the code in the postinst file had already been written. It would be interesting to know when those "invalidate_nscd" calls were added. Maybe an older Debian version is still affected by this, while Buster is safe?
So I agree that this should be fixed asap in x2gobroker-ssh, but it should be fixed by removing the nscd code altogether.
So if we remove it, we're likely to get another bug report tagged "severity: important", just coming from the other direction, begging us to add this code again.
No, those people begging for it can be instructed to check their distro. And to run the flush manually. Or even to simply reboot.
The proper way to handle this, IMO, is a two-step process.
- Apply my patch ASAP and release a fixed package, to un-break current preseeded installations.
- Turn the blind execution if the condition is triggered into an interactive, preseedable question, and check for sssd as well. This is what Debian does e.g. when updating libc - you get a popup asking you if it is okay to restart affected services, or if you would like to do it manually at a later date. So in addition to
x2gobroker-ssh x2gobroker-ssh/group-x2gobroker-users string x2gobroker-users x2gobroker-ssh x2gobroker-ssh/group-does-not-exist note x2gobroker-ssh x2gobroker-ssh/use-existing-group boolean true|false x2gobroker-ssh x2gobroker-ssh/create-group boolean true|false x2gobroker-ssh x2gobroker-ssh/manual-setup-required boolean true|false x2gobroker-ssh x2gobroker-ssh/del-last-group-x2gobroker-users boolean true|false x2gobroker-ssh x2gobroker-ssh/last-group-x2gobroker-users string x2gobroker-users
we'd have new preseedable entries like
x2gobroker-ssh x2gobroker-ssh/flush-nscd-groups boolean true|false x2gobroker-ssh x2gobroker-ssh/flush-sssd-groups boolean true|false
Whoever tries to run an unattended installation using preseeds can then choose whichever option suits them best. And if you're worried that the attempt to flush the cache has a negative impact, even with the triggering condition fixed, we can make those options default to "false" and add a notice like "this should not be neccessary unless your groupadd implementation is faulty" in the popup message.
-Stefan
-- BAUR-ITCS UG (haftungsbeschränkt) Geschäftsführer: Stefan Baur Eichenäckerweg 10, 89081 Ulm | Registergericht Ulm, HRB 724364 Fon/Fax 0731 40 34 66-36/-35 | USt-IdNr.: DE268653243
For the record,
Uli has contacted me off-list and indicated this is a course of action he can live with.
In the meantime, I decided to dig through X2Go's git repo.
This is the commit that introduced the faulty nscd cache flushing code in x2goserver-ssh:
<https://code.x2go.org/gitweb?p=x2gobroker.git;a=commit;h=9e44861e4a29897228cb70a95d6853dbe85779b0>
author Mike Gabriel <mike.gabriel@das-netzwerkteam.de> Wed, 1 Apr 2015 03:50:45 +0000 (05:50 +0200) committer Mike Gabriel <mike.gabriel@das-netzwerkteam.de> Wed, 1 Apr 2015 03:50:45 +0000 (05:50 +0200) commit 9e44861e4a29897228cb70a95d6853dbe85779b0 tree 9bba4aeb6d2ffa2ac5bfe47a63e2f20fbb4c6a2a parent 2cd0cdc8b73967b87d53c615a5952a83309ba63f
The previous commit that touched this file is:
author Mike Gabriel <mike.gabriel@das-netzwerkteam.de> Thu, 11 Sep 2014 23:29:14 +0000 (01:29 +0200) committer Mike Gabriel <mike.gabriel@das-netzwerkteam.de> Thu, 11 Sep 2014 23:29:37 +0000 (01:29 +0200) commit 67d9cfd0ba235c919af84aa9b0d647d0e8a47968 tree 88ed368b14cb2e440f7eebedec9a342257317ff3 parent 806224f2caab8db9c7accb7fa4c5e587d34cd5ca
So some time between September 11, 2014, and April 1, 2015, Mike#1 noticed that groupadd did not behave as expected, and added this code. Which means Wheezy (7) and Jessie (8) are the most likely candidates where the issue might have cropped up, as there's a good chance Mike#1 was using testing instead of stable, being so close to the official release (which happened on April 25./26., 2015). Squeeze (6) was already out of support by September 11, 2014, which means it's unlikely that whatever happened, happened on Squeeze.
Anyone curious enough to go through the groupadd source code for the binaries that shipped in these two Debian releases in said time frame?
-Stefan
Am 19.04.20 um 15:30 schrieb Stefan Baur:
Am 17.04.20 um 21:59 schrieb Ulrich Sibiller:
On Fri, Apr 17, 2020 at 8:36 PM Stefan Baur <X2Go-ML-1@baur-itcs.de> wrote:
Currently, all automated Debian installs for x2gobroker-ssh are failing due to this faulty piece of code.
So, let's make this clear: Code, which should not be there at all ist now breaking installation. That's exactly my point... It should not be there.
Code that has been put there to solve an actual issue, but which has a flaw in its detection routine as to when it should trigger.
I have just checked on my Debian 10. Here's a snipped from /usr/sbin/groupadd: ... printf (gtx("Adding group `%s' (GID %d) ...\n"),$new_name,$new_gid) if $verbose; &invalidate_nscd("group"); my $groupadd = &which('groupadd'); &systemcall($groupadd, '-g', $new_gid, $new_name); &invalidate_nscd("group"); print (gtx("Done.\n")) if $verbose; ...
So this is where this kind of code belongs to. Debian has everything in place.
And it seems to be broken, or this code in the postinst script wouldn't be needed. Or maybe it was only added to groupadd after the code in the postinst file had already been written. It would be interesting to know when those "invalidate_nscd" calls were added. Maybe an older Debian version is still affected by this, while Buster is safe?
So I agree that this should be fixed asap in x2gobroker-ssh, but it should be fixed by removing the nscd code altogether.
So if we remove it, we're likely to get another bug report tagged "severity: important", just coming from the other direction, begging us to add this code again.
No, those people begging for it can be instructed to check their distro. And to run the flush manually. Or even to simply reboot.
The proper way to handle this, IMO, is a two-step process.
- Apply my patch ASAP and release a fixed package, to un-break current preseeded installations.
- Turn the blind execution if the condition is triggered into an interactive, preseedable question, and check for sssd as well. This is what Debian does e.g. when updating libc - you get a popup asking you if it is okay to restart affected services, or if you would like to do it manually at a later date. So in addition to
x2gobroker-ssh x2gobroker-ssh/group-x2gobroker-users string x2gobroker-users x2gobroker-ssh x2gobroker-ssh/group-does-not-exist note x2gobroker-ssh x2gobroker-ssh/use-existing-group boolean true|false x2gobroker-ssh x2gobroker-ssh/create-group boolean true|false x2gobroker-ssh x2gobroker-ssh/manual-setup-required boolean true|false x2gobroker-ssh x2gobroker-ssh/del-last-group-x2gobroker-users boolean true|false x2gobroker-ssh x2gobroker-ssh/last-group-x2gobroker-users string x2gobroker-users
we'd have new preseedable entries like
x2gobroker-ssh x2gobroker-ssh/flush-nscd-groups boolean true|false x2gobroker-ssh x2gobroker-ssh/flush-sssd-groups boolean true|false
Whoever tries to run an unattended installation using preseeds can then choose whichever option suits them best. And if you're worried that the attempt to flush the cache has a negative impact, even with the triggering condition fixed, we can make those options default to "false" and add a notice like "this should not be neccessary unless your groupadd implementation is faulty" in the popup message.
-Stefan
-- BAUR-ITCS UG (haftungsbeschränkt) Geschäftsführer: Stefan Baur Eichenäckerweg 10, 89081 Ulm | Registergericht Ulm, HRB 724364 Fon/Fax 0731 40 34 66-36/-35 | USt-IdNr.: DE268653243
On So 19 Apr 2020 20:20:52 CEST, Stefan Baur wrote:
Anyone curious enough to go through the groupadd source code for the binaries that shipped in these two Debian releases in said time frame?
Would it make sense filing a bug against the appropriate Debian package???
Mike
DAS-NETZWERKTEAM c\o Technik- und Ökologiezentrum Eckernförde Mike Gabriel, Marienthaler Str. 17, 24340 Eckernförde mobile: +49 (1520) 1976 148 landline: +49 (4351) 850 8940
GnuPG Fingerprint: 9BFB AEE8 6C0A A5FF BF22 0782 9AF4 6B30 2577 1B31 mail: mike.gabriel@das-netzwerkteam.de, http://das-netzwerkteam.de
Am 20.04.20 um 20:42 schrieb Mike Gabriel:
On So 19 Apr 2020 20:20:52 CEST, Stefan Baur wrote:
Anyone curious enough to go through the groupadd source code for the binaries that shipped in these two Debian releases in said time frame?
Would it make sense filing a bug against the appropriate Debian package???
Not if the code Uli has quoted has since been put there to fix the issue you saw back when you added the workaround to our postinst script.
We should do "our" homework before filing a bug report and confusing the maintainer(s) of groupadd, by reporting an issue that has already been fixed since.
So $SOMEONE needs to look at the groupadd sources to confirm or rule out that the lines Uli spotted have been put there after you wrote the workaround, and we shouldn't place that burden on the nscd maintainer(s).
If it turns out that the code has already been there, yet you saw a need to flush the nscd cache with your workaround, we need to create a list of steps to reproduce the issue you ran into.
Because from looking at the code, as it is now, your workaround would seem unnecessary - *if* the function in groupadd works as intended.
If not, we need to prove that it fails, in a reproducible way. Then a bug report against groupadd's package would be warranted.
Kind Regards, Stefan Baur
-- BAUR-ITCS UG (haftungsbeschränkt) Geschäftsführer: Stefan Baur Eichenäckerweg 10, 89081 Ulm | Registergericht Ulm, HRB 724364 Fon/Fax 0731 40 34 66-36/-35 | USt-IdNr.: DE268653243
tag #1458 pending fixed #1458 0.0.4.2 thanks Hi! X2Go issue #1458 (src:x2gobroker) reported by you has been fixed in X2Go Git. You can see the changelog below, and you can check the diff of the fix at: https://code.x2go.org/gitweb?p=x2gobroker.git;a=commitdiff;h=52a8aeef7311e38... The issue will most likely be fixed in src:x2gobroker (0.0.4.2). light+love X2Go Git Admin (on behalf of the sender of this mail) --- commit 52a8aeef7311e384889d07537183e6b4c8b695a7 Author: Mihai Moldovan <ionic@ionic.de> Date: Tue Apr 21 14:44:23 2020 +0200 debian/x2gobroker-ssh.postinst: only reload nscd configuration if daemon is installed and actually running. Fixes: #1458. Based on a patch by Stefan Baur. diff --git a/debian/changelog b/debian/changelog index 85985e9..1d07af6 100644 --- a/debian/changelog +++ b/debian/changelog @@ -13,6 +13,9 @@ x2gobroker (0.0.4.2-0x2go1) UNRELEASED; urgency=medium * New upstream version (0.0.4.2): * debian/control: + Add psproc dependency to x2gobroker-ssh. We'll need it later on. + * debian/x2gobroker-ssh.postinst: + + Only reload nscd configuration if daemon is installed and actually + running. Fixes: #1458. Based on a patch by Stefan Baur. -- X2Go Release Manager <git-admin@x2go.org> Mon, 22 Apr 2019 12:31:49 +0200
Processing commands for control@bugs.x2go.org:
tag #1458 pending Bug #1458 [x2gobroker-ssh] unattended Debian installations (using preseed) fail when x2gobroker-ssh is involved Added tag(s) pending. fixed #1458 0.0.4.2 Bug #1458 [x2gobroker-ssh] unattended Debian installations (using preseed) fail when x2gobroker-ssh is involved There is no source info for the package 'x2gobroker-ssh' at version '0.0.4.2' with architecture '' Unable to make a source version for version '0.0.4.2' Marked as fixed in versions 0.0.4.2. thanks Stopping processing here.
Please contact me if you need assistance.
1458: bugs.x2go.org/cgi-bin/bugreport.cgi?bug=1458 X2Go Bug Tracking System Contact owner@bugs.x2go.org with problems
-
Mihai Moldovan -
Mike Gabriel -
Stefan Baur -
Ulrich Sibiller -
Ulrich Sibiller -
X2Go Bug Tracking System