Re: IPFire 2.29 - Core Update 185 is available for testing

Re: IPFire 2.29 - Core Update 185 is available for testing

[Adolf Belka]

[-- Attachment #1: Type: text/plain, Size: 1736 bytes --]

Hi Michael,

First issue found. Due to a boo-boo made by me related to wsdd.

I had submitted what I thought was a patch set for the samba initscript together with an update of the samba lfs file to add wsdd as a dependency. Unfortunately it turned out I had provided a single patch with both changes in the same patch

You raised the question about not having the samba initscript calling the wsdd initscript. I updated that script and submitted it but thought the change of adding the wsdd as a dependency had been merged so did not send a v2 of that change.

So wsdd is not getting installed when samba is being updated.

I will submit a new patch for the samba lfs update to add wsdd as a dependency.

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>
> 

Re: IPFire 2.29 - Core Update 185 is available for testing

[Michael Tremer]

[-- Attachment #1: Type: text/plain, Size: 2016 bytes --]

Hello Adolf,

I also slightly cramp when I see emails regarding to these announcements.

But this isn’t a big problem at all. It is an add-ons so we don’t even have to touch the updater.

-Michael

> On 25 Mar 2024, at 11:10, Adolf Belka <adolf.belka(a)ipfire.org> wrote:
> 
> Hi Michael,
> 
> First issue found. Due to a boo-boo made by me related to wsdd.
> 
> I had submitted what I thought was a patch set for the samba initscript together with an update of the samba lfs file to add wsdd as a dependency. Unfortunately it turned out I had provided a single patch with both changes in the same patch
> 
> You raised the question about not having the samba initscript calling the wsdd initscript. I updated that script and submitted it but thought the change of adding the wsdd as a dependency had been merged so did not send a v2 of that change.
> 
> So wsdd is not getting installed when samba is being updated.
> 
> I will submit a new patch for the samba lfs update to add wsdd as a dependency.
> 
> 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>

Re: IPFire 2.29 - Core Update 185 is available for testing

[Adolf Belka]

[-- Attachment #1: Type: text/plain, Size: 3350 bytes --]

Hi Michael & Stefan,

On 25/03/2024 12:12, Michael Tremer wrote:
> Hello Adolf,
> 
> I also slightly cramp when I see emails regarding to these announcements.

Sorry for causing you that feeling.

Unfortunately, this next issue may require bigger changes.

Doing an upgrade with my existing suricata providers worked without any problem.

I then created a new CU184 vm and added PT Attack into it and selected the rules.

Then I ran the CU185 Testing update and the WUI page basically stuck at the running the CU185 and post scripts section.

I checked via the console and the update had completed with no errors in the update logfile.

However the WUI page fails to connect. It times out.

I tried this twice more with and without PT Attack in the suricata providers. Whenever P Attack was included the WUI page stops functioning.

Then on a vm that was timing out for the WUI page I stopped suricata via the console. The WUI page then became accessible again. I have tested this a second time and confirmed that a non responding WUI becomes responding again when suricata is stopped.

I have no idea what is causing the problem but I have been able to reproduce it several times now with my vm systems.

I will add this info into the Suricata bug that has been raised by Michael earlier.
https://bugzilla.ipfire.org/show_bug.cgi?id=13638

Regards,

Adolf.

> 
> But this isn’t a big problem at all. It is an add-ons so we don’t even have to touch the updater.
> 
> -Michael
> 
>> On 25 Mar 2024, at 11:10, Adolf Belka <adolf.belka(a)ipfire.org> wrote:
>>
>> Hi Michael,
>>
>> First issue found. Due to a boo-boo made by me related to wsdd.
>>
>> I had submitted what I thought was a patch set for the samba initscript together with an update of the samba lfs file to add wsdd as a dependency. Unfortunately it turned out I had provided a single patch with both changes in the same patch
>>
>> You raised the question about not having the samba initscript calling the wsdd initscript. I updated that script and submitted it but thought the change of adding the wsdd as a dependency had been merged so did not send a v2 of that change.
>>
>> So wsdd is not getting installed when samba is being updated.
>>
>> I will submit a new patch for the samba lfs update to add wsdd as a dependency.
>>
>> 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>
> 

Re: IPFire 2.29 - Core Update 185 is available for testing

[Adolf Belka]

[-- Attachment #1: Type: text/plain, Size: 2824 bytes --]

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>
> 

Re: IPFire 2.29 - Core Update 185 is available for testing

[Nick Howitt]

[-- Attachment #1: Type: text/plain, Size: 3976 bytes --]

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
> 
> 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>
>>

Re: IPFire 2.29 - Core Update 185 is available for testing

[Adolf Belka]

[-- Attachment #1: Type: text/plain, Size: 4909 bytes --]

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
>>
>> 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

Re: IPFire 2.29 - Core Update 185 is available for testing

[Michael Tremer]

[-- Attachment #1: Type: text/plain, Size: 6360 bytes --]

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(a)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
>>> 
>>> 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

Re: IPFire 2.29 - Core Update 185 is available for testing

[Adolf Belka]

[-- Attachment #1: Type: text/plain, Size: 7511 bytes --]

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(a)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
>>>>
>>>> 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
> 
> 

Re: IPFire 2.29 - Core Update 185 is available for testing

[Michael Tremer]

[-- Attachment #1: Type: text/plain, Size: 8239 bytes --]

Hello,

> On 26 Mar 2024, at 11:37, Adolf Belka <adolf.belka(a)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(a)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
>>>>> 
>>>>> 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

Re: IPFire 2.29 - Core Update 185 is available for testing

[Adolf Belka]

[-- Attachment #1: Type: text/plain, Size: 9256 bytes --]

Hi Michael,

On 26/03/2024 12:45, Michael Tremer wrote:
> Hello,
> 
>> On 26 Mar 2024, at 11:37, Adolf Belka <adolf.belka(a)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(a)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
> 

Re: IPFire 2.29 - Core Update 185 is available for testing

[Nick Howitt]

[-- Attachment #1: Type: text/plain, Size: 8449 bytes --]



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(a)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
>>>>>
>>>>> 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
>>
>>

Re: IPFire 2.29 - Core Update 185 is available for testing

[Michael Tremer]

[-- Attachment #1: Type: text/plain, Size: 9625 bytes --]

Does this work?

https://git.ipfire.org/?p=ipfire-2.x.git;a=commitdiff;h=c2df627c8c29d43d1acfbdf60878f6a3339151e1

> On 26 Mar 2024, at 12:35, Adolf Belka <adolf.belka(a)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(a)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(a)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

Re: IPFire 2.29 - Core Update 185 is available for testing

[Adolf Belka]

[-- Attachment #1: Type: text/plain, Size: 11035 bytes --]

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=c2df627c8c29d43d1acfbdf60878f6a3339151e1
> 
>> On 26 Mar 2024, at 12:35, Adolf Belka <adolf.belka(a)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(a)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(a)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
> 
> 

Re: IPFire 2.29 - Core Update 185 is available for testing

[Michael Tremer]

[-- Attachment #1: Type: text/plain, Size: 11460 bytes --]

I ran this multiple times on my machine and it worked.

> On 27 Mar 2024, at 10:05, Adolf Belka <adolf.belka(a)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=c2df627c8c29d43d1acfbdf60878f6a3339151e1
>>> On 26 Mar 2024, at 12:35, Adolf Belka <adolf.belka(a)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(a)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(a)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

Re: IPFire 2.29 - Core Update 185 is available for testing

[Adolf Belka]

[-- Attachment #1: Type: text/plain, Size: 12319 bytes --]

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(a)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=c2df627c8c29d43d1acfbdf60878f6a3339151e1
>>>> On 26 Mar 2024, at 12:35, Adolf Belka <adolf.belka(a)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(a)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(a)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
> 

-- 
Sent from my laptop