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
1) Does not recognise that the entries already exist in the 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 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
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.
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
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
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
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.
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. 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 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
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.
But is it pretty? Not quite, so let’s dive into a little bit of shell :)
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.
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.
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.
-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
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.
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.
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.
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.
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
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
Hi Michael,
On 26/03/2024 12:45, Michael Tremer wrote:
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.
:+1:
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”:
Ah, I have heard of that. Thanks.
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 :)
Interestingly enough it is still giving me the same problem of an additional two entries for LOGDROPHOSTILEIN=on if the entry was already present in the file and the same for the OUT version.
If the file started with no LOGDROPHOSTILEIN or LOGDROPHOSTILEOUT entry then the update givers me the correct result of one entry for each.
I just looked at the update-core-upgrade-185.log and it has the following two lines.
./update.sh: line 120: [: LOGDROPHOSTILEIN=on: unary operator expected ./update.sh: line 121: [: LOGDROPHOSTILEOUT=on: unary operator expected
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.
Regards,
Adolf.
-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
Does this work?
https://git.ipfire.org/?p=ipfire-2.x.git;a=commitdiff;h=c2df627c8c29d43d1acf...
On 26 Mar 2024, at 12:35, Adolf Belka adolf.belka@ipfire.org wrote:
Hi Michael,
On 26/03/2024 12:45, Michael Tremer wrote:
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.
:+1:
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”:
Ah, I have heard of that. Thanks.
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 :)
Interestingly enough it is still giving me the same problem of an additional two entries for LOGDROPHOSTILEIN=on if the entry was already present in the file and the same for the OUT version.
If the file started with no LOGDROPHOSTILEIN or LOGDROPHOSTILEOUT entry then the update givers me the correct result of one entry for each.
I just looked at the update-core-upgrade-185.log and it has the following two lines.
./update.sh: line 120: [: LOGDROPHOSTILEIN=on: unary operator expected ./update.sh: line 121: [: LOGDROPHOSTILEOUT=on: unary operator expected
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.
Regards,
Adolf.
-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 > > 1) Does not recognise that the entries already exist in the 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 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
Hi Michael,
On 26/03/2024 15:44, Michael Tremer wrote:
Does this work?
Progress has been made but not fixed.
Now there were only one additional entry added to the end of the file for LOGDROPHOSTILEIN & LOGDROPHOSTILEOUT but the entries were in the file already.
SHOWTABLES=on SHOWCOLORS=on DROPNEWNOTSYN=off DROPSPOOFEDMARTIAN=on DROPWIRELESSINPUT=off SHOWDROPDOWN=on DROPOUTGOING=on DROPWIRELESSFORWARD=on DROPPORTSCAN=on DROPFORWARD=on DROPINPUT=on DROPHOSTILE=on FWPOLICY=DROP LOGDROPHOSTILEIN=on LOGDROPCTINVALID=on FWPOLICY2=DROP DROPSAMBA=on MASQUERADE_BLUE=on MASQUERADE_GREEN=on FWPOLICY1=DROP SHOWREMARK=on LOGDROPHOSTILEOUT=on MASQUERADE_ORANGE=on DROPPROXY=off LOGDROPHOSTILEIN=on LOGDROPHOSTILEOUT=on
It acts like the code is not seeing the entries earlier in the file. However if I run the code directly on the file from the command line it does find the earlier lines and doesn't add anything.
The permissions and ownership of the optionsfw/settings file is the same as all the other files in that area so nothing abnormal there that I can find.
This time there were no error messages in the update-core-upgrade-185.log
Real puzzler.
Do you or anyone else find the same thing or is it only my system?
Regards, Adolf.
https://git.ipfire.org/?p=ipfire-2.x.git;a=commitdiff;h=c2df627c8c29d43d1acf...
On 26 Mar 2024, at 12:35, Adolf Belka adolf.belka@ipfire.org wrote:
Hi Michael,
On 26/03/2024 12:45, Michael Tremer wrote:
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.
:+1:
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”:
Ah, I have heard of that. Thanks.
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 :)
Interestingly enough it is still giving me the same problem of an additional two entries for LOGDROPHOSTILEIN=on if the entry was already present in the file and the same for the OUT version.
If the file started with no LOGDROPHOSTILEIN or LOGDROPHOSTILEOUT entry then the update givers me the correct result of one entry for each.
I just looked at the update-core-upgrade-185.log and it has the following two lines.
./update.sh: line 120: [: LOGDROPHOSTILEIN=on: unary operator expected ./update.sh: line 121: [: LOGDROPHOSTILEOUT=on: unary operator expected
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.
Regards,
Adolf.
-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 >> >> 1) Does not recognise that the entries already exist in the 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 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
I ran this multiple times on my machine and it worked.
On 27 Mar 2024, at 10:05, Adolf Belka adolf.belka@ipfire.org wrote:
Hi Michael,
On 26/03/2024 15:44, Michael Tremer wrote:
Does this work?
Progress has been made but not fixed.
Now there were only one additional entry added to the end of the file for LOGDROPHOSTILEIN & LOGDROPHOSTILEOUT but the entries were in the file already.
SHOWTABLES=on SHOWCOLORS=on DROPNEWNOTSYN=off DROPSPOOFEDMARTIAN=on DROPWIRELESSINPUT=off SHOWDROPDOWN=on DROPOUTGOING=on DROPWIRELESSFORWARD=on DROPPORTSCAN=on DROPFORWARD=on DROPINPUT=on DROPHOSTILE=on FWPOLICY=DROP LOGDROPHOSTILEIN=on LOGDROPCTINVALID=on FWPOLICY2=DROP DROPSAMBA=on MASQUERADE_BLUE=on MASQUERADE_GREEN=on FWPOLICY1=DROP SHOWREMARK=on LOGDROPHOSTILEOUT=on MASQUERADE_ORANGE=on DROPPROXY=off LOGDROPHOSTILEIN=on LOGDROPHOSTILEOUT=on
It acts like the code is not seeing the entries earlier in the file. However if I run the code directly on the file from the command line it does find the earlier lines and doesn't add anything.
The permissions and ownership of the optionsfw/settings file is the same as all the other files in that area so nothing abnormal there that I can find.
This time there were no error messages in the update-core-upgrade-185.log
Real puzzler.
Do you or anyone else find the same thing or is it only my system?
Regards, Adolf.
https://git.ipfire.org/?p=ipfire-2.x.git;a=commitdiff;h=c2df627c8c29d43d1acf...
On 26 Mar 2024, at 12:35, Adolf Belka adolf.belka@ipfire.org wrote:
Hi Michael,
On 26/03/2024 12:45, Michael Tremer wrote:
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.
:+1:
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”:
Ah, I have heard of that. Thanks.
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 :)
Interestingly enough it is still giving me the same problem of an additional two entries for LOGDROPHOSTILEIN=on if the entry was already present in the file and the same for the OUT version.
If the file started with no LOGDROPHOSTILEIN or LOGDROPHOSTILEOUT entry then the update givers me the correct result of one entry for each.
I just looked at the update-core-upgrade-185.log and it has the following two lines.
./update.sh: line 120: [: LOGDROPHOSTILEIN=on: unary operator expected ./update.sh: line 121: [: LOGDROPHOSTILEOUT=on: unary operator expected
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.
Regards,
Adolf.
-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 >>> >>> 1) Does not recognise that the entries already exist in the 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 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
Hi Michael,
On 27/03/2024 11:14, Michael Tremer wrote:
I ran this multiple times on my machine and it worked.
Okay, then let's leave it as it is. I clearly have some sort of corner situation. The code is nicer now anyway.
Having two entries does not cause any problems. I have checked in the iptables and the LOGDROPHOSTILEIN and LOGDROPHOSTILEOUT only have one entry each in the iptables so the logging will occur correctly.
If I press the Save button on the FW Options page then the settings file is updated correctly anyway.
So even if some other people end up with two entries everything will still work correctly anyway.
So considering this issue fixed now.
Regards, Adolf.
On 27 Mar 2024, at 10:05, Adolf Belka adolf.belka@ipfire.org wrote:
Hi Michael,
On 26/03/2024 15:44, Michael Tremer wrote:
Does this work?
Progress has been made but not fixed.
Now there were only one additional entry added to the end of the file for LOGDROPHOSTILEIN & LOGDROPHOSTILEOUT but the entries were in the file already.
SHOWTABLES=on SHOWCOLORS=on DROPNEWNOTSYN=off DROPSPOOFEDMARTIAN=on DROPWIRELESSINPUT=off SHOWDROPDOWN=on DROPOUTGOING=on DROPWIRELESSFORWARD=on DROPPORTSCAN=on DROPFORWARD=on DROPINPUT=on DROPHOSTILE=on FWPOLICY=DROP LOGDROPHOSTILEIN=on LOGDROPCTINVALID=on FWPOLICY2=DROP DROPSAMBA=on MASQUERADE_BLUE=on MASQUERADE_GREEN=on FWPOLICY1=DROP SHOWREMARK=on LOGDROPHOSTILEOUT=on MASQUERADE_ORANGE=on DROPPROXY=off LOGDROPHOSTILEIN=on LOGDROPHOSTILEOUT=on
It acts like the code is not seeing the entries earlier in the file. However if I run the code directly on the file from the command line it does find the earlier lines and doesn't add anything.
The permissions and ownership of the optionsfw/settings file is the same as all the other files in that area so nothing abnormal there that I can find.
This time there were no error messages in the update-core-upgrade-185.log
Real puzzler.
Do you or anyone else find the same thing or is it only my system?
Regards, Adolf.
https://git.ipfire.org/?p=ipfire-2.x.git;a=commitdiff;h=c2df627c8c29d43d1acf...
On 26 Mar 2024, at 12:35, Adolf Belka adolf.belka@ipfire.org wrote:
Hi Michael,
On 26/03/2024 12:45, Michael Tremer wrote:
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.
:+1:
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”:
Ah, I have heard of that. Thanks.
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 :)
Interestingly enough it is still giving me the same problem of an additional two entries for LOGDROPHOSTILEIN=on if the entry was already present in the file and the same for the OUT version.
If the file started with no LOGDROPHOSTILEIN or LOGDROPHOSTILEOUT entry then the update givers me the correct result of one entry for each.
I just looked at the update-core-upgrade-185.log and it has the following two lines.
./update.sh: line 120: [: LOGDROPHOSTILEIN=on: unary operator expected ./update.sh: line 121: [: LOGDROPHOSTILEOUT=on: unary operator expected
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.
Regards,
Adolf.
-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 >>>> >>>> 1) Does not recognise that the entries already exist in the 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 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
On 26/03/2024 11:37, Adolf Belka 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.
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.
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.
I fell over here once and sed may have the same issue. If there is no line terminator (I can never remember if it is CR, LF or both) at the end of a file, it will append to the last line rather than adding a new line. vi always adds a line terminator but java data files don't seem to have one, in my experience.
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 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.
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. 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 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
-
Adolf Belka -
Michael Tremer -
Nick Howitt