[RFC PATCH] ovpnmain.cgi: Replace the ncp-disable with data-ciphers server entry

[RFC PATCH] ovpnmain.cgi: Replace the ncp-disable with data-ciphers server entry

[Adolf Belka]

- If a backup from before openvpn-2.6 is restored then the server.conf file has
   ncp-disable still in it. Also data-ciphers in server.conf and DATACIPHERS in settings
   will not be present.
- The existing code checks if DATACIPHERS is empty and if it is then it puts ncp-disable
   into server.conf which we no longer need to have.
- This patch changes this code section so that if DATACIPHERS is empty then it has the
   default ciphers added into server.conf and then also updates the DATACIPHERS entry
   in the settings file.
- I have made this an RFC patch as the patch does work but it might not be the correct
   or best way to go about this.
- If it is accepted then the previous patches I did for backup.pl and update.sh can be
   reverted.
- The change was tested out with the reverted backup.pl and all old backup versions
   were successfully restored correctly and worked as expected. Also tested out the
   change with a backup from CU197 and that restore also worked correctly.

Tested-by: Adolf Belka <adolf.belka@ipfire.org>
Signed-off-by: Adolf Belka <adolf.belka@ipfire.org>
---
 html/cgi-bin/ovpnmain.cgi | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/html/cgi-bin/ovpnmain.cgi b/html/cgi-bin/ovpnmain.cgi
index dfe7f8ad5..8c908d725 100644
--- a/html/cgi-bin/ovpnmain.cgi
+++ b/html/cgi-bin/ovpnmain.cgi
@@ -291,8 +291,14 @@ sub writeserverconf {
     print CONF "status $RW_STATUS 30\n";
 
 	# Cryptography
+
+	# Previous ncp-disable server conf will have an empty DATACIPHERS entry
+	# This will occur with restores from prior to OpenVPN-2.6
+	# Replace the empty DATACIPHERS entry with the default value
 	if ($vpnsettings{'DATACIPHERS'} eq '') {
-		print CONF "ncp-disable\n";
+		print CONF "data-ciphers " . "AES-256-GCM:AES-128-GCM:CHACHA20-POLY1305" . "\n";
+		$vpnsettings{'DATACIPHERS'} = "AES-256-GCM|AES-128-GCM|CHACHA20-POLY1305";
+		&General::writehash("${General::swroot}/ovpn/settings", \%vpnsettings);
 	} else {
 		print CONF "data-ciphers " . $vpnsettings{'DATACIPHERS'} =~ s/\|/:/gr . "\n";
 	}
-- 
2.51.0

Re: [RFC PATCH] ovpnmain.cgi: Replace the ncp-disable with data-ciphers server entry

[Michael Tremer]

Hello Adolf,

Sorry for digging this up so late…

You are right that we cannot use ncp-disable at all any more with newer clients. However, there is no chance for a user to disable NCP on the server side any more. So we can simply remove the case of not having any ciphers set because that should never ever exist. I added this here:

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

I had this in the back of my mind that this needed fixing, but I completely forgot that you sent this.

You still sent me down the right way.

-Michael

> On 28 Aug 2025, at 11:29, Adolf Belka <adolf.belka@ipfire.org> wrote:
> 
> - If a backup from before openvpn-2.6 is restored then the server.conf file has
>   ncp-disable still in it. Also data-ciphers in server.conf and DATACIPHERS in settings
>   will not be present.
> - The existing code checks if DATACIPHERS is empty and if it is then it puts ncp-disable
>   into server.conf which we no longer need to have.
> - This patch changes this code section so that if DATACIPHERS is empty then it has the
>   default ciphers added into server.conf and then also updates the DATACIPHERS entry
>   in the settings file.
> - I have made this an RFC patch as the patch does work but it might not be the correct
>   or best way to go about this.
> - If it is accepted then the previous patches I did for backup.pl and update.sh can be
>   reverted.
> - The change was tested out with the reverted backup.pl and all old backup versions
>   were successfully restored correctly and worked as expected. Also tested out the
>   change with a backup from CU197 and that restore also worked correctly.
> 
> Tested-by: Adolf Belka <adolf.belka@ipfire.org>
> Signed-off-by: Adolf Belka <adolf.belka@ipfire.org>
> ---
> html/cgi-bin/ovpnmain.cgi | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/html/cgi-bin/ovpnmain.cgi b/html/cgi-bin/ovpnmain.cgi
> index dfe7f8ad5..8c908d725 100644
> --- a/html/cgi-bin/ovpnmain.cgi
> +++ b/html/cgi-bin/ovpnmain.cgi
> @@ -291,8 +291,14 @@ sub writeserverconf {
>     print CONF "status $RW_STATUS 30\n";
> 
> # Cryptography
> +
> + # Previous ncp-disable server conf will have an empty DATACIPHERS entry
> + # This will occur with restores from prior to OpenVPN-2.6
> + # Replace the empty DATACIPHERS entry with the default value
> if ($vpnsettings{'DATACIPHERS'} eq '') {
> - print CONF "ncp-disable\n";
> + print CONF "data-ciphers " . "AES-256-GCM:AES-128-GCM:CHACHA20-POLY1305" . "\n";
> + $vpnsettings{'DATACIPHERS'} = "AES-256-GCM|AES-128-GCM|CHACHA20-POLY1305";
> + &General::writehash("${General::swroot}/ovpn/settings", \%vpnsettings);
> } else {
> print CONF "data-ciphers " . $vpnsettings{'DATACIPHERS'} =~ s/\|/:/gr . "\n";
> }
> -- 
> 2.51.0
> 
>