From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Tremer To: development@lists.ipfire.org Subject: Re: [PATCH] OpenVPN: Fix to prevent exceedance of OpenSSLs max. validity. Date: Tue, 14 Nov 2017 14:11:56 +0000 Message-ID: <1510668716.4838.424.camel@ipfire.org> In-Reply-To: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============3596421966802609917==" List-Id: --===============3596421966802609917== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable 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(a)lists.ipfire.org". https://stackoverflow.com/questions/2563632/how-can-i-merge-two-commits-into-= one 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/p= ipe > 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=3Dpeople/ummeegge/ipfire- > 2.x.git;a=3Dcommit;h=3D3e58db4871f707f6ea79e6f8ca219ee03008fe76 is it OK=E2= =80=A6 Am > currently not sure what i did wrong there.=20 > Michael, if the format is useless let it me know will try then to send it > again... >=20 >=20 > Best, >=20 > Erik >=20 >=20 > Am 12.11.2017 um 13:15 schrieb Michael Tremer: >=20 > > Hi, > >=20 > > 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. > > >=20 > > > Fixes: #10482 > > > --- > > > html/cgi-bin/ovpnmain.cgi | 14 ++++++++++++++ > > > 1 file changed, 14 insertions(+) > > >=20 > > > 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; > > > } > > >=20 > > > + # Check that OpenSSL maximum of valid days won=C2=B4t be exceeded > > > + if (length($cgiparams{'DAYS_VALID'}) > 6) { > > > + $errormessage =3D $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; > > > + } > > > + > >=20 > > 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. > >=20 > > > if ($cgiparams{'ENABLED'} !~ /^(on|off)$/) { > > > $errormessage =3D $Lang::tr{'invalid input'}; > > > goto VPNCONF_ERROR; > > > @@ -4221,6 +4229,12 @@ if ($cgiparams{'TYPE'} eq 'net') { > > > goto VPNCONF_ERROR; > > > } > > >=20 > > > + # Check that OpenSSL maximum of valid days won=C2=B4t be > > > exceeded > > > + if (length($cgiparams{'DAYS_VALID'}) > 6) { > > > + $errormessage =3D $Lang::tr{'invalid input for > > > valid till days'}; > > > + goto VPNCONF_ERROR; > > > + } > > > + > > > # Replace empty strings with a . > > > (my $ou =3D $cgiparams{'CERT_OU'}) =3D~ s/^\s*$/\./; > > > (my $city =3D $cgiparams{'CERT_CITY'}) =3D~ s/^\s*$/\./; > >=20 > > -Michael >=20 >=20 --===============3596421966802609917== Content-Type: application/pgp-signature Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="signature.asc" MIME-Version: 1.0 LS0tLS1CRUdJTiBQR1AgU0lHTkFUVVJFLS0tLS0KCmlRSXpCQUFCQ2dBZEZpRUU1L3JXNWwzR0dl Mnlwa3R4Z0hudy8yK1FDUWNGQWxvSythd0FDZ2tRZ0hudy8yK1EKQ1Fkd0tnLzhES0drNkxvM3RZ ZGtkL1QvOGZMcjVNMVYvL2dVQS9jNU1GK0hNMTRqSzZWTmVmYzlGRTVtSk5RMQpiNDk4anZ1Q1Uy NFVMNHQxbTlKNGV3TUJPVEhPM1VFZWxDTUtLNGM5Q1pNRllLcWNZdUVpRlB3b0p4TmNaNlRPCkFE alpSSTJ2ampUNkNzdTBRdWFwdmVJQWlVeWZnRFZYMUQ4d0t6WlNnZTVQaHlnQnNYRHhFZlE5WFpB U24xTlgKbElvTithcTl0RzJZMWlxSURmQ3J6WWRCK0pxaGJvdXpUVGVQRmo4bnBQQU9iVmdYZTUr WmtnVnoxNXgxck5HWQpicE82NDJVeXptNDR0TjgwbVZDdUpFZ3Y2dUo5ZHhqMnh0YmpZWlpwTlFQ WnliRFhvdVlGT0cxM25DbzZmSnYrCkhFZEovSVd1RVpXNEEyRStwZjFXcDBxaFlrNko1em9sMTA4 WXZaaEV3bEdFK0hUOFBmVi9Ea1dIT0tWaFVtd1AKcG9wMDMxbHhZWHoxeHpPL3JZaXlPdXV2SDBp b0VPT1RHKzVaRW5WZis0Q2NnQ1JoNHUxRTVPaDBGMW1wa2ZBZApNTUhGN3VHTU5WTjluNWpwT1FV QVk5bzlOa3VWTTlPYVpDckgzT2owdmdGTi8wQTQwQ0x0elFzYUtkOEtWSDRHCm9YbU9OOUxVZWFn emp0UEh2Vkt2aEcyVEVQbXczSEdoWUI4UUNlVzNFQUZGT0JNUjZpbS82czBPV0E4S3hrRUkKR3Z1 bVZ3RmR0N3lna1dFRnlOK0FaT0xFdVFxeCtUbkNQak90Rm9FcXBHQU5BQ2N2WURNbm1nbGo3YkE2 cDFvQwpIZzVtWnN3VElXZ1pjbVpuL0I3VkdoNXhhZDVVZVhXSlhzWXRyQVg0dFFkYVZjVy9wSVk9 Cj01NCtjCi0tLS0tRU5EIFBHUCBTSUdOQVRVUkUtLS0tLQo= --===============3596421966802609917==--