OpenVPN: Fix for N2N plausibility checks

Message ID 20200415132404.25688-1-ummeegge@ipfire.org
State Accepted
Commit 73735ad99c93cbe76f789f14cfa445d8b11310a0
Headers
Series OpenVPN: Fix for N2N plausibility checks |

Commit Message

ummeegge April 15, 2020, 1:24 p.m. UTC
  Fixes #12335
If no N2N name has been set, no directory and config has been created so it can not be deleted.

'goto VPNCONF_ERROR;' has been missing for N2N checks.
Fixed also code formatting.

Signed-off-by: Erik Kapfer <ummeegge@ipfire.org>
---
 html/cgi-bin/ovpnmain.cgi | 55 ++++++++++++++++++++++++-----------------------
 1 file changed, 28 insertions(+), 27 deletions(-)
  

Comments

Michael Tremer April 16, 2020, 1:44 p.m. UTC | #1
Hi,

Just for protocol, this patch is difficult to read.

I would not have noticed that only two lines have been added.

However, let’s just go ahead with it.

-Michael

> On 15 Apr 2020, at 14:24, Erik Kapfer <ummeegge@ipfire.org> wrote:
> 
> Fixes #12335
> If no N2N name has been set, no directory and config has been created so it can not be deleted.
> 
> 'goto VPNCONF_ERROR;' has been missing for N2N checks.
> Fixed also code formatting.
> 
> Signed-off-by: Erik Kapfer <ummeegge@ipfire.org>
> ---
> html/cgi-bin/ovpnmain.cgi | 55 ++++++++++++++++++++++++-----------------------
> 1 file changed, 28 insertions(+), 27 deletions(-)
> 
> diff --git a/html/cgi-bin/ovpnmain.cgi b/html/cgi-bin/ovpnmain.cgi
> index 00ecd77a0..1e61ddb42 100644
> --- a/html/cgi-bin/ovpnmain.cgi
> +++ b/html/cgi-bin/ovpnmain.cgi
> @@ -3765,41 +3765,42 @@ if ($cgiparams{'TYPE'} eq 'host') {
> #CCD End
> 
> 	
> - if ($cgiparams{'TYPE'} !~ /^(host|net)$/) {
> -	    $errormessage = $Lang::tr{'connection type is invalid'};
> -	    if ($cgiparams{'TYPE'} eq 'net') {
> -      unlink ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}/$cgiparams{'NAME'}.conf") or die "Removing Configfile fail: $!";
> -	    rmdir ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}") || die "Removing Directory fail: $!";
> -      }
> -	    goto VPNCONF_ERROR;
> +	if ($cgiparams{'TYPE'} !~ /^(host|net)$/) {
> +		$errormessage = $Lang::tr{'connection type is invalid'};
> +		if ($cgiparams{'TYPE'} eq 'net') {
> +			unlink ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}/$cgiparams{'NAME'}.conf") or die "Removing Configfile fail: $!";
> +			rmdir ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}") || die "Removing Directory fail: $!";
> +			goto VPNCONF_ERROR;
> +		}
> +		goto VPNCONF_ERROR;
> 	}
> 
> -
> 	if ($cgiparams{'NAME'} !~ /^[a-zA-Z0-9]+$/) {
> -	    $errormessage = $Lang::tr{'name must only contain characters'};
> -      if ($cgiparams{'TYPE'} eq 'net') {
> -      unlink ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}/$cgiparams{'NAME'}.conf") or die "Removing Configfile fail: $!";
> -	    rmdir ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}") || die "Removing Directory fail: $!";
> -      }
> -      goto VPNCONF_ERROR;
> -  }
> +		$errormessage = $Lang::tr{'name must only contain characters'};
> +		if ($cgiparams{'TYPE'} eq 'net') {
> +			goto VPNCONF_ERROR;
> +		}
> +		goto VPNCONF_ERROR;
> +	}
> 
> 	if ($cgiparams{'NAME'} =~ /^(host|01|block|private|clear|packetdefault)$/) {
> -	    $errormessage = $Lang::tr{'name is invalid'};
> -	    if ($cgiparams{'TYPE'} eq 'net') {
> -      unlink ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}/$cgiparams{'NAME'}.conf") or die "Removing Configfile fail: $!";
> -	    rmdir ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}") || die "Removing Directory fail: $!";
> -      }
> -	    goto VPNCONF_ERROR;
> +		$errormessage = $Lang::tr{'name is invalid'};
> +		if ($cgiparams{'TYPE'} eq 'net') {
> +			unlink ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}/$cgiparams{'NAME'}.conf") or die "Removing Configfile fail: $!";
> +			rmdir ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}") || die "Removing Directory fail: $!";
> +			goto VPNCONF_ERROR;
> +		}
> +		goto VPNCONF_ERROR;
> 	}
> 
> 	if (length($cgiparams{'NAME'}) >60) {
> -	    $errormessage = $Lang::tr{'name too long'};
> -	    if ($cgiparams{'TYPE'} eq 'net') {
> -      unlink ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}/$cgiparams{'NAME'}.conf") or die "Removing Configfile fail: $!";
> -	    rmdir ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}") || die "Removing Directory fail: $!";
> -      }
> -	    goto VPNCONF_ERROR;
> +		$errormessage = $Lang::tr{'name too long'};
> +		if ($cgiparams{'TYPE'} eq 'net') {
> +			unlink ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}/$cgiparams{'NAME'}.conf") or die "Removing Configfile fail: $!";
> +			rmdir ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}") || die "Removing Directory fail: $!";
> +			goto VPNCONF_ERROR;
> +		}
> +		goto VPNCONF_ERROR;
> 	}
> 
> ###
> -- 
> 2.12.2
>
  

