Re: [PATCH] sslh: Update to version 1.20

[Michael Tremer <michael.tremer@ipfire.org>]

[-- Attachment #1: Type: text/plain, Size: 16982 bytes --]

Hi,

The parameter is “in-reply-to”. You got an extra “a” there.

-Michael

> On 10 May 2019, at 12:54, ummeegge <ummeegge(a)ipfire.org> wrote:
> 
> Hi Michael,
> and thanks for your feedback. Made a V2 patch now but i get an 
> 
> $ git send-email -v2 -1 --in-replay-to 690B105B-733E-4283-9D05-9967D09BA86F(a)ipfire.org
> fatal: ambiguous argument '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 <command> [<revision>...] -- [<file>...]'
> format-patch -o /tmp/qnlL0LbJPZ -v2 -1 --in-replay-to 690B105B-733E-4283-9D05-9967D09BA86F(a)ipfire.org: command returned error: 128
> 
> am currently not sure how to fix this. Have nevertheless pushed the new version 
> (hopefully all wanted is included) to Git -->
> https://git.ipfire.org/?p=people/ummeegge/ipfire-2.x.git;a=commit;h=314975a6703f78d8e4c0cb9071819597ad72d48e
> 
> Since my time is currently a little less it might be great if you can
> review it in that way.
> 
> Best,
> 
> Erik
> 
> 
> On Mi, 2019-05-01 at 12:13 +0100, Michael Tremer wrote:
>> Hi,
>> 
>> Apologies for the late reply again… I am trying to catch up on my
>> rather large inbox.
>> 
>>> On 26 Apr 2019, at 05:55, ummeegge <ummeegge(a)ipfire.org> wrote:
>>> 
>>> Hi Michael and thanks for looking into this.
>>> 
>>> On Mi, 2019-04-24 at 12:04 +0100, Michael Tremer wrote:
>>>> Hi,
>>>> 
>>>> Thanks for working on this. I have a couple of questions and
>>>> remarks.
>>>> 
>>>>> On 23 Apr 2019, at 08:06, Erik Kapfer <ummeegge(a)ipfire.org>
>>>>> wrote:
>>>>> 
>>>>> - Added USELIBCAP=1 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.
>>>>> 
>>>>> Signed-off-by: Erik Kapfer <ummeegge(a)ipfire.org>
>>>>> ---
>>>>> 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(-)
>>>>> 
>>>>> 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 @@
>>>>> ###############################################################
>>>>> ####
>>>>> ############
>>>>> #                                                              
>>>>> 
>>>>>          #
>>>>> # IPFire.org - A linux based
>>>>> firewall                                         #
>>>>> -# Copyright (C) 2007-2018  IPFire Team  <info(a)ipfire.org>     
>>>>> 
>>>>>           #
>>>>> +# Copyright (C) 2007-2019  IPFire Team  <info(a)ipfire.org>     
>>>>> 
>>>>>           #
>>>>> #                                                              
>>>>> 
>>>>>          #
>>>>> # 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 @@
>>>>> 
>>>>> include Config
>>>>> 
>>>>> -VER        = 1.7a
>>>>> +VER        = 1.20
>>>>> 
>>>>> THISAPP    = sslh-$(VER)
>>>>> DL_FILE    = $(THISAPP).tar.gz
>>>>> @@ -32,7 +32,7 @@ DL_FROM    = $(URL_IPFIRE)
>>>>> DIR_APP    = $(DIR_SRC)/$(THISAPP)
>>>>> TARGET     = $(DIR_INFO)/$(THISAPP)
>>>>> PROG       = sslh
>>>>> -PAK_VER    = 5
>>>>> +PAK_VER    = 6
>>>>> 
>>>>> DEPS       = ""
>>>>> 
>>>>> @@ -44,7 +44,7 @@ objects = $(DL_FILE)
>>>>> 
>>>>> $(DL_FILE) = $(DL_FROM)/$(DL_FILE)
>>>>> 
>>>>> -$(DL_FILE)_MD5 = ee124654412198a5e11fe28acf10634d
>>>>> +$(DL_FILE)_MD5 = 0db26ed2825b1ef6c83959a988279912
>>>>> 
>>>>> install : $(TARGET)
>>>>> 
>>>>> @@ -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="$(CFLAGS)" $(MAKETUNING)
>>>>> USELIBWRAP=
>>>>> -	cd $(DIR_APP) && install -v -m 755 sslh /usr/sbin
>>>>> +	cd $(DIR_APP) && make CFLAGS="$(CFLAGS)" $(MAKETUNING)
>>>>> USELIBCAP=1 USELIBWRAP=
>>>>> +	cd $(DIR_APP) && install -v -m 755 sslh-fork
>>>>> /usr/sbin/sslh
>>>>> 
>>>>> 	#install initscripts
>>>>> 	$(call INSTALL_INITSCRIPT,sslh)
>>>> 
>>>> It looks all fine up to this point.
>>>> 
>>>>> 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 @@
>>>>> 
>>>>> # Based on sysklogd script from LFS-3.1 and earlier.
>>>>> # Rewritten by Gerard Beekmans  - gerard(a)linuxfromscratch.org
>>>>> +#
>>>>> +# $LastChangedBy: ummeegge - ummeegge(a)ipfire.org $
>>>>> +# $Date: 2019-04-04 04:35:09 -0500 (Thu, 04 Apr 2019) $
>>>>> +#
>>>>> +#############################################################
>>>>> +#
>>>> 
>>>> You do not need to include authorship headers because Git will
>>>> take
>>>> care of this.
>>> 
>>> Done.
>>> 
>>>> 
>>>>> 
>>>>> . /etc/sysconfig/rc
>>>>> . $rc_functions
>>>>> 
>>>>> +DAEMON="/usr/sbin/sslh"
>>>>> +
>>>>> +# Check for external IP address and provide it to listening
>>>>> option
>>>>> +EXTERNAL_IP_ADDRESS="$(</var/ipfire/red/local-ipaddress)"
>>>>> +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
>>>>> +}
>>>> 
>>>> 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.
>>> 
>>> 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 ?
>> 
>> 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.
>> 
>>> 
>>>> 
>>>> What if the IP address on RED changes? How is sslh notified?
>>> 
>>> I think it won´t 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 ?
>> 
>> 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’t be interrupted. I think that is what we need here, isn’t it?
>> 
>>> 
>>> 
>>>> 
>>>>> +
>>>>> +# Loopback interface
>>>>> +LO="127.0.0.1"
>>>>> +# Used TCP ports
>>>>> +LISTENPORT="443"
>>>>> +SSHPORT="222"
>>>>> +TLSPORT="444"
>>>>> +OPENVPNPORT=“1194"
>>>> 
>>>> Can we use hard-coded port numbers here?
>>> 
>>> 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.
>> 
>> I would say that 443 probably masks traffic best and is most likely
>> to be open even in more restricted environments.
>> 
>> OpenVPN has to be running in TCP mode, but that is something the user
>> has to configure. It isn’t even certain if they are using OpenVPN
>> here. So that should be optional.
>> 
>>> 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?!
>> 
>> The Web UI? No, that should not be open by default. People should use
>> OpenVPN to access the firewall.
>> 
>>> 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?!
>> 
>> We do not know how critical the application of the user is. So
>> assuming it is most critical is best.
>> 
>>> 
>>>> 
>>>>> +
>>>>> +# Configuration options
>>>>> +DAEMON_OPTS="
>>>>> +--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
>>>> 
>>>> No need for this check really. We don’t have it anywhere else.
>>> 
>>> Done.
>>> 
>>>> 
>>>>> +
>>>>> +# Check for external IP
>>>>> +EXTERNAL_IP_FUNCT
>>>> 
>>>> This does not really need to be a function because the if
>>>> statement
>>>> above isn’t either.
>>>> 
>>>> And it should not be evaluated in the stop case. If the system is
>>>> not
>>>> online, this service can never be stopped.
>>>> 
>>>> I think it is best for this code to have it in the start section
>>>> where is was before - unless I missed something here.
>>> 
>>> 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.
>> 
>> You won’t be able to stop the service then if the script is ended in
>> that block.
>> 
>> I think it should only be evaluated in the start block. The check
>> isn’t needed for stopping the service.
>> 
>>> 
>>>> 
>>>>> +
>>>>> case "$1" in
>>>>> 	start)
>>>>> 		boot_mesg "Starting SSLH Deamon..."
>>>>> -
>>>>> -		LOCAL_IP_ADDRESS="$(</var/ipfire/red/local-
>>>>> 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
>>>>> 		;;
>>>>> 
>>>>> 	stop)
>>>>> 		boot_mesg "Stopping SSLH Deamon..."
>>>>> -		killproc /usr/sbin/sslh
>>>>> +		killproc ${DAEMON}
>>>>> 		evaluate_retval
>>>>> 		;;
>>>>> 
>>>>> @@ -38,7 +75,7 @@ case "$1" in
>>>>> 		;;
>>>>> 
>>>>> 	status)
>>>>> -		statusproc /usr/sbin/sslh
>>>>> +		statusproc ${DAEMON}
>>>>> 		;;
>>>>> 
>>>>> 	*)
>>>>> 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
>>>> 
>>>> Can you change this to use getent? There are a couple of examples
>>>> in
>>>> some other scripts (e.g. tor).
>>> 
>>> Done.
>>> 
>>>> 
>>>> You should also check if the group existed already.
>>> 
>>> I think the getent solution -->
>>> if ! getent group sslh &>/dev/null; then
>>> ...
>>> will do this.
>> 
>> OK!
>> 
>>>> 
>>>>> +
>>>>> ln -s /etc/init.d/sslh /etc/rc.d/init.d/networking/red.up/50-
>>>>> sslh
>>>> 
>>>> Why is this link not in the package?
>>> 
>>> 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) ?
>> 
>> I think it is easiest to have it in the package so it will be deleted
>> when the package is being removed.
>> 
>> This might become a script now - see above.
>> 
>>>> 
>>>> Should we not check if the service is running and only then
>>>> restart? 
>>> 
>>> Via pgrep check in start) ?
>> 
>> Not in start, but in the red.up script.
>> 
>> That way we would only touch the service when it is active. If it
>> isn’t started, we need to figure out if it should be started - or
>> check if the boot process brought it up - even though RED wasn’t up.
>> 
>>>> What happens when the service is disabled at boot time?
>>> 
>>> You mean disabled via WUI ?
>> 
>> Yes.
>> 
>>> 
>>>> 
>>>>> +
>>>>> +# 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
>>>> 
>>>> We don’t delete users. This will cause that log files or so will
>>>> have
>>>> no owner any more.
>>>> 
>>>> This won’t 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.
>>> 
>>> Done.
>> 
>> Thanks!
>> 
>>>> 
>>>>> +
>>>>> 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
>>>>> -- 
>>>>> 2.12.2
>>>>> 
>>>> 
>>>> 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.
>>> 
>>> 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.
>> 
>> If you are using it, it should of course be updated.
>> 
>> 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.
>> 
>> Maybe we should have awesome documentation on the Wiki so that more
>> people are aware of it and therefore use it.
>> 
>> -Michael
>> 
>>> 
>>>> 
>>>> Best,
>>>> -Michael
>>>> 
>>> 
>>> Best,
>>> 
>>> Erik
>> 
>> 
>