[2/3] backup.pl: Remove the previous code for adding legacty provider to n2n

Message ID 20230607142150.18407-2-adolf.belka@ipfire.org
State New
Headers
Series [1/3] ovpnmain.cgi: Updated fix for Bug#13137 |

Commit Message

Adolf Belka June 7, 2023, 2:21 p.m. UTC
  - This code is no longer needed with the code in the ovpnmain.cgi patch in this patch set.

Tested-by: Adolf Belka <adolf.belka@ipfire.org>
Signed-off-by: Adolf Belka <adolf.belka@ipfire.org>
---
 config/backup/backup.pl | 15 ---------------
 1 file changed, 15 deletions(-)
  

Comments

Michael Tremer June 10, 2023, 10:16 a.m. UTC | #1
I did not merge this, as I believe we need this, because:

We won’t rewrite the OpenVPN configuration files on update, so it might be a good idea to just add the line and if someone edits the connection it might be removed.

That should work I believe and -legacy should not have any side effects when enabled but not needed.

Best,
-Michael

> On 7 Jun 2023, at 15:21, Adolf Belka <adolf.belka@ipfire.org> wrote:
> 
> - This code is no longer needed with the code in the ovpnmain.cgi patch in this patch set.
> 
> Tested-by: Adolf Belka <adolf.belka@ipfire.org>
> Signed-off-by: Adolf Belka <adolf.belka@ipfire.org>
> ---
> config/backup/backup.pl | 15 ---------------
> 1 file changed, 15 deletions(-)
> 
> diff --git a/config/backup/backup.pl b/config/backup/backup.pl
> index 8d990c0f1..60138a58a 100644
> --- a/config/backup/backup.pl
> +++ b/config/backup/backup.pl
> @@ -190,21 +190,6 @@ restore_backup() {
> # Update OpenVPN CRL
> /etc/fcron.daily/openvpn-crl-updater
> 
> - # Update OpenVPN N2N Client Configs
> - ## Add providers legacy default line to n2n client config files
> - # Check if ovpnconfig exists and is not empty
> - if [ -s /var/ipfire/ovpn/ovpnconfig ]; then
> -       # Identify all n2n connections
> -       for y in $(awk -F',' '/net/ { print $3 }' /var/ipfire/ovpn/ovpnconfig); do
> -           # Add the legacy option to all N2N client conf files if it does not already exist
> - if [ $(grep -c "Open VPN Client Config" /var/ipfire/ovpn/n2nconf/${y}/${y}.conf) -eq 1 ] ; then
> - if [ $(grep -c "providers legacy default" /var/ipfire/ovpn/n2nconf/${y}/${y}.conf) -eq 0 ] ; then
> - echo "providers legacy default" >> /var/ipfire/ovpn/n2nconf/${y}/${y}.conf
> - fi
> - fi
> -       done
> - fi
> -
> return 0
> }
> 
> -- 
> 2.40.1
>
  
Adolf Belka June 10, 2023, 11:16 a.m. UTC | #2
Hi Michael,

On 10/06/2023 12:16, Michael Tremer wrote:
> I did not merge this, as I believe we need this, because:
> 
> We won’t rewrite the OpenVPN configuration files on update, so it might be a good idea to just add the line and if someone edits the connection it might be removed.
The code in the backup.pl put the line into the config irrespective of 
the certificate being legacy or not.

With the ovpnmain.cgi code patch of this patch set, it now only adds the 
providers legacy default to the config file if the cert is legacy when 
downloading the connection set. This is now done for both n2n and 
roadwarrior connection sets.
> 
> That should work I believe and -legacy should not have any side effects when enabled but not needed.
That is something I have not tested out but I think you are correct, it 
shouldn't have any side affects.

I think it is good to go now and I can always do any additional minor 
tunings later in CU176 and onwards, otherwise we will be here for ever.

Regards,

