Bug#1110033: unblock: openssh/1:10.0p1-6
Hi Colin,
Thanks for your reply.
As mentioned on IRC, I added the unblock, but I'm leaving the bug open for
now, to see if we want to add additional changes.
At least this will allow the change to get into trixie, and should allow us to
collect feedback from users who upgrade in the coming days.
On Fri, Aug 01, 2025 at 12:06:50PM +0100, Colin Watson wrote:
> On Wed, Jul 30, 2025 at 03:22:29PM +0000, Ivo De Decker wrote:
> > On Mon, Jul 28, 2025 at 12:54:40PM +0100, Colin Watson wrote:
> > > The self-diversion approach is a bit alarming, but it limits the scope
> > > of the workaround code to just the affected upgrade scenarios, and the
> > > code is mechanically simple enough even if it requires some careful
> > > thinking. I can't think of any better approaches.
> >
> > I'm leaning towards unblocking this, as it's probably the least bad option. I
> > wonder if there are any corner cases where the result of this change is worse
> > than not doing it.
> >
> > Fortunately, I haven't been able to come up with such a case yet.
> >
> > Some questions:
>
> Thanks. I agree that these are all reasonable questions to consider.
>
> > Can you think of any scenario where the system would end up without a
> > /usr/sbin/sshd binary?
>
> I think this is impossible. The dpkg-divert calls themselves are run with
> --no-rename, so they only touch the dpkg database; dpkg is going to unpack
> either to /usr/sbin/sshd or to /usr/sbin/sshd.session-split, but in either
> case it does so atomically; and we atomically move the diverted file to the
> real location immediately after removing the diversion.
OK.
> > What happens if the system crashes during the upgrade, after the diversion is
> > added, but before it is removed? Will sshd work after reboot (it's possible
> > that sshd wouldn't work in this scenario without the change anyway)? If not,
> > will it work after the upgrade is finished (by an admin connected in a
> > different way)?
>
> I just tested this by restarting the container after the "dpkg --unpack"
> step in the test procedure I gave in my original message to this bug, and
> sshd continues to work fine. Makes sense, since /usr/sbin/sshd hasn't
> changed yet in this scenario.
>
> In fact, this change arguably makes things _more_ resilient in this scenario
> for this particular upgrade: it used to be that if you unpacked the new
> openssh-server before the new libc6 then a system crash would leave you
> without a working sshd until openssh-server is configured again, but with
> this change it works. (The complexity cost is high enough that I wouldn't
> suggest dropping the version guards, though.)
>
> I think the least obvious case is where the system crashes between
> dpkg-divert and mv in the postinst, and that's hard to test precisely. But
> I believe in that case dpkg will still think the package needs to be
> configured, so it will try the dpkg-divert --remove call again, and I've
> tested that that exits zero with just a warning message if the diversion has
> already been removed. So that case should work fine too.
OK.
> > Can you think of a scenario where dpkg thinks the upgrade of openssh-server is
> > done, but the diversion is still there? In that case, even (purging and)
> > reinstalling openssh-server won't help, because the code removing the
> > diversion will no longer be triggered.
>
> While it's hard to give a categorical "that can't happen", I can't currently
> think of such a scenario. I suppose the postinst might be a little more
> resilient against such problems if I did this:
>
> diff --git a/debian/openssh-server.postinst b/debian/openssh-server.postinst
> index c0d43006d..2a68f6f85 100644
> --- a/debian/openssh-server.postinst
> +++ b/debian/openssh-server.postinst
> @@ -116,7 +116,7 @@ if [ "$action" = configure ]; then
> systemctl disable ssh.service
> fi
> # begin-remove-after: released:forky
> - if dpkg --compare-versions "$2" lt-nl 1:9.8p1-1~; then
> + if [ -e /usr/sbin/sshd.session-split ]; then
> # We're ready to restart the listener process so that it
> # executes sshd-session rather than sshd for new
> # connections, so we can remove this diversion now. This
> @@ -128,9 +128,7 @@ if [ "$action" = configure ]; then
> # name.
> dpkg-divert --package openssh-client --remove --no-rename \
> --divert /usr/sbin/sshd.session-split /usr/sbin/sshd
> - if [ -e /usr/sbin/sshd.session-split ]; then
> - mv -f /usr/sbin/sshd.session-split /usr/sbin/sshd
> - fi
> + mv -f /usr/sbin/sshd.session-split /usr/sbin/sshd
> fi
> # end-remove-after
> fi
>
> I haven't tested this as yet, but do you think it would be better? It
> seemed clearest to use the same condition in the preinst and postinst, but I
> could be persuaded either way.
I'm inclined to prefer the version that removes the diversion in all cases
where /usr/sbin/sshd.session-split exists. If that exists, it means the
diversion is still there, and it must be removed, even if the postinst doesn't
think we're upgrading from an older version. If it doesn't exist, there's no
harm in having this code in the postinst.
Maybe it could also be useful to add some specific output when this is
happening. That could make it easier to debug things if unexpected corner
cases were to show up. I don't really have a good suggestion of the conditions
under which it would be good to give additional output (without alarming users
in the standard scenario), though.
Thanks,
Ivo
Reply to: