Hello,
On 26 Mar 2024, at 11:37, Adolf Belka adolf.belka@ipfire.org wrote:
Hi,
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 supposed 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 any idea for why.
But is it pretty? Not quite, so let’s dive into a little bit of shell :)
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 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 sometimes a problem is easier than it feels.
And after all, the code that you proposed does work. There is nothing wrong with that in that regard.
On 25 Mar 2024, at 16:29, Adolf Belka adolf.belka@ipfire.org 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 this scriptlet is safe?
If you have one line and not the other in the file you will end up with three 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 =on or =off 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 ^LOGDROPHOSTILEIN= and ^LOGDROPHOSTILEOUT=
if ! grep "^LOGDROPHOSTILEIN=" /var/ipfire/optionsfw/settings; then sed -i '$ a\LOGDROPHOSTILEIN=on' /var/ipfire/optionsfw/settings /usr/local/bin/firewallctrl fi if ! grep "^LOGDROPHOSTILEOUT=" /var/ipfire/optionsfw/settings; then sed -i '$ a\LOGDROPHOSTILEOUT=on' /var/ipfire/optionsfw/settings /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=on" /var/ipfire/optionsfw/settings) ]
This will run a sub shell $(…) with grep in it which is not necessary since if alone can determine the exit code of a command.
if ! grep "^LOGDROPHOSTILEIN=" /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 shortcut to “test”:
https://www.man7.org/linux/man-pages/man1/test.1.html
sed -i '$ a\LOGDROPHOSTILEOUT=on' /var/ipfire/optionsfw/settings
This could have simply been an echo: echo "LOGDROPHOSTILEOUT=on” >> /var/ipfire/optionsfw/settings That would even be executed by the shell without forking any subprocesses and therefore be the fastest option. Not very important if there is only two strings to write, but sometimes there are loops that are being called thousands of times… You are going to save a lot of time and will also have easier 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.
No need this time.
Reloading the firewall once would have been fine for me, even if there 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 have found from the update as the original script does not cause the same result when I manually run it.
However, definitely want to change the script anyway to make sure that I don't end up with both =on and =off fore the same setting which might occur 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.
Thank you. I will merge it shortly.
You have merged my v2 example but it still had the square brackets and the subshell and the sed etc.
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 update which really isn’t an issue whatsoever :)
-Michael
Adolf.
-Michael
Regards, Adolf.
It does, however, cost another firewall restart, which could be 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 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 added # into the optionsfw settings file and apply to firewall if ! [ $(grep "LOGDROPHOSTILEIN=on" /var/ipfire/optionsfw/settings) ] && \ ! [ $(grep "LOGDROPHOSTILEOUT=on" /var/ipfire/optionsfw/settings) ]; then sed -i '$ a\LOGDROPHOSTILEIN=on' /var/ipfire/optionsfw/settings sed -i '$ a\LOGDROPHOSTILEOUT=on' /var/ipfire/optionsfw/settings /usr/local/bin/firewallctrl fi
If I do an update with a Core Update 183 version that has the LOGDROPHOSTILEIN and LOGDROPHOSTILEOUT entries in optionsfw/settings missing then the update adds in the two lines shown. So working correctly.
However if the Core Update 183 has the two entries already in the optionsfw/settings file then the above code ends up with two more copies of each put into the file as following.
FWPOLICY=DROP SHOWTABLES=on DROPPROXY=off LOGDROPHOSTILEIN=on LOGDROPHOSTILEOUT=on LOGDROPHOSTILEIN=on LOGDROPHOSTILEOUT=on
However if I take a vm with optionsfw/settings containing the two entries 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 missing 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
- Does not recognise that the entries already exist in the settings file.
- 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 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 the brand 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!
IPFire_
IPFire 2.29 - Core Update 185 is available for testing
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 system and a good amount of package updates. Test it while it's still hot!
Read The Full Post On Our Blog https://www.ipfire.org/blog/ipfire-2-29-core-update-185-is-available-for-testing?utm_medium=email&utm_source=blog-announcement
The IPFire Project, c/o Lightning Wire Labs GmbH, Gerhardstraße 8, 45711 Datteln, Germany
Unsubscribe https://www.ipfire.org/unsubscribe
-- Sent from my laptop