Adolf.
> 
> Best,
> -Michael
> 
>> On 7 Jun 2023, at 15:21, Adolf Belka <adolf.belka@ipfire.org> wrote:
>>
>> - This code is no longer needed with the code in the ovpnmain.cgi patch in this patch set.
>>
>> Tested-by: Adolf Belka <adolf.belka@ipfire.org>
>> Signed-off-by: Adolf Belka <adolf.belka@ipfire.org>
>> ---
>> config/backup/backup.pl | 15 ---------------
>> 1 file changed, 15 deletions(-)
>>
>> diff --git a/config/backup/backup.pl b/config/backup/backup.pl
>> index 8d990c0f1..60138a58a 100644
>> --- a/config/backup/backup.pl
>> +++ b/config/backup/backup.pl
>> @@ -190,21 +190,6 @@ restore_backup() {
>> # Update OpenVPN CRL
>> /etc/fcron.daily/openvpn-crl-updater
>>
>> - # Update OpenVPN N2N Client Configs
>> - ## Add providers legacy default line to n2n client config files
>> - # Check if ovpnconfig exists and is not empty
>> - if [ -s /var/ipfire/ovpn/ovpnconfig ]; then
>> -       # Identify all n2n connections
>> -       for y in $(awk -F',' '/net/ { print $3 }' /var/ipfire/ovpn/ovpnconfig); do
>> -           # Add the legacy option to all N2N client conf files if it does not already exist
>> - if [ $(grep -c "Open VPN Client Config" /var/ipfire/ovpn/n2nconf/${y}/${y}.conf) -eq 1 ] ; then
>> - if [ $(grep -c "providers legacy default" /var/ipfire/ovpn/n2nconf/${y}/${y}.conf) -eq 0 ] ; then
>> - echo "providers legacy default" >> /var/ipfire/ovpn/n2nconf/${y}/${y}.conf
>> - fi
>> - fi
>> -       done
>> - fi
>> -
>> return 0
>> }
>>
>> -- 
>> 2.40.1
>>
>
  
Michael Tremer June 10, 2023, 11:28 a.m. UTC | #3
Hello,

> On 10 Jun 2023, at 12:16, Adolf Belka <adolf.belka@ipfire.org> wrote:
> 
> Hi Michael,
> 
> On 10/06/2023 12:16, Michael Tremer wrote:
>> I did not merge this, as I believe we need this, because:
>> We won’t rewrite the OpenVPN configuration files on update, so it might be a good idea to just add the line and if someone edits the connection it might be removed.
> The code in the backup.pl put the line into the config irrespective of the certificate being legacy or not.
> 
> With the ovpnmain.cgi code patch of this patch set, it now only adds the providers legacy default to the config file if the cert is legacy when downloading the connection set. This is now done for both n2n and roadwarrior connection sets.

Yes, this is true, but we won’t run the CGI during the update.

Any connections that have legacy certificates won’t work after installing the new version of OpenSSL. So we need the legacy provider enabled (just to be safe).

>> That should work I believe and -legacy should not have any side effects when enabled but not needed.
> That is something I have not tested out but I think you are correct, it shouldn't have any side affects.
> 
> I think it is good to go now and I can always do any additional minor tunings later in CU176 and onwards, otherwise we will be here for ever.

I would rather like to get it right than being fast, but at this point I don’t know what else we can do. So *fingers crossed*.

Let’s release either tomorrow or Monday. Depending on how much I am going to enjoy the nice weather this weekend :)

-Michael

