Merge ~ahasenack/ubuntu/+source/openssh:openssh-split-unique-gssapi into ~canonical-server/ubuntu/+source/openssh:openssh-split-unique-gssapi

[Andreas Hasenack]

This was merged already. This MP is for history purpposes.

It's based on work from dannf to split the gssapi-new-unique and fedora patches so they apply more easily. These are the first two commits from Dannf, and start on top of pkg/ubuntu/jammy-devel:

98efccb32 Separate out the changes that comes from Fedora's keyex patch modifications
1ab8bcf2b add gssapi-new-unique.patch

Then come my changes which do:
- new bin:openssh-server-default-ccache package, produced from a new build with the two extra gssapi patches applied
- update-alternatives support for bin:openssh-server and bin:openssh-server-default-ccache
- 3 new DEP8 tests for this specific new functionality, and another run of an existing DEP8 test ("regress") but with the new openssh-server-default-ccache package instead of just openssh-server

The versioning scheme I'm envisioning is:

  ${base}+ccache1

Where ${base} is the jammy ubuntu version of the package, i.e., 1:8.9p1-3ubuntu0.1 currently.

For PPAs, it will be the usual ${base}+ccache1~ppaN

Updates to this extra packaging will increase the ccache1 suffix to ccache2, ccache3, etc.

I welcome suggestions on this versioning as well :)

Launchpad recipe: https://code.launchpad.net/~ahasenack/+recipe/openssh-ccache-testing

Using another ppa, which is what I plan to use for real for development of this package:
https://code.launchpad.net/~ahasenack/+archive/ubuntu/openssh-ccache-testing

[Andreas Hasenack (ahasenack)]

I got a suggestion that works around https://bugs.launchpad.net/ubuntu/+source/git-build-recipe/+bug/2020386: move the two patches we need to apply manually to debian/*, out of debian/patches, but still inside debian/. That will be preserved when launchpad builds the package from a recipe and converts it to a native package. I'll work on that, but it's a minimal change to this MP, and shouldn't affect the rest of the review points.

[Andreas Hasenack (ahasenack)]

The recipe build worked with this branch: https://code.launchpad.net/~ahasenack/+recipe/openssh-ccache-testing

[Andreas Hasenack (ahasenack)]

