From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Tremer To: development@lists.ipfire.org Subject: Re: IPFire 2.29 - Core Update 185 is available for testing Date: Tue, 26 Mar 2024 14:44:47 +0000 Message-ID: <3B903CAE-8F97-43E9-A0E1-D9E556F61501@ipfire.org> In-Reply-To: <5b7cd6e0-baa1-4ef7-ab2a-3a38e98ac064@ipfire.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============4384429437397814279==" List-Id: --===============4384429437397814279== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Does this work? https://git.ipfire.org/?p=3Dipfire-2.x.git;a=3Dcommitdiff;h=3Dc2df627c8c29d43= d1acfbdf60878f6a3339151e1 > On 26 Mar 2024, at 12:35, Adolf Belka wrote: >=20 > Hi Michael, >=20 > On 26/03/2024 12:45, Michael Tremer wrote: >> Hello, >>> On 26 Mar 2024, at 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 I= ran the code twice on a test system to see if it actually does what it is su= pposed to do. I felt it did. >>> If I ran the code myself on my vm system, it did the right things. It was= when I ran the CU185 update that it went wrong but I couldn't come up with a= ny idea for why. >>>=20 >>>> But is it pretty? Not quite, so let=E2=80=99s dive into a little bit of = shell :) >>> Well I am definitely not an expert at bash. Whenever I want to do somethi= ng I always have to search on the internet so that might cause me to have ugl= y code. I try and learn as I go along, if I can remember. >> That is why I thought I would give a little bit of feedback. I think somet= imes a problem is easier than it feels. > :+1: >> And after all, the code that you proposed does work. There is nothing wron= g with that in that regard. >>>>> On 25 Mar 2024, at 16:29, Adolf Belka wrote: >>>>>=20 >>>>> Hi Nick, >>>>>=20 >>>>> 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 i= f this scriptlet is safe? >>>>>>=20 >>>>>> If you have one line and not the other in the file you will end up wit= h three lines, the original plus two new. Also, if someone has preffed the li= nes off, they will gain two lines preffed on. >>>>>>=20 >>>>> Good point. If the lines are present with =3Don or =3Doff then the opti= ons have been saved and the update code would not be needed. >>>>>> Perhaps it is safer to run the tests independently, just checking for = ^LOGDROPHOSTILEIN=3D and ^LOGDROPHOSTILEOUT=3D >>>>>>=20 >>>>>> 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. >>>> 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 nece= ssary 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 signs = and is a lot easier to read. >>> The [] brackets were something I read some time ago that I should have in= place when I did my original internet searching so have always used them in = other places in IPFire. >>> Happy to be corrected and change to this approach. >> [ is actually a command. It is somewhere in /usr/bin or /bin and a shortcu= t to =E2=80=9Ctest=E2=80=9D: > Ah, I have heard of that. Thanks. >> https://www.man7.org/linux/man-pages/man1/test.1.html >>>>>> sed -i '$ a\LOGDROPHOSTILEOUT=3Don' /var/ipfire/optionsfw/settings >>>> This could have simply been an echo: >>>> echo "LOGDROPHOSTILEOUT=3Don=E2=80=9D >> /var/ipfire/optionsfw/settings >>>> That would even be executed by the shell without forking any subprocesse= s and therefore be the fastest option. Not very important if there is only tw= o strings to write, but sometimes there are loops that are being called thous= ands of times=E2=80=A6 You are going to save a lot of time and will also have= easier code to read. >>>=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. >> No need this time. >>>> Reloading the firewall once would have been fine for me, even if there h= ave 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 ha= ve found from the update as the original script does not cause the same resul= t when I manually run it. >>>>>=20 >>>>> However, definitely want to change the script anyway to make sure that = I don't end up with both =3Don and =3Doff fore the same setting which might o= ccur if someone has already adjusted their preferences. >>>>>=20 >>>>> I will probably have to submit a patch for the modification and then te= st 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 th= e subshell and the sed etc. >>>=20 >>> Should I do an update to the v2 version to get rid of the square brackets= etc as discussed above. >> I think we can leave it. This will waste 10ms of time when running the upd= ate which really isn=E2=80=99t an issue whatsoever :) >=20 > Interestingly enough it is still giving me the same problem of an additiona= l two entries for LOGDROPHOSTILEIN=3Don if the entry was already present in t= he file and the same for the OUT version. >=20 > If the file started with no LOGDROPHOSTILEIN or LOGDROPHOSTILEOUT entry the= n the update givers me the correct result of one entry for each. >=20 > I just looked at the update-core-upgrade-185.log and it has the following t= wo lines. >=20 > ./update.sh: line 120: [: LOGDROPHOSTILEIN=3Don: unary operator expected > ./update.sh: line 121: [: LOGDROPHOSTILEOUT=3Don: unary operator expected >=20 > Maybe that is what is causing me the problem. Any suggestions of what would= be causing those errors. The quoted line numbers don't show the text that is= shown in the error. >=20 > Regards, >=20 > Adolf. >>=20 >> -Michael >>>=20 >>> Adolf. >>>> -Michael >>>>>=20 >>>>> Regards, >>>>> Adolf. >>>>>> It does, however, cost another firewall restart, which could be evaded= with a few more lines of script. >>>>>>=20 >>>>>> Regards, >>>>>>=20 >>>>>> Nick >>>>>>=20 >>>>>> On 25/03/2024 15:02, Adolf Belka wrote: >>>>>>>=20 >>>>>>> Hi All, >>>>>>>=20 >>>>>>> I am having difficulty understanding something that is happening with= the Core Update to 185. >>>>>>>=20 >>>>>>> I added the following code into the update.sh script >>>>>>>=20 >>>>>>> # Check if the drop hostile in and out logging options need to be add= ed >>>>>>> # into the optionsfw settings file and apply to firewall >>>>>>> if ! [ $(grep "LOGDROPHOSTILEIN=3Don" /var/ipfire/optionsfw/settings)= ] && \ >>>>>>> ! [ $(grep "LOGDROPHOSTILEOUT=3Don" /var/ipfire/optionsfw/setting= s) ]; then >>>>>>> sed -i '$ a\LOGDROPHOSTILEIN=3Don' /var/ipfire/optionsfw/set= tings >>>>>>> sed -i '$ a\LOGDROPHOSTILEOUT=3Don' /var/ipfire/optionsfw/se= ttings >>>>>>> /usr/local/bin/firewallctrl >>>>>>> fi >>>>>>>=20 >>>>>>> If I do an update with a Core Update 183 version that has the LOGDROP= HOSTILEIN and LOGDROPHOSTILEOUT entries in optionsfw/settings missing then th= e update adds in the two lines shown. So working correctly. >>>>>>>=20 >>>>>>> However if the Core Update 183 has the two entries already in the opt= ionsfw/settings file then the above code ends up with two more copies of each= put into the file as following. >>>>>>>=20 >>>>>>> FWPOLICY=3DDROP >>>>>>> SHOWTABLES=3Don >>>>>>> DROPPROXY=3Doff >>>>>>> LOGDROPHOSTILEIN=3Don >>>>>>> LOGDROPHOSTILEOUT=3Don >>>>>>> LOGDROPHOSTILEIN=3Don >>>>>>> LOGDROPHOSTILEOUT=3Don >>>>>>>=20 >>>>>>> However if I take a vm with optionsfw/settings containing the two ent= ries 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 mi= ssing and I run the script manually then it adds in one entry for each. >>>>>>>=20 >>>>>>> So I do not understand at all why the code I put into the update.sh f= ile >>>>>>>=20 >>>>>>> 1) Does not recognise that the entries already exist in the settings = file. >>>>>>> 2) Then prints the entries twice. >>>>>>>=20 >>>>>>> when it is run in the update.sh via an upgrade. >>>>>>>=20 >>>>>>> Any help with understanding what is going wrong with the code I wrote= would be very much appreciated. >>>>>>>=20 >>>>>>> Regards, >>>>>>> Adolf. >>>>>>>=20 >>>>>>> On 25/03/2024 10:15, IPFire Project wrote: >>>>>>>> This update is another testing version for IPFire: It comes with the= brand release of the IPFire IPS, a number of bug fixes across the entire sys= tem 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 >>>>>>>>=20 >>>>>>>>=20 >>>>>>>> IPFire_ >>>>>>>>=20 >>>>>>>>=20 >>>>>>>> IPFire 2.29 - Core Update 185 is available for testing >>>>>>>>=20 >>>>>>>> This update is another testing version for IPFire: It comes with the= brand release of the IPFire IPS, a number of bug fixes across the entire sys= tem and a good amount of package updates. Test it while it's still hot! >>>>>>>>=20 >>>>>>>> Read The Full Post On Our Blog >>>>>>>>=20 >>>>>>>> The IPFire Project, c/o Lightning Wire Labs GmbH, Gerhardstra=C3=9Fe= 8, 45711 Datteln, Germany >>>>>>>>=20 >>>>>>>> Unsubscribe >>>>>>>>=20 >>>>>=20 >>>>> --=20 >>>>> Sent from my laptop --===============4384429437397814279==--