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 4ZSLWJ6Cthz32wj for ; Wed, 2 Apr 2025 10:21:32 +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 4ZSLWF2NBjz2yHd for ; Wed, 2 Apr 2025 10:21:29 +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 4ZSLWD515mzNV; Wed, 2 Apr 2025 10:21:28 +0000 (UTC) DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=ipfire.org; s=202003ed25519; t=1743589288; 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=6ej1QGB58XvVNA6hteX/YgpEfzpFJwROUTx7NJtvYm0=; b=5zzTOU8Jug1NMLMMhNjWleLOvK9enbkrWGwgsLPwRqrKYar0jsCuvDxYkNS4I68J8yE91r C5gLrNmStue4BbAg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ipfire.org; s=202003rsa; t=1743589288; 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=6ej1QGB58XvVNA6hteX/YgpEfzpFJwROUTx7NJtvYm0=; b=Id3ehPyKxrb0lF/A5NynWQ1v39XvT7kPjTQi+kkF04gfv+2KUa7UT84hcfVOiAZ99Y0hRc 0eCVICMAnReOPutlDnkOBMicRwaq08zeFwWSYIukJoHCuSPrFeVqJbGz8Z2RPD4K5H4ZzS +wMpEjcqcIX2ZnnUpuDUJE5q/GBGjpudkVRBh3ZhHPOFU32uuBi3KB4sbPscogcIZSARXd wZxQVNCcEPAw6d8idQEEPbqgtSOBwUyHOdFu6Kx0YpFYSCU9LOWQX0my9p7mK8SR3TylkX vrpHGoJwftxhrc7k5SU8tHCTNKvenZh9mJZ+9DtSWm54iVm4Kckav1Ztv6ZxHA== 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: <20250401180802.19784-2-adolf.belka@ipfire.org> Date: Wed, 2 Apr 2025 11:21:28 +0100 Cc: development@lists.ipfire.org Content-Transfer-Encoding: quoted-printable Message-Id: <1065A659-E2A7-4CB5-9F2C-3E8E40A0DE04@ipfire.org> References: <20250401180802.19784-1-adolf.belka@ipfire.org> <20250401180802.19784-2-adolf.belka@ipfire.org> To: Adolf Belka > 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. > + 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 >=20 >=20