From mboxrd@z Thu Jan 1 00:00:00 1970 From: Adolf Belka To: development@lists.ipfire.org Subject: Re: IPFire 2.29 - Core Update 185 is available for testing Date: Tue, 26 Mar 2024 12:37:02 +0100 Message-ID: <9bcf2953-8bdd-4f4a-9c01-08a1bbdb5a80@ipfire.org> In-Reply-To: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============8949355295872125387==" List-Id: --===============8949355295872125387== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hi, On 26/03/2024 11:55, Michael Tremer wrote: > Hello everyone, >=20 > I am not entirely sure what corner-case we are talking about here, but I ra= n the code twice on a test system to see if it actually does what it is suppo= sed to do. I felt it did. If I ran the code myself on my vm system, it did the right things. It was whe= n I ran the CU185 update that it went wrong but I couldn't come up with any i= dea for why. >=20 > But is it pretty? Not quite, so let=E2=80=99s dive into a little bit of she= ll :) Well I am definitely not an expert at bash. Whenever I want to do something I= always have to search on the internet so that might cause me to have ugly co= de. I try and learn as I go along, if I can remember. >=20 >> On 25 Mar 2024, at 16:29, Adolf Belka wrote: >> >> Hi Nick, >> >> On 25/03/2024 16:49, Nick Howitt wrote: >>> I don't have the answer to why it is adding the lines, but can I ask if t= his scriptlet is safe? >>> >>> If you have one line and not the other in the file you will end up with t= hree lines, the original plus two new. Also, if someone has preffed the lines= off, they will gain two lines preffed on. >>> >> Good point. If the lines are present with =3Don or =3Doff then the options= have been saved and the update code would not be needed. >>> Perhaps it is safer to run the tests independently, just checking for ^LO= GDROPHOSTILEIN=3D and ^LOGDROPHOSTILEOUT=3D >>> >>> if ! grep "^LOGDROPHOSTILEIN=3D" /var/ipfire/optionsfw/settings; then >>> sed -i '$ a\LOGDROPHOSTILEIN=3Don' /var/ipfire/optionsfw/settings >>> /usr/local/bin/firewallctrl >>> fi >>> if ! grep "^LOGDROPHOSTILEOUT=3D" /var/ipfire/optionsfw/settings; then >>> sed -i '$ a\LOGDROPHOSTILEOUT=3Don' /var/ipfire/optionsfw/settings >>> /usr/local/bin/firewallctrl >>> fi >>> >=20 > This is probably the cleanest version that we have seen here. >=20 > Formerly the if statement looked like this: >=20 >>>> if ! [ $(grep "LOGDROPHOSTILEIN=3Don" /var/ipfire/optionsfw/settings) ] >=20 > This will run a sub shell $(=E2=80=A6) with grep in it which is not necessa= ry since if alone can determine the exit code of a command. >=20 >>> if ! grep "^LOGDROPHOSTILEIN=3D" /var/ipfire/optionsfw/settings; then >=20 > So this eliminates the sub shell, a couple of brackets and dollar signs and= is a lot easier to read. The [] brackets were something I read some time ago that I should have in pla= ce when I did my original internet searching so have always used them in othe= r places in IPFire. Happy to be corrected and change to this approach. >=20 >>> sed -i '$ a\LOGDROPHOSTILEOUT=3Don' /var/ipfire/optionsfw/settings >=20 > This could have simply been an echo: >=20 > echo "LOGDROPHOSTILEOUT=3Don=E2=80=9D >> /var/ipfire/optionsfw/settings >=20 > That would even be executed by the shell without forking any subprocesses a= nd therefore be the fastest option. Not very important if there is only two s= trings to write, but sometimes there are loops that are being called thousand= s of times=E2=80=A6 You are going to save a lot of time and will also have ea= sier code to read. The search I did gave examples of sed, grep, awk but not echo. It does seem easier when you point it out. Will also change to that. >=20 > Reloading the firewall once would have been fine for me, even if there have= been no changes whatsoever. >=20 >> I will look at making that update. >> The only problem is I can't easily test that it solves the problem I have = found from the update as the original script does not cause the same result w= hen I manually run it. >> >> However, definitely want to change the script anyway to make sure that I d= on't end up with both =3Don and =3Doff fore the same setting which might occu= r if someone has already adjusted their preferences. >> >> I will probably have to submit a patch for the modification and then test = it in the CU185 Testing release when it is updated. >=20 > Thank you. I will merge it shortly. You have merged my v2 example but it still had the square brackets and the su= bshell and the sed etc. Should I do an update to the v2 version to get rid of the square brackets etc= as discussed above. Adolf. >=20 > -Michael >=20 >> >> Regards, >> Adolf. >>> It does, however, cost another firewall restart, which could be evaded wi= th a few more lines of script. >>> >>> Regards, >>> >>> Nick >>> >>> On 25/03/2024 15:02, Adolf Belka wrote: >>>> >>>> Hi All, >>>> >>>> I am having difficulty understanding something that is happening with th= e Core Update to 185. >>>> >>>> I added the following code into the update.sh script >>>> >>>> # Check if the drop hostile in and out logging options need to be added >>>> # into the optionsfw settings file and apply to firewall >>>> if ! [ $(grep "LOGDROPHOSTILEIN=3Don" /var/ipfire/optionsfw/settings) ] = && \ >>>> ! [ $(grep "LOGDROPHOSTILEOUT=3Don" /var/ipfire/optionsfw/settings)= ]; then >>>> sed -i '$ a\LOGDROPHOSTILEIN=3Don' /var/ipfire/optionsfw/setti= ngs >>>> sed -i '$ a\LOGDROPHOSTILEOUT=3Don' /var/ipfire/optionsfw/sett= ings >>>> /usr/local/bin/firewallctrl >>>> fi >>>> >>>> If I do an update with a Core Update 183 version that has the LOGDROPHOS= TILEIN and LOGDROPHOSTILEOUT entries in optionsfw/settings missing then the u= pdate adds in the two lines shown. So working correctly. >>>> >>>> However if the Core Update 183 has the two entries already in the option= sfw/settings file then the above code ends up with two more copies of each pu= t into the file as following. >>>> >>>> FWPOLICY=3DDROP >>>> SHOWTABLES=3Don >>>> DROPPROXY=3Doff >>>> LOGDROPHOSTILEIN=3Don >>>> LOGDROPHOSTILEOUT=3Don >>>> LOGDROPHOSTILEIN=3Don >>>> LOGDROPHOSTILEOUT=3Don >>>> >>>> However if I take a vm with optionsfw/settings containing the two entrie= s already and run the update code shown above manually via a script on the vm= then it does not add any extra lines in. If the vm has the two entries missi= ng and I run the script manually then it adds in one entry for each. >>>> >>>> So I do not understand at all why the code I put into the update.sh file >>>> >>>> 1) Does not recognise that the entries already exist in the settings fil= e. >>>> 2) Then prints the entries twice. >>>> >>>> when it is run in the update.sh via an upgrade. >>>> >>>> Any help with understanding what is going wrong with the code I wrote wo= uld be very much appreciated. >>>> >>>> Regards, >>>> Adolf. >>>> >>>> On 25/03/2024 10:15, IPFire Project wrote: >>>>> This update is another testing version for IPFire: It comes with the br= and release of the IPFire IPS, a number of bug fixes across the entire system= and a good amount of package updates. Test it while it's still hot! >>>>> =E2=80=8C =E2=80=8C =E2=80=8C =E2=80=8C =E2=80=8C =E2=80=8C =E2=80=8C = =E2=80=8C =E2=80=8C =E2=80=8C =E2=80=8C =E2=80=8C =E2=80=8C =E2=80=8C =E2=80= =8C =E2=80=8C =E2=80=8C =E2=80=8C >>>>> >>>>> >>>>> IPFire_ >>>>> >>>>> >>>>> IPFire 2.29 - Core Update 185 is available for testing >>>>> >>>>> This update is another testing version for IPFire: It comes with the br= and release of the IPFire IPS, a number of bug fixes across the entire system= and a good amount of package updates. Test it while it's still hot! >>>>> >>>>> Read The Full Post On Our Blog >>>>> >>>>> The IPFire Project, c/o Lightning Wire Labs GmbH, Gerhardstra=C3=9Fe 8,= 45711 Datteln, Germany >>>>> >>>>> Unsubscribe >>>>> >> >> --=20 >> Sent from my laptop >=20 >=20 --===============8949355295872125387==--