Re: [PATCH v2 1/2] vpnmain.cgi: Fix for 2nd part of bug10595

[Adolf Belka <adolf.belka@ipfire.org>]

[-- Attachment #1: Type: text/plain, Size: 3804 bytes --]

Hi Michael,

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

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

Sure no problem.

Regards,
Adolf.
> 
> -Michael
> 
>> 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 fixed. 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 allow spaces.
>> - Having modified that then the subroutines getsubjectfromcert and getCNfromcert 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 version of this
>>    patch set has the safe approach suggested by @Michael.
>> - I am open to any suggestions for improvements to how I implemented the use 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 => 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 = `/usr/bin/openssl x509 -text -in $_[0]`;
>> - $temp =~ /Subject:.*CN\s*=\s*(.*)[\n]/;
>> - $temp = $1;
>> - $temp =~ s+/Email+, E+;
>> - $temp =~ s/ ST = / S = /;
>> - $temp =~ s/,//g;
>> - $temp =~ s/\'//g;
>> + my @output = &General::system_output("/usr/bin/openssl", "x509", "-text", "-in", "$_[0]");
>> + my $temp;
>> + foreach my $line (@output) {
>> + $line =~ /Subject:.*CN\s*=\s*(.*)[\n]/;
>> + $temp = $1;
>> + $temp =~ s+/Email+, E+;
>> + $temp =~ s/ ST = / S = /;
>> + $temp =~ s/,//g;
>> + $temp =~ s/\'//g;
>> + }
>> return $temp;
>> }
>> ###
>> @@ -259,11 +262,14 @@ sub getCNfromcert ($) {
>> ###
>> sub getsubjectfromcert ($) {
>> #&General::log("ipsec", "Extracting subject from $_[0]...");
>> - my $temp = `/usr/bin/openssl x509 -text -in $_[0]`;
>> - $temp =~ /Subject: (.*)[\n]/;
>> - $temp = $1;
>> - $temp =~ s+/Email+, E+;
>> - $temp =~ s/ ST = / S = /;
>> + my @output = &General::system_output("/usr/bin/openssl", "x509", "-text", "-in", "$_[0]");
>> + my $temp;
>> + foreach my $line (@output) {
>> + $line =~ /Subject: (.*)[\n]/;
>> + $temp = $1;
>> + $temp =~ s+/Email+, E+;
>> + $temp =~ s/ ST = / S = /;
>> + }
>> 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 = $Lang::tr{'name must only contain characters'};
>> + if ($cgiparams{'CA_NAME'} !~ /^[a-zA-Z0-9 ]*$/) {
>> + $errormessage = $Lang::tr{'ca name must only contain characters and spaces'};
>> goto UPLOADCA_ERROR;
>> }
>>
>> -- 
>> 2.48.1
>>
>