Re: [PATCH 1/6] ovpnmain.cgi: Fix for bug#11048 - insecure download icon shown for connections with a password

[Michael Tremer <michael.tremer@ipfire.org>]

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

Hello Adolf,

No problem. I took care of this when merging the patchset.

-Michael

> On 27 Sep 2023, at 09:20, Adolf Belka <adolf.belka(a)ipfire.org> wrote:
> 
> Hi All,
> 
> Yesterday I submitted this updated patch set for bug#11048 (better late than never).
> 
> One of the patches is adding update code for the fix to the update.sh script in Core Update 180, as that was still the version in next.
> 
> As CU180 has now been released for Testing, then that update code will likely need to be in the update.sh script for CU181.
> 
> 
> Do I need to resubmit a v2 version of my patch set, once next has been updated to CU181, or can the patch set still be merged without problems as it is?
> 
> 
> Regards,
> 
> Adolf
> 
> 
> On 25/09/2023 18:41, Adolf Belka wrote:
>> - At long last I have re-visited the patch submission for bug #11048 and fixed the issues
>>    that caused the problems last time I evaluated it in Testing.
>> - 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 enters no-pass if the connection type is host and the password is null, pass if
>>    the connection type is host and the password has characters. If the connection type is
>>    net then no-pass is used as net2net connections dop not have encrypted certificates.
>> - The code has been changed to show a different icon for unencrypted and encrypted
>>    certificates.
>> - Separate patches are provided for the language file change, the provision of a new icon
>>    and the code for the update.sh script for the Core Update to update all existing
>>    connections, if any exist, to have either pass or no-pass in index 41.
>> - This patch set was a joint collaboration between Erik Kapfer and Adolf Belka
>> - Patch set, including the code for the Core Update 180 update.sh script has been tested
>>    on a vm testbed
>> Fixes: Bug#11048
>> Tested-by: Adolf Belka <adolf.belka(a)ipfire.org>
>> Suggested-by: Adolf Belka <adolf.belka(a)ipfire.org>
>> Suggested-by: Erik Kapfer <ummeegge(a)ipfire.org>
>> Signed-off-by: Adolf Belka <adolf.belka(a)ipfire.org>
>> ---
>>  html/cgi-bin/ovpnmain.cgi | 75 +++++++++++++++++++++++----------------
>>  1 file changed, 44 insertions(+), 31 deletions(-)
>> diff --git a/html/cgi-bin/ovpnmain.cgi b/html/cgi-bin/ovpnmain.cgi
>> index 5afe54f55..eb89c5095 100755
>> --- a/html/cgi-bin/ovpnmain.cgi
>> +++ b/html/cgi-bin/ovpnmain.cgi
>> @@ -4370,9 +4370,15 @@ 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";
>> +               } elsif ($cgiparams{'TYPE'} eq 'net') {
>> +                       $confighash{$key}[41] = "no-pass";
>> +               }
>> +       }
>>       $confighash{$key}[42] = 'HOTP/T30/6';
>>   $confighash{$key}[43] = $cgiparams{'OTP_STATE'};
>> @@ -5512,20 +5518,24 @@ END
>>  }
>>    -    print <<END;
>> - <td align='center' $col1>$active</td>
>> +       if ($confighash{$key}[41] eq "pass") {
>> +               print <<END;
>> +                       <td align='center' $col1>$active</td>
>>  - <form method='post' name='frm${key}a'><td align='center' $col>
>> -    <input type='image'  name='$Lang::tr{'dl client arch'}' src='/images/openvpn.png' alt='$Lang::tr{'dl client arch'}' title='$Lang::tr{'dl client arch'}' border='0' />
>> -    <input type='hidden' name='ACTION' value='$Lang::tr{'dl client arch'}' />
>> -    <input type='hidden' name='KEY' value='$key' />
>> - </td></form>
>> +                       <form method='post' name='frm${key}a'><td align='center' $col>
>> +                           <input type='image'  name='$Lang::tr{'dl client arch'}' src='/images/openvpn_encrypted.png'
>> +                                       alt='$Lang::tr{'dl client arch'}' title='$Lang::tr{'dl client arch'}' border='0' />
>> +                           <input type='hidden' name='ACTION' value='$Lang::tr{'dl client arch'}' />
>> +                           <input type='hidden' name='MODE' value='secure' />
>> +                           <input type='hidden' name='KEY' value='$key' />
>> +                       </td></form>
>>  END
>> - ;
>>  - if ($confighash{$key}[41] eq "no-pass") {
>> +       ; } elsif ($confighash{$key}[41] eq "no-pass") {
>>   print <<END;
>> - <form method='post' name='frm${key}g'><td align='center' $col>
>> +                       <td align='center' $col1>$active</td>
>> +
>> +                       <form method='post' name='frm${key}a'><td align='center' $col>
>>   <input type='image'  name='$Lang::tr{'dl client arch insecure'}' src='/images/openvpn.png'
>>   alt='$Lang::tr{'dl client arch insecure'}' title='$Lang::tr{'dl client arch insecure'}' border='0' />
>>   <input type='hidden' name='ACTION' value='$Lang::tr{'dl client arch'}' />
>> @@ -5533,7 +5543,7 @@ END
>>   <input type='hidden' name='KEY' value='$key' />
>>   </td></form>
>>  END
>> - } else {
>> + ; } else {
>>   print "<td $col>&nbsp;</td>";
>>   }
>>  @@ -5609,30 +5619,33 @@ END
>>      # If the config file contains entries, print Key to action icons
>>      if ( $id ) {
>>      print <<END;
>> -    <table border='0'>
>> -    <tr>
>> +       <table width='85%' border='0'>
>> +       <tr>
>>   <td class='boldbase'>&nbsp; <b>$Lang::tr{'legend'}:</b></td>
>> - <td>&nbsp; <img src='/images/on.gif' alt='$Lang::tr{'click to disable'}' /></td>
>> - <td class='base'>$Lang::tr{'click to disable'}</td>
>> +              <td>&nbsp; &nbsp; <img src='/images/openvpn.png' alt='?RELOAD'/></td>
>> +              <td class='base'>$Lang::tr{'dl client arch insecure'}</td>
>> +              <td>&nbsp; &nbsp; <img src='/images/openvpn_encrypted.png' alt='?RELOAD'/></td>
>> +              <td class='base'>$Lang::tr{'dl client arch'}</td>
>>   <td>&nbsp; &nbsp; <img src='/images/info.gif' alt='$Lang::tr{'show certificate'}' /></td>
>>   <td class='base'>$Lang::tr{'show certificate'}</td>
>> +              <td>&nbsp; &nbsp; <img src='/images/qr-code.png' alt='$Lang::tr{'show otp qrcode'}'/></td>
>> +              <td class='base'>$Lang::tr{'show otp qrcode'}</td>
>> +       </tr>
>> +       <tr>
>> +              <td>&nbsp; </td>
>> +              <td>&nbsp; &nbsp; <img src='/images/media-floppy.png' alt='?FLOPPY' /></td>
>> +              <td class='base'>$Lang::tr{'download certificate'}</td>
>> +              <td>&nbsp; <img src='/images/off.gif' alt='?OFF' /></td>
>> +              <td class='base'>$Lang::tr{'click to enable'}</td>
>> +              <td>&nbsp; <img src='/images/on.gif' alt='$Lang::tr{'click to disable'}' /></td>
>> +              <td class='base'>$Lang::tr{'click to disable'}</td> 
>> +
>>   <td>&nbsp; &nbsp; <img src='/images/edit.gif' alt='$Lang::tr{'edit'}' /></td>
>>   <td class='base'>$Lang::tr{'edit'}</td>
>>   <td>&nbsp; &nbsp; <img src='/images/delete.gif' alt='$Lang::tr{'remove'}' /></td>
>>   <td class='base'>$Lang::tr{'remove'}</td>
>> -    </tr>
>> -    <tr>
>> - <td>&nbsp; </td>
>> - <td>&nbsp; <img src='/images/off.gif' alt='?OFF' /></td>
>> - <td class='base'>$Lang::tr{'click to enable'}</td>
>> - <td>&nbsp; &nbsp; <img src='/images/media-floppy.png' alt='?FLOPPY' /></td>
>> - <td class='base'>$Lang::tr{'download certificate'}</td>
>> - <td>&nbsp; &nbsp; <img src='/images/openvpn.png' alt='?RELOAD'/></td>
>> - <td class='base'>$Lang::tr{'dl client arch'}</td>
>> - <td>&nbsp; &nbsp; <img src='/images/qr-code.png' alt='$Lang::tr{'show otp qrcode'}'/></td>
>> - <td class='base'>$Lang::tr{'show otp qrcode'}</td>
>> - </tr>
>> -    </table><br>
>> +       </tr>
>> +       </table><br>
>>  END
>>      ;
>>      }