From mboxrd@z Thu Jan  1 00:00:00 1970
From: Michael Tremer <michael.tremer@ipfire.org>
To: development@lists.ipfire.org
Subject:
 Re: [PATCH] OpenVPN: Fix to prevent exceedance of OpenSSLs max. validity.
Date: Sun, 12 Nov 2017 12:15:53 +0000
Message-ID: <1510488953.3441.3.camel@ipfire.org>
In-Reply-To: <1510393507-15218-1-git-send-email-erik.kapfer@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============0793600146986202414=="
List-Id: <development.lists.ipfire.org>

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

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 ent=
ry 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 "Rem=
oving 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 =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*$/\./;

-Michael

--===============0793600146986202414==
Content-Type: application/pgp-signature
Content-Transfer-Encoding: base64
Content-Disposition: attachment; filename="signature.asc"
MIME-Version: 1.0

LS0tLS1CRUdJTiBQR1AgU0lHTkFUVVJFLS0tLS0KCmlRSXpCQUFCQ2dBZEZpRUU1L3JXNWwzR0dl
Mnlwa3R4Z0hudy8yK1FDUWNGQWxvSU8za0FDZ2tRZ0hudy8yK1EKQ1FlWWd4QUFvMHJZbXUrQU1C
b1ZYc0ZEYWYwMU9TbndPN0xpR0swakM0NDZwdHN2Q2R2RzdGb0IvZlg0Y3VaOApTSkdHa3JQWFhE
dEpLdTJKVGZndThsSStyS2xtNUNwVXZ1c05uVTlmWm4wbzgyM0JpellVVFZIMFB5K2NWVEhTClJh
ZGR3eVhVQm5DMXhCNms1YmxwWjNjR1hsc0FVb3BtTkE1UjAxNi9yUStxSWdkWHZDNy9heW5jeFhi
WGFHYnUKVXprNzdIa0JDUDd5VEVyQ1dlQ1hSanl2bjZ3Qzd0RmZLTnVkcEJ6a21jR0hGUlEvcU1B
UzN2MzhoWHZzZnZUWQpFMk5hRHVCZHVvVWk1UWRIMEthVUZOMmpoZFdoeEYwRWRwRmluZ2NrQ0hq
dC9RQms4U3M0dHhTR1NQVndwbkc4CmlldHdhNFJZQ2Q2TlVCQlJzY0hHa2NKMG9RSjhNZTVWWUR0
VkdEV01wZitWK0hCZFB0SmxzeGZtNHBNQnozTUIKUU12cTF1MmhNOGk1d1JSTlUwcnhhQXJvdDJT
RTFiSHoxSHpDRWpjcitjb0psR3B3dEJkbGtFb0tKS3N6SkF3VQpqdnJRY3JhN1g4UHBSckZtTTZN
ZThyd0t0QStsS2NQTkRrOGZFNFB3RWJXV25KN3lQM2V4VWFyYXoxaDFveFJMCno1anJpOC81SlZN
R28rUlFtcHNhbjJXZE9KdDdXZG1rT1RaQkQ5b2RBTitXQnZ0RDRTME45TXJMS0ttZ1RuZkMKb3Yz
aUJIU1hqK2F3eW44UWU3bFN5U1ozNEZHSlk3R0NFQ1gwUTFVWmZ5Q2dpM3NIZzNrQW9wU2FtN09l
dk5wVAorNlpQYitpcEV6MThiK28wTW9BYVVJS3JTSVJyc3lVVnJtUUZxaWVwMzdBRENiRnNHQ3M9
Cj1WUFpWCi0tLS0tRU5EIFBHUCBTSUdOQVRVUkUtLS0tLQo=

--===============0793600146986202414==--