> 
> Regards,
> 
> Adolf.
>> Best,
>> -Michael
>>> On 7 Jun 2023, at 15:21, Adolf Belka <adolf.belka@ipfire.org> wrote:
>>> 
>>> - This code is no longer needed with the code in the ovpnmain.cgi patch in this patch set.
>>> 
>>> Tested-by: Adolf Belka <adolf.belka@ipfire.org>
>>> Signed-off-by: Adolf Belka <adolf.belka@ipfire.org>
>>> ---
>>> config/backup/backup.pl | 15 ---------------
>>> 1 file changed, 15 deletions(-)
>>> 
>>> diff --git a/config/backup/backup.pl b/config/backup/backup.pl
>>> index 8d990c0f1..60138a58a 100644
>>> --- a/config/backup/backup.pl
>>> +++ b/config/backup/backup.pl
>>> @@ -190,21 +190,6 @@ restore_backup() {
>>> # Update OpenVPN CRL
>>> /etc/fcron.daily/openvpn-crl-updater
>>> 
>>> - # Update OpenVPN N2N Client Configs
>>> - ## Add providers legacy default line to n2n client config files
>>> - # Check if ovpnconfig exists and is not empty
>>> - if [ -s /var/ipfire/ovpn/ovpnconfig ]; then
>>> -       # Identify all n2n connections
>>> -       for y in $(awk -F',' '/net/ { print $3 }' /var/ipfire/ovpn/ovpnconfig); do
>>> -           # Add the legacy option to all N2N client conf files if it does not already exist
>>> - if [ $(grep -c "Open VPN Client Config" /var/ipfire/ovpn/n2nconf/${y}/${y}.conf) -eq 1 ] ; then
>>> - if [ $(grep -c "providers legacy default" /var/ipfire/ovpn/n2nconf/${y}/${y}.conf) -eq 0 ] ; then
>>> - echo "providers legacy default" >> /var/ipfire/ovpn/n2nconf/${y}/${y}.conf
>>> - fi
>>> - fi
>>> -       done
>>> - fi
>>> -
>>> return 0
>>> }
>>> 
>>> -- 
>>> 2.40.1
>>> 
> 
> -- 
> Sent from my laptop
  
Adolf Belka June 11, 2023, 1:17 p.m. UTC | #4
Hi Michael,

On 10/06/2023 13:28, Michael Tremer wrote:
> Hello,
> 
>> On 10 Jun 2023, at 12:16, Adolf Belka <adolf.belka@ipfire.org> wrote:
>>
>> Hi Michael,
>>
>> On 10/06/2023 12:16, Michael Tremer wrote:
>>> I did not merge this, as I believe we need this, because:
>>> We won’t rewrite the OpenVPN configuration files on update, so it might be a good idea to just add the line and if someone edits the connection it might be removed.
>> The code in the backup.pl put the line into the config irrespective of the certificate being legacy or not.
>>
>> With the ovpnmain.cgi code patch of this patch set, it now only adds the providers legacy default to the config file if the cert is legacy when downloading the connection set. This is now done for both n2n and roadwarrior connection sets.
> 
> Yes, this is true, but we won’t run the CGI during the update.
> 
> Any connections that have legacy certificates won’t work after installing the new version of OpenSSL. So we need the legacy provider enabled (just to be safe).

Okay, understand where you are coming from.Good catch.

I have also now tested out a n2n connection created with openssl-3.x with and without the providers legacy default line in the client conf.
Can confirm that it works in both cases, so having the legacy line added dose not cause any problems with the openssl-3.x n2n client connection working.

> 
>>> That should work I believe and -legacy should not have any side effects when enabled but not needed.
>> That is something I have not tested out but I think you are correct, it shouldn't have any side affects.
>>
>> I think it is good to go now and I can always do any additional minor tunings later in CU176 and onwards, otherwise we will be here for ever.
> 
> I would rather like to get it right than being fast, but at this point I don’t know what else we can do. So *fingers crossed*.
> 
> Let’s release either tomorrow or Monday. Depending on how much I am going to enjoy the nice weather this weekend :)

Enjoy the nice weather.

Regards,
Adolf.

