Hello Adolf,
> On 2 Apr 2025, at 11:41, Adolf Belka <[email protected]> wrote:
>
> Hi Michael,
>
> On 02/04/2025 12:21, Michael Tremer wrote:
>>> On 1 Apr 2025, at 19:07, Adolf Belka <[email protected]> wrote:
>>>
>>> - As the serial number is incremented now for each new cert that is
>>> created, then when a
>>> client cert is deleted from the ipsec list in the wui then that cert must
>>> be revoked
>>> otherwise it will still be listed in the .index file as a valid
>>> certificate and then
>>> the certificate name and DN could never be used again.
>>> - Running the revoke command when deleting a client cert leaves the details
>>> in the .index
>>> file but the same name can then be re-used and will get a new serial
>>> number etc.
>>>
>>> Fixes: bug13737
>>> Tested-by: Adolf Belka <[email protected]>
>>> Signed-off-by: Adolf Belka <[email protected]>
>>> ---
>>> html/cgi-bin/vpnmain.cgi | 30 +++++++++++++++++++-----------
>>> 1 file changed, 19 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/html/cgi-bin/vpnmain.cgi b/html/cgi-bin/vpnmain.cgi
>>> index 85119a81d..1c9f9243b 100644
>>> --- a/html/cgi-bin/vpnmain.cgi
>>> +++ b/html/cgi-bin/vpnmain.cgi
>>> @@ -1595,17 +1595,25 @@ END
>>> &General::readhash("${General::swroot}/vpn/settings", \%vpnsettings);
>>> &General::readhasharray("${General::swroot}/vpn/config", \%confighash);
>>>
>>> - if ($confighash{$cgiparams{'KEY'}}) {
>>> - unlink
>>> ("${General::swroot}/certs/$confighash{$cgiparams{'KEY'}}[1]cert.pem");
>>> - unlink ("${General::swroot}/certs/$confighash{$cgiparams{'KEY'}}[1].p12");
>>> - delete $confighash{$cgiparams{'KEY'}};
>>> - &General::writehasharray("${General::swroot}/vpn/config", \%confighash);
>>> - &writeipsecfiles();
>>> - &General::system('/usr/local/bin/ipsecctrl', 'D', $cgiparams{'KEY'}) if
>>> (&vpnenabled);
>>> - } else {
>>> - $errormessage = $Lang::tr{'invalid key'};
>>> - }
>>> - &General::firewall_reload();
>>> + if ($confighash{$cgiparams{'KEY'}}) {
>>> + # Revoke the removed certificate
>>> + if (!$errormessage) {
>>> + &General::log("charon", "Revoking the removed
>>> client cert...");
>>> + my $opt = " ca -revoke
>>> ${General::swroot}/certs/$confighash{$cgiparams{'KEY'}}[1]cert.pem";
>>> + $errormessage = &callssl($opt);
>> This is another chance to perform some shell command execution because the
>> $cgiparams{'KEY’} input is potentially unchecked.
>> Can we change the validate the key before? It should be a number. In the if
>> statement above, we are only checking if it is actually set.
>
> I will look at doing that. However it might have to wait till I am back from
> visiting my family. I might be able to do it while visiting my son but can't
> be sure I will get the time.
No rush, this is not that urgent, but of course we should not forget about it…
-Michael
>
> Will put it on my list.
>
> Regards,
>
> Adolf.
>
>>> + unlink
>>> ("${General::swroot}/certs/$confighash{$cgiparams{'KEY'}}[1]cert.pem");
>>> + unlink
>>> ("${General::swroot}/certs/$confighash{$cgiparams{'KEY'}}[1].p12");
>>> + delete $confighash{$cgiparams{'KEY'}};
>>> +
>>> &General::writehasharray("${General::swroot}/vpn/config", \%confighash);
>>> + &writeipsecfiles();
>>> + &General::system('/usr/local/bin/ipsecctrl', 'D',
>>> $cgiparams{'KEY'}) if (&vpnenabled);
>>> + } else {
>>> + goto VPNCONF_ERROR;
>>> + }
>>> + } else {
>>> + $errormessage = $Lang::tr{'invalid key'};
>>> + }
>>> + &General::firewall_reload();
>>> ###
>>> ### Choose between adding a host-net or net-net connection
>>> ###
>>> --
>>> 2.49.0
