[Buildroot] [PATCH 1/1] package/openssh: refactor S50sshd

[Buildroot] [PATCH 1/1] package/openssh: refactor S50sshd

[Nicolas Cavallari]

Make it look more like the example S01syslogd shell script in
docs/manual/adding-packages-directory.txt.

Functionnally, it changes the following:
- Options can be defined in /etc/default/sshd
- "S50sshd stop" will no longer kill active SSH sessions or sshd daemons
  that run inside a container or chroot.  It is now safe to stop or
  restart openssh inside an SSH session.
- "S50sshd restart" now sleeps between stop and start, reducing the
  probability of failures caused by sshd taking too much time to stop.
- "S50sshd reload" will send a SIGHUP instead of restarting sshd.
- /var/lock/sshd is no longer created.  The daemon does not use it.
  The only reference to /var/lock is in contrib/redhat/sshd.init, which
  uses it as a way to test if sshd is (supposed to be) running.

Signed-off-by: Nicolas Cavallari <nicolas.cavallari@green-communications.fr>
---
 package/openssh/S50sshd | 75 +++++++++++++++++++++++++++--------------
 1 file changed, 49 insertions(+), 26 deletions(-)

diff --git a/package/openssh/S50sshd b/package/openssh/S50sshd
index 22da41d1ca..c3f1cd2906 100644
--- a/package/openssh/S50sshd
+++ b/package/openssh/S50sshd
@@ -1,7 +1,12 @@
 #!/bin/sh
-#
-# sshd        Starts sshd.
-#
+
+DAEMON="sshd"
+PIDFILE="/var/run/$DAEMON.pid"
+
+SSHD_ARGS=""
+
+# shellcheck source=/dev/null
+[ -r "/etc/default/$DAEMON" ] && . "/etc/default/$DAEMON"
 
 # Make sure the ssh-keygen progam exists
 [ -f /usr/bin/ssh-keygen ] || exit 0
@@ -12,36 +17,54 @@ start() {
 	# Create any missing keys
 	/usr/bin/ssh-keygen -A
 
-	printf "Starting sshd: "
-	/usr/sbin/sshd
-	touch /var/lock/sshd
-	echo "OK"
+	printf 'Starting %s: ' "$DAEMON"
+	# shellcheck disable=SC2086 # we need the word splitting
+	start-stop-daemon -S -q -p "$PIDFILE" -x "/usr/sbin/$DAEMON" \
+		-- $SSHD_ARGS
+	status=$?
+	if [ "$status" -eq 0 ]; then
+		echo "OK"
+	else
+		echo "FAIL"
+	fi
+	return "$status"
 }
+
 stop() {
-	printf "Stopping sshd: "
-	killall sshd
-	rm -f /var/lock/sshd
-	echo "OK"
+	printf 'Stopping %s: ' "$DAEMON"
+	start-stop-daemon -K -q -p "$PIDFILE" -x "/usr/sbin/$DAEMON"
+	status=$?
+	if [ "$status" -eq 0 ]; then
+		rm -f "$PIDFILE"
+		echo "OK"
+	else
+		echo "FAIL"
+	fi
+	return "$status"
 }
+
 restart() {
 	stop
+	sleep 1
 	start
 }
 
+reload() {
+	printf 'Reloading %s: ' "$DAEMON"
+	start-stop-daemon -K -s HUP -q -p "$PIDFILE" -x "/usr/sbin/$DAEMON"
+	status=$?
+	if [ "$status" -eq 0 ]; then
+		echo "OK"
+	else
+		echo "FAIL"
+	fi
+	return "$status"
+}
+
 case "$1" in
-  start)
-	start
-	;;
-  stop)
-	stop
-	;;
-  restart|reload)
-	restart
-	;;
-  *)
-	echo "Usage: $0 {start|stop|restart}"
-	exit 1
+	start|stop|restart|reload)
+		"$1";;
+	*)
+		echo "Usage: $0 {start|stop|restart|reload}"
+		exit 1
 esac
-
-exit $?
-
-- 
2.42.0

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/openssh: refactor S50sshd

[Thomas Petazzoni via buildroot]

Hello Nicolas,

On Thu, 23 Nov 2023 11:39:52 +0100
Nicolas Cavallari <nicolas.cavallari@green-communications.fr> wrote:

> Make it look more like the example S01syslogd shell script in
> docs/manual/adding-packages-directory.txt.
> 
> Functionnally, it changes the following:
> - Options can be defined in /etc/default/sshd
> - "S50sshd stop" will no longer kill active SSH sessions or sshd daemons
>   that run inside a container or chroot.  It is now safe to stop or
>   restart openssh inside an SSH session.
> - "S50sshd restart" now sleeps between stop and start, reducing the
>   probability of failures caused by sshd taking too much time to stop.
> - "S50sshd reload" will send a SIGHUP instead of restarting sshd.
> - /var/lock/sshd is no longer created.  The daemon does not use it.
>   The only reference to /var/lock is in contrib/redhat/sshd.init, which
>   uses it as a way to test if sshd is (supposed to be) running.
> 
> Signed-off-by: Nicolas Cavallari <nicolas.cavallari@green-communications.fr>
> ---
>  package/openssh/S50sshd | 75 +++++++++++++++++++++++++++--------------
>  1 file changed, 49 insertions(+), 26 deletions(-)

Sorry for the slow feedback. There's been more recent discussion
related to init scripts, and these lead to changes in S01syslogd, and a
following improvement of S50sshd to align it with the changes in
S01syslogd. Therefore your patch is in fact no longer needed I believe,
but don't hesitate to check if S50sshd is OK.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot