From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.haj.ipfire.org (localhost [127.0.0.1]) by mail02.haj.ipfire.org (Postfix) with ESMTP id 4ZSRBt3s12z333x for ; Wed, 2 Apr 2025 13:52:38 +0000 (UTC) Received: from mail01.ipfire.org (mail01.haj.ipfire.org [172.28.1.202]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) client-signature RSA-PSS (4096 bits)) (Client CN "mail01.haj.ipfire.org", Issuer "R10" (verified OK)) by mail02.haj.ipfire.org (Postfix) with ESMTPS id 4ZSRBp6vBSz2yRh for ; Wed, 2 Apr 2025 13:52:34 +0000 (UTC) Received: from [127.0.0.1] (localhost [127.0.0.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail01.ipfire.org (Postfix) with ESMTPSA id 4ZSRBp2VBvz6p; Wed, 2 Apr 2025 13:52:34 +0000 (UTC) DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=ipfire.org; s=202003ed25519; t=1743601954; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=5VXH6NAyKWQmY1+36fGbe0tIG0d7IxAQcxbmIafk2J8=; b=4G6IYq3SppbmYxLKZfruyDjb54UYLxSw8TrEiShswQ60cse+XTsu4nnZYg6pdjxdu4fNNt 8GpKewKn2utEJhBg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ipfire.org; s=202003rsa; t=1743601954; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=5VXH6NAyKWQmY1+36fGbe0tIG0d7IxAQcxbmIafk2J8=; b=c8NEux4Oc8dGYEMH/wyYxiPaCZM+VYhmxYmRmN6migkLdrxp8tXWHp+6ZyVnZHtj+tNWiF FdguQ5BNfTsw3k0w9H2oOdXxLdTD3isyiYjwwcaxQbf2oX3ycMUqNM5Dp8qQVEmvq8+fk4 HxdyLZtY5Q6SFndksJh5+1lSzdJZhM/D50/6q9PiQ7xBMoWbypmQBqwi1T8AdWaQYpkOuu 28hkDOBIAofohcQLUqMRWu2B1xqantnad0XdQI79Tm7I01Eo3yZ3Nk1WnfrRw5XZvrfkig 8gRQyIQEj3xj1b5yN7Bzu0Qg0qKuCqCXCFhd5tKjb0d8EoJgwsWOGSoJCsiXrw== Content-Type: text/plain; charset=utf-8 Precedence: list List-Id: List-Subscribe: , List-Unsubscribe: , List-Post: List-Help: Sender: Mail-Followup-To: Mime-Version: 1.0 Subject: Re: [PATCH 2/6] vpnmain.cgi: Fixes bug13737 - revoke any deleted client certificate From: Michael Tremer In-Reply-To: <12f44533-ba3b-4d91-83a5-c97200cccdd4@ipfire.org> Date: Wed, 2 Apr 2025 14:52:33 +0100 Cc: "IPFire: Development-List" Content-Transfer-Encoding: quoted-printable Message-Id: <70277736-47FC-4746-9C88-134F734817DA@ipfire.org> References: <20250401180802.19784-1-adolf.belka@ipfire.org> <20250401180802.19784-2-adolf.belka@ipfire.org> <1065A659-E2A7-4CB5-9F2C-3E8E40A0DE04@ipfire.org> <12f44533-ba3b-4d91-83a5-c97200cccdd4@ipfire.org> To: Adolf Belka Hello Adolf, > On 2 Apr 2025, at 11:41, Adolf Belka wrote: >=20 > Hi Michael, >=20 > On 02/04/2025 12:21, Michael Tremer wrote: >>> On 1 Apr 2025, at 19:07, Adolf Belka wrote: >>>=20 >>> - 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. >>>=20 >>> Fixes: bug13737 >>> Tested-by: Adolf Belka >>> Signed-off-by: Adolf Belka >>> --- >>> html/cgi-bin/vpnmain.cgi | 30 +++++++++++++++++++----------- >>> 1 file changed, 19 insertions(+), 11 deletions(-) >>>=20 >>> 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); >>>=20 >>> - 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 =3D $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 =3D " ca -revoke = ${General::swroot}/certs/$confighash{$cgiparams{'KEY'}}[1]cert.pem"; >>> + $errormessage =3D &callssl($opt); >> This is another chance to perform some shell command execution = because the $cgiparams{'KEY=E2=80=99} 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. >=20 > 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=E2=80=A6 -Michael >=20 > Will put it on my list. >=20 > Regards, >=20 > Adolf. >=20 >>> + 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 =3D $Lang::tr{'invalid key'}; >>> + } >>> + &General::firewall_reload(); >>> ### >>> ### Choose between adding a host-net or net-net connection >>> ### >>> --=20 >>> 2.49.0