Tests are green, just s390x that is queued up (only arch with a queue, according to https://autopkgtest.ubuntu.com/running):

Results: (from http://autopkgtest.ubuntu.com/results/autopkgtest-jammy-ahasenack-openssh-ccache/?format=plain)
  openssh @ amd64:
    http://autopkgtest.ubuntu.com/results/autopkgtest-jammy-ahasenack-openssh-ccache/jammy/amd64/o/openssh/20230523_191309_762b2@/log.gz
    23.05.23 19:13:09 ✅ Triggers: openssh/1:8.9p1-3ubuntu0.1+ccache1~ppa1
  openssh @ arm64:
    http://autopkgtest.ubuntu.com/results/autopkgtest-jammy-ahasenack-openssh-ccache/jammy/arm64/o/openssh/20230523_192947_f6453@/log.gz
    23.05.23 19:29:47 ✅ Triggers: openssh/1:8.9p1-3ubuntu0.1+ccache1~ppa1
  openssh @ armhf:
    http://autopkgtest.ubuntu.com/results/autopkgtest-jammy-ahasenack-openssh-ccache/jammy/armhf/o/openssh/20230523_191329_f6453@/log.gz
    23.05.23 19:13:29 ✅ Triggers: openssh/1:8.9p1-3ubuntu0.1+ccache1~ppa1
  openssh @ ppc64el:
    http://autopkgtest.ubuntu.com/results/autopkgtest-jammy-ahasenack-openssh-ccache/jammy/ppc64el/o/openssh/20230523_190557_762b2@/log.gz
    23.05.23 19:05:57 ✅ Triggers: openssh/1:8.9p1-3ubuntu0.1+ccache1~ppa1
Running: (none)
Waiting:
    Q-num pkg release arch ppa trigger
    80 openssh jammy s390x ahasenack/openssh-ccache openssh/1:8.9p1-3ubuntu0.1+ccache1~ppa1

[dann frazier (dannf)]

Thanks for this work Andreas - esp. on the test cases! Aside from a couple of nits inline, one concern I have is that the unpatched sshd will fail to start if KerberosUniqueCCache is set:

line 1: Bad configuration option: KerberosUniqueCCache

Would it make sense to have openssh-server-default-ccache install a config file, say:
  /etc/ssh/sshd_config.d/openssh-server-default.ccache.conf.disabled
That contains a commented out #KerberosUniqueCCache config (would aide discoverability),and then a symlink that allows the "Include /etc/ssh/sshd_config.d/*.conf" to pick it up:
    openssh-server-default.ccache.conf -> openssh-server-default-ccache.conf.disabled

The symlink would always be removed in uninstall, leaving any KerberosUniqueCCache setting impotent.

Also, do we need to restart sshd after an o-s-d-c install or removal?

[Andreas Hasenack (ahasenack)]

I'll check options regarding the config file. We already have a "ExecStartPre=/usr/sbin/sshd -t" line in the systemd unit that checks the config, and will not start sshd if there are problems, although I'm not sure how much that protects us from a restart.

Regarding the restart after o-s-d-c, I think with socket activation we don't need that, but I'll double check.

[Andreas Hasenack (ahasenack)]

So, I was confused and socket activation is not enbaled in jammy, even though the package ships a ssh.socket unit.

Even so, it doesn't look like a restart is needed, because of the architecture of how sshd works nowadays. There is a main listener process, that spawns sshd children to handle incoming connections. These spawned children will be fresh processes, and they will pick up whatever update-alternatives set the symlink to.

That being said, a restart sounds cleaner, and I'll probably do that.

Regarding the config file, I'm still thinking about options. One problem is that we can switch between the sshd implementations with the update-alternatives --config command, it doesn't have to be with installing and removing packages. Maybe add that config file snippet you suggested as another update-alternatives slave, like the manpage? I'll think about it, but even that would not totally prevent errors. People could still add this option to the main sshd_config file.
If there were a "hook" mechanism we could use, so that whenever update-alternatives is called, we can check if the current config file is ok for the currently selected sshd server, then we could use that.

[Andreas Hasenack (ahasenack)]

The "errors in config file can break sshd" bug is known already, and not specific to this new option we are adding: https://bugs.launchpad.net/bugs/1913810 Still no clean solution in general.

[dann frazier (dannf)]

On Fri, May 26, 2023 at 7:55 AM Andreas Hasenack
<email address hidden> wrote:
> Regarding the config file, I'm still thinking about options. One problem is that we can switch between the sshd implementations with the update-alternatives --config command, it doesn't have to be with installing and removing packages. Maybe add that config file snippet you suggested as another update-alternatives slave, like the manpage? I'll think about it, but even that would not totally prevent errors.

That's a good point. Ideally we'd have
/etc/ssh/sshd_config.d/openssh-server-default.ccache.conf ->
openssh-server-default-ccache.conf.real if the o-s-d-c sshd
alternative is selected, but have *no*
/etc/ssh/sshd_config.d/openssh-server-default.ccache.conf link at all
if the standard sshd alternative is selected. What happens when only 1
alternative option provides a given slave?

btw, instead of just being prescriptive about the problem I was
looking to solve with the snippet + symlink was with configuration
file handling, let me be explicit about the problem I was trying to
solve - I'm not sure if it was obvious. If someone makes
o-s-d-c-specific config changes, we should aim to keep track of them
even after o-s-d-c is removed (but not purged). Here is the use/test
case I had in mind:

1) User installs o-s-d-c
2) User makes some o-s-d-c changes that normal o-s does not parse
(i.e. setting KerberosUniqueCCache)
3) User removes o-s-d-c
# Here o-s should not fail because of KerberosUniqueCCache
4) User reinstalls o-s-d-c
# Here the settings the user made in step #2 should have persisted
5) User purges o-s-d-c
# Now the settings in step #2 should be cleared

> People could still add this option to the main sshd_config file.
> If there were a "hook" mechanism we could use, so that whenever update-alternatives is called, we can check if the current config file is ok for the currently selected sshd server, then we could use that.