> 
> -Michael
> 
>>
>> Regards,
>>
>> Adolf.
>>> Best,
>>> -Michael
>>>> On 7 Jun 2023, at 15:21, Adolf Belka <adolf.belka@ipfire.org> wrote:
>>>>
>>>> - This code is no longer needed with the code in the ovpnmain.cgi patch in this patch set.
>>>>
>>>> Tested-by: Adolf Belka <adolf.belka@ipfire.org>
>>>> Signed-off-by: Adolf Belka <adolf.belka@ipfire.org>
>>>> ---
>>>> config/backup/backup.pl | 15 ---------------
>>>> 1 file changed, 15 deletions(-)
>>>>
>>>> diff --git a/config/backup/backup.pl b/config/backup/backup.pl
>>>> index 8d990c0f1..60138a58a 100644
>>>> --- a/config/backup/backup.pl
>>>> +++ b/config/backup/backup.pl
>>>> @@ -190,21 +190,6 @@ restore_backup() {
>>>> # Update OpenVPN CRL
>>>> /etc/fcron.daily/openvpn-crl-updater
>>>>
>>>> - # Update OpenVPN N2N Client Configs
>>>> - ## Add providers legacy default line to n2n client config files
>>>> - # Check if ovpnconfig exists and is not empty
>>>> - if [ -s /var/ipfire/ovpn/ovpnconfig ]; then
>>>> -       # Identify all n2n connections
>>>> -       for y in $(awk -F',' '/net/ { print $3 }' /var/ipfire/ovpn/ovpnconfig); do
>>>> -           # Add the legacy option to all N2N client conf files if it does not already exist
>>>> - if [ $(grep -c "Open VPN Client Config" /var/ipfire/ovpn/n2nconf/${y}/${y}.conf) -eq 1 ] ; then
>>>> - if [ $(grep -c "providers legacy default" /var/ipfire/ovpn/n2nconf/${y}/${y}.conf) -eq 0 ] ; then
>>>> - echo "providers legacy default" >> /var/ipfire/ovpn/n2nconf/${y}/${y}.conf
>>>> - fi
>>>> - fi
>>>> -       done
>>>> - fi
>>>> -
>>>> return 0
>>>> }
>>>>
>>>> -- 
>>>> 2.40.1
>>>>
>>
>> -- 
>> Sent from my laptop
> 
>
  
Michael Tremer June 11, 2023, 4:29 p.m. UTC | #5
Thanks for confirming.

I will schedule the release for tomorrow then as there hasn’t been any issues any more.

Thanks to everyone who helped to *finally* get this over the line and I will keep my fingers crossed that we found all issues.

Best,
-Michael

