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

Hi,

yes, it would have been better to send a patch against next instead of a patch for the other patch. That makes it a lot harder to see what has actually changed.

I suppose you can squash both commits together to one and then just send it by using "git send-email -1 --to development@lists.ipfire.org".

https://stackoverflow.com/questions/2563632/how-can-i-merge-two-commits-into...

Best, -Michael

On Tue, 2017-11-14 at 14:20 +0100, ummeegge wrote:

Hi Michael, have now seen the format of the sended patch --> https://lists.ipfire.org/pipe rmail/development/2017-November/003732.html is somehow broken (seems that sendmail and me have some incompatibilities :-| ) whereby in Git --> https://g it.ipfire.org/?p=people/ummeegge/ipfire- 2.x.git;a=commit;h=3e58db4871f707f6ea79e6f8ca219ee03008fe76 is it OK… Am currently not sure what i did wrong there. Michael, if the format is useless let it me know will try then to send it again...

Best,

Erik

Am 12.11.2017 um 13:15 schrieb Michael Tremer:

...

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'}.co nf") 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