Patch

diff --git a/html/cgi-bin/ovpnmain.cgi b/html/cgi-bin/ovpnmain.cgi
index 00ecd77a0..1e61ddb42 100644
--- a/html/cgi-bin/ovpnmain.cgi
+++ b/html/cgi-bin/ovpnmain.cgi
@@ -3765,41 +3765,42 @@  if ($cgiparams{'TYPE'} eq 'host') {
 #CCD End
 
 	
- if ($cgiparams{'TYPE'} !~ /^(host|net)$/) {
-	    $errormessage = $Lang::tr{'connection type is invalid'};
-	    if ($cgiparams{'TYPE'} eq 'net') {
-      unlink ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}/$cgiparams{'NAME'}.conf") or die "Removing Configfile fail: $!";
-	    rmdir ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}") || die "Removing Directory fail: $!";
-      }
-	    goto VPNCONF_ERROR;
+	if ($cgiparams{'TYPE'} !~ /^(host|net)$/) {
+		$errormessage = $Lang::tr{'connection type is invalid'};
+		if ($cgiparams{'TYPE'} eq 'net') {
+			unlink ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}/$cgiparams{'NAME'}.conf") or die "Removing Configfile fail: $!";
+			rmdir ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}") || die "Removing Directory fail: $!";
+			goto VPNCONF_ERROR;
+		}
+		goto VPNCONF_ERROR;
 	}
 
-
 	if ($cgiparams{'NAME'} !~ /^[a-zA-Z0-9]+$/) {
-	    $errormessage = $Lang::tr{'name must only contain characters'};
-      if ($cgiparams{'TYPE'} eq 'net') {
-      unlink ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}/$cgiparams{'NAME'}.conf") or die "Removing Configfile fail: $!";
-	    rmdir ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}") || die "Removing Directory fail: $!";
-      }
-      goto VPNCONF_ERROR;
-  }
+		$errormessage = $Lang::tr{'name must only contain characters'};
+		if ($cgiparams{'TYPE'} eq 'net') {
+			goto VPNCONF_ERROR;
+		}
+		goto VPNCONF_ERROR;
+	}
 
 	if ($cgiparams{'NAME'} =~ /^(host|01|block|private|clear|packetdefault)$/) {
-	    $errormessage = $Lang::tr{'name is invalid'};
-	    if ($cgiparams{'TYPE'} eq 'net') {
-      unlink ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}/$cgiparams{'NAME'}.conf") or die "Removing Configfile fail: $!";
-	    rmdir ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}") || die "Removing Directory fail: $!";
-      }
-	    goto VPNCONF_ERROR;
+		$errormessage = $Lang::tr{'name is invalid'};
+		if ($cgiparams{'TYPE'} eq 'net') {
+			unlink ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}/$cgiparams{'NAME'}.conf") or die "Removing Configfile fail: $!";
+			rmdir ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}") || die "Removing Directory fail: $!";
+			goto VPNCONF_ERROR;
+		}
+		goto VPNCONF_ERROR;
 	}
 
 	if (length($cgiparams{'NAME'}) >60) {
-	    $errormessage = $Lang::tr{'name too long'};
-	    if ($cgiparams{'TYPE'} eq 'net') {
-      unlink ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}/$cgiparams{'NAME'}.conf") or die "Removing Configfile fail: $!";
-	    rmdir ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}") || die "Removing Directory fail: $!";
-      }
-	    goto VPNCONF_ERROR;
+		$errormessage = $Lang::tr{'name too long'};
+		if ($cgiparams{'TYPE'} eq 'net') {
+			unlink ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}/$cgiparams{'NAME'}.conf") or die "Removing Configfile fail: $!";
+			rmdir ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}") || die "Removing Directory fail: $!";
+			goto VPNCONF_ERROR;
+		}
+		goto VPNCONF_ERROR;
 	}
 
 ###