From mboxrd@z Thu Jan  1 00:00:00 1970
From: Adolf Belka <adolf.belka@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 13:02:31 +0100
Message-ID: <3ec1fc50-aacf-4184-bec3-58ed486a258a@ipfire.org>
In-Reply-To: <112D821F-E7E7-43F4-BCFD-3BCB4B6C9CE6@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============8135597183276833582=="
List-Id: <development.lists.ipfire.org>

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

Hi Michael,

On 05/03/2025 12:49, Michael Tremer wrote:
> Hello,
>=20
> It seems that this patch does not apply because of conflicts with the loggi=
ng changes.

Ah, yes.
>=20
> Could you please rebase it and submit it again?

Sure no problem.

Regards,
Adolf.
>=20
> -Michael
>=20
>> On 3 Mar 2025, at 14:09, Adolf Belka <adolf.belka(a)ipfire.org> wrote:
>>
>> - Bug10595 had two parts in it and was closed after the first part was fix=
ed. 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 a=
llow spaces.
>> - Having modified that then the subroutines getsubjectfromcert and getCNfr=
omcert required
>>    modifications otherwise the openssl statement only got a filename with =
the first
>>    portion of the ca name until the first space was encountered. This v2 v=
ersion of this
>>    patch set has the safe approach suggested by @Michael.
>> - I am open to any suggestions for improvements to how I implemented the u=
se 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
>>
>> 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
>>
>> 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", "-tex=
t", "-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", "-tex=
t", "-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);
>>
>> - 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 sp=
aces'};
>> goto UPLOADCA_ERROR;
>> }
>>
>> --=20
>> 2.48.1
>>
>=20


--===============8135597183276833582==--