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

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

[Adolf Belka]

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

- 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

[PATCH v2 2/2] en.pl: Update the wording for the check on the CA Name for upload

[Adolf Belka]

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

- This changes the wording to allowing characters and spaces.

Fixes: Bug10595 part 2
Tested-by: Adolf Belka <adolf.belka(a)ipfire.org>
Signed-off-by: Adolf Belka <adolf.belka(a)ipfire.org>
---
 doc/language_issues.de | 1 +
 doc/language_issues.en | 1 +
 doc/language_issues.es | 1 +
 doc/language_issues.fr | 1 +
 doc/language_issues.it | 1 +
 doc/language_issues.nl | 1 +
 doc/language_issues.pl | 1 +
 doc/language_issues.ru | 1 +
 doc/language_issues.tr | 1 +
 doc/language_missings  | 8 ++++++++
 langs/en/cgi-bin/en.pl | 1 +
 11 files changed, 18 insertions(+)

diff --git a/doc/language_issues.de b/doc/language_issues.de
index 7883bef76..8626edafd 100644
--- a/doc/language_issues.de
+++ b/doc/language_issues.de
@@ -930,6 +930,7 @@ WARNING: untranslated string: access point name = Access Point Name
 WARNING: untranslated string: access point name is invalid = Access Point Name is invalid
 WARNING: untranslated string: access point name is required = Access Point Name is required
 WARNING: untranslated string: aliases default interface = - Default Interface -
+WARNING: untranslated string: ca name must only contain characters and spaces = unknown string
 WARNING: untranslated string: cake profile bridged-llcsnap 32 = Bridged LLC SNAP (32 bytes)
 WARNING: untranslated string: cake profile bridged-ptm 19 = Bridged PTM (19 bytes)
 WARNING: untranslated string: cake profile bridged-vcmux 24 = Bridged VC-MUX (24 bytes)
diff --git a/doc/language_issues.en b/doc/language_issues.en
index a1730ac7b..f8e25ead3 100644
--- a/doc/language_issues.en
+++ b/doc/language_issues.en
@@ -360,6 +360,7 @@ WARNING: untranslated string: bytes received = Bytes Received
 WARNING: untranslated string: bytes sent = Bytes Sent
 WARNING: untranslated string: ca certificate = CA Certificate
 WARNING: untranslated string: ca name = CA name
+WARNING: untranslated string: ca name must only contain characters and spaces = unknown string
 WARNING: untranslated string: cached = cached
 WARNING: untranslated string: cached memory = Cached Memory  
 WARNING: untranslated string: cached swap = Cached Swap
diff --git a/doc/language_issues.es b/doc/language_issues.es
index 0a89279d5..30cd7afff 100644
--- a/doc/language_issues.es
+++ b/doc/language_issues.es
@@ -1003,6 +1003,7 @@ WARNING: untranslated string: access point name = Access Point Name
 WARNING: untranslated string: access point name is invalid = Access Point Name is invalid
 WARNING: untranslated string: access point name is required = Access Point Name is required
 WARNING: untranslated string: bypassed = Bypassed
+WARNING: untranslated string: ca name must only contain characters and spaces = unknown string
 WARNING: untranslated string: cpu frequency = CPU frequency
 WARNING: untranslated string: data transfer = Data Transfer
 WARNING: untranslated string: dhcp fixed ip address in dynamic range = Fixed IP Address in dynamic range
diff --git a/doc/language_issues.fr b/doc/language_issues.fr
index 7f9349bc0..72067d4f8 100644
--- a/doc/language_issues.fr
+++ b/doc/language_issues.fr
@@ -968,6 +968,7 @@ WARNING: translation string unused: zoneconf val vlan tag assignment error
 WARNING: translation string unused: zoneconf val vlan tag range error
 WARNING: translation string unused: zoneconf val zoneslave amount error
 WARNING: untranslated string: bypassed = Bypassed
+WARNING: untranslated string: ca name must only contain characters and spaces = unknown string
 WARNING: untranslated string: core notice 3 = available.
 WARNING: untranslated string: data transfer = Data Transfer
 WARNING: untranslated string: enable disable client = unknown string
diff --git a/doc/language_issues.it b/doc/language_issues.it
index 16371b566..a6c3a20a9 100644
--- a/doc/language_issues.it
+++ b/doc/language_issues.it
@@ -970,6 +970,7 @@ WARNING: untranslated string: available = available
 WARNING: untranslated string: block = Block
 WARNING: untranslated string: broken = Broken
 WARNING: untranslated string: bypassed = Bypassed
+WARNING: untranslated string: ca name must only contain characters and spaces = unknown string
 WARNING: untranslated string: cake profile bridged-llcsnap 32 = Bridged LLC SNAP (32 bytes)
 WARNING: untranslated string: cake profile bridged-ptm 19 = Bridged PTM (19 bytes)
 WARNING: untranslated string: cake profile bridged-vcmux 24 = Bridged VC-MUX (24 bytes)
diff --git a/doc/language_issues.nl b/doc/language_issues.nl
index f647d50a8..d13efb067 100644
--- a/doc/language_issues.nl
+++ b/doc/language_issues.nl
@@ -972,6 +972,7 @@ WARNING: untranslated string: available = available
 WARNING: untranslated string: block = Block
 WARNING: untranslated string: broken = Broken
 WARNING: untranslated string: bypassed = Bypassed
+WARNING: untranslated string: ca name must only contain characters and spaces = unknown string
 WARNING: untranslated string: cake profile bridged-llcsnap 32 = Bridged LLC SNAP (32 bytes)
 WARNING: untranslated string: cake profile bridged-ptm 19 = Bridged PTM (19 bytes)
 WARNING: untranslated string: cake profile bridged-vcmux 24 = Bridged VC-MUX (24 bytes)
diff --git a/doc/language_issues.pl b/doc/language_issues.pl
index a3acc61af..0cc94937a 100644
--- a/doc/language_issues.pl
+++ b/doc/language_issues.pl
@@ -897,6 +897,7 @@ WARNING: untranslated string: bit = bit
 WARNING: untranslated string: block = Block
 WARNING: untranslated string: broken = Broken
 WARNING: untranslated string: bypassed = Bypassed
+WARNING: untranslated string: ca name must only contain characters and spaces = unknown string
 WARNING: untranslated string: cake profile bridged-llcsnap 32 = Bridged LLC SNAP (32 bytes)
 WARNING: untranslated string: cake profile bridged-ptm 19 = Bridged PTM (19 bytes)
 WARNING: untranslated string: cake profile bridged-vcmux 24 = Bridged VC-MUX (24 bytes)
diff --git a/doc/language_issues.ru b/doc/language_issues.ru
index e946c22df..83be26b32 100644
--- a/doc/language_issues.ru
+++ b/doc/language_issues.ru
@@ -892,6 +892,7 @@ WARNING: untranslated string: bit = bit
 WARNING: untranslated string: block = Block
 WARNING: untranslated string: broken = Broken
 WARNING: untranslated string: bypassed = Bypassed
+WARNING: untranslated string: ca name must only contain characters and spaces = unknown string
 WARNING: untranslated string: cake profile bridged-llcsnap 32 = Bridged LLC SNAP (32 bytes)
 WARNING: untranslated string: cake profile bridged-ptm 19 = Bridged PTM (19 bytes)
 WARNING: untranslated string: cake profile bridged-vcmux 24 = Bridged VC-MUX (24 bytes)
diff --git a/doc/language_issues.tr b/doc/language_issues.tr
index c0cb2703a..9c64615dc 100644
--- a/doc/language_issues.tr
+++ b/doc/language_issues.tr
@@ -957,6 +957,7 @@ WARNING: untranslated string: autonomous system = Autonomous System
 WARNING: untranslated string: available = available
 WARNING: untranslated string: broken = Broken
 WARNING: untranslated string: bypassed = Bypassed
+WARNING: untranslated string: ca name must only contain characters and spaces = unknown string
 WARNING: untranslated string: cake profile bridged-llcsnap 32 = Bridged LLC SNAP (32 bytes)
 WARNING: untranslated string: cake profile bridged-ptm 19 = Bridged PTM (19 bytes)
 WARNING: untranslated string: cake profile bridged-vcmux 24 = Bridged VC-MUX (24 bytes)
diff --git a/doc/language_missings b/doc/language_missings
index 92a78b090..f8a825c5d 100644
--- a/doc/language_missings
+++ b/doc/language_missings
@@ -39,6 +39,7 @@
 < cake profile pppoe-ptm 27
 < cake profile pppoe-vcmux 32
 < cake profile raw 0
+< ca name must only contain characters or spaces
 < Captive heading terms
 < Captive heading voucher
 < Captive invalid coupon
@@ -122,6 +123,7 @@
 < access point name is required
 < addon
 < bypassed
+< ca name must only contain characters or spaces
 < cpu frequency
 < data transfer
 < dhcp fixed ip address in dynamic range
@@ -179,6 +181,7 @@
 < bewan adsl pci st
 < bewan adsl usb
 < bypassed
+< ca name must only contain characters or spaces
 < data transfer
 < extrahd because it it outside the allowed mount path
 < fwdfw syn flood protection
@@ -261,6 +264,7 @@
 < cake profile pppoe-ptm 27
 < cake profile pppoe-vcmux 32
 < cake profile raw 0
+< ca name must only contain characters or spaces
 < Captive
 < Captive 1day
 < Captive 1month
@@ -804,6 +808,7 @@
 < cake profile pppoe-ptm 27
 < cake profile pppoe-vcmux 32
 < cake profile raw 0
+< ca name must only contain characters or spaces
 < capabilities
 < Captive
 < Captive 1day
@@ -1387,6 +1392,7 @@
 < cake profile pppoe-ptm 27
 < cake profile pppoe-vcmux 32
 < cake profile raw 0
+< ca name must only contain characters or spaces
 < capabilities
 < Captive
 < Captive 1day
@@ -2403,6 +2409,7 @@
 < cake profile pppoe-ptm 27
 < cake profile pppoe-vcmux 32
 < cake profile raw 0
+< ca name must only contain characters or spaces
 < capabilities
 < Captive
 < Captive 1day
@@ -3400,6 +3407,7 @@
 < cake profile pppoe-ptm 27
 < cake profile pppoe-vcmux 32
 < cake profile raw 0
+< ca name must only contain characters or spaces
 < Captive delete logo
 < core update
 < cpu frequency
diff --git a/langs/en/cgi-bin/en.pl b/langs/en/cgi-bin/en.pl
index 197f44633..afea8c5fb 100644
--- a/langs/en/cgi-bin/en.pl
+++ b/langs/en/cgi-bin/en.pl
@@ -530,6 +530,7 @@
 'bytes sent' => 'Bytes Sent',
 'ca certificate' => 'CA Certificate',
 'ca name' => 'CA name',
+'ca name must only contain characters or spaces' => 'CA Name must only contain characters or spaces.',
 'cache management' => 'Cache management',
 'cache size' => 'Cache size (MB):',
 'cached' => 'cached',
-- 
2.48.1

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

[Michael Tremer]

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

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:
> 
> - 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
> 

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

[Adolf Belka]

[-- 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
>>
>