Perfect. I am happy that we found a solution for this.
> On 29 Aug 2025, at 17:50, Adolf Belka <adolf.be...@ipfire.org> wrote:
>
> Hi Michael,
>
> On 29/08/2025 12:42, Michael Tremer wrote:
>> Hello Adolf,
>> Thank you for looking at this in detail. I was a little bit confused first,
>> but I think I am now on the same page:
>> * The issue is that we write ncp-disable into the configuration file if no
>> data ciphers have been selected.
>> This should have worked fine in OpenVPN 2.5 (for which this code was
>> written), but since ncp-disable was now removed in OpenVPN 2.6, we can no
>> longer do this.
>> Normally we would never land here, because there would always be a default
>> for DATACIPHERS set in
>> https://git.ipfire.org/?p=ipfire-2.x.git;a=blob;f=html/cgi-bin/ovpnmain.cgi;hb=198025111e37a80944dbab9ddd57967945e27949#l117
>> Since we have now modified this, we can no longer rely on any of these
>> defaults being set. That is a real shame.
>> But I don’t see how we can fix this now at all. The problem lies in browsers
>> not setting us any values if a checkbox has not been selected. There are a
>> thousand hacks how to achieve that and they are all opening up another large
>> list of new problems. So I am not very keen on doing this.
>> I am proposing this as a solution now:
>>
>> https://git.ipfire.org/?p=ipfire-2.x.git;a=commitdiff;h=351113e21eecd730b33a2d73c1bb74eff9fcb845
>> It is not very pretty, but it works reliably without any Javascript stunts
>> or other hacks in the browser. We just need to make ensure that we are
>> extending it whenever we are adding more checkboxes.
>> Please test and let me know if this resolves the issue(s).
>
> Have tested this out with the latest CU197 build.
>
> Everything worked as I expected it to.
>
> I started with a working client in CU196.
>
> I then updated to CU197 and the update worked as expected and the connection
> worked.
>
> I then restored a client connection that had previously been created in CU197
> and that also worked fine.
>
> I then did a restore of the CU196 version and the connection also worked.
>
> I then modified a connection to use different ciphers in the data-ciphers
> table and backed that up. I then restored one of the other backups with the
> standard default data-ciphers. Then I restored the backup with the modified
> data-ciphers and those were correctly restored.
>
> So in terms of my standard client connections from CU196, everything is
> working as expected both after an update to CU197 or by restoring a CU196
> based backup into CU197.
>
> While working on various things today I have found another thing that needs
> to be resolved when doing a restore of a client with multiple routes to be
> pushed but I will do a separate email on that.
>
> Regards,
>
> Adolf.
>
>> All the best,
>> -Michael
>>> On 27 Aug 2025, at 12:55, Adolf Belka <adolf.be...@ipfire.org> wrote:
>>>
>>> Hi Michael,
>>>
>>> On 26/08/2025 20:02, Adolf Belka wrote:
>>>> Hi Michael,
>>>> On 26/08/2025 12:45, Michael Tremer wrote:
>>>>> Hello Adolf,
>>>>>
>>>>> Hmm, maybe Stefan’s patch is not providing the full solution.
>>>>>
>>>>> I created a new function some time ago which is called “set_defaults” and
>>>>> the idea is that it would populate any fields that have not been
>>>>> initialized. That way, if we add anything new, there should always be a
>>>>> good default.
>>>>>
>>>>> I understand why Stefan is turning of that initialisation, but this does
>>>>> seem to create some more consequences.
>>>>>
>>>>> We should never have any server configurations left with ncp-disable,
>>>>> because that will not work any more. I thought regenerating the
>>>>> configuration files through the CGI should take care of this.
>>>> If Stefan's patch is not present then yes it takes care of it for old
>>>> restores with ncp-disable still in the server.conf file.
>>>> If Stefan's patch is present then the default settings don't get used as
>>>> the restore already has the settings values defined. However the old
>>>> settings file has no DATACIPHERS entry in it and in ovpnmain.cgi there is
>>>> still a section of code that checks if DATACIPHERS='' and if yes then it
>>>> writes ncp-disable into server.conf and leaves DATACIPHERS='' and this
>>>> causes the openvpn-rw daemon to fail to start.
>>>> That code is in lines 294 to 298. I suspect those lines were not intended
>>>> to be still in there.
>>>> However this means that when an old (non ncp) restore is done then
>>>> ncp-disable is written into the server.conf file.
>>>> Maybe if that section of code checks if DATACIPHERS='' and if it is then
>>>> enters the default values for DATACIPHERS then maybe my "fixes" in
>>>> backup.pl and update.sh might not be needed. I could have a look at that.
>>>> I have a little bit of time now to work on IPFire as my nephew and his
>>>> family are now going back home so I can do some testing out of those
>>>> scenarios.
>>>
>>> I modified the section of code to remove the ncp-disable entry and if the
>>> DATACIPHERS value is empty then I am entering the default DATACIPHERS set.
>>>
>>> Doing this and then removing the backup.pl modification I made worked for
>>> all restores of old backups that were still using ncp-disable.
>>>
>>> I also tested out doing restores of backups with the new config with
>>> DATACIPHERS and those also all worked, giving the expected entries.
>>>
>>> In all cases this was with Stefan's patch in place.
>>>
>>> Based on this then the changes I made previously to backup.pl and update.sh
>>> would no longer be needed and everything would be dealt with in
>>> ovpnmain.cgi.
>>>
>>> However, I am not certain if the changes I have made are the best way to
>>> deal with this issue or not.
>>>
>>> So I will submit a patch with the changes as an RFC patch so it can be
>>> confirmed as the right way or if this would be considered a hack approach.
>>>
>>> Open for feedback.
>>>
>>> Regards,
>>>
>>> Adolf.
>>>
>>>>>
>>>>> Maybe we need to rethink how we can make set_defaults() work so that we
>>>>> don’t have to add more and more hacks?!
>>>> If this can be done then that would probably be a good idea.
>>>> Regards,
>>>> Adolf.
>>>>>
>>>>> Best,
>>>>> -Michael
>>>>>
>>>>>> On 25 Aug 2025, at 09:51, Adolf Belka <adolf.be...@ipfire.org> wrote:
>>>>>>
>>>>>> Hi Stefan,
>>>>>>
>>>>>> On 23/08/2025 14:55, Adolf Belka wrote:
>>>>>>> Hi Stefan,
>>>>>>> I tried out the CU197 Testing update with this patch in place. It works
>>>>>>> fine for a new install, where there is no existing settings file but
>>>>>>> for updates or when a restore from an old backup is being done then a
>>>>>>> settings file already exists and then the default settings are not
>>>>>>> applied and this results in the settings file having no CIPHERS entry
>>>>>>> but having a fallback DCIPHER entry.
>>>>>>> In the update where the OpenVPN RW server is stopped before updating
>>>>>>> and started again afterwards this causes the server to fail to start as
>>>>>>> there is no CIPHER entry. When a restore from backup is done then the
>>>>>>> same thing happens with no CIPHERS entry, just a DCIPHER one but as the
>>>>>>> server is running when the restore is done, it stays running with the
>>>>>>> old settings but if the Save button is pressed then it Stops because
>>>>>>> the settings file now has no CIPHERS entry.
>>>>>>> Not sure how to fix this at the moment. Maybe it needs to be if the
>>>>>>> settings file exists and it contains a CIPHERS entry but I am not sure
>>>>>>> that is the right approach or not.
>>>>>>
>>>>>> Figured out how to fix this. Your patch in ovpnmain.cgi stays the same.
>>>>>> I just needed to add in some additional lines into backup.pl and the
>>>>>> CU197 update.sh file.
>>>>>>
>>>>>> I just check if ncp-disable is present in server.conf and if it is then
>>>>>> delete it and then add in the default
>>>>>> DATACIPHERS=AES-256-GCM|AES-128-GCM|CHACHA20-POLY1305 into the settings
>>>>>> file.
>>>>>>
>>>>>> If ncp-disable is in the server.conf then the restore is from prior to
>>>>>> openvpn-2.6 and there will be no DATACIPHERS entry.
>>>>>>
>>>>>> I have tested this out with the backup.pl changes and your patch in
>>>>>> place and everything works correctly again. I will submit patches for
>>>>>> these additional changes.
>>>>>>
>>>>>> Regards,
>>>>>>
>>>>>> Adolf.
>>>>>>
>>>>>>
>>>>>>> Regards,
>>>>>>> Adolf.
>>>>>>> On 19/08/2025 20:39, Stefan Schantl wrote:
>>>>>>>> Only apply the default settings in case nothing has been configured
>>>>>>>> yet,
>>>>>>>> otherwise existing settings may get overwritten.
>>>>>>>>
>>>>>>>> Signed-off-by: Stefan Schantl <stefan.scha...@ipfire.org>
>>>>>>>> ---
>>>>>>>> html/cgi-bin/ovpnmain.cgi | 2 +-
>>>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>>
>>>>>>>> diff --git a/html/cgi-bin/ovpnmain.cgi b/html/cgi-bin/ovpnmain.cgi
>>>>>>>> index 83f9fdc02..a2f95dc9a 100644
>>>>>>>> --- a/html/cgi-bin/ovpnmain.cgi
>>>>>>>> +++ b/html/cgi-bin/ovpnmain.cgi
>>>>>>>> @@ -132,7 +132,7 @@ my $col="";
>>>>>>>> "MAX_CLIENTS" => 100,
>>>>>>>> "MSSFIX" => "off",
>>>>>>>> "TLSAUTH" => "on",
>>>>>>>> -});
>>>>>>>> +}) unless (%vpnsettings);
>>>>>>>> # Load CGI parameters
>>>>>>>> &Header::getcgihash(\%cgiparams, {'wantfile' => 1, 'filevar' =>
>>>>>>>> 'FH'});
