From mboxrd@z Thu Jan  1 00:00:00 1970
From: ummeegge <ummeegge@ipfire.org>
To: development@lists.ipfire.org
Subject:
 Re: [PATCH] OpenVPN: Fix to prevent exceedance of OpenSSLs max. validity.
Date: Tue, 14 Nov 2017 14:20:42 +0100
Message-ID: <F556C7BD-A067-4437-9069-AC46E5E01B20@ipfire.org>
In-Reply-To: <1510488953.3441.3.camel@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============3158416984916808022=="
List-Id: <development.lists.ipfire.org>

--===============3158416984916808022==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

Hi Michael,
have now seen the format of the sended patch --> https://lists.ipfire.org/pip=
ermail/development/2017-November/003732.html is somehow broken (seems that se=
ndmail and me have some incompatibilities :-| ) whereby in Git --> https://gi=
t.ipfire.org/?p=3Dpeople/ummeegge/ipfire-2.x.git;a=3Dcommit;h=3D3e58db4871f70=
7f6ea79e6f8ca219ee03008fe76 is it OK=E2=80=A6 Am currently not sure what i di=
d wrong there.=20
Michael, if the format is useless let it me know will try then to send it aga=
in...


Best,

Erik


Am 12.11.2017 um 13:15 schrieb Michael Tremer:

> 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 en=
try 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'}.conf") or die "Removing Configfile fail: $!";
>> +		rmdir ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}") || die "Re=
moving 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


--===============3158416984916808022==--