openssh bugs - Jan 2019 - [Bug 2953] New: Race during daemon reload may cause to fail to listen on configured ports

https://bugzilla.mindrot.org/show_bug.cgi?id=2953

            Bug ID: 2953
           Summary: Race during daemon reload may cause to fail to listen
                    on configured ports
           Product: Portable OpenSSH
           Version: 7.2p2
          Hardware: amd64
                OS: Linux
            Status: NEW
          Severity: minor
          Priority: P5
         Component: sshd
          Assignee: unassigned-bugs at mindrot.org
          Reporter: mkoutny at suse.com

Created attachment 3222
  --> https://bugzilla.mindrot.org/attachment.cgi?id=3222&action=edit
Prevent restarting while children are listening

This was observed on 7.2p2 on SLES 12 SP2 (4.4.121-92.98-default) but
based
on the code review, I expect the current version is affected too.

There is a short window when forked children are still referencing the
listen
socket after a new client connects. When the parent handles SIGHUP and
reloads
itself, it may fail to bind the new sockets if it hits this window.

Linux manual page socket(7) on SO_REUSEADDR:
> For AF_INET sockets this means that a socket may bind, except when
> there is an active listening socket bound to the address.
I'm attaching a patch (against master, 4a526941) that tackles this
(verified on
reproducer on the system mentioned above).

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2953

Jakub Jelen <jjelen at redhat.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jjelen at redhat.com

--- Comment #1 from Jakub Jelen <jjelen at redhat.com> ---
I confirm that this happens also with our systems when they are under
heavy load. See following bug for simple reproducer

https://bugzilla.redhat.com/show_bug.cgi?id=1643176

This was temporarily resolved by configuring systemd service to
automatically restart the sshd if it failed, but I agree that this
sounds like a better idea except for the issue with blocking for
undefined time for unresponsive (for whatever reason) children.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2953

--- Comment #2 from Michal Koutn? <mkoutny at suse.com> ---
(In reply to Jakub Jelen from comment #1)
> [...] except for the issue with blocking for undefined time for
unresponsive
> (for whatever reason) children.
If the children were unresponsive, it'd make the sshd server eventually
unresponsive nowadays too (because of MaxStartups). Or are you
referring to SIGHUP responsiveness?
That's indeed harmed with this but should the child have issues
reaching close_listen_socks() there are perhaps more severe issues with
the system (IMO).

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2953

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #3222|0                           |1
        is obsolete|                            |
             Status|NEW                         |ASSIGNED
                 CC|                            |djm at mindrot.org,
                   |                            |dtucker at dtucker.net
           Assignee|unassigned-bugs at mindrot.org |djm at mindrot.org
   Attachment #3245|                            |ok?(dtucker at dtucker.net)
              Flags|                            |

--- Comment #3 from Damien Miller <djm at mindrot.org> ---
Created attachment 3245
  --> https://bugzilla.mindrot.org/attachment.cgi?id=3245&action=edit
reuse startup_pipes; fix race with rexeced children too

I don't like the complexity of having another set of fds just for
handling the listen_socks. Instead, this reuses the existing
startup_pipes for signalling, but writing a char over them when the
child process has finished "early startup".

"early startup" includes closing listen_socks but also, for the rexec
case, receiving the re-executed child's state from sshd. This avoids
another race condition: at SIGHUP the listener sshd was exiting before
the child received its configuration.

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2953

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |2915


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2915
[Bug 2915] Tracking bug for 8.0 release
-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2953

--- Comment #4 from Damien Miller <djm at mindrot.org> ---
This patch (with a couple of tweaks) has been committed as
76a24b3fa193a and will be in openssh-8.0. Thanks!

commit 76a24b3fa193a9ca3e47a8779d497cb06500798b (HEAD -> master,
origin/master, origin/HEAD)
Author: djm at openbsd.org <djm at openbsd.org>
Date:   Fri Mar 1 02:32:39 2019 +0000

    upstream: Fix two race conditions in sshd relating to SIGHUP:

    1. Recently-forked child processes will briefly remain listening to
      listen_socks. If the main server sshd process completes its
restart
      via execv() before these sockets are closed by the child
processes
      then it can fail to listen at the desired addresses/ports and/or
      fail to restart.

    2. When a SIGHUP is received, there may be forked child processes
that
      are awaiting their reexecution state. If the main server sshd
      process restarts before passing this state, these child processes
      will yield errors and use a fallback path of reading the current
      sshd_config from the filesystem rather than use the one that sshd
      was started with.

    To fix both of these cases, we reuse the startup_pipes that are
shared
    between the main server sshd and forked children. Previously this
was
    used solely to implement tracking of pre-auth child processes for
    MaxStartups, but this extends the messaging over these pipes to
include
    a child->parent message that the parent process is safe to restart.
This
    message is sent from the child after it has completed its
preliminaries:
    closing listen_socks and receiving its reexec state.

    bz#2953, reported by Michal Koutn?; ok markus@ dtucker@

    OpenBSD-Commit-ID: 7df09eacfa3ce13e9a7b1e9f17276ecc924d65ab

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2953

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2953

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #5 from Damien Miller <djm at mindrot.org> ---
Close bugs fixed in openssh-8.1 release cycle

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2953

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #3245|ok?(dtucker at dtucker.net)    |
              Flags|                            |

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.