Bug 272282

Summary:

freebsd-update tries to restart sshd although it is not running

Product:

Base System

Reporter:

Daniel Tameling <tamelingdaniel>

Component:

bin

Assignee:

Ed Maste <emaste>

Status:

Closed FIXED

Severity:

Affects Some People

CC:

emaste, grahamperrin, jlduran, otis

Priority:

---

Version:

13.2-RELEASE

Hardware:

Any

OS:

Any

See Also:

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=263489

Bug Depends on:

Bug Blocks:

271607

Attachments:

Description	Flags
rc.subr(8): for service status don't check whether it is enabled	none

Description Daniel Tameling 2023-06-29 12:42:41 UTC

I have not enabled sshd and the output of service sshd status is

Cannot 'status' sshd. Set sshd_enable to YES in /etc/rc.conf or use 'onestatus' instead of 'status'.

But this command evaluates as true:

# if service sshd status >/dev/null 2>/dev/null; then echo "Restarting sshd after upgrade"; fi
Restarting sshd after upgrade

This is the same code that is used in freebsd-update, which in an update a few days ago then tried to restart sshd. Thus, the update progress contained a line with "Set sshd_enable to YES in /etc/rc.conf or use 'onerestart' instead of 'restart'.", which was confusing.

Comment 1 Daniel Tameling 2023-07-01 09:16:30 UTC

I think there are two ways to fix this: 1) freebsd-update could use service sshd onestatus 2) change rc.subr so that it always executes the status branch.

What ends up happening in rc.subr is that for all services that define a rcvar, there is a check whether the service is enabled. If this fails, it tells us to enable the service in rc.conf and returns early with an exit code of 0. One can circumvent this check with onestatus, but I think for the status command this check doesn't really make sense. A little later we would be told that the service is not running anyway, this time returning with a non-zero exit code.

I created a parch that omits the check for the status command.

Comment 2 Daniel Tameling 2023-07-01 09:17:56 UTC

Created attachment 243096 [details]
rc.subr(8): for service status don't check whether it is enabled

Comment 3 Jose Luis Duran freebsd_committer

2023-08-08 21:44:00 UTC

This is a corner case of the fix to bug #263489. Maybe also tag @rc.

Comment 4 Juraj Lutter freebsd_committer

2023-09-16 16:43:16 UTC

Something similar happens also while updating a poudriere jail:

root@ampex:~ # poudriere jail -j 132raa64 -u
[00:00:00] Upgrading using http
Looking up update.FreeBSD.org mirrors... 2 mirrors found.
Fetching metadata signature for 13.2-RELEASE from update1.freebsd.org... done.
Fetching metadata index... done.
Fetching 2 metadata patches.. done.
...
Installing updates...
Restarting sshd after upgrade
Performing sanity check on sshd configuration.
Stopping sshd.
Waiting for PIDS: 1214.
Performing sanity check on sshd configuration.
Starting sshd.
...

And indeed, sshd on the host was restarted.

Sep 16 15:54:04 ampex kernel: Stopping sshd.
Sep 16 15:54:42 ampex sshd[1214]: Server listening on :: port 22.
Sep 16 15:54:42 ampex kernel: Performing sanity check on sshd configuration.
Sep 16 15:54:42 ampex kernel: Starting sshd.
Sep 16 15:54:42 ampex sshd[1214]: Server listening on 0.0.0.0 port 22.

Comment 5 commit-hook freebsd_committer

2023-09-18 09:51:05 UTC

A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=8ee97b1928e75f7f81a870ffb063010778e0a129

commit 8ee97b1928e75f7f81a870ffb063010778e0a129
Author:     Ed Maste <emaste@FreeBSD.org>
AuthorDate: 2023-09-16 20:46:16 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2023-09-18 09:47:01 +0000

    freebsd-update: do not restart sshd when updating jail/basedir

    In 6cd1bc531609 for PR 263489 I changed freebsd-update to restart sshd
    after upgrade, to avoid an upgrade-related incompatibility that made it
    impossible to login.

    This is intended to avoid losing access to remote hosts, and ought not
    apply to upgrading jails (from outside).

    PR:             263489, 272282
    Reported by:    otis
    Reviewed by:    otis, kevans
    MFC after:      3 days
    Sponsored by:   The FreeBSD Foundation
    Fixes: 6cd1bc531609 ("freebsd-update: restart sshd after upgrade")
    Differential Revision: https://reviews.freebsd.org/D41890

 usr.sbin/freebsd-update/freebsd-update.sh | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comment 6 commit-hook freebsd_committer

