rxrbln / t2sde

T2 SDE Linux
https://t2sde.org
Other
341 stars 48 forks source link

sshd: init kills process on restart #137

Open paulwratt opened 1 year ago

paulwratt commented 1 year ago

.. when there is an invalid sshd_config


I made a change to test for a legitimate sshd_config before sshd start , but also don't kill process on restart if the config is not legal.


I post the change tonight (in about 6-8 hours)

rxrbln commented 1 year ago

Well, most people only run a dhcp client per machine, which is probably why this was never an issue for 20 years since the ROCK Linux days. As usual I think we should keep it stupid simple, and my fix fixes the SIGTERM, as well as elegantly using the dhclient provided means even a dhclient per interface. Both nice improvements over the 20yo state. I don't think we should try to optimize for mixing rocknet and manual fiddling. This will usually anyway not match nor work, so we should not clutter the code necessarily. IMHO if one manually fiddles around, like I myself for testing, one can also just manually clean up their stuff.

Actually one thing I would re-write in rock net is saving the ifup state, and using that for ifdow. Currently the configurationstate is always re-evaluated so that editing the configuration between ifup and if down results in if down using newly edited and thus potentially not matching commands. No a daily issue, but certainly another nice improvement to be done eventually just for such cases.

paulwratt commented 1 year ago

This is not related to dhclient issue BTW, it looks like a crash from the console. Its probably never been an issue because no-one thought to test for a broken sshd_config.


You probably saw me ask about this on Discord, but here is the svn di.

While trying to get SSHd to work for me, I missed a value from one of the options and the init script " spat the dummy " (ie borked) and then killed the SSHd process, which I am figuring on a production server is NOT something you want (because then you can not fix it remotely ).

In this version I chose to leave -T > /dev/null in (instead of -t ), just incase someone actually wants to see parsed config out from the init.

Index: sshd.init
===================================================================
--- sshd.init   (revision 65522)
+++ sshd.init   (working copy)
@@ -17,6 +17,13 @@
 main_begin

     block_begin(start, `Starting sshd')
+   /usr/sbin/sshd -T > /dev/null || error=$?   
+   if [ ! $error -eq 0 ]; then
+       echo "Can't start sshd: Error in sshd_config !!"\
+           "Please edit sshd_config and verify"
+       status
+       exit
+   fi  
    if ! test -e /etc/ssh/ssh_host_key -o \
          -e /etc/ssh/ssh_host_rsa_key -o \
          -e /etc/ssh/ssh_host_ecdsa_key -o \
@@ -39,6 +46,13 @@
     block_end

     block_begin(restart, `Restarting sshd')
+   /usr/sbin/sshd -T > /dev/null || error=$?   
+   if [ ! $error -eq 0 ]; then
+       echo "Can't start sshd: Error in sshd_config !!"\
+           "Please edit sshd_config and verify"
+       status
+       exit
+   fi  
    check(`if [ -s /var/run/sshd.pid ]; then
        kill -HUP $(cat /var/run/sshd.pid)
    else
paulwratt commented 1 year ago

I asked about this on Discord, because I think there might be a way to slim it down or fit in into a check function, except that it needs to exit if it fails, after doing a status (to show the FAIL )