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. 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
