[PATCH] ovpnmain.cgi: Fix for bug#11048 - insecure download icon for connections with a password
- The insecure package download icon is shown if entry 41 in /var/ipfire/ovpn/ovpnconfig is set to no-pass. The code block on ovpnmain.cgi that deals with this checks if the connection is a host and if the first password entry is a null. Then it adds no-pass to ovpnconfig. - The same block of code is also used for when he connection is edited. However at this stage the password entry is back to null because the password value is only kept until the connection has been saved. Therefore doing an edit results in the password value being taken as null even for connections with a password. - This fix checks the password value only if entry 41 in the ovpnconfig file is a null. If it is a null then it enters no-pass if the password is a null and it enters pass if the password contains characters. This way the entry 41 always contains either pass or no-pass, except when the connection is being first added and saved. - When adding this fix into a Core Update the update.sh script will need to check if ovpnconfig exists and then add pass to all lines that have a null at entry 41. This will only fix those connections that have not already been edited. Any connections already edited will have no-pass at entry 41 of ovpnconfig and will therefore show the insecure package download icon. - The only way I can think of dealing with entries that already have no-pass added is to go through all .p12 files in the certs directory and if there is a connection entry with that name then to change the no-pass to a pass. However that will only work if the connection name has been set the same as the certificate name, which is not a requirement. - So I think we can only fix those coneections that have never been edited. Any connections with passwords that have already been edited and containg no-pass in ovpnconfig and showing the insecure package download icon will have to be manually dealt with by users. - I think that should still be okay because they currently have two icons when they shouldn't and that will continue to be the case if they don't carry out a manual edit of ovpnconfig. - Maybe someone can think of an alternative way of identifying all connections using a password so that they can have entry 41 changed to pass. I haven't been able to do that so far - Looking forward to feedback
Tested-by: Adolf Belka adolf.belka@ipfire.org Signed-off-by: Adolf Belka adolf.belka@ipfire.org --- html/cgi-bin/ovpnmain.cgi | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/html/cgi-bin/ovpnmain.cgi b/html/cgi-bin/ovpnmain.cgi index 42a7354fc..2586a1796 100644 --- a/html/cgi-bin/ovpnmain.cgi +++ b/html/cgi-bin/ovpnmain.cgi @@ -4326,9 +4326,13 @@ if ($cgiparams{'TYPE'} eq 'net') { $confighash{$key}[39] = $cgiparams{'DAUTH'}; $confighash{$key}[40] = $cgiparams{'DCIPHER'};
- if (($cgiparams{'TYPE'} eq 'host') && ($cgiparams{'CERT_PASS1'} eq "")) { - $confighash{$key}[41] = "no-pass"; - } + if ($confighash{$key}[41] eq "") { + if (($cgiparams{'TYPE'} eq 'host') && ($cgiparams{'CERT_PASS1'} eq "")) { + $confighash{$key}[41] = "no-pass"; + } elsif (($cgiparams{'TYPE'} eq 'host') && ($cgiparams{'CERT_PASS1'} ne "")) { + $confighash{$key}[41] = "pass"; + } + }
$confighash{$key}[42] = 'HOTP/T30/6'; $confighash{$key}[43] = $cgiparams{'OTP_STATE'};
Hi All,
I have marked this patch as superseded as it became clear that also the N2N entries in ovpnconfig needed to be explicitly specified as no-pass.
Also with the no password insecure option two icons were still shown for the download. There should only be one icon for each situation. One downloads the package with no password option and the certificates built into the .ovpn file and the other downloads the package with the .p12 file protected by a password.
A new patch will be put together covering all above and tested and then submitted together with a tested script for giving all connections, N2N, password protected hosts and hosts with no password in ovpnconfig an explicit status of pass or no-pass.
Regards, Adolf.
On 10/02/2023 19:13, Adolf Belka wrote:
- The insecure package download icon is shown if entry 41 in /var/ipfire/ovpn/ovpnconfig is set to no-pass. The code block on ovpnmain.cgi that deals with this checks if the connection is a host and if the first password entry is a null. Then it adds no-pass to ovpnconfig.
- The same block of code is also used for when he connection is edited. However at this stage the password entry is back to null because the password value is only kept until the connection has been saved. Therefore doing an edit results in the password value being taken as null even for connections with a password.
- This fix checks the password value only if entry 41 in the ovpnconfig file is a null. If it is a null then it enters no-pass if the password is a null and it enters pass if the password contains characters. This way the entry 41 always contains either pass or no-pass, except when the connection is being first added and saved.
- When adding this fix into a Core Update the update.sh script will need to check if ovpnconfig exists and then add pass to all lines that have a null at entry 41. This will only fix those connections that have not already been edited. Any connections already edited will have no-pass at entry 41 of ovpnconfig and will therefore show the insecure package download icon.
- The only way I can think of dealing with entries that already have no-pass added is to go through all .p12 files in the certs directory and if there is a connection entry with that name then to change the no-pass to a pass. However that will only work if the connection name has been set the same as the certificate name, which is not a requirement.
- So I think we can only fix those coneections that have never been edited. Any connections with passwords that have already been edited and containg no-pass in ovpnconfig and showing the insecure package download icon will have to be manually dealt with by users.
- I think that should still be okay because they currently have two icons when they shouldn't and that will continue to be the case if they don't carry out a manual edit of ovpnconfig.
- Maybe someone can think of an alternative way of identifying all connections using a password so that they can have entry 41 changed to pass. I haven't been able to do that so far
- Looking forward to feedback
Tested-by: Adolf Belka adolf.belka@ipfire.org Signed-off-by: Adolf Belka adolf.belka@ipfire.org
html/cgi-bin/ovpnmain.cgi | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/html/cgi-bin/ovpnmain.cgi b/html/cgi-bin/ovpnmain.cgi index 42a7354fc..2586a1796 100644 --- a/html/cgi-bin/ovpnmain.cgi +++ b/html/cgi-bin/ovpnmain.cgi @@ -4326,9 +4326,13 @@ if ($cgiparams{'TYPE'} eq 'net') { $confighash{$key}[39] = $cgiparams{'DAUTH'}; $confighash{$key}[40] = $cgiparams{'DCIPHER'};
- if (($cgiparams{'TYPE'} eq 'host') && ($cgiparams{'CERT_PASS1'} eq "")) {
$confighash{$key}[41] = "no-pass";- }
if ($confighash{$key}[41] eq "") {
if (($cgiparams{'TYPE'} eq 'host') && ($cgiparams{'CERT_PASS1'} eq "")) {
$confighash{$key}[41] = "no-pass";} elsif (($cgiparams{'TYPE'} eq 'host') && ($cgiparams{'CERT_PASS1'} ne "")) {
$confighash{$key}[41] = "pass";}
}
$confighash{$key}[42] = 'HOTP/T30/6'; $confighash{$key}[43] = $cgiparams{'OTP_STATE'};
-
Adolf Belka