2023-09-21 14:40:25 UTC

A commit in branch stable/14 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=cce342e21357833892083e865710c4a05603b7b5

commit cce342e21357833892083e865710c4a05603b7b5
Author:     Ed Maste <emaste@FreeBSD.org>
AuthorDate: 2023-09-16 20:46:16 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2023-09-21 14:38:48 +0000

    freebsd-update: do not restart sshd when updating jail/basedir

    In 6cd1bc531609 for PR 263489 I changed freebsd-update to restart sshd
    after upgrade, to avoid an upgrade-related incompatibility that made it
    impossible to login.

    This is intended to avoid losing access to remote hosts, and ought not
    apply to upgrading jails (from outside).

    PR:             263489, 272282
    Reported by:    otis
    Reviewed by:    otis, kevans
    MFC after:      3 days
    Sponsored by:   The FreeBSD Foundation
    Fixes: 6cd1bc531609 ("freebsd-update: restart sshd after upgrade")
    Differential Revision: https://reviews.freebsd.org/D41890

    (cherry picked from commit 8ee97b1928e75f7f81a870ffb063010778e0a129)

 usr.sbin/freebsd-update/freebsd-update.sh | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comment 7 commit-hook freebsd_committer

2023-09-21 14:41:31 UTC

A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=877d15d57931350dcde264de1dabe7fccf769388

commit 877d15d57931350dcde264de1dabe7fccf769388
Author:     Ed Maste <emaste@FreeBSD.org>
AuthorDate: 2023-09-16 20:46:16 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2023-09-21 14:40:03 +0000

    freebsd-update: do not restart sshd when updating jail/basedir

    In 6cd1bc531609 for PR 263489 I changed freebsd-update to restart sshd
    after upgrade, to avoid an upgrade-related incompatibility that made it
    impossible to login.

    This is intended to avoid losing access to remote hosts, and ought not
    apply to upgrading jails (from outside).

    PR:             263489, 272282
    Reported by:    otis
    Reviewed by:    otis, kevans
    MFC after:      3 days
    Sponsored by:   The FreeBSD Foundation
    Fixes: 6cd1bc531609 ("freebsd-update: restart sshd after upgrade")
    Differential Revision: https://reviews.freebsd.org/D41890

    (cherry picked from commit 8ee97b1928e75f7f81a870ffb063010778e0a129)
    (cherry picked from commit cce342e21357833892083e865710c4a05603b7b5)

 usr.sbin/freebsd-update/freebsd-update.sh | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comment 8 commit-hook freebsd_committer

2023-09-21 23:31:22 UTC

A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=ba793728a840041e93e38bcbff4a7233dc63b722

commit ba793728a840041e93e38bcbff4a7233dc63b722
Author:     Daniel Tameling <tamelingdaniel@gmail.com>
AuthorDate: 2023-07-01 08:43:40 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2023-09-21 23:28:40 +0000

    rc.subr: don't require service to be enabled for `status`

    For a service that sets an rcvar, there is a check whether it has been
    enabled before the actual command is executed. If the check fails, one
    gets a message to enable it and the returned exit status is 0.
    However, this is usually undesirable for the status command, which is
    a) supposed to check whether the service is running anyway and
    b) returns a non-zero exit code if that is not the case.
    Thus, skip the check for the status command.

    PR:             272282
    Reviewed by:    emaste
    MFC after:      3 days

 libexec/rc/rc.subr | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comment 9 commit-hook freebsd_committer

2023-09-23 13:09:51 UTC

A commit in branch releng/14.0 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=78534afbcab6e97565c06041fecfe28587e13f2b

commit 78534afbcab6e97565c06041fecfe28587e13f2b
Author:     Ed Maste <emaste@FreeBSD.org>
AuthorDate: 2023-09-16 20:46:16 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2023-09-23 13:08:37 +0000

    freebsd-update: do not restart sshd when updating jail/basedir

    In 6cd1bc531609 for PR 263489 I changed freebsd-update to restart sshd
    after upgrade, to avoid an upgrade-related incompatibility that made it
    impossible to login.

    This is intended to avoid losing access to remote hosts, and ought not
    apply to upgrading jails (from outside).

    PR:             263489, 272282
    Reported by:    otis
    Reviewed by:    otis, kevans
    Sponsored by:   The FreeBSD Foundation
    Fixes: 6cd1bc531609 ("freebsd-update: restart sshd after upgrade")
    Differential Revision: https://reviews.freebsd.org/D41890

    (cherry picked from commit 8ee97b1928e75f7f81a870ffb063010778e0a129)
    (cherry picked from commit cce342e21357833892083e865710c4a05603b7b5)

    Approved by:    re (gjb)

 usr.sbin/freebsd-update/freebsd-update.sh | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comment 10 commit-hook freebsd_committer

2023-09-24 13:20:51 UTC

A commit in branch stable/14 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=f701d9da1d94891dd2edad607a989cad6eb10313

commit f701d9da1d94891dd2edad607a989cad6eb10313
Author:     Daniel Tameling <tamelingdaniel@gmail.com>
AuthorDate: 2023-07-01 08:43:40 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2023-09-24 13:17:43 +0000

    rc.subr: don't require service to be enabled for `status`

    For a service that sets an rcvar, there is a check whether it has been
    enabled before the actual command is executed. If the check fails, one
    gets a message to enable it and the returned exit status is 0.
    However, this is usually undesirable for the status command, which is
    a) supposed to check whether the service is running anyway and
    b) returns a non-zero exit code if that is not the case.
    Thus, skip the check for the status command.

    PR:             272282
    Reviewed by:    emaste

    (cherry picked from commit ba793728a840041e93e38bcbff4a7233dc63b722)

 libexec/rc/rc.subr | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comment 11 commit-hook freebsd_committer

2023-09-24 14:22:05 UTC

A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=95d4529fb06b6121a87ee06b0ab982fe49e3ce82

commit 95d4529fb06b6121a87ee06b0ab982fe49e3ce82
Author:     Daniel Tameling <tamelingdaniel@gmail.com>
AuthorDate: 2023-07-01 08:43:40 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2023-09-24 14:07:40 +0000

    rc.subr: don't require service to be enabled for `status`

    For a service that sets an rcvar, there is a check whether it has been
    enabled before the actual command is executed. If the check fails, one
    gets a message to enable it and the returned exit status is 0.
    However, this is usually undesirable for the status command, which is
    a) supposed to check whether the service is running anyway and
    b) returns a non-zero exit code if that is not the case.
    Thus, skip the check for the status command.

    PR:             272282
    Reviewed by:    emaste

    (cherry picked from commit ba793728a840041e93e38bcbff4a7233dc63b722)
    (cherry picked from commit f701d9da1d94891dd2edad607a989cad6eb10313)

 libexec/rc/rc.subr | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comment 12 commit-hook freebsd_committer

2023-09-24 16:20:25 UTC

A commit in branch releng/14.0 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=20f6af198e728262472bd347d68ea264b8cf37eb

commit 20f6af198e728262472bd347d68ea264b8cf37eb
Author:     Daniel Tameling <tamelingdaniel@gmail.com>
AuthorDate: 2023-07-01 08:43:40 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2023-09-24 16:18:41 +0000

    rc.subr: don't require service to be enabled for `status`

    For a service that sets an rcvar, there is a check whether it has been
    enabled before the actual command is executed. If the check fails, one
    gets a message to enable it and the returned exit status is 0.
    However, this is usually undesirable for the status command, which is
    a) supposed to check whether the service is running anyway and
    b) returns a non-zero exit code if that is not the case.
    Thus, skip the check for the status command.

    PR:             272282
    Reviewed by:    emaste

    (cherry picked from commit ba793728a840041e93e38bcbff4a7233dc63b722)
    (cherry picked from commit f701d9da1d94891dd2edad607a989cad6eb10313)

    Approved by:    re (gjb)

 libexec/rc/rc.subr | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)