From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nick Howitt To: development@lists.ipfire.org Subject: Re: IPFire 2.29 - Core Update 185 is available for testing Date: Tue, 26 Mar 2024 12:36:55 +0000 Message-ID: <9a20e3a5-9af7-481f-a24e-27222ed0a6b9@howitts.co.uk> In-Reply-To: <9bcf2953-8bdd-4f4a-9c01-08a1bbdb5a80@ipfire.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============6023602205137522623==" List-Id: --===============6023602205137522623== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable On 26/03/2024 11:37, Adolf Belka wrote: >=20 > Hi, >=20 > On 26/03/2024 11:55, Michael Tremer wrote: >> Hello everyone, >> >> I am not entirely sure what corner-case we are talking about here, but=20 >> I ran the code twice on a test system to see if it actually does what=20 >> it is supposed to do. I felt it did. > If I ran the code myself on my vm system, it did the right things. It=20 > was when I ran the CU185 update that it went wrong but I couldn't come=20 > up with any idea for why. >=20 >> >> But is it pretty? Not quite, so let=E2=80=99s dive into a little bit of sh= ell :) > Well I am definitely not an expert at bash. Whenever I want to do=20 > something I always have to search on the internet so that might cause me=20 > to have ugly code. I try and learn as I go along, if I can remember. >> >>> 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=20 >>>> if this scriptlet is safe? >>>> >>>> If you have one line and not the other in the file you will end up=20 >>>> with three lines, the original plus two new. Also, if someone has=20 >>>> preffed the lines off, they will gain two lines preffed on. >>>> >>> Good point. If the lines are present with =3Don or =3Doff then the=20 >>> options have been saved and the update code would not be needed. >>>> Perhaps it is safer to run the tests independently, just checking=20 >>>> for ^LOGDROPHOSTILEIN=3D and ^LOGDROPHOSTILEOUT=3D >>>> >>>> if ! grep "^LOGDROPHOSTILEIN=3D" /var/ipfire/optionsfw/settings; then >>>> =C2=A0=C2=A0=C2=A0=C2=A0 sed -i '$ a\LOGDROPHOSTILEIN=3Don' /var/ipfire/= optionsfw/settings >>>> =C2=A0=C2=A0=C2=A0=C2=A0 /usr/local/bin/firewallctrl >>>> fi >>>> if ! grep "^LOGDROPHOSTILEOUT=3D" /var/ipfire/optionsfw/settings; then >>>> =C2=A0=C2=A0=C2=A0=C2=A0 sed -i '$ a\LOGDROPHOSTILEOUT=3Don' /var/ipfire= /optionsfw/settings >>>> =C2=A0=C2=A0=C2=A0=C2=A0 /usr/local/bin/firewallctrl >>>> fi >>>> >> >> This is probably the cleanest version that we have seen here. >> >> Formerly the if statement looked like this: >> >>>>> if ! [ $(grep "LOGDROPHOSTILEIN=3Don" /var/ipfire/optionsfw/settings) ] >> >> This will run a sub shell $(=E2=80=A6) with grep in it which is not necess= ary=20 >> since if alone can determine the exit code of a command. >> >>>> if ! grep "^LOGDROPHOSTILEIN=3D" /var/ipfire/optionsfw/settings; then >> >> So this eliminates the sub shell, a couple of brackets and dollar=20 >> signs and is a lot easier to read. > The [] brackets were something I read some time ago that I should have=20 > in place when I did my original internet searching so have always used=20 > them in other places in IPFire. > Happy to be corrected and change to this approach. >> >>>> sed -i '$ a\LOGDROPHOSTILEOUT=3Don' /var/ipfire/optionsfw/settings >> >> This could have simply been an echo: >> >> =C2=A0=C2=A0 echo "LOGDROPHOSTILEOUT=3Don=E2=80=9D >> /var/ipfire/optionsf= w/settings >> >> That would even be executed by the shell without forking any=20 >> subprocesses and therefore be the fastest option. Not very important=20 >> if there is only two strings to write, but sometimes there are loops=20 >> that are being called thousands of times=E2=80=A6 You are going to save a = lot=20 >> of time and will also have easier code to read. I fell over here once and sed may have the same issue. If there is no=20 line terminator (I can never remember if it is CR, LF or both) at the=20 end of a file, it will append to the last line rather than adding a new=20 line. vi always adds a line terminator but java data files don't seem to=20 have one, in my experience. >=20 > 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. >> >> Reloading the firewall once would have been fine for me, even if there=20 >> have been no changes whatsoever. >> >>> I will look at making that update. >>> The only problem is I can't easily test that it solves the problem I=20 >>> have found from the update as the original script does not cause the=20 >>> same result when I manually run it. >>> >>> However, definitely want to change the script anyway to make sure=20 >>> that I don't end up with both =3Don and =3Doff fore the same setting=20 >>> which might occur if someone has already adjusted their preferences. >>> >>> I will probably have to submit a patch for the modification and then=20 >>> test it in the CU185 Testing release when it is updated. >> >> Thank you. I will merge it shortly. > You have merged my v2 example but it still had the square brackets and=20 > the subshell and the sed etc. >=20 > Should I do an update to the v2 version to get rid of the square=20 > brackets etc as discussed above. >=20 > Adolf. >> >> -Michael >> >>> >>> Regards, >>> Adolf. >>>> It does, however, cost another firewall restart, which could be=20 >>>> evaded with 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=20 >>>>> with the 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=20 >>>>> added >>>>> # into the optionsfw settings file and apply to firewall >>>>> if ! [ $(grep "LOGDROPHOSTILEIN=3Don" /var/ipfire/optionsfw/settings)=20 >>>>> ] && \ >>>>> =C2=A0=C2=A0=C2=A0=C2=A0 ! [ $(grep "LOGDROPHOSTILEOUT=3Don"=20 >>>>> /var/ipfire/optionsfw/settings) ]; then >>>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 sed -i '$ a\LOGD= ROPHOSTILEIN=3Don'=20 >>>>> /var/ipfire/optionsfw/settings >>>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 sed -i '$ a\LOGD= ROPHOSTILEOUT=3Don'=20 >>>>> /var/ipfire/optionsfw/settings >>>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 /usr/local/bin/f= irewallctrl >>>>> fi >>>>> >>>>> If I do an update with a Core Update 183 version that has the=20 >>>>> LOGDROPHOSTILEIN and LOGDROPHOSTILEOUT entries in=20 >>>>> optionsfw/settings missing then the update adds in the two lines=20 >>>>> shown. So working correctly. >>>>> >>>>> However if the Core Update 183 has the two entries already in the=20 >>>>> optionsfw/settings file then the above code ends up with two more=20 >>>>> copies of each put 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=20 >>>>> entries already and run the update code shown above manually via a=20 >>>>> script on the vm then it does not add any extra lines in. If the vm=20 >>>>> has the two entries missing and I run the script manually then it=20 >>>>> adds in one entry for each. >>>>> >>>>> So I do not understand at all why the code I put into the update.sh=20 >>>>> file >>>>> >>>>> 1) Does not recognise that the entries already exist in the=20 >>>>> settings file. >>>>> 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=20 >>>>> wrote would 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=20 >>>>>> the brand release of the IPFire IPS, a number of bug fixes across=20 >>>>>> the entire system and a good amount of package updates. Test it=20 >>>>>> 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 >>>>>> >>>>>> >>>>>> =C2=A0=C2=A0 IPFire_ >>>>>> >>>>>> >>>>>> =C2=A0=C2=A0 IPFire 2.29 - Core Update 185 is available for testing >>>>>> >>>>>> This update is another testing version for IPFire: It comes with=20 >>>>>> the brand release of the IPFire IPS, a number of bug fixes across=20 >>>>>> the entire system and a good amount of package updates. Test it=20 >>>>>> while it's still hot! >>>>>> >>>>>> Read The Full Post On Our Blog=20 >>>>>> >>>>>> >>>>>> The IPFire Project, c/o Lightning Wire Labs GmbH, Gerhardstra=C3=9Fe 8= ,=20 >>>>>> 45711 Datteln, Germany >>>>>> >>>>>> Unsubscribe >>>>>> >>> >>> --=20 >>> Sent from my laptop >> >> --===============6023602205137522623==--