* [PATCH 1/2] vpnmain.cgi: Fixes bug12298 - IPSec password cannot use semicolon
@ 2025-03-09 14:12 Adolf Belka
2025-03-09 14:12 ` [PATCH 2/2] language files: Update to include a message about a double quotation mark Adolf Belka
2025-03-10 9:56 ` [PATCH 1/2] vpnmain.cgi: Fixes bug12298 - IPSec password cannot use semicolon Michael Tremer
0 siblings, 2 replies; 5+ messages in thread
From: Adolf Belka @ 2025-03-09 14:12 UTC (permalink / raw)
To: development; +Cc: Adolf Belka
- The password for the pkcs12 certificate is passed to the open ssl command via $opt but
it is not quoted and so the ; is taken as the end of the command rather than as part
of the password. This also means that a pkcs12 file is not created and the .pem
intermediate file is what is left in the directory.
- This patch makes the -passout option quoted in the same way as the -name and -caname
options.
- Based on being the same as the name and caname parts in $opt, I believe that this should
not give rise to a vulnerability but I am open to being corrected.
- By quoting the -passout then the password must not contain double quotation marks, ",
so a test for the password containing a " has been added.
- The message about the use of the double quotation mark has been added to the english,
dutch and german language files. Feel free to correct if what I have used is not
correct. Those are in the other patch of this patch set.
- Tested out on my testbed system. I was able to create a pkcs12 certificate with a
password containing a variety of characters, including the semicolon, and getting
a message that the password contains a double quotation mark when I used that.
Fixes: bug12298
Tested-by: Adolf Belka <adolf.belka@ipfire.org>
Signed-off-by: Adolf Belka <adolf.belka@ipfire.org>
---
html/cgi-bin/vpnmain.cgi | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
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 c9bbbb494..8106ee24e
--- a/html/cgi-bin/vpnmain.cgi
+++ b/html/cgi-bin/vpnmain.cgi
@@ -2149,6 +2149,10 @@ END
$errormessage = $Lang::tr{'password too short'};
goto VPNCONF_ERROR;
}
+ if ($cgiparams{'CERT_PASS1'} =~ /["]/) {
+ $errormessage = $Lang::tr{'password has quotation mark'};
+ goto VPNCONF_ERROR;
+ }
if ($cgiparams{'CERT_PASS1'} ne $cgiparams{'CERT_PASS2'}) {
$errormessage = $Lang::tr{'passwords do not match'};
goto VPNCONF_ERROR;
@@ -2226,7 +2230,7 @@ END
$opt .= " -inkey ${General::swroot}/certs/$cgiparams{'NAME'}key.pem";
$opt .= " -in ${General::swroot}/certs/$cgiparams{'NAME'}cert.pem";
$opt .= " -name \"$cgiparams{'NAME'}\"";
- $opt .= " -passout pass:$cgiparams{'CERT_PASS1'}";
+ $opt .= " -passout pass:\"$cgiparams{'CERT_PASS1'}\"";
$opt .= " -certfile ${General::swroot}/ca/cacert.pem";
$opt .= " -caname \"$vpnsettings{'ROOTCERT_ORGANIZATION'} CA\"";
$opt .= " -out ${General::swroot}/certs/$cgiparams{'NAME'}.p12";
--
2.48.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] language files: Update to include a message about a double quotation mark
2025-03-09 14:12 [PATCH 1/2] vpnmain.cgi: Fixes bug12298 - IPSec password cannot use semicolon Adolf Belka
@ 2025-03-09 14:12 ` Adolf Belka
2025-03-10 9:56 ` [PATCH 1/2] vpnmain.cgi: Fixes bug12298 - IPSec password cannot use semicolon Michael Tremer
1 sibling, 0 replies; 5+ messages in thread
From: Adolf Belka @ 2025-03-09 14:12 UTC (permalink / raw)
To: development; +Cc: Adolf Belka
Fixes: bug12298
Tested-by: Adolf Belka <adolf.belka@ipfire.org>
Signed-off-by: Adolf Belka <adolf.belka@ipfire.org>
---
doc/language_issues.en | 1 +
doc/language_issues.es | 1 +
doc/language_issues.fr | 1 +
doc/language_issues.it | 1 +
doc/language_issues.pl | 1 +
doc/language_issues.ru | 1 +
doc/language_issues.tr | 1 +
doc/language_missings | 6 ++++++
langs/de/cgi-bin/de.pl | 1 +
langs/en/cgi-bin/en.pl | 1 +
langs/nl/cgi-bin/nl.pl | 1 +
11 files changed, 16 insertions(+)
diff --git a/doc/language_issues.en b/doc/language_issues.en
index a1730ac7b..02c4e6bfd 100644
--- a/doc/language_issues.en
+++ b/doc/language_issues.en
@@ -1480,6 +1480,7 @@ WARNING: untranslated string: pap or chap = PAP or CHAP
WARNING: untranslated string: parentclass = Parentclass
WARNING: untranslated string: parentclass add = Add parentclass
WARNING: untranslated string: password = Password:
+WARNING: untranslated string: password has quotation mark = Password contains an illegal double quotation mark.
WARNING: untranslated string: password not set = Password not set.
WARNING: untranslated string: password too short = Password is too short.
WARNING: untranslated string: passwords do not match = Passwords do not match.
diff --git a/doc/language_issues.es b/doc/language_issues.es
index 0a89279d5..60177bf49 100644
--- a/doc/language_issues.es
+++ b/doc/language_issues.es
@@ -1066,6 +1066,7 @@ WARNING: untranslated string: openvpn cert expires soon = Expires Soon
WARNING: untranslated string: openvpn cert has expired = Expired
WARNING: untranslated string: ovpn roadwarrior server = OpenVPN Roadwarrior Server
WARNING: untranslated string: pakfire ago = ago.
+WARNING: untranslated string: password has quotation mark = Password contains an illegal double quotation mark.
WARNING: untranslated string: processors = Processors
WARNING: untranslated string: reg_file_data_sampling = Register File Data Sampling (RFDS)
WARNING: untranslated string: regenerate host certificate = Renew Host Certificate
diff --git a/doc/language_issues.fr b/doc/language_issues.fr
index 7f9349bc0..7cf937d51 100644
--- a/doc/language_issues.fr
+++ b/doc/language_issues.fr
@@ -1014,6 +1014,7 @@ WARNING: untranslated string: load average = Load Average
WARNING: untranslated string: oops something went wrong = Oops, something went wrong...
WARNING: untranslated string: ovpn roadwarrior server = OpenVPN Roadwarrior Server
WARNING: untranslated string: pakfire ago = ago.
+WARNING: untranslated string: password has quotation mark = Password contains an illegal double quotation mark.
WARNING: untranslated string: processors = Processors
WARNING: untranslated string: reg_file_data_sampling = Register File Data Sampling (RFDS)
WARNING: untranslated string: routing config added = unknown string
diff --git a/doc/language_issues.it b/doc/language_issues.it
index 16371b566..1595e79d9 100644
--- a/doc/language_issues.it
+++ b/doc/language_issues.it
@@ -1269,6 +1269,7 @@ WARNING: untranslated string: pakfire tree = Repository
WARNING: untranslated string: pakfire tree stable = Stable
WARNING: untranslated string: pakfire tree testing = Testing
WARNING: untranslated string: pakfire tree unstable = Unstable
+WARNING: untranslated string: password has quotation mark = Password contains an illegal double quotation mark.
WARNING: untranslated string: please reboot to apply your changes = Please reboot to apply your changes
WARNING: untranslated string: pptp netconfig = My Net Config
WARNING: untranslated string: pptp peer = Peer
diff --git a/doc/language_issues.pl b/doc/language_issues.pl
index a3acc61af..935241881 100644
--- a/doc/language_issues.pl
+++ b/doc/language_issues.pl
@@ -1464,6 +1464,7 @@ WARNING: untranslated string: pakfire tree = Repository
WARNING: untranslated string: pakfire tree stable = Stable
WARNING: untranslated string: pakfire tree testing = Testing
WARNING: untranslated string: pakfire tree unstable = Unstable
+WARNING: untranslated string: password has quotation mark = Password contains an illegal double quotation mark.
WARNING: untranslated string: please reboot to apply your changes = Please reboot to apply your changes
WARNING: untranslated string: pptp netconfig = My Net Config
WARNING: untranslated string: pptp peer = Peer
diff --git a/doc/language_issues.ru b/doc/language_issues.ru
index e946c22df..4e9dda4b4 100644
--- a/doc/language_issues.ru
+++ b/doc/language_issues.ru
@@ -1457,6 +1457,7 @@ WARNING: untranslated string: pakfire tree = Repository
WARNING: untranslated string: pakfire tree stable = Stable
WARNING: untranslated string: pakfire tree testing = Testing
WARNING: untranslated string: pakfire tree unstable = Unstable
+WARNING: untranslated string: password has quotation mark = Password contains an illegal double quotation mark.
WARNING: untranslated string: please reboot to apply your changes = Please reboot to apply your changes
WARNING: untranslated string: pptp netconfig = My Net Config
WARNING: untranslated string: pptp peer = Peer
diff --git a/doc/language_issues.tr b/doc/language_issues.tr
index c0cb2703a..2746a108a 100644
--- a/doc/language_issues.tr
+++ b/doc/language_issues.tr
@@ -1183,6 +1183,7 @@ WARNING: untranslated string: pakfire tree = Repository
WARNING: untranslated string: pakfire tree stable = Stable
WARNING: untranslated string: pakfire tree testing = Testing
WARNING: untranslated string: pakfire tree unstable = Unstable
+WARNING: untranslated string: password has quotation mark = Password contains an illegal double quotation mark.
WARNING: untranslated string: please reboot to apply your changes = Please reboot to apply your changes
WARNING: untranslated string: processor vulnerability mitigations = Processor Vulnerability Mitigations
WARNING: untranslated string: processors = Processors
diff --git a/doc/language_missings b/doc/language_missings
index 92a78b090..c30f09827 100644
--- a/doc/language_missings
+++ b/doc/language_missings
@@ -150,6 +150,7 @@
< openvpn cert expires soon
< openvpn cert has expired
< ovpn roadwarrior server
+< password has quotation mark
< processors
< regenerate host certificate
< reg_file_data_sampling
@@ -193,6 +194,7 @@
< load average
< oops something went wrong
< ovpn roadwarrior server
+< password has quotation mark
< processors
< reg_file_data_sampling
< scanned
@@ -593,6 +595,7 @@
< pakfire tree testing
< pakfire tree unstable
< pak update
+< password has quotation mark
< please reboot to apply your changes
< pptp netconfig
< pptp peer
@@ -2064,6 +2067,7 @@
< pakfire tree testing
< pakfire tree unstable
< pak update
+< password has quotation mark
< please reboot to apply your changes
< pptp netconfig
< pptp peer
@@ -3084,6 +3088,7 @@
< pakfire tree testing
< pakfire tree unstable
< pak update
+< password has quotation mark
< please reboot to apply your changes
< pptp netconfig
< pptp peer
@@ -3595,6 +3600,7 @@
< pakfire tree testing
< pakfire tree unstable
< pak update
+< password has quotation mark
< please reboot to apply your changes
< processors
< processor vulnerability mitigations
diff --git a/langs/de/cgi-bin/de.pl b/langs/de/cgi-bin/de.pl
index 5f89c7010..5cac132b9 100644
--- a/langs/de/cgi-bin/de.pl
+++ b/langs/de/cgi-bin/de.pl
@@ -2044,6 +2044,7 @@
'password' => 'Passwort:',
'password contains illegal characters' => 'Passwort enthält ungültige(s) Zeichen.',
'password crypting key' => 'Schlüssel wird mit dem Passwort chiffriert',
+'password has quotation mark' => 'Kennwort enthält ein unzulässiges doppeltes Anführungszeichen.',
'password not set' => 'Passwort nicht angegeben.',
'password too short' => 'Passwort ist zu kurz.',
'passwords do not match' => 'Die Passwörter stimmen nicht überein.',
diff --git a/langs/en/cgi-bin/en.pl b/langs/en/cgi-bin/en.pl
index 197f44633..8c105150a 100644
--- a/langs/en/cgi-bin/en.pl
+++ b/langs/en/cgi-bin/en.pl
@@ -2108,6 +2108,7 @@
'password' => 'Password:',
'password contains illegal characters' => 'Password contains illegal characters.',
'password crypting key' => 'Password crypting the key',
+'password has quotation mark' => 'Password contains an illegal double quotation mark.',
'password not set' => 'Password not set.',
'password too short' => 'Password is too short.',
'passwords do not match' => 'Passwords do not match.',
diff --git a/langs/nl/cgi-bin/nl.pl b/langs/nl/cgi-bin/nl.pl
index beb1b6e36..8b8979972 100644
--- a/langs/nl/cgi-bin/nl.pl
+++ b/langs/nl/cgi-bin/nl.pl
@@ -1712,6 +1712,7 @@
'password' => 'Wachtwoord:',
'password contains illegal characters' => 'Wachtwoord bevat ongeldige tekens.',
'password crypting key' => 'Wachtwoord codeert de sleutel',
+'password has quotation mark' => 'Wachtwoord bevat een ongeldig dubbel aanhalingsteken.',
'password not set' => 'Wachtwoord niet ingesteld.',
'password too short' => 'Wachtwoord is te kort.',
'passwords do not match' => 'Wachtwoorden komen niet overeen.',
--
2.48.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] vpnmain.cgi: Fixes bug12298 - IPSec password cannot use semicolon
2025-03-09 14:12 [PATCH 1/2] vpnmain.cgi: Fixes bug12298 - IPSec password cannot use semicolon Adolf Belka
2025-03-09 14:12 ` [PATCH 2/2] language files: Update to include a message about a double quotation mark Adolf Belka
@ 2025-03-10 9:56 ` Michael Tremer
2025-03-10 10:48 ` Adolf Belka
1 sibling, 1 reply; 5+ messages in thread
From: Michael Tremer @ 2025-03-10 9:56 UTC (permalink / raw)
To: Adolf Belka; +Cc: development
Hello Adolf,
Oh this is bad. Not the patch, what we have there before…
I would say I will accept this patch as it is because it slightly mitigates the problem. However, there is still a chance to run shell commands using backticks and $(…).
Would you be able to rewrite this command to use one of the &General::system* commands? That way, there will no command injection be possible any more and we can in theory allow quotes and semicolons again.
Best,
-Michael
> On 9 Mar 2025, at 14:12, Adolf Belka <adolf.belka@ipfire.org> wrote:
>
> - The password for the pkcs12 certificate is passed to the open ssl command via $opt but
> it is not quoted and so the ; is taken as the end of the command rather than as part
> of the password. This also means that a pkcs12 file is not created and the .pem
> intermediate file is what is left in the directory.
> - This patch makes the -passout option quoted in the same way as the -name and -caname
> options.
> - Based on being the same as the name and caname parts in $opt, I believe that this should
> not give rise to a vulnerability but I am open to being corrected.
> - By quoting the -passout then the password must not contain double quotation marks, ",
> so a test for the password containing a " has been added.
> - The message about the use of the double quotation mark has been added to the english,
> dutch and german language files. Feel free to correct if what I have used is not
> correct. Those are in the other patch of this patch set.
> - Tested out on my testbed system. I was able to create a pkcs12 certificate with a
> password containing a variety of characters, including the semicolon, and getting
> a message that the password contains a double quotation mark when I used that.
>
> Fixes: bug12298
> Tested-by: Adolf Belka <adolf.belka@ipfire.org>
> Signed-off-by: Adolf Belka <adolf.belka@ipfire.org>
> ---
> html/cgi-bin/vpnmain.cgi | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
> 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 c9bbbb494..8106ee24e
> --- a/html/cgi-bin/vpnmain.cgi
> +++ b/html/cgi-bin/vpnmain.cgi
> @@ -2149,6 +2149,10 @@ END
> $errormessage = $Lang::tr{'password too short'};
> goto VPNCONF_ERROR;
> }
> + if ($cgiparams{'CERT_PASS1'} =~ /["]/) {
> + $errormessage = $Lang::tr{'password has quotation mark'};
> + goto VPNCONF_ERROR;
> + }
> if ($cgiparams{'CERT_PASS1'} ne $cgiparams{'CERT_PASS2'}) {
> $errormessage = $Lang::tr{'passwords do not match'};
> goto VPNCONF_ERROR;
> @@ -2226,7 +2230,7 @@ END
> $opt .= " -inkey ${General::swroot}/certs/$cgiparams{'NAME'}key.pem";
> $opt .= " -in ${General::swroot}/certs/$cgiparams{'NAME'}cert.pem";
> $opt .= " -name \"$cgiparams{'NAME'}\"";
> - $opt .= " -passout pass:$cgiparams{'CERT_PASS1'}";
> + $opt .= " -passout pass:\"$cgiparams{'CERT_PASS1'}\"";
> $opt .= " -certfile ${General::swroot}/ca/cacert.pem";
> $opt .= " -caname \"$vpnsettings{'ROOTCERT_ORGANIZATION'} CA\"";
> $opt .= " -out ${General::swroot}/certs/$cgiparams{'NAME'}.p12";
> --
> 2.48.1
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] vpnmain.cgi: Fixes bug12298 - IPSec password cannot use semicolon
2025-03-10 9:56 ` [PATCH 1/2] vpnmain.cgi: Fixes bug12298 - IPSec password cannot use semicolon Michael Tremer
@ 2025-03-10 10:48 ` Adolf Belka
2025-03-10 11:15 ` Michael Tremer
0 siblings, 1 reply; 5+ messages in thread
From: Adolf Belka @ 2025-03-10 10:48 UTC (permalink / raw)
To: Michael Tremer; +Cc: IPFire: Development-List
Hi Michael,
On 10/03/2025 10:56, Michael Tremer wrote:
> Hello Adolf,
>
> Oh this is bad. Not the patch, what we have there before…
>
> I would say I will accept this patch as it is because it slightly mitigates the problem. However, there is still a chance to run shell commands using backticks and $(…).
>
> Would you be able to rewrite this command to use one of the &General::system* commands? That way, there will no command injection be possible any more and we can in theory allow quotes and semicolons again.
I had wondered about that and did try at first to use the
&General::system_output command but I couldn't get it to work. Even went
to the stage of creating my own mini program to use system_output but it
didn't like the use of $opt being a string with multiple commands in it
separated by a space.
I then saw that there were already other parts of $opt that were quoted
and so I thought it was alright to use that approach.
I now understand that those other entries are also not good to use.
I should have trusted my first instinct that this looked the same as the
other bug fix in vpnmain.cgi where I got &General::system_output working.
I will go back and find a way to use system_output to do what is
required here.
After finding a solution I will also provide patches for all the other
locations in vpnmain.cgi that use a similar approach. Then I can look if
there are similar things in other .cgi files.
Regards,
Adolf.
>
> Best,
> -Michael
>
>> On 9 Mar 2025, at 14:12, Adolf Belka <adolf.belka@ipfire.org> wrote:
>>
>> - The password for the pkcs12 certificate is passed to the open ssl command via $opt but
>> it is not quoted and so the ; is taken as the end of the command rather than as part
>> of the password. This also means that a pkcs12 file is not created and the .pem
>> intermediate file is what is left in the directory.
>> - This patch makes the -passout option quoted in the same way as the -name and -caname
>> options.
>> - Based on being the same as the name and caname parts in $opt, I believe that this should
>> not give rise to a vulnerability but I am open to being corrected.
>> - By quoting the -passout then the password must not contain double quotation marks, ",
>> so a test for the password containing a " has been added.
>> - The message about the use of the double quotation mark has been added to the english,
>> dutch and german language files. Feel free to correct if what I have used is not
>> correct. Those are in the other patch of this patch set.
>> - Tested out on my testbed system. I was able to create a pkcs12 certificate with a
>> password containing a variety of characters, including the semicolon, and getting
>> a message that the password contains a double quotation mark when I used that.
>>
>> Fixes: bug12298
>> Tested-by: Adolf Belka <adolf.belka@ipfire.org>
>> Signed-off-by: Adolf Belka <adolf.belka@ipfire.org>
>> ---
>> html/cgi-bin/vpnmain.cgi | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>> 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 c9bbbb494..8106ee24e
>> --- a/html/cgi-bin/vpnmain.cgi
>> +++ b/html/cgi-bin/vpnmain.cgi
>> @@ -2149,6 +2149,10 @@ END
>> $errormessage = $Lang::tr{'password too short'};
>> goto VPNCONF_ERROR;
>> }
>> + if ($cgiparams{'CERT_PASS1'} =~ /["]/) {
>> + $errormessage = $Lang::tr{'password has quotation mark'};
>> + goto VPNCONF_ERROR;
>> + }
>> if ($cgiparams{'CERT_PASS1'} ne $cgiparams{'CERT_PASS2'}) {
>> $errormessage = $Lang::tr{'passwords do not match'};
>> goto VPNCONF_ERROR;
>> @@ -2226,7 +2230,7 @@ END
>> $opt .= " -inkey ${General::swroot}/certs/$cgiparams{'NAME'}key.pem";
>> $opt .= " -in ${General::swroot}/certs/$cgiparams{'NAME'}cert.pem";
>> $opt .= " -name \"$cgiparams{'NAME'}\"";
>> - $opt .= " -passout pass:$cgiparams{'CERT_PASS1'}";
>> + $opt .= " -passout pass:\"$cgiparams{'CERT_PASS1'}\"";
>> $opt .= " -certfile ${General::swroot}/ca/cacert.pem";
>> $opt .= " -caname \"$vpnsettings{'ROOTCERT_ORGANIZATION'} CA\"";
>> $opt .= " -out ${General::swroot}/certs/$cgiparams{'NAME'}.p12";
>> --
>> 2.48.1
>>
>>
>
>
--
Sent from my laptop
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] vpnmain.cgi: Fixes bug12298 - IPSec password cannot use semicolon
2025-03-10 10:48 ` Adolf Belka
@ 2025-03-10 11:15 ` Michael Tremer
0 siblings, 0 replies; 5+ messages in thread
From: Michael Tremer @ 2025-03-10 11:15 UTC (permalink / raw)
To: Adolf Belka; +Cc: IPFire: Development-List
Hello,
> On 10 Mar 2025, at 10:48, Adolf Belka <adolf.belka@ipfire.org> wrote:
>
> Hi Michael,
>
> On 10/03/2025 10:56, Michael Tremer wrote:
>> Hello Adolf,
>> Oh this is bad. Not the patch, what we have there before…
>> I would say I will accept this patch as it is because it slightly mitigates the problem. However, there is still a chance to run shell commands using backticks and $(…).
>> Would you be able to rewrite this command to use one of the &General::system* commands? That way, there will no command injection be possible any more and we can in theory allow quotes and semicolons again.
>
> I had wondered about that and did try at first to use the &General::system_output command but I couldn't get it to work. Even went to the stage of creating my own mini program to use system_output but it didn't like the use of $opt being a string with multiple commands in it separated by a space.
>
> I then saw that there were already other parts of $opt that were quoted and so I thought it was alright to use that approach.
It actually might be. The custom system functions will however guarantee it and I would prefer to have a unified way across the entire code base than custom solutions here or there, because we might break them over time.
> I now understand that those other entries are also not good to use.
> I should have trusted my first instinct that this looked the same as the other bug fix in vpnmain.cgi where I got &General::system_output working.
>
> I will go back and find a way to use system_output to do what is required here.
>
> After finding a solution I will also provide patches for all the other locations in vpnmain.cgi that use a similar approach. Then I can look if there are similar things in other .cgi files.
Yes please. There should not be too many I hope. I think we deliberately skipped a few places where the command line was static and did not contain any variables as those should always be safe and we urgently needed to ship the fixes that were reported to us back then.
Feel free to ask any questions in case you get stuck.
-Michael
>
> Regards,
> Adolf.
>
>
>> Best,
>> -Michael
>>> On 9 Mar 2025, at 14:12, Adolf Belka <adolf.belka@ipfire.org> wrote:
>>>
>>> - The password for the pkcs12 certificate is passed to the open ssl command via $opt but
>>> it is not quoted and so the ; is taken as the end of the command rather than as part
>>> of the password. This also means that a pkcs12 file is not created and the .pem
>>> intermediate file is what is left in the directory.
>>> - This patch makes the -passout option quoted in the same way as the -name and -caname
>>> options.
>>> - Based on being the same as the name and caname parts in $opt, I believe that this should
>>> not give rise to a vulnerability but I am open to being corrected.
>>> - By quoting the -passout then the password must not contain double quotation marks, ",
>>> so a test for the password containing a " has been added.
>>> - The message about the use of the double quotation mark has been added to the english,
>>> dutch and german language files. Feel free to correct if what I have used is not
>>> correct. Those are in the other patch of this patch set.
>>> - Tested out on my testbed system. I was able to create a pkcs12 certificate with a
>>> password containing a variety of characters, including the semicolon, and getting
>>> a message that the password contains a double quotation mark when I used that.
>>>
>>> Fixes: bug12298
>>> Tested-by: Adolf Belka <adolf.belka@ipfire.org>
>>> Signed-off-by: Adolf Belka <adolf.belka@ipfire.org>
>>> ---
>>> html/cgi-bin/vpnmain.cgi | 6 +++++-
>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>> 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 c9bbbb494..8106ee24e
>>> --- a/html/cgi-bin/vpnmain.cgi
>>> +++ b/html/cgi-bin/vpnmain.cgi
>>> @@ -2149,6 +2149,10 @@ END
>>> $errormessage = $Lang::tr{'password too short'};
>>> goto VPNCONF_ERROR;
>>> }
>>> + if ($cgiparams{'CERT_PASS1'} =~ /["]/) {
>>> + $errormessage = $Lang::tr{'password has quotation mark'};
>>> + goto VPNCONF_ERROR;
>>> + }
>>> if ($cgiparams{'CERT_PASS1'} ne $cgiparams{'CERT_PASS2'}) {
>>> $errormessage = $Lang::tr{'passwords do not match'};
>>> goto VPNCONF_ERROR;
>>> @@ -2226,7 +2230,7 @@ END
>>> $opt .= " -inkey ${General::swroot}/certs/$cgiparams{'NAME'}key.pem";
>>> $opt .= " -in ${General::swroot}/certs/$cgiparams{'NAME'}cert.pem";
>>> $opt .= " -name \"$cgiparams{'NAME'}\"";
>>> - $opt .= " -passout pass:$cgiparams{'CERT_PASS1'}";
>>> + $opt .= " -passout pass:\"$cgiparams{'CERT_PASS1'}\"";
>>> $opt .= " -certfile ${General::swroot}/ca/cacert.pem";
>>> $opt .= " -caname \"$vpnsettings{'ROOTCERT_ORGANIZATION'} CA\"";
>>> $opt .= " -out ${General::swroot}/certs/$cgiparams{'NAME'}.p12";
>>> --
>>> 2.48.1
>>>
>>>
>
> --
> Sent from my laptop
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-03-10 11:15 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-03-09 14:12 [PATCH 1/2] vpnmain.cgi: Fixes bug12298 - IPSec password cannot use semicolon Adolf Belka
2025-03-09 14:12 ` [PATCH 2/2] language files: Update to include a message about a double quotation mark Adolf Belka
2025-03-10 9:56 ` [PATCH 1/2] vpnmain.cgi: Fixes bug12298 - IPSec password cannot use semicolon Michael Tremer
2025-03-10 10:48 ` Adolf Belka
2025-03-10 11:15 ` Michael Tremer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox