Re: [PATCH] OpenVPN: Fix to prevent exceedance of OpenSSLs max. validity.

[Michael Tremer <michael.tremer@ipfire.org>]

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

Hi,

On Sat, 2017-11-11 at 10:45 +0100, Erik Kapfer wrote:
> - If the OpenSSL maximum of '999999' will be exceeded over the WUI, the entry in
> 	OpenVPNs database index.txt will be written without a timestamp
> 	and crashes the database which blocks the creation of new clients.
> 	To prevent this, a check has been set which restricts the data field
> 	of 'valid til days' to '6' numerics.
> 
> Fixes: #10482
> ---
>  html/cgi-bin/ovpnmain.cgi | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/html/cgi-bin/ovpnmain.cgi b/html/cgi-bin/ovpnmain.cgi
> index ceb88c1..8f45f04 100644
> --- a/html/cgi-bin/ovpnmain.cgi
> +++ b/html/cgi-bin/ovpnmain.cgi
> @@ -4039,6 +4039,14 @@ if ($cgiparams{'TYPE'} eq 'net') {
>        		goto VPNCONF_ERROR;
>  	}
>  
> +	# Check that OpenSSL maximum of valid days won´t be exceeded
> +	if (length($cgiparams{'DAYS_VALID'}) > 6) {
> +		$errormessage = $Lang::tr{'invalid input for valid till days'};
> +		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;
> +	}
> +

I think it would be better just to check if DAYS_VALID is less then
999999. Checking the length of the string wasn't really obvious for me
what was actually going to be achieved here.

>  	if ($cgiparams{'ENABLED'} !~ /^(on|off)$/) {
>  	    $errormessage = $Lang::tr{'invalid input'};
>  	    goto VPNCONF_ERROR;
> @@ -4221,6 +4229,12 @@ if ($cgiparams{'TYPE'} eq 'net') {
>  		goto VPNCONF_ERROR;
>  	    }
>  
> +		# Check that OpenSSL maximum of valid days won´t be exceeded
> +		if (length($cgiparams{'DAYS_VALID'}) > 6) {
> +			$errormessage = $Lang::tr{'invalid input for valid till days'};
> +			goto VPNCONF_ERROR;
> +		}
> +
>  	    # Replace empty strings with a .
>  	    (my $ou = $cgiparams{'CERT_OU'}) =~ s/^\s*$/\./;
>  	    (my $city = $cgiparams{'CERT_CITY'}) =~ s/^\s*$/\./;

-Michael

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]