Perhaps we can also modify the manpage part of the patch to clarify
that this setting should only be defined in the o-s-d-c-specific
snippet, and not any other sshd config file. That, combined with
including a commented out KerberoseUniqueCCache setting in the snippet
for discoverability feels like a complete solution to me.

Another option might be to just patch the standard sshd to accept but
ignore this config option (and ideally emit a warning instead of
silently ignoring it). But that might cause an upgrade path problem if
we want to retain the option to drop this patch in the future. That
is, if a 22.04 PPA user decided to dist-upgrade to a 24.04 that
included neither this feature nor a patch to recognize and ignore
KerberoseUniqueCCache, then it'd be good for that the KUCC config to
be quarantined off to avoid causing 24.04's sshd to fail to start.

  -dann

[Andreas Hasenack (ahasenack)]

I recognized your "breakage" scenario, step (3) specifically:

3) User removes o-s-d-c
# Here o-s should not fail because of KerberosUniqueCCache

I just don't like to have a solution based solely on *where* that config option was added (a specific file in a .d directory).

> Another option might be to just patch the standard sshd to accept but
> ignore this config option (and ideally emit a warning instead of
> silently ignoring it).

I was briefly excited about this, when I saw that ssh has an option called IgnoreUnknown, but it's only for the client... :/

I'm toying now with a script for ExecStartPre in ssh's service unit file. It could check if the sshd that is about to start recognizes KerberosUniqueCCache or not, and then comment it in the config file if necessary to allow sshd to start. This would avoid the catastrophe of remotely killing sshd and the only means to login on to the system. But won't address your step (4):

4) User reinstalls o-s-d-c
# Here the settings the user made in step #2 should have persisted

I.e., it wouldn't restore KerberosUniqueCCache.

I'm also not a big fan of changing config files automatically like that, but hey, it allows sshd to start.

Finally, I'm also contemplating again the decision to use update-alternatives. I'm wondering what would be different if we had instead conflicting server packages. I think not much, because both packages would still be using the same config files, and we would have the same problem of a config option that only one of the packages understands. The only change is the point in which the sshd binary changes, now it's at install/remove time, where we do have hooks (preinst/postinst), and not via update-alternatives.

> Perhaps we can also modify the manpage part of the patch

Going back to what this option actually does:

KerberosUniqueCCache:
no (default in patched sshd): new behavior introduced by this patch
yes: same behavior as unpatched sshd

We don't really need this option at the moment, do we? If you don't want the new behavior, stick with openssh-server. If you want the new behavior, install openssh-server-default-ccache. This option really only makes more sense if you have just one server package, and then you can enable or disable the new behavior.

We could perhaps not include it. At the risk of being different and carry regrets about this decision for quite a while.

To summarize, some of the ideas we had, in no particular order, hope I didn't miss any:

a) constrict the KerberosUniqueCCache option to a specific config file, which we can then manipulate (via alternatives, or something else). Document that users should only change the option in that specific file.

b) ExecStartPre and remove the option from the config (wherever it was found) if the current sshd doesn't understand it. Prevents a failing-to-start-sshd, but makes a change to the config file "behind the user's back". Should only happen if mistakes were made. Somewhat easy mistakes, however.

c) Change the handling of unknown options in unpatched sshd so that if KerberosUniqueCCache is present and unknown, the server won't fail to start (like IgnoreUnknown in the ssh client). Patch on top of patch.

d...

[Andreas Hasenack (ahasenack)]

I pushed a check script, this is option (b) from my previous comment.

Unfortunately sshd -t doesn't list all files that might have problems. Looks like it checks the main config file at /etc/ssh/sshd_config, then processes the include directive, and then just highlights one of the included files.

For example, here sshd_config is not even listed, because I added the bad option at the end:
root@j-sshd-dev:~# echo "KerberosUniqueCCache no" >> /etc/ssh/sshd_config
root@j-sshd-dev:~# echo "KerberosUniqueCCache no" >> /etc/ssh/sshd_config.d/gssapi.conf
root@j-sshd-dev:~# echo "KerberosUniqueCCache no" >> /etc/ssh/sshd_config.d/gssapi-2.conf

root@j-sshd-dev:~# sshd -t
/etc/ssh/sshd_config.d/gssapi-2.conf: line 1: Bad configuration option: KerberosUniqueCCache
/etc/ssh/sshd_config.d/gssapi-2.conf: terminating, 1 bad configuration options

If I add it to the top, befire sshd_config includes sshd_config.d/*.conf, then it's listed too:

root@j-sshd-dev:~# vi /etc/ssh/sshd_config
root@j-sshd-dev:~# sshd -t
/etc/ssh/sshd_config: line 1: Bad configuration option: KerberosUniqueCCache
/etc/ssh/sshd_config.d/gssapi-2.conf: line 1: Bad configuration option: KerberosUniqueCCache
/etc/ssh/sshd_config.d/gssapi-2.conf: terminating, 1 bad configuration options

But gssapi.conf is not listed.

If I remove gssapi-2.conf, then gssapi.conf is listed:

root@j-sshd-dev:~# rm /etc/ssh/sshd_config.d/gssapi-2.conf
root@j-sshd-dev:~# sshd -t
/etc/ssh/sshd_config: line 1: Bad configuration option: KerberosUniqueCCache
/etc/ssh/sshd_config.d/gssapi.conf: line 1: Bad configuration option: KerberosUniqueCCache
/etc/ssh/sshd_config.d/gssapi.conf: terminating, 1 bad configuration options

Anyway, in this state, it's able to clean the config up:

# systemctl restart ssh

logs:
May 26 19:57:08 j-sshd-dev systemd[1]: Starting OpenBSD Secure Shell server...
May 26 19:57:08 j-sshd-dev sshd-config-check[50945]: WARNING: Disabling KerberosUniqueCCache option in /etc/ssh/sshd_config
May 26 19:57:08 j-sshd-dev sshd-config-check[50945]: WARNING: Disabling KerberosUniqueCCache option in /etc/ssh/sshd_config.d/gssapi.conf
May 26 19:57:08 j-sshd-dev sshd-config-check[50945]: Testing again
May 26 19:57:09 j-sshd-dev sshd[50960]: Server listening on 0.0.0.0 port 22.
May 26 19:57:09 j-sshd-dev sshd[50960]: Server listening on :: port 22.
May 26 19:57:09 j-sshd-dev systemd[1]: Started OpenBSD Secure Shell server.

[dann frazier (dannf)]

On Fri, May 26, 2023 at 12:25 PM Andreas Hasenack
<email address hidden> wrote:
>
> I recognized your "breakage" scenario, step (3) specifically:
>
> 3) User removes o-s-d-c
> # Here o-s should not fail because of KerberosUniqueCCache
>
> I just don't like to have a solution based solely on *where* that config option was added (a specific file in a .d directory).
>
> > Another option might be to just patch the standard sshd to accept but
> > ignore this config option (and ideally emit a warning instead of
> > silently ignoring it).
>
> I was briefly excited about this, when I saw that ssh has an option called IgnoreUnknown, but it's only for the client... :/
>
> I'm toying now with a script for ExecStartPre in ssh's service unit file. It could check if the sshd that is about to start recognizes KerberosUniqueCCache or not, and then comment it in the config file if necessary to allow sshd to start. This would avoid the catastrophe of remotely killing sshd and the only means to login on to the system. But won't address your step (4):
>
> 4) User reinstalls o-s-d-c
> # Here the settings the user made in step #2 should have persisted
>
> I.e., it wouldn't restore KerberosUniqueCCache.
>
> I'm also not a big fan of changing config files automatically like that, but hey, it allows sshd to start.
>
> Finally, I'm also contemplating again the decision to use update-alternatives. I'm wondering what would be different if we had instead conflicting server packages. I think not much, because both packages would still be using the same config files, and we would have the same problem of a config option that only one of the packages understands. The only change is the point in which the sshd binary changes, now it's at install/remove time, where we do have hooks (preinst/postinst), and not via update-alternatives.
>
> > Perhaps we can also modify the manpage part of the patch
>
> Going back to what this option actually does:
>
> KerberosUniqueCCache:
> no (default in patched sshd): new behavior introduced by this patch
> yes: same behavior as unpatched sshd
>
> We don't really need this option at the moment, do we? If you don't want the new behavior, stick with openssh-server. If you want the new behavior, install openssh-server-default-ccache. This option really only makes more sense if you have just one server package, and then you can enable or disable the new behavior.
>
> We could perhaps not include it. At the risk of being different and carry regrets about this decision for quite a while.

I actually quite like that idea. There's really no reason to try and
support an o-s-d-c that behaves exactly like o-s, and it simplifies
the packaging and upgrade/crossgrade paths a great deal. While it does
mean we presumably need to carry a 3rd patch that reverts support for
KerberosUniqueCCache, I think that should be trivial to maintain.

  -dann

[Andreas Hasenack (ahasenack)]

> I actually quite like that idea. There's really no reason to try and
> support an o-s-d-c that behaves exactly like o-s, and it simplifies
> the packaging and upgrade/crossgrade paths a great deal. While it does
> mean we presumably need to carry a 3rd patch that reverts support for
> KerberosUniqueCCache, I think that should be trivial to maintain.

Ok, let me just push this extra dep8 test for the ExecStartPre check scenario (which we can easily drop/skip), and then look at what it would take to not add the KerberosUniqueCCache option to the config (or bypass it completely).

[Andreas Hasenack (ahasenack)]

> Ok, let me just push this extra dep8 test for the ExecStartPre check scenario
> (which we can easily drop/skip), and then look at what it would take to not
> add the KerberosUniqueCCache option to the config (or bypass it completely).

It doesn't look hard to drop the KerberosUniqueCCache option, and have openssh-server-default-ccache work as if that option was set to "no" all the time (which is the default). But it just occurred to me that it might be desirable to be able to deploy the same sshd_config file to all systems, and not have to differentiate it between ubuntu and, say, fedora, just because ubuntu does not support the config option written out.

[dann frazier (dannf)]

Yeah, that could be convenient. But it doesn't seem reasonable for a user to assume they can always use the same sshd_config cross-distro, if only because sshd versions will differ. And I don't really see a way we can support that while retaining a "return to non-o-s-d-c" crossgrade/upgrade path.

[Andreas Hasenack (ahasenack)]

I pushed restart fixes, so now ssh gets properly restarted upon removal/installation of the -default-ccache package.

[Andreas Hasenack (ahasenack)]

@dannf, I pushed to https://code.launchpad.net/~ahasenack/ubuntu/+source/openssh/+git/openssh/+ref/openssh-split-unique-gssapi-drop-kerberosuniqueccache the changes, on top, to remove the KerberosUniqueCCache config option. I ran the DEP8 tests locally with that modification and it passed, but I haven't exercised that branch as much as the current one here.

So we have these two options out there now:
- this MP: it implements (b) from a previous comment, which "sanitizes" any config file which has KerberosUniqueCCache when that is the sole reason sshd is failing to start;
- the openssh-split-unique-gssapi-drop-kerberosuniqueccache branch, which removes the KerberosUniqueCCache option entirely (but keeps the behavior), so that both openssh-server and openssh-server-default-ccache will NOT recognize it and sshd will fail to start if that option is present, with either package.

[Andreas Hasenack (ahasenack)]

I opted to revert the sshd-config-check change, and its related dep8 test, because it's surprising behavior. I think in the end we should be transparent: openssh-server-default-ccache supports KerberosUniqueCCache, and openssh-server does not. I just pushed, and will trigger a recipe build.

The DEP8 tests in the infrastructure are unfortunately still swamped (https://autopkgtest.ubuntu.com/running shows thousands of packages in the queue). It will take days to run these tests. I'll run them manually locally for now.

[Andreas Hasenack (ahasenack)]

Added README.default-ccache giving more details about the patched sshd server, and d/NEWS highlighting the changes.

[Andreas Hasenack (ahasenack)]

This was delivered as 1:8.9p1-3ubuntu0.1+ccache1. Further MPs will be against this state, in another branch. Closing the MP.