Bug#726472: [Pkg-samba-maint] Bug#726472: share passwords not working after upgrade from samba3
Hi Steve, On Fri, Nov 01, 2013 at 04:31:36PM -0700, Steve Langasek wrote: On Fri, Nov 01, 2013 at 01:20:35PM +0100, Ivo De Decker wrote: With the conflicts, samba-libs can only be installed once the old packages (samba, winbind, samba-common-bin [for smbpasswd and net] and libpam-smbpass) are no longer present. During the upgrade, this means they will already be upgraded (on new installations, there is no issue). There is no race condition, however, as none of these new versions will work, because they are all linked against libraries in samba-libs, which don't exist yet. So smbd and winbind will not be running, and will be unable to run (even if the admin tries to start them). The pam module also fails silently (this doesn't prevent login). This doesn't prevent login *in the default configuration*. You cannot assume that the pam module is only being used via the stock pam profile - if someone has configured their machine to use pam_smbpass as the primary login method, this will break horribly. I added the code from samba-libs.preinst to libpam-smbpass.preinst as well. Given that Jelmer convinced me yesterday to add a link instead of doing a simple move, the code is idempotent, so it doesn't matter if it runs multiple times in multiple maintainer scripts. When the preinst fails, libpam-smbpass will not be upgraded, and the old version will stay on the system. In samba 3.6, libpam-smbpass was self-contained and didn't need any shared libraries from other samba packages. This means that the old libpam-smbpass module will keep working, even when the upgrade fails: users are still able to login to pam-enabled services (like ssh) using credentials that are only stored in passdb.tdb. The only scenario where this is relevant, is when the tdb-files in /var/lib/samba/private already exist before the upgrade. This is only possible if we broke something in some older version of samba (I still haven't found any evidence of anything referencing /var/lib/samba/private in our old packages). In that case, the only thing we can do is fail with a clear message, instead of silently using the wrong user data. With these latest changes, the pam module will keep working, even in this case. That said, given that a race condition during the upgrade is easily reproducible, I am pretty convinced that the problem in the original submission of this bug was caused by such a race condition. That case should be fixed by the earlier changes. Cheers, Ivo -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#726472: [Pkg-samba-maint] Bug#726472: share passwords not working after upgrade from samba3
On Sat, 2013-11-02 at 11:27 +0100, Ivo De Decker wrote: Hi Steve, I added the code from samba-libs.preinst to libpam-smbpass.preinst as well. Given that Jelmer convinced me yesterday to add a link instead of doing a simple move, the code is idempotent, so it doesn't matter if it runs multiple times in multiple maintainer scripts. When the preinst fails, libpam-smbpass will not be upgraded, and the old version will stay on the system. In samba 3.6, libpam-smbpass was self-contained and didn't need any shared libraries from other samba packages. This means that the old libpam-smbpass module will keep working, even when the upgrade fails: users are still able to login to pam-enabled services (like ssh) using credentials that are only stored in passdb.tdb. The only scenario where this is relevant, is when the tdb-files in /var/lib/samba/private already exist before the upgrade. This is only possible if we broke something in some older version of samba (I still haven't found any evidence of anything referencing /var/lib/samba/private in our old packages). In that case, the only thing we can do is fail with a clear message, instead of silently using the wrong user data. With these latest changes, the pam module will keep working, even in this case. That said, given that a race condition during the upgrade is easily reproducible, I am pretty convinced that the problem in the original submission of this bug was caused by such a race condition. That case should be fixed by the earlier changes. Thank you so much for you diligent attention to this difficult problem. Andrew Bartlett -- Andrew Bartletthttp://samba.org/~abartlet/ Authentication Developer, Samba Team http://samba.org -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#726472: [Pkg-samba-maint] Bug#726472: share passwords not working after upgrade from samba3
Hi Andrew, On Thu, Oct 31, 2013 at 09:26:43AM +1300, Andrew Bartlett wrote: If these assumptions are correct (can someone confirm that?), we only need to deal with passdb.tdb. If we can find a way to work around that race condition, we could do that move as well. Could we ensure the pam module is disabled in .preinst and conditionally re-installed in a .postinst? Excellent question! We can do this with conflicts in samba-libs. Also, is this .postinst on the right package anyway? Shouldn't it be on whatever package actually references passdb.tdb, such as samba-libs (presumably that owns libpdb)? Correct. I have patches (which I will test some more and commit soon), which do the move in the samba-libs preinst. Together with a conflict on the earlier version of the packages which accessed the tdb files directly, this should fix the problem: With the conflicts, samba-libs can only be installed once the old packages (samba, winbind, samba-common-bin [for smbpasswd and net] and libpam-smbpass) are no longer present. During the upgrade, this means they will already be upgraded (on new installations, there is no issue). There is no race condition, however, as none of these new versions will work, because they are all linked against libraries in samba-libs, which don't exist yet. So smbd and winbind will not be running, and will be unable to run (even if the admin tries to start them). The pam module also fails silently (this doesn't prevent login). In the samba-libs preinst, we can fix the location of the files without any risk, as there is nothing on the system which accesses the affected tdb files. The move happens before samba-libs is installed, so before any of the binaries mentioned above are able to run, the tdb files are already in the right location. Cheers, Ivo -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#726472: [Pkg-samba-maint] Bug#726472: share passwords not working after upgrade from samba3
On Fri, Nov 01, 2013 at 01:20:35PM +0100, Ivo De Decker wrote: With the conflicts, samba-libs can only be installed once the old packages (samba, winbind, samba-common-bin [for smbpasswd and net] and libpam-smbpass) are no longer present. During the upgrade, this means they will already be upgraded (on new installations, there is no issue). There is no race condition, however, as none of these new versions will work, because they are all linked against libraries in samba-libs, which don't exist yet. So smbd and winbind will not be running, and will be unable to run (even if the admin tries to start them). The pam module also fails silently (this doesn't prevent login). This doesn't prevent login *in the default configuration*. You cannot assume that the pam module is only being used via the stock pam profile - if someone has configured their machine to use pam_smbpass as the primary login method, this will break horribly. -- Steve Langasek Give me a lever long enough and a Free OS Debian Developer to set it on, and I can move the world. Ubuntu Developerhttp://www.debian.org/ slanga...@ubuntu.com vor...@debian.org signature.asc Description: Digital signature
Bug#726472: [Pkg-samba-maint] Bug#726472: share passwords not working after upgrade from samba3
Hi Andrew, On Wed, Oct 30, 2013 at 11:34:25AM +1300, Andrew Bartlett wrote: That'll also cause some confusion though, as those files will be in sysstatedir on debian but in privatedir on other systems... I'm not sure that will work either. There are really only 3 databases that matter, because schannel_store.tdb will eventually regenerate (client machines forced to 'log in' with a NETLOGON serverAuthenticate). passdb.tdb, secrets.tdb, idmap2.tdb. We don't necessarily need to move them all at the same time (although moving only some of them would probably cause even more confusion). passdb.tdb is what is tripping us up and got us here, but secrets.tdb will cause us more pain in 'fixing' this. The issue is secrets.tdb must be in the same directory as secrets.ldb, because we keep them in sync when secrets.ldb is updated. This allows -P to work in tools no matter the code origin. Is secrets.tdb used outside of smbd? The only case I know of is smbpasswd, running as root, so that shouldn't be an issue. If there are no other uses outside smbd, there is no race condition when we move it in samba.postinst, because smbd won't be running. As for idmap2.tdb, it seems that's only being used from winbindd, and from the net command, running as root. So if we move that in winbind.postinst, it should be fine too. If these assumptions are correct (can someone confirm that?), we only need to deal with passdb.tdb. If we can find a way to work around that race condition, we could do that move as well. Cheers, Ivo -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#726472: [Pkg-samba-maint] Bug#726472: share passwords not working after upgrade from samba3
On Wed, 2013-10-30 at 10:22 +0100, Ivo De Decker wrote: Hi Andrew, On Wed, Oct 30, 2013 at 11:34:25AM +1300, Andrew Bartlett wrote: That'll also cause some confusion though, as those files will be in sysstatedir on debian but in privatedir on other systems... I'm not sure that will work either. There are really only 3 databases that matter, because schannel_store.tdb will eventually regenerate (client machines forced to 'log in' with a NETLOGON serverAuthenticate). passdb.tdb, secrets.tdb, idmap2.tdb. We don't necessarily need to move them all at the same time (although moving only some of them would probably cause even more confusion). passdb.tdb is what is tripping us up and got us here, but secrets.tdb will cause us more pain in 'fixing' this. The issue is secrets.tdb must be in the same directory as secrets.ldb, because we keep them in sync when secrets.ldb is updated. This allows -P to work in tools no matter the code origin. Is secrets.tdb used outside of smbd? The only case I know of is smbpasswd, running as root, so that shouldn't be an issue. If there are no other uses outside smbd, there is no race condition when we move it in samba.postinst, because smbd won't be running. Yes, it is. Any passdb interaction will first try to generate a domain SID in secrets.tdb. As for idmap2.tdb, it seems that's only being used from winbindd, and from the net command, running as root. So if we move that in winbind.postinst, it should be fine too. That is much more likely to be safe. If these assumptions are correct (can someone confirm that?), we only need to deal with passdb.tdb. If we can find a way to work around that race condition, we could do that move as well. Could we ensure the pam module is disabled in .preinst and conditionally re-installed in a .postinst? Also, is this .postinst on the right package anyway? Shouldn't it be on whatever package actually references passdb.tdb, such as samba-libs (presumably that owns libpdb)? Andrew Bartlett -- Andrew Bartlett http://samba.org/~abartlet/ Authentication Developer, Samba Team http://samba.org Samba Developer, Catalyst IT http://catalyst.net.nz -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#726472: [Pkg-samba-maint] Bug#726472: share passwords not working after upgrade from samba3
On Tue, 2013-10-29 at 00:35 +0100, Jelmer Vernooij wrote: On Mon, Oct 28, 2013 at 10:00:12PM +0100, Ivo De Decker wrote: Hi, On Mon, Oct 21, 2013 at 10:37:49PM +1300, Andrew Bartlett wrote: Ok. I think we need to undo this /var/lib/samba/private nonsense. It is a pointless and imperfect migration (as shown by this bug report), and the only rationale upstream ever gave for keeping these files in a separate private directory is some stupid and ancient target OS that couldn't properly set per-file permissions. Debian users have been using /var/lib/samba exclusively for the better part of a decade; migrating to this private/ directory adds no value for our users. In the alternate, the only reason this happened now is because we are finally having the Debian packages follow where upstream has decided to put the files. Having different packages moving files around to different places only increases user confusion, and creates 'only on Debian' bugs. For example, a significant number of issues came about as folks tried to divine if a particular TDB was short, medium or long-term, when no such separation existed in the code. We (upstream) have gone to significant effort to integrate the FHS changes that have been proposed via Debian, I can only ask that having got to a mutually agreed state, that Debian not change it again, having once more Debian packages special and different. I had a long talk with Jelmer yesterday about this bug. Here are a number of considerations from that discussion: First of all, it certainly isn't clear that the issue is caused by a mistake in unstable some time ago. I haven't been able to find any indication for that (yet) in any of the samba versions available on snapshot.debian.org. We've been using /etc/samba as private dir from the first 3.2 upload to the last 3.6 upload and there is no apparent usage of /var/lib/samba/private anywhere in the code of any of these versions. It is quite possible that the issue is triggered by a race condition in the tdb-handling (especially for passdb.tdb), which can result in the creation of the wrong tdb file during the upgrade, which messes up our move. This could be caused by the usage of the 'smbpasswd' command or the pam-smbpass pam module. I haven't had the time yet to create a situation where I can trigger the race condition. There are a number of ways to work around these possible race conditions during at the time of the move. An incomplete list of ideas (some of these would have to be combined to get to a complete fix): - Patch the passdb tdb module to do the move, instead of having it in samba.postinst. This isn't completely unprecedented, as the move of MACHINE.SID was supported by the samba code. - Track the fact that the move was done, by adding some field to the new tdb file. This might cause issues in tools that don't handle unknown fields. The samba4 upgrade script might be one of those. - Track the fact that the move was done, by making sure the old tdb can't be used anymore. This could be done by changing the version number of the old tdb to something non-existent. - Do some kind of merge of both tdb files, if they both exist. - When both tdb files exist, have a debconf question that gives some information about both (eg number of users), and ask the user to choose between them. Any decent implementation will require some careful planning and probably quite some discussion as well. It is clear that the new samba 4.x packages provide a nice step forward compared to the samba 3.6 packages, and these changes shouldn't be blocked by this problem. Therefore we are proposing to postpone this move for now. Moving the private dir to /var/lib/samba would cause an issue with the files kept by samba4 in /var/lib/samba/private. So the only obvious way forward would be to have a new version of our previous patch, which moves the 4 affected files back to /var/lib/samba. I really regret having to propose this, as I very much would like to avoid reintroducing a patch like that, but at this point, I don't really see any other short term options. So the proposal is to reintroduce something like the old patch, and try to get that version into shape for testing/jessie. After that, we can figure out how to get this move done the right way. An alternative solution might be to just move those four files that already existed back in 3.x back from privatedir to sysstatedir and to not revert the entire move to privatedir. That'll also cause some confusion though, as those files will be in sysstatedir on debian but in privatedir on other systems... I'm not sure that will work either. There are really only 3 databases that matter, because schannel_store.tdb will eventually regenerate
Bug#726472: [Pkg-samba-maint] Bug#726472: share passwords not working after upgrade from samba3
On Mon, Oct 28, 2013 at 10:00:12PM +0100, Ivo De Decker wrote: Hi, On Mon, Oct 21, 2013 at 10:37:49PM +1300, Andrew Bartlett wrote: Ok. I think we need to undo this /var/lib/samba/private nonsense. It is a pointless and imperfect migration (as shown by this bug report), and the only rationale upstream ever gave for keeping these files in a separate private directory is some stupid and ancient target OS that couldn't properly set per-file permissions. Debian users have been using /var/lib/samba exclusively for the better part of a decade; migrating to this private/ directory adds no value for our users. In the alternate, the only reason this happened now is because we are finally having the Debian packages follow where upstream has decided to put the files. Having different packages moving files around to different places only increases user confusion, and creates 'only on Debian' bugs. For example, a significant number of issues came about as folks tried to divine if a particular TDB was short, medium or long-term, when no such separation existed in the code. We (upstream) have gone to significant effort to integrate the FHS changes that have been proposed via Debian, I can only ask that having got to a mutually agreed state, that Debian not change it again, having once more Debian packages special and different. I had a long talk with Jelmer yesterday about this bug. Here are a number of considerations from that discussion: First of all, it certainly isn't clear that the issue is caused by a mistake in unstable some time ago. I haven't been able to find any indication for that (yet) in any of the samba versions available on snapshot.debian.org. We've been using /etc/samba as private dir from the first 3.2 upload to the last 3.6 upload and there is no apparent usage of /var/lib/samba/private anywhere in the code of any of these versions. It is quite possible that the issue is triggered by a race condition in the tdb-handling (especially for passdb.tdb), which can result in the creation of the wrong tdb file during the upgrade, which messes up our move. This could be caused by the usage of the 'smbpasswd' command or the pam-smbpass pam module. I haven't had the time yet to create a situation where I can trigger the race condition. There are a number of ways to work around these possible race conditions during at the time of the move. An incomplete list of ideas (some of these would have to be combined to get to a complete fix): - Patch the passdb tdb module to do the move, instead of having it in samba.postinst. This isn't completely unprecedented, as the move of MACHINE.SID was supported by the samba code. - Track the fact that the move was done, by adding some field to the new tdb file. This might cause issues in tools that don't handle unknown fields. The samba4 upgrade script might be one of those. - Track the fact that the move was done, by making sure the old tdb can't be used anymore. This could be done by changing the version number of the old tdb to something non-existent. - Do some kind of merge of both tdb files, if they both exist. - When both tdb files exist, have a debconf question that gives some information about both (eg number of users), and ask the user to choose between them. Any decent implementation will require some careful planning and probably quite some discussion as well. It is clear that the new samba 4.x packages provide a nice step forward compared to the samba 3.6 packages, and these changes shouldn't be blocked by this problem. Therefore we are proposing to postpone this move for now. Moving the private dir to /var/lib/samba would cause an issue with the files kept by samba4 in /var/lib/samba/private. So the only obvious way forward would be to have a new version of our previous patch, which moves the 4 affected files back to /var/lib/samba. I really regret having to propose this, as I very much would like to avoid reintroducing a patch like that, but at this point, I don't really see any other short term options. So the proposal is to reintroduce something like the old patch, and try to get that version into shape for testing/jessie. After that, we can figure out how to get this move done the right way. An alternative solution might be to just move those four files that already existed back in 3.x back from privatedir to sysstatedir and to not revert the entire move to privatedir. That'll also cause some confusion though, as those files will be in sysstatedir on debian but in privatedir on other systems... Cheers, Jelmer -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#726472: [Pkg-samba-maint] Bug#726472: share passwords not working after upgrade from samba3
At Sat, 19 Oct 2013 10:16:31 -0700, Steve Langasek wrote: Ok. I think we need to undo this /var/lib/samba/private nonsense. It is a pointless and imperfect migration (as shown by this bug report), and the only rationale upstream ever gave for keeping these files in a separate private directory is some stupid and ancient target OS that couldn't properly set per-file permissions. Debian users have been using /var/lib/samba exclusively for the better part of a decade; migrating to this private/ directory adds no value for our users. I disagree. Putting private stuff in /var/lib/samba/private might be nonsense, but this is what upsteam decided and using the same location as upstream is far from pointless. In my opinion it is the other way around and it would be a pointless deviation from upstream to continue putting the files in /var/lib/samba instead of /var/lib/samba/private. And the only reason I can think of that the migration is imperfect is that we already messed this up earlier, but that would only be an argument for not continuing with this deviation in my eyes. And if we indeed messed up then we need to clean it up regardless of what directory we are going to put the files in. Kind regards, Jeroen Dekkers -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
