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 v2 1/2] vpnmain.cgi: Fix for 2nd part of bug10595
Date: Wed, 05 Mar 2025 11:49:48 +0000
Message-ID: <112D821F-E7E7-43F4-BCFD-3BCB4B6C9CE6@ipfire.org>
In-Reply-To: <20250303140908.12961-1-adolf.belka@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============6788781765117954503=="
List-Id: <development.lists.ipfire.org>

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

Hello,

It seems that this patch does not apply because of conflicts with the logging=
 changes.

Could you please rebase it and submit it again?

-Michael

> On 3 Mar 2025, at 14:09, Adolf Belka <adolf.belka(a)ipfire.org> wrote:
>=20
> - Bug10595 had two parts in it and was closed after the first part was fixe=
d. The second
>   part was still unfixed at that time. I cam across it when checking out an=
 open bug on
>   a similar issue with OpenVPN.
> - I found the section that checks on the CA Name and modified it to also al=
low spaces.
> - Having modified that then the subroutines getsubjectfromcert and getCNfro=
mcert required
>   modifications otherwise the openssl statement only got a filename with th=
e first
>   portion of the ca name until the first space was encountered. This v2 ver=
sion of this
>   patch set has the safe approach suggested by @Michael.
> - I am open to any suggestions for improvements to how I implemented the us=
e of the
>   &General::system_output function
> - Tested this change out on my vm and it worked fine. I was able to upload =
a ca
>   certificate into IPSec and use spaces in the CA Name.
> - Changed the test for the CA_NAME to allow spaces. Change also made to en.=
pl file
>=20
> Fixes: Bug10595 part 2
> Tested-by: Adolf Belka <adolf.belka(a)ipfire.org>
> Signed-off-by: Adolf Belka <adolf.belka(a)ipfire.org>
> ---
> html/cgi-bin/vpnmain.cgi | 34 ++++++++++++++++++++--------------
> 1 file changed, 20 insertions(+), 14 deletions(-)
> mode change 100755 =3D> 100644 html/cgi-bin/vpnmain.cgi
>=20
> diff --git a/html/cgi-bin/vpnmain.cgi b/html/cgi-bin/vpnmain.cgi
> old mode 100755
> new mode 100644
> index a1d0f0e2a..c11fb0dc8
> --- a/html/cgi-bin/vpnmain.cgi
> +++ b/html/cgi-bin/vpnmain.cgi
> @@ -245,13 +245,16 @@ sub callssl ($) {
> ###
> sub getCNfromcert ($) {
> #&General::log("ipsec", "Extracting name from $_[0]...");
> - my $temp =3D `/usr/bin/openssl x509 -text -in $_[0]`;
> - $temp =3D~ /Subject:.*CN\s*=3D\s*(.*)[\n]/;
> - $temp =3D $1;
> - $temp =3D~ s+/Email+, E+;
> - $temp =3D~ s/ ST =3D / S =3D /;
> - $temp =3D~ s/,//g;
> - $temp =3D~ s/\'//g;
> + my @output =3D &General::system_output("/usr/bin/openssl", "x509", "-text=
", "-in", "$_[0]");
> + my $temp;
> + foreach my $line (@output) {
> + $line =3D~ /Subject:.*CN\s*=3D\s*(.*)[\n]/;
> + $temp =3D $1;
> + $temp =3D~ s+/Email+, E+;
> + $temp =3D~ s/ ST =3D / S =3D /;
> + $temp =3D~ s/,//g;
> + $temp =3D~ s/\'//g;
> + }
> return $temp;
> }
> ###
> @@ -259,11 +262,14 @@ sub getCNfromcert ($) {
> ###
> sub getsubjectfromcert ($) {
> #&General::log("ipsec", "Extracting subject from $_[0]...");
> - my $temp =3D `/usr/bin/openssl x509 -text -in $_[0]`;
> - $temp =3D~ /Subject: (.*)[\n]/;
> - $temp =3D $1;
> - $temp =3D~ s+/Email+, E+;
> - $temp =3D~ s/ ST =3D / S =3D /;
> + my @output =3D &General::system_output("/usr/bin/openssl", "x509", "-text=
", "-in", "$_[0]");
> + my $temp;
> + foreach my $line (@output) {
> + $line =3D~ /Subject: (.*)[\n]/;
> + $temp =3D $1;
> + $temp =3D~ s+/Email+, E+;
> + $temp =3D~ s/ ST =3D / S =3D /;
> + }
> return $temp;
> }
> ###
> @@ -644,8 +650,8 @@ END
> } elsif ($cgiparams{'ACTION'} eq $Lang::tr{'upload ca certificate'}) {
> &General::readhasharray("${General::swroot}/vpn/caconfig", \%cahash);
>=20
> - if ($cgiparams{'CA_NAME'} !~ /^[a-zA-Z0-9]+$/) {
> - $errormessage =3D $Lang::tr{'name must only contain characters'};
> + if ($cgiparams{'CA_NAME'} !~ /^[a-zA-Z0-9 ]*$/) {
> + $errormessage =3D $Lang::tr{'ca name must only contain characters and spa=
ces'};
> goto UPLOADCA_ERROR;
> }
>=20
> --=20
> 2.48.1
>=20


--===============6788781765117954503==--