> On 11 Jun 2023, at 14:17, Adolf Belka <adolf.belka@ipfire.org> wrote:
> 
> Hi Michael,
> 
>> On 10/06/2023 13:28, Michael Tremer wrote:
>> Hello,
>>>> On 10 Jun 2023, at 12:16, Adolf Belka <adolf.belka@ipfire.org> wrote:
>>> 
>>> Hi Michael,
>>> 
>>> On 10/06/2023 12:16, Michael Tremer wrote:
>>>> I did not merge this, as I believe we need this, because:
>>>> We won’t rewrite the OpenVPN configuration files on update, so it might be a good idea to just add the line and if someone edits the connection it might be removed.
>>> The code in the backup.pl put the line into the config irrespective of the certificate being legacy or not.
>>> 
>>> With the ovpnmain.cgi code patch of this patch set, it now only adds the providers legacy default to the config file if the cert is legacy when downloading the connection set. This is now done for both n2n and roadwarrior connection sets.
>> Yes, this is true, but we won’t run the CGI during the update.
>> Any connections that have legacy certificates won’t work after installing the new version of OpenSSL. So we need the legacy provider enabled (just to be safe).
> 
> Okay, understand where you are coming from.Good catch.
> 
> I have also now tested out a n2n connection created with openssl-3.x with and without the providers legacy default line in the client conf.
> Can confirm that it works in both cases, so having the legacy line added dose not cause any problems with the openssl-3.x n2n client connection working.
> 
>>>> That should work I believe and -legacy should not have any side effects when enabled but not needed.
>>> That is something I have not tested out but I think you are correct, it shouldn't have any side affects.
>>> 
>>> I think it is good to go now and I can always do any additional minor tunings later in CU176 and onwards, otherwise we will be here for ever.
>> I would rather like to get it right than being fast, but at this point I don’t know what else we can do. So *fingers crossed*.
>> Let’s release either tomorrow or Monday. Depending on how much I am going to enjoy the nice weather this weekend :)
> 
> Enjoy the nice weather.
> 
> Regards,
> Adolf.
> 
>> -Michael
>>> 
>>> Regards,
>>> 
>>> Adolf.
>>>> Best,
>>>> -Michael
>>>>> On 7 Jun 2023, at 15:21, Adolf Belka <adolf.belka@ipfire.org> wrote:
>>>>> 
>>>>> - This code is no longer needed with the code in the ovpnmain.cgi patch in this patch set.
>>>>> 
>>>>> Tested-by: Adolf Belka <adolf.belka@ipfire.org>
>>>>> Signed-off-by: Adolf Belka <adolf.belka@ipfire.org>
>>>>> ---
>>>>> config/backup/backup.pl | 15 ---------------
>>>>> 1 file changed, 15 deletions(-)
>>>>> 
>>>>> diff --git a/config/backup/backup.pl b/config/backup/backup.pl
>>>>> index 8d990c0f1..60138a58a 100644
>>>>> --- a/config/backup/backup.pl
>>>>> +++ b/config/backup/backup.pl
>>>>> @@ -190,21 +190,6 @@ restore_backup() {
>>>>> # Update OpenVPN CRL
>>>>> /etc/fcron.daily/openvpn-crl-updater
>>>>> 
>>>>> - # Update OpenVPN N2N Client Configs
>>>>> - ## Add providers legacy default line to n2n client config files
>>>>> - # Check if ovpnconfig exists and is not empty
>>>>> - if [ -s /var/ipfire/ovpn/ovpnconfig ]; then
>>>>> -       # Identify all n2n connections
>>>>> -       for y in $(awk -F',' '/net/ { print $3 }' /var/ipfire/ovpn/ovpnconfig); do
>>>>> -           # Add the legacy option to all N2N client conf files if it does not already exist
>>>>> - if [ $(grep -c "Open VPN Client Config" /var/ipfire/ovpn/n2nconf/${y}/${y}.conf) -eq 1 ] ; then
>>>>> - if [ $(grep -c "providers legacy default" /var/ipfire/ovpn/n2nconf/${y}/${y}.conf) -eq 0 ] ; then
>>>>> - echo "providers legacy default" >> /var/ipfire/ovpn/n2nconf/${y}/${y}.conf
>>>>> - fi
>>>>> - fi
>>>>> -       done
>>>>> - fi
>>>>> -
>>>>> return 0
>>>>> }
>>>>> 
>>>>> -- 
>>>>> 2.40.1
>>>>> 
>>> 
>>> -- 
>>> Sent from my laptop
  

Patch

diff --git a/config/backup/backup.pl b/config/backup/backup.pl
index 8d990c0f1..60138a58a 100644
--- a/config/backup/backup.pl
+++ b/config/backup/backup.pl
@@ -190,21 +190,6 @@  restore_backup() {
 	# Update OpenVPN CRL
 	/etc/fcron.daily/openvpn-crl-updater
 	
-	# Update OpenVPN N2N Client Configs
-	## Add providers legacy default line to n2n client config files
-	# Check if ovpnconfig exists and is not empty
-	if [ -s /var/ipfire/ovpn/ovpnconfig ]; then
-	       # Identify all n2n connections
-	       for y in $(awk -F',' '/net/ { print $3 }' /var/ipfire/ovpn/ovpnconfig); do
-	           # Add the legacy option to all N2N client conf files if it does not already exist
-			if [ $(grep -c "Open VPN Client Config" /var/ipfire/ovpn/n2nconf/${y}/${y}.conf) -eq 1 ] ; then
-				if [ $(grep -c "providers legacy default" /var/ipfire/ovpn/n2nconf/${y}/${y}.conf) -eq 0 ] ; then
-					echo "providers legacy default" >> /var/ipfire/ovpn/n2nconf/${y}/${y}.conf
-				fi
-			fi
-	       done
-	fi
-
 	return 0
 }