From mboxrd@z Thu Jan 1 00:00:00 1970 From: ummeegge To: development@lists.ipfire.org Subject: Re: [PATCH] sslh: Update to version 1.20 Date: Sun, 12 May 2019 06:35:43 +0200 Message-ID: In-Reply-To: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============5383771994285500426==" List-Id: --===============5383771994285500426== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hi Michael, On Sa, 2019-05-11 at 11:06 +0100, Michael Tremer wrote: > Hi, >=20 > The parameter is =E2=80=9Cin-reply-to=E2=80=9D. You got an extra =E2=80=9Ca= =E2=80=9D there. autsch... Thanks for the hint. Have send it to the list now. Best, Erik >=20 > -Michael >=20 > > On 10 May 2019, at 12:54, ummeegge wrote: > >=20 > > Hi Michael, > > and thanks for your feedback. Made a V2 patch now but i get an=20 > >=20 > > $ git send-email -v2 -1 --in-replay-to=20 > > 690B105B-733E-4283-9D05-9967D09BA86F(a)ipfire.org > > fatal: ambiguous argument=20 > > '690B105B-733E-4283-9D05-9967D09BA86F(a)ipfire.org': unknown revision > > or path not in the working tree. > > Use '--' to separate paths from revisions, like this: > > 'git [...] -- [...]' > > format-patch -o /tmp/qnlL0LbJPZ -v2 -1 --in-replay-to=20 > > 690B105B-733E-4283-9D05-9967D09BA86F(a)ipfire.org: command returned > > error: 128 > >=20 > > am currently not sure how to fix this. Have nevertheless pushed the > > new version=20 > > (hopefully all wanted is included) to Git --> > >=20 https://git.ipfire.org/?p=3Dpeople/ummeegge/ipfire-2.x.git;a=3Dcommit;h=3D314= 975a6703f78d8e4c0cb9071819597ad72d48e > >=20 > > Since my time is currently a little less it might be great if you > > can > > review it in that way. > >=20 > > Best, > >=20 > > Erik > >=20 > >=20 > > On Mi, 2019-05-01 at 12:13 +0100, Michael Tremer wrote: > > > Hi, > > >=20 > > > Apologies for the late reply again=E2=80=A6 I am trying to catch up on = my > > > rather large inbox. > > >=20 > > > > On 26 Apr 2019, at 05:55, ummeegge wrote: > > > >=20 > > > > Hi Michael and thanks for looking into this. > > > >=20 > > > > On Mi, 2019-04-24 at 12:04 +0100, Michael Tremer wrote: > > > > > Hi, > > > > >=20 > > > > > Thanks for working on this. I have a couple of questions and > > > > > remarks. > > > > >=20 > > > > > > On 23 Apr 2019, at 08:06, Erik Kapfer > > > > > > wrote: > > > > > >=20 > > > > > > - Added USELIBCAP=3D1 to enable the possibility for > > > > > > transparent > > > > > > option. > > > > > > - Wrote configuration directives in initscript into > > > > > > variable > > > > > > for > > > > > > better overview. > > > > > > - Introduce chroot directive in start parameter. > > > > > > - Added new user and group sslh (will be deleted if > > > > > > uninstall). > > > > > > - Changed EXTERNAL_IP_FUNCT to serve data also for > > > > > > configuration > > > > > > block but use it also as check as before. > > > > > > - Added symlinks in sslh paks since the initscripts LFS do > > > > > > not > > > > > > serves it in old installation (a reboot does not started > > > > > > sslh > > > > > > again). > > > > > > - Deleted sslh symlinks in initscripts LFS since they are > > > > > > served > > > > > > via sslh paks and are not needed anymore. > > > > > >=20 > > > > > > Signed-off-by: Erik Kapfer > > > > > > --- > > > > > > lfs/initscripts | 3 -- > > > > > > lfs/sslh | 12 ++++---- > > > > > > src/initscripts/packages/sslh | 65 > > > > > > +++++++++++++++++++++++++++++++++---------- > > > > > > src/paks/sslh/install.sh | 13 +++++++++ > > > > > > src/paks/sslh/uninstall.sh | 9 ++++++ > > > > > > 5 files changed, 79 insertions(+), 23 deletions(-) > > > > > >=20 > > > > > > diff --git a/lfs/initscripts b/lfs/initscripts > > > > > > index 055e106d0..3173a04e4 100644 > > > > > > --- a/lfs/initscripts > > > > > > +++ b/lfs/initscripts > > > > > > @@ -136,9 +136,6 @@ $(TARGET) : > > > > > > ln -sf > > > > > > ../init.d/client175 /etc/rc.d/rc0.d/K34client175 > > > > > > ln -sf > > > > > > ../init.d/client175 /etc/rc.d/rc3.d/S66client175 > > > > > > ln -sf > > > > > > ../init.d/client175 /etc/rc.d/rc6.d/K34client175 > > > > > > - ln -sf ../init.d/sslh /etc/rc.d/rc3.d/S98sslh > > > > > > - ln -sf ../init.d/sslh /etc/rc.d/rc0.d/K02sslh > > > > > > - ln -sf ../init.d/sslh /etc/rc.d/rc6.d/K02sslh > > > > > > ln -sf > > > > > > ../init.d/vdradmin /etc/rc.d/rc3.d/S99vdradmin > > > > > > ln -sf > > > > > > ../init.d/vdradmin /etc/rc.d/rc0.d/K01vdradmin > > > > > > ln -sf > > > > > > ../init.d/vdradmin /etc/rc.d/rc6.d/K01vdradmin > > > > > > diff --git a/lfs/sslh b/lfs/sslh > > > > > > index 100cec065..dedd10272 100644 > > > > > > --- a/lfs/sslh > > > > > > +++ b/lfs/sslh > > > > > > @@ -1,7 +1,7 @@ > > > > > > ########################################################### > > > > > > #### > > > > > > #### > > > > > > ############ > > > > > > # =20 > > > > > > =20 > > > > > >=20 > > > > > > # > > > > > > # IPFire.org - A linux based > > > > > > firewall # > > > > > > -# Copyright (C) 2007-2018 IPFire Team =20 > > > > > > =20 > > > > > >=20 > > > > > > # > > > > > > +# Copyright (C) 2007-2019 IPFire Team =20 > > > > > > =20 > > > > > >=20 > > > > > > # > > > > > > # =20 > > > > > > =20 > > > > > >=20 > > > > > > # > > > > > > # This program is free software: you can redistribute it > > > > > > and/or > > > > > > modify # > > > > > > # it under the terms of the GNU General Public License as > > > > > > published > > > > > > by # > > > > > > @@ -24,7 +24,7 @@ > > > > > >=20 > > > > > > include Config > > > > > >=20 > > > > > > -VER =3D 1.7a > > > > > > +VER =3D 1.20 > > > > > >=20 > > > > > > THISAPP =3D sslh-$(VER) > > > > > > DL_FILE =3D $(THISAPP).tar.gz > > > > > > @@ -32,7 +32,7 @@ DL_FROM =3D $(URL_IPFIRE) > > > > > > DIR_APP =3D $(DIR_SRC)/$(THISAPP) > > > > > > TARGET =3D $(DIR_INFO)/$(THISAPP) > > > > > > PROG =3D sslh > > > > > > -PAK_VER =3D 5 > > > > > > +PAK_VER =3D 6 > > > > > >=20 > > > > > > DEPS =3D "" > > > > > >=20 > > > > > > @@ -44,7 +44,7 @@ objects =3D $(DL_FILE) > > > > > >=20 > > > > > > $(DL_FILE) =3D $(DL_FROM)/$(DL_FILE) > > > > > >=20 > > > > > > -$(DL_FILE)_MD5 =3D ee124654412198a5e11fe28acf10634d > > > > > > +$(DL_FILE)_MD5 =3D 0db26ed2825b1ef6c83959a988279912 > > > > > >=20 > > > > > > install : $(TARGET) > > > > > >=20 > > > > > > @@ -77,8 +77,8 @@ $(subst %,%_MD5,$(objects)) : > > > > > > $(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects)) > > > > > > @$(PREBUILD) > > > > > > @rm -rf $(DIR_APP) && cd $(DIR_SRC) && tar zxf > > > > > > $(DIR_DL)/$(DL_FILE) > > > > > > - cd $(DIR_APP) && make CFLAGS=3D"$(CFLAGS)" $(MAKETUNING) > > > > > > USELIBWRAP=3D > > > > > > - cd $(DIR_APP) && install -v -m 755 sslh /usr/sbin > > > > > > + cd $(DIR_APP) && make CFLAGS=3D"$(CFLAGS)" $(MAKETUNING) > > > > > > USELIBCAP=3D1 USELIBWRAP=3D > > > > > > + cd $(DIR_APP) && install -v -m 755 sslh-fork > > > > > > /usr/sbin/sslh > > > > > >=20 > > > > > > #install initscripts > > > > > > $(call INSTALL_INITSCRIPT,sslh) > > > > >=20 > > > > > It looks all fine up to this point. > > > > >=20 > > > > > > diff --git a/src/initscripts/packages/sslh > > > > > > b/src/initscripts/packages/sslh > > > > > > index 43e58f392..0935b1114 100644 > > > > > > --- a/src/initscripts/packages/sslh > > > > > > +++ b/src/initscripts/packages/sslh > > > > > > @@ -3,31 +3,68 @@ > > > > > >=20 > > > > > > # Based on sysklogd script from LFS-3.1 and earlier. > > > > > > # Rewritten by Gerard Beekmans -=20 > > > > > > gerard(a)linuxfromscratch.org > > > > > > +# > > > > > > +# $LastChangedBy: ummeegge - ummeegge(a)ipfire.org $ > > > > > > +# $Date: 2019-04-04 04:35:09 -0500 (Thu, 04 Apr 2019) $ > > > > > > +# > > > > > > +########################################################## > > > > > > ### > > > > > > +# > > > > >=20 > > > > > You do not need to include authorship headers because Git > > > > > will > > > > > take > > > > > care of this. > > > >=20 > > > > Done. > > > >=20 > > > > >=20 > > > > > >=20 > > > > > > . /etc/sysconfig/rc > > > > > > . $rc_functions > > > > > >=20 > > > > > > +DAEMON=3D"/usr/sbin/sslh" > > > > > > + > > > > > > +# Check for external IP address and provide it to > > > > > > listening > > > > > > option > > > > > > +EXTERNAL_IP_ADDRESS=3D"$( > > > > > +EXTERNAL_IP_FUNCT() { > > > > > > + if [ -z "${EXTERNAL_IP_ADDRESS}" ]; then > > > > > > + echo_failure > > > > > > + boot_mesg -n "FAILURE:\n\nCould not > > > > > > determine" > > > > > > ${FAILURE} > > > > > > + boot_mesg -n " your external IP address." > > > > > > + boot_mesg "" ${NORMAL} > > > > > > + exit 1 > > > > > > + fi > > > > > > +} > > > > >=20 > > > > > Although you are starting the service rather late in the boot > > > > > process, it might indeed happen that this goes wrong. How do > > > > > we > > > > > recover from this? The service should be started eventually > > > > > when > > > > > RED > > > > > comes up. > > > >=20 > > > > I tried to leave the most options pretty much the same as they > > > > where > > > > before whereby the numbers of the symlinks are a part of this > > > > the > > > > testings did not delivered problems with this but better to > > > > stay > > > > save > > > > there. > > > > The network initscript starts at 'S20' where i think RED will > > > > be > > > > started too what you think is a good starting point for sslh ? > > >=20 > > > If there is a cable plugged into RED and it actually comes up > > > this is > > > a good time to start the service. But what if the DSL modem takes > > > longer to sync or so? It should still be started in red.up then. > > >=20 > > > >=20 > > > > >=20 > > > > > What if the IP address on RED changes? How is sslh notified? > > > >=20 > > > > I think it won=C2=B4t currently. Have seen some configurations which > > > > uses > > > > 0.0.0.0 for the --listen option but this can cause bigger > > > > troubles > > > > if > > > > '443' is the --listen port here i think. As far as i can see > > > > only a > > > > restart of sslh would fix this currently. > > > > May you have an idea here ? > > >=20 > > > Have a script in red.up which restarts the whole service - but > > > only > > > if the IP address has actually changed so that standing > > > connections > > > won=E2=80=99t be interrupted. I think that is what we need here, isn=E2= =80=99t > > > it? > > >=20 > > > >=20 > > > >=20 > > > > >=20 > > > > > > + > > > > > > +# Loopback interface > > > > > > +LO=3D"127.0.0.1" > > > > > > +# Used TCP ports > > > > > > +LISTENPORT=3D"443" > > > > > > +SSHPORT=3D"222" > > > > > > +TLSPORT=3D"444" > > > > > > +OPENVPNPORT=3D=E2=80=9C1194" > > > > >=20 > > > > > Can we use hard-coded port numbers here? > > > >=20 > > > > Here arises also some more questions for me. > > > > We can grep for them in the configuration files but also may a > > > > check > > > > for the correct usage can be made since e.g. OpenVPN needs to > > > > run > > > > with > > > > TCP for example. > > >=20 > > > I would say that 443 probably masks traffic best and is most > > > likely > > > to be open even in more restricted environments. > > >=20 > > > OpenVPN has to be running in TCP mode, but that is something the > > > user > > > has to configure. It isn=E2=80=99t even certain if they are using OpenV= PN > > > here. So that should be optional. > > >=20 > > > > Should we really provide TCP 444 in here for external access ? > > > > I > > > > think > > > > also SSH might be more secure via VPN but may a little more > > > > acceptable > > > > if no VPN is in usage?! > > >=20 > > > The Web UI? No, that should not be open by default. People should > > > use > > > OpenVPN to access the firewall. > > >=20 > > > > The main topic in the tests we made in the forum where mainly > > > > for > > > > LAN > > > > instances on seperated machines (mostly webservers) where those > > > > questions may be not that critical?! > > >=20 > > > We do not know how critical the application of the user is. So > > > assuming it is most critical is best. > > >=20 > > > >=20 > > > > >=20 > > > > > > + > > > > > > +# Configuration options > > > > > > +DAEMON_OPTS=3D" > > > > > > +--user sslh > > > > > > +--listen ${EXTERNAL_IP_ADDRESS}:${LISTENPORT} > > > > > > +--ssh ${LO}:${SSHPORT} > > > > > > +--tls ${LO}:${TLSPORT} > > > > > > +--openvpn ${LO}:${OPENVPNPORT} > > > > > > +--pidfile /var/run/sslh.pid > > > > > > +-C /var/empty > > > > > > +" > > > > > > + > > > > > > +# Check for binary > > > > > > +if ! [ -x "$(command -v ${DAEMON})" ]; then > > > > > > + echo "Error: could not find ${DAEMON}" >&2 > > > > > > + exit 1 > > > > > > +fi > > > > >=20 > > > > > No need for this check really. We don=E2=80=99t have it anywhere > > > > > else. > > > >=20 > > > > Done. > > > >=20 > > > > >=20 > > > > > > + > > > > > > +# Check for external IP > > > > > > +EXTERNAL_IP_FUNCT > > > > >=20 > > > > > This does not really need to be a function because the if > > > > > statement > > > > > above isn=E2=80=99t either. > > > > >=20 > > > > > And it should not be evaluated in the stop case. If the > > > > > system is > > > > > not > > > > > online, this service can never be stopped. > > > > >=20 > > > > > I think it is best for this code to have it in the start > > > > > section > > > > > where is was before - unless I missed something here. > > > >=20 > > > > Have externalised the investigation of the RED address to > > > > provide > > > > it > > > > also for the --listen option in the configuration block. To > > > > keep > > > > the > > > > start) case also a little cleaner have decided to set the > > > > external > > > > IP > > > > check completely outside of the start) case and set the check > > > > before > > > > but this is fast changed. > > >=20 > > > You won=E2=80=99t be able to stop the service then if the script is end= ed > > > in > > > that block. > > >=20 > > > I think it should only be evaluated in the start block. The check > > > isn=E2=80=99t needed for stopping the service. > > >=20 > > > >=20 > > > > >=20 > > > > > > + > > > > > > case "$1" in > > > > > > start) > > > > > > boot_mesg "Starting SSLH Deamon..." > > > > > > - > > > > > > - LOCAL_IP_ADDRESS=3D"$( > > > > > ipaddress)" > > > > > > - if [ -z "${LOCAL_IP_ADDRESS}" ]; then > > > > > > - echo_failure > > > > > > - boot_mesg -n "FAILURE:\n\nCould not > > > > > > determine" > > > > > > ${FAILURE} > > > > > > - boot_mesg -n " your external IP > > > > > > address." > > > > > > - boot_mesg "" ${NORMAL} > > > > > > - exit 1 > > > > > > - fi > > > > > > - > > > > > > - loadproc /usr/sbin/sslh -u nobody \ > > > > > > - -p "${LOCAL_IP_ADDRESS}:443" -s > > > > > > localhost:222 > > > > > > -l localhost:444 > > > > > > + loadproc ${DAEMON} ${DAEMON_OPTS} > > > > > > evaluate_retval > > > > > > ;; > > > > > >=20 > > > > > > stop) > > > > > > boot_mesg "Stopping SSLH Deamon..." > > > > > > - killproc /usr/sbin/sslh > > > > > > + killproc ${DAEMON} > > > > > > evaluate_retval > > > > > > ;; > > > > > >=20 > > > > > > @@ -38,7 +75,7 @@ case "$1" in > > > > > > ;; > > > > > >=20 > > > > > > status) > > > > > > - statusproc /usr/sbin/sslh > > > > > > + statusproc ${DAEMON} > > > > > > ;; > > > > > >=20 > > > > > > *) > > > > > > diff --git a/src/paks/sslh/install.sh > > > > > > b/src/paks/sslh/install.sh > > > > > > index 626884bdd..df7cafc78 100644 > > > > > > --- a/src/paks/sslh/install.sh > > > > > > +++ b/src/paks/sslh/install.sh > > > > > > @@ -23,5 +23,18 @@ > > > > > > # > > > > > > . /opt/pakfire/lib/functions.sh > > > > > > extract_files > > > > > > + > > > > > > +# Add user and group for sslh if not already done > > > > > > +if ! grep -q sslh /etc/passwd; then > > > > > > + groupadd sslh; > > > > > > + useradd -g sslh -M -s /sbin/nologin sslh > > > > > > +fi > > > > >=20 > > > > > Can you change this to use getent? There are a couple of > > > > > examples > > > > > in > > > > > some other scripts (e.g. tor). > > > >=20 > > > > Done. > > > >=20 > > > > >=20 > > > > > You should also check if the group existed already. > > > >=20 > > > > I think the getent solution --> > > > > if ! getent group sslh &>/dev/null; then > > > > ... > > > > will do this. > > >=20 > > > OK! > > >=20 > > > > >=20 > > > > > > + > > > > > > ln -s /etc/init.d/sslh > > > > > > /etc/rc.d/init.d/networking/red.up/50- > > > > > > sslh > > > > >=20 > > > > > Why is this link not in the package? > > > >=20 > > > > You mean in the LFS ? This is also a part of the old > > > > implementation, > > > > might it be an idea to have it in the initscript (incl. > > > > deletion of > > > > the > > > > symlink) ? > > >=20 > > > I think it is easiest to have it in the package so it will be > > > deleted > > > when the package is being removed. > > >=20 > > > This might become a script now - see above. > > >=20 > > > > >=20 > > > > > Should we not check if the service is running and only then > > > > > restart?=20 > > > >=20 > > > > Via pgrep check in start) ? > > >=20 > > > Not in start, but in the red.up script. > > >=20 > > > That way we would only touch the service when it is active. If it > > > isn=E2=80=99t started, we need to figure out if it should be started - = or > > > check if the boot process brought it up - even though RED wasn=E2=80=99t > > > up. > > >=20 > > > > > What happens when the service is disabled at boot time? > > > >=20 > > > > You mean disabled via WUI ? > > >=20 > > > Yes. > > >=20 > > > >=20 > > > > >=20 > > > > > > + > > > > > > +# Set symlink for runlevels > > > > > > +ln -svf ../init.d/sslh /etc/rc.d/rc0.d/K02sslh > > > > > > +ln -svf ../init.d/sslh /etc/rc.d/rc3.d/S98sslh > > > > > > +ln -svf ../init.d/sslh /etc/rc.d/rc6.d/K02sslh > > > > > > + > > > > > > start_service --background ${NAME} > > > > > > diff --git a/src/paks/sslh/uninstall.sh > > > > > > b/src/paks/sslh/uninstall.sh > > > > > > index dca34ccbd..05bb27945 100644 > > > > > > --- a/src/paks/sslh/uninstall.sh > > > > > > +++ b/src/paks/sslh/uninstall.sh > > > > > > @@ -23,5 +23,14 @@ > > > > > > # > > > > > > . /opt/pakfire/lib/functions.sh > > > > > > stop_service ${NAME} > > > > > > + > > > > > > +# Delete user and group sslh > > > > > > +if grep -q sslh /etc/passwd; then > > > > > > + userdel sslh > > > > > > + groupdel sslh > > > > > > +fi > > > > >=20 > > > > > We don=E2=80=99t delete users. This will cause that log files or so > > > > > will > > > > > have > > > > > no owner any more. > > > > >=20 > > > > > This won=E2=80=99t be an issue because we check if the user exists = in > > > > > the > > > > > install script and if it does we are not trying to create it > > > > > any > > > > > more. > > > >=20 > > > > Done. > > >=20 > > > Thanks! > > >=20 > > > > >=20 > > > > > > + > > > > > > remove_files > > > > > > rm -f /etc/rc.d/init.d/networking/red.up/50-sslh > > > > > > +# Delete initscript symlinks > > > > > > +rm -f /etc/rc.d/rc?.d/???sslh > > > > > > --=20 > > > > > > 2.12.2 > > > > > >=20 > > > > >=20 > > > > > Overall this looks good and makes this a better add-on. It > > > > > looks > > > > > like > > > > > many things to change, but they are all small and should not > > > > > take > > > > > too > > > > > long. > > > >=20 > > > > This Addon is really a old one and i was not sure if it should > > > > be > > > > updated again or if it would be dropped since stunnel do a > > > > similar > > > > job > > > > i think. But am happy if it can be a better one and the time to > > > > bring > > > > it back to life was not wasted. > > >=20 > > > If you are using it, it should of course be updated. > > >=20 > > > There is a little bit of work to be done and some add-ons are > > > really > > > just the package itself with little integration, but I think it > > > is a > > > good idea to make high-quality add-on out of this. > > >=20 > > > Maybe we should have awesome documentation on the Wiki so that > > > more > > > people are aware of it and therefore use it. > > >=20 > > > -Michael > > >=20 > > > >=20 > > > > >=20 > > > > > Best, > > > > > -Michael > > > > >=20 > > > >=20 > > > > Best, > > > >=20 > > > > Erik > > >=20 > > >=20 >=20 >=20 --===============5383771994285500426==--