Please consider dropping /etc/network/if-up.d/openssh-server

Bug #1674330 reported by Martin Pitt on 2017-03-20
14
This bug affects 1 person
Affects Status Importance Assigned to Milestone
openssh (Ubuntu)
Low
David Britton

Bug Description

The /etc/network/if-up.d/openssh-server hack was introduced ten years ago [1] as a response to bug
103436
. At least from today's perspective this isn't justified:

I can't seem to be able to actually reproduce that issue: I can start a VM with no network interfaces, remove the above hack, then start sshd, then bring up an ethernet interface, and I can connect to ssh via ethernet just fine. Also, e. g. Fedora has no counterpart of this hack, and these days a lot of people would complain if that would cause problems, as hotpluggable/roaming network devices are everywhere.

The hack introduces a race: you run into connection errors after bringing up a new interface as sshd stops listening briefly while being reloaded. That's the reason why I looked at it, as this regularly happens in upstream's cockpit integration tests.

Also, /etc/network/if-up.d/ isn't being run when using networkd/netplan, i. e. in more recent Ubuntnu cloud instances. So far this doesn't seem to have caused any issues.

I asked the original reporter of bug 103436 for some details, and to check whether that hack is still necessary. There is actually a proposed patch upstream [2] to use IP_FREEBIND, which is the modern solution to listening to all "future" interfaces as well. But at least for the majority of cases it seems to work fine without that even.

So I wonder if it's time to bury that hack?

[1] https://anonscm.debian.org/cgit/pkg-ssh/openssh.git/commit/?id=ba6b55ed6
[2] https://bugzilla.mindrot.org/show_bug.cgi?id=2512

Martin Pitt (pitti) on 2017-03-20
Changed in openssh (Ubuntu):
importance: Undecided → Low

On Mon, 20 Mar 2017 13:26:35 -0000 Launchpad Bug Tracker
<email address hidden> wrote:
> You have been subscribed to a public bug by Martin Pitt (pitti):
>
> The /etc/network/if-up.d/openssh-server hack was introduced ten
> years ago [1] as a response to bug 103436. At least from today's
> perspective this isn't justified:
>
> I can't seem to be able to actually reproduce that issue: I can
> start a VM with no network interfaces, remove the above hack, then
> start sshd, then bring up an ethernet interface, and I can connect
> to ssh via ethernet just fine.

sshd has no internal support to open and close listening addresses on
its own, so I suspect you're wrong. Why don't you try the actual use
case, which is changing addresses rather than an initial open.

However, I haven't used ubuntu in at least eight years and have no
way to help you.

> Also, e. g. Fedora has no
> counterpart of this hack, and these days a lot of people would
> complain if that would cause problems,

How many people regularly ssh into their laptops on multiple
networks? I would guess very few.

> The hack introduces a race: you run into connection errors after
> bringing up a new interface as sshd stops listening briefly while
> being reloaded.

Well, yah, but when you change networks you're also not listening to
the network. This isn't a race, this is just expected behavior. Even
if sshd did this on its own this would happen.

And it isn't a "hack", this is exactly what ifup/down scripts are for.

Perry
--
Perry E. Metzger <email address hidden>

Colin Watson (cjwatson) wrote :

On Mon, Mar 20, 2017 at 05:14:07PM -0000, Perry E. Metzger wrote:
> And it isn't a "hack", this is exactly what ifup/down scripts are for.

They're useful for giving sysadmins the flexibility to do this sort of
thing locally without too much work, but doing service restarts on
if-{up,down} is an awfully big hammer that's generally better handled
some other way if possible.

Not being the maintainer and not using Ubuntu any more, you might be
unaware of how much work this hack has been to maintain over the years.
I'd certainly support removing it if it can be demonstrated to be safe
to do so (in which I do include your original use case). For example:

  https://bugs.debian.org/502444
  https://bugs.debian.org/756547
  https://bugs.launchpad.net/bugs/1584393

On Sat, 25 Mar 2017 09:57:26 -0000 Colin Watson
<email address hidden> wrote:
> On Mon, Mar 20, 2017 at 05:14:07PM -0000, Perry E. Metzger wrote:
> > And it isn't a "hack", this is exactly what ifup/down scripts are
> > for.
>
> They're useful for giving sysadmins the flexibility to do this sort
> of thing locally without too much work, but doing service restarts
> on if-{up,down} is an awfully big hammer that's generally better
> handled some other way if possible.

So why don't you get a laptop and try it out? Using a virtual machine
will not tell you what the behavior is if the network address is
forcibly changed on the machine, and there are other confounding
circumstances here like loss of network carrier when you change
location etc. (It may be possible to conduct a principled experiment
with virtual machines but it will not be particularly easy.)

You will have to make sure that the daemon continues to permit remote
logins on every new address it acquires.

> Not being the maintainer and not using Ubuntu any more, you might be
> unaware of how much work this hack has been to maintain over the
> years.

Many things are unpleasant to maintain but provide necessary
functionality. Again, what you should do is conduct an actual test.

Perry
--
Perry E. Metzger <email address hidden>

Download full text (4.3 KiB)

Hi,
trying to get new life in bugs dormant for quite a while - in addition hte had not too much consensus before.

I'll just try to recreate the case in a virt env (as it is easy for everyone to reproduce) in the way I understood the case - I beg your pardon if that isn't the case you meant - please help if that is true.

# get a simple guest
$ uvt-kvm create --password=ubuntu artful-testnosshhook release=artful arch=amd64 label=daily
# gets it's IP
$ virsh domifaddr artful-testnosshhook
 Name MAC address Protocol Address
-------------------------------------------------------------------------------
 vnet1 52:54:00:38:7b:4e ipv4 192.168.122.236/24
# Login via SSH
$ ssh ubuntu@192.168.122.236
# Login via Console in case network/ssh connection dies
$ virsh console artful-testnosshhook

# Initially both logins work and there is no ifupdown installed (so hooks won't be working)
ubuntu@artful-testnosshhook:~$ dpkg -S /sbin/ifup
dpkg-query: no path found matching pattern /sbin/ifup
ubuntu@artful-testnosshhook:~$ ll /sbin/ifup
ls: cannot access '/sbin/ifup': No such file or directory

# Before changing IPs we have the binds on 0.0.0.0 and :::, as well as my own active ssh connection
$ sudo netstat -apn | grep ssh
tcp 0 0 0.0.0.0:22 0.0.0.0:* LISTEN 967/sshd
tcp 0 0 192.168.122.236:22 192.168.122.1:36062 ESTABLISHED 1043/sshd: ubuntu [
tcp6 0 0 :::22 :::* LISTEN 967/sshd

# Currently I have
$ ip addr show
2: ens3:
   inet 192.168.122.236/24 brd 192.168.122.255 scope global dynamic ens3

#Thet next IP is free, so just bluntly change the IP (without up/down/dhcp this time)
$ ip addr add 192.168.122.237/24 broadcast 192.168.122.255 dev ens3
$ sudo ip addr del 192.168.122.236/24 dev ens3

Of course my ssh session is dead, ssh is still listening on all interfaces but didn't pick up .237 as the ssh code just doesn't do so (like IP_FREEBIND).

So let's be more fair and use dhcp to reassign (more realistic case).
# Edit the range so the current IP is no more in it and then restart (Host)
# I had a range like this <range start='192.168.122.2' end='192.168.122.200'/>
#And started with leases:
$ virsh net-dhcp-leases default
2017-09-22 10:39:55 52:54:00:38:7b:4e ipv4 192.168.122.30/24 artful-testnosshhook
# Now lets add a new range and remove the current one without brekaing the bridge (destroy/start would break it)
$ virsh net-update default add ip-dhcp-range "<range start='192.168.122.220' end='192.168.122.230'/>" --live --config
$ virsh net-update default delete ip-dhcp-range "<range start='192.168.122.2' end='192.168.122.200'/>" --live --config

# Then in the guest renew the lease and networking
# this will only ADD the new ip
$ sudo dhclient -r ens3; sudo dhclient -v ens3
# so instead do
$ sudo systemctl restart systemd-networkd
# After a few seconds we have a new IP and ssh is gone on "the old IP"
# but a login to the new one works.

# Now this might be due to restarting systemd-networkd
# It could be "too much" for the test still and its dependencies reload sshd?
# So instea...

Read more...

Andreas Hasenack (ahasenack) wrote :

Hm, cosmic rays, my comment wasn't saved :/

I also did some testing and the only case that won't work without either the hook, or using the IP_FREEBIND socket option, is when ListenAddress is set to an IP that doesn't exist yet. Meaning, sshd won't listen on that IP when it comes up.

Maybe people caught in this corner case should use 0.0.0.0 (or the IPv6 equivalent) for ListenAddress and firewall rules to control which traffic can reach the sshd daemon. That's the only way in artful now anyway.

Ok, that said:
1. the hook is no more active
2. it works without it by sshd re-binding interfaces when it comes up when it is an open ListenAddress

@cjwatson - I think you can drop this Delta when you merge openssh next time, what do you think?

Colin Watson (cjwatson) wrote :

It's not a "delta", it's code in the packaging. Feel free to send me a tested patch to drop the relevant pieces of it and I'll consider it.

David Britton (davidpbritton) wrote :

This is a debdiff for Bionic applicable to 7.6p1-4. I built the binary packages in pbuilder
and they build, upgraded and installed successfully.

Tested that the ifup hook was removed on upgrade, and also on install it was never installed.

I went through a series of tests on a laptop with a wireless interface and desktop with some somewhat complicated network layout.

I did not run into any unexpected results in the testing. After every network event I could trigger, I did not see a daemon restart, and also the ssh server was still reachable on all exposed interfaces I tried, as it was on 127.0.0.1.

Thanks for your consideration of this patch.

Changed in openssh (Ubuntu):
status: New → In Progress
assignee: nobody → David Britton (davidpbritton)

The attachment "openssh-7.6p1-4ubuntu1.debdiff" seems to be a debdiff. The ubuntu-sponsors team has been subscribed to the bug report so that they can review and hopefully sponsor the debdiff. If the attachment isn't a patch, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are member of the ~ubuntu-sponsors, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issue please contact him.]

tags: added: patch
Launchpad Janitor (janitor) wrote :
Download full text (4.0 KiB)

This bug was fixed in the package openssh - 1:7.9p1-1

---------------
openssh (1:7.9p1-1) unstable; urgency=medium

  * New upstream release (https://www.openssh.com/txt/release-7.9):
    - ssh(1), sshd(8): allow most port numbers to be specified using service
      names from getservbyname(3) (typically /etc/services; closes:
      #177406).
    - ssh(1): allow the IdentityAgent configuration directive to accept
      environment variable names. This supports the use of multiple agent
      sockets without needing to use fixed paths.
    - sshd(8): support signalling sessions via the SSH protocol. A limited
      subset of signals is supported and only for login or command sessions
      (i.e. not subsystems) that were not subject to a forced command via
      authorized_keys or sshd_config.
    - ssh(1): support "ssh -Q sig" to list supported signature options.
      Also "ssh -Q help" to show the full set of supported queries.
    - ssh(1), sshd(8): add a CASignatureAlgorithms option for the client and
      server configs to allow control over which signature formats are
      allowed for CAs to sign certificates. For example, this allows
      banning CAs that sign certificates using the RSA-SHA1 signature
      algorithm.
    - sshd(8), ssh-keygen(1): allow key revocation lists (KRLs) to revoke
      keys specified by SHA256 hash.
    - ssh-keygen(1): allow creation of key revocation lists directly from
      base64-encoded SHA256 fingerprints. This supports revoking keys using
      only the information contained in sshd(8) authentication log messages.
    - ssh(1), ssh-keygen(1): avoid spurious "invalid format" errors when
      attempting to load PEM private keys while using an incorrect
      passphrase.
    - sshd(8): when a channel closed message is received from a client,
      close the stderr file descriptor at the same time stdout is closed.
      This avoids stuck processes if they were waiting for stderr to close
      and were insensitive to stdin/out closing (closes: #844494).
    - ssh(1): allow ForwardX11Timeout=0 to disable the untrusted X11
      forwarding timeout and support X11 forwarding indefinitely.
      Previously the behaviour of ForwardX11Timeout=0 was undefined.
    - sshd(8): when compiled with GSSAPI support, cache supported method
      OIDs regardless of whether GSSAPI authentication is enabled in the
      main section of sshd_config. This avoids sandbox violations if GSSAPI
      authentication was later enabled in a Match block.
    - sshd(8): do not fail closed when configured with a text key revocation
      list that contains a too-short key.
    - ssh(1): treat connections with ProxyJump specified the same as ones
      with a ProxyCommand set with regards to hostname canonicalisation
      (i.e. don't try to canonicalise the hostname unless
      CanonicalizeHostname is set to 'always').
    - ssh(1): fix regression in OpenSSH 7.8 that could prevent public-key
      authentication using certificates hosted in a ssh-agent(1) or against
      sshd(8) from OpenSSH <7.8 (LP: #1790963).
    - All: support building against the openssl-1.1 API (releases 1.1.0g and
      later). The openssl-1.0 AP...

Read more...

Changed in openssh (Ubuntu):
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.