[v3,1/2] vpnmain.cgi: Fix for 2nd part of bug10595
Commit Message
- 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. This v3 version has been
re based to another patch submission that modified lines in a similar place and
prevented a merge to work.
- 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@ipfire.org>
Signed-off-by: Adolf Belka <adolf.belka@ipfire.org>
---
html/cgi-bin/vpnmain.cgi | 34 ++++++++++++++++++++--------------
1 file changed, 20 insertions(+), 14 deletions(-)
Comments
Thank you. Merged.
> On 6 Mar 2025, at 11:32, Adolf Belka <adolf.belka@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. This v3 version has been
> re based to another patch submission that modified lines in a similar place and
> prevented a merge to work.
> - 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@ipfire.org>
> Signed-off-by: Adolf Belka <adolf.belka@ipfire.org>
> ---
> html/cgi-bin/vpnmain.cgi | 34 ++++++++++++++++++++--------------
> 1 file changed, 20 insertions(+), 14 deletions(-)
>
> diff --git a/html/cgi-bin/vpnmain.cgi b/html/cgi-bin/vpnmain.cgi
> index c9bbbb494..0c69efb17 100755
> --- a/html/cgi-bin/vpnmain.cgi
> +++ b/html/cgi-bin/vpnmain.cgi
> @@ -245,13 +245,16 @@ sub callssl ($) {
> ###
> sub getCNfromcert ($) {
> #&General::log("charon", "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("charon", "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
>
>
@@ -245,13 +245,16 @@ sub callssl ($) {
###
sub getCNfromcert ($) {
#&General::log("charon", "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("charon", "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;
}