From mboxrd@z Thu Jan 1 00:00:00 1970 From: Adolf Belka To: development@lists.ipfire.org Subject: Re: [PATCH] ovpnmain.cgi: Fix for bug#11048 - insecure download icon for connections with a password Date: Fri, 10 Mar 2023 18:20:34 +0100 Message-ID: <24f424c5-19f2-beae-f09d-4d3d97691567@ipfire.org> In-Reply-To: <20230210181343.17763-1-adolf.belka@ipfire.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============6453471677059412161==" List-Id: --===============6453471677059412161== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hi All, I have marked this patch as superseded as it became clear that also the=20 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=20 the download. There should only be one icon for each situation. One=20 downloads the package with no password option and the certificates built=20 into the .ovpn file and the other downloads the package with the .p12=20 file protected by a password. A new patch will be put together covering all above and tested and then=20 submitted together with a tested script for giving all connections, N2N,=20 password protected hosts and hosts with no password in ovpnconfig an=20 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/ov= pn/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. How= ever 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 p= assword value > being taken as null even for connections with a password. > - This fix checks the password value only if entry 41 in the ovpnconfig fil= e 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 conta= ins 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 en= try 41. This will > only fix those connections that have not already been edited. Any conne= ctions already > edited will have no-pass at entry 41 of ovpnconfig and will therefore s= how the insecure > package download icon. > - The only way I can think of dealing with entries that already have no-pa= ss added is to > go through all .p12 files in the certs directory and if there is a conn= ection 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 ov= pnconfig and > showing the insecure package download icon will have to be manually dea= lt with by users. > - I think that should still be okay because they currently have two icons w= hen 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 connecti= ons 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 >=20 > Tested-by: Adolf Belka > Signed-off-by: Adolf Belka > --- > html/cgi-bin/ovpnmain.cgi | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) >=20 > 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] =3D $cgiparams{'DAUTH'}; > $confighash{$key}[40] =3D $cgiparams{'DCIPHER'}; > =20 > - if (($cgiparams{'TYPE'} eq 'host') && ($cgiparams{'CERT_PASS1'} eq "")) { > - $confighash{$key}[41] =3D "no-pass"; > - } > + if ($confighash{$key}[41] eq "") { > + if (($cgiparams{'TYPE'} eq 'host') && ($cgiparams{'CERT_PASS1'} eq "")= ) { > + $confighash{$key}[41] =3D "no-pass"; > + } elsif (($cgiparams{'TYPE'} eq 'host') && ($cgiparams{'CERT_PASS1'} n= e "")) { > + $confighash{$key}[41] =3D "pass"; > + } > + } > =20 > $confighash{$key}[42] =3D 'HOTP/T30/6'; > $confighash{$key}[43] =3D $cgiparams{'OTP_STATE'}; --=20 Sent from my laptop --===============6453471677059412161==--