From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Tremer To: development@lists.ipfire.org Subject: Re: [PATCH] OpenVPN: Delete RRD dir if connection is deleted Date: Thu, 23 Apr 2020 21:03:00 +0100 Message-ID: <3C82876E-084B-49D2-8AB7-9CF0B9A44AAD@ipfire.org> In-Reply-To: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0475802339807020658==" List-Id: --===============0475802339807020658== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hi, So where are we on this issue? Is the patch ready to be accepted? How do we delete the files that should already have been deleted? -Michael > On 11 Apr 2020, at 13:52, ummeegge wrote: >=20 > Hi Michael, >=20 > Am Samstag, den 11.04.2020, 13:24 +0100 schrieb Michael Tremer: >> Hi, >>=20 >>> On 11 Apr 2020, at 12:59, ummeegge wrote: >>>=20 >>> Hi Michael, >>>=20 >>> Am Samstag, den 11.04.2020, 11:46 +0100 schrieb Michael Tremer: >>>> Hi, >>>>=20 >>>> This is a good find. >>>>=20 >>>> Did you have a connection that had a space in the common name? >>>> Potentially it is that. >>>=20 >>> No, the connections doesn=C2=B4t have spaces.=20 >>>=20 >>>>=20 >>>> Changing the code to use the common name should be trivial. Maybe >>>> just try printing the path it is trying to delete. Are the files >>>> maybe not accessible by =E2=80=9Cnobody=E2=80=9D? >>>=20 >>> They are pretty much all root:root . If i change the permissions to >>> nobody:nobdy i can delete all of them (by deleting X509) via a >>>=20 >>> @@ -1288,6 +1277,9 @@ >>> while ($file =3D glob("${General::swroot}/ovpn/n2nconf/*")) { >>> system ("rm -rf $file"); >>> } >>> + while ($file =3D glob("/var/log/rrd/collectd/localhost/openvpn- >>> *")) { >>> + system ("rm -rf $file"); >>> + } >>>=20 >>> which would spare this code --> >>>=20 > https://git.ipfire.org/?p=3Dipfire-2.x.git;a=3Dcommitdiff;h=3De1297cbb76596= 18c526fdc1ab07e97f57f55fd78 >>> . Haven=C2=B4t checked that yet for the deletion of only one >>> connection... >>=20 >> If they belong to root, the web UI won=E2=80=99t have permissions to delete >> them. > Have changed the permissions via chown -R and tried to delete then via > single connection but also via X509 deletion (deleting all) with no > luck. > Nevertheless, the RRD creation should chown then openvpn-* directories=20 > too which it currently do not. >=20 >>=20 >> That is something we will have to handle in openvpnctrl then. > Yes. >=20 >>=20 >>> Might it be possible that openvpnctrl handles there something >>> incorrect ? >>=20 >> Is there any code to handle it? And if so, why is the CGI calling >> =E2=80=9Crm=E2=80=9D? > It is held in the already existing coding style --> > https://git.ipfire.org/?p=3Dipfire-2.x.git;a=3Dblob;f=3Dhtml/cgi-bin/ovpnma= in.cgi;h=3De76a688fe7dcda0b77bf716eb2538342cd775b00;hb=3Drefs/heads/core142#l= 1231 > which should prevent the rmdir/unlink part for every connection i think. >=20 >=20 > Best, >=20 >=20 > Erik >=20 >=20 >>=20 >>=20 >> -Michael >>=20 >>>=20 >>> Best, >>>=20 >>>=20 >>> Erik >>>=20 >>>>=20 >>>> -Michael >>>>=20 >>>>> On 11 Apr 2020, at 09:06, ummeegge wrote: >>>>>=20 >>>>> Hi all, >>>>> this patch does only works if the common name is the same then >>>>> the >>>>> connection name. Have encountered that the rrd creation for >>>>> OpenVPN >>>>> uses the common name of the certificate not the connection name >>>>> --> >>>>>=20 >>>>> # root @ ipfire-server in /var/log/rrd/collectd/localhost >>>>> [8:34:50]=20 >>>>> $ ls >>>>> cpu-0 disk-loop0 iptables-filter- >>>>> PSCAN processes-charon processes-spamd >>>>> cpu-1 disk- >>>>> sda load processes- >>>>> java processes-squid >>>>> cpu- >>>>> 2 entropy memory proces >>>>> ses- >>>>> mpd processes-squidguard >>>>> cpu-3 interface openvpn- >>>>> rwonecert processes-nmbd processes-sshd >>>>> cpufreq iptables-filter-NEWNOTSYN openvpn- >>>>> rwtwocert processes-openvpn sensors-coretemp-isa-0000 >>>>> disk-dm-0 iptables-filter- >>>>> POLICYFWD ping processes-qemu sensors- >>>>> f71869-isa-0290 >>>>> disk-dm-1 iptables-filter- >>>>> POLICYIN processes processes-rtorrent swap >>>>> disk-dm-2 iptables-filter-POLICYOUT processes- >>>>> asterisk processes-smbd >>>>>=20 >>>>> $ cat /var/ipfire/ovpn/ovpnconfig=20 >>>>> 1,on,rwonename,rwonecert,host,cert,,,,,,,,,,,,,,,,,,,,,,,,,,,,d >>>>> ynam >>>>> ic >>>>> 2,on,rwtwoname,rwtwocert,host,cert,,,,,,,,,,,,,,,,,,,,,,,,,,,,d >>>>> ynam >>>>> ic,,,,,,,,,,, >>>>>=20 >>>>> strangely enough if i set the element index to [2] it doesn=C2=B4t >>>>> work. >>>>> Currently not sure why that=C2=B4s happen. >>>>>=20 >>>>> It is better to revert this patch. >>>>>=20 >>>>> Best, >>>>>=20 >>>>> Erik >>>>>=20 >>>>> Am Samstag, den 28.03.2020, 10:45 +0100 schrieb ummeegge: >>>>>> Hi Peter, >>>>>>=20 >>>>>> Am Samstag, den 28.03.2020, 09:25 +0000 schrieb Peter M=C3=BCller: >>>>>>> Reviewed-by: Peter M=C3=BCller >>>>>>>=20 >>>>>>> In my opinion, this fixes #11713. >>>>>>=20 >>>>>> Haven=C2=B4t seen that one, yes i think so. >>>>>> Have found another one in here -->=20 >>>>>>=20 >>>>>=20 >>>>>=20 >>>=20 >>>=20 > https://git.ipfire.org/?p=3Dipfire-2.x.git;a=3Dblob;f=3Dhtml/cgi-bin/ovpnma= in.cgi;h=3De76a688fe7dcda0b77bf716eb2538342cd775b00;hb=3DHEAD#l1224 >>>>>> which can not be solved in this way. Need to have another >>>>>> look >>>>>> into >>>>>> this. >>>>>> Will send a separate patch then for "delete all RRDs if X509 >>>>>> is >>>>>> deleted". >>>>>>=20 >>>>>> Need a little more time. >>>>>>=20 >>>>>> Best, >>>>>>=20 >>>>>> Erik >>>>>>=20 >>>>>>>=20 >>>>>>>> Signed-off-by: Erik Kapfer >>>>>>>> --- >>>>>>>> html/cgi-bin/ovpnmain.cgi | 2 +- >>>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>>>>=20 >>>>>>>> diff --git a/html/cgi-bin/ovpnmain.cgi b/html/cgi- >>>>>>>> bin/ovpnmain.cgi >>>>>>>> index ce9524df7..00ecd77a0 100644 >>>>>>>> --- a/html/cgi-bin/ovpnmain.cgi >>>>>>>> +++ b/html/cgi-bin/ovpnmain.cgi >>>>>>>> @@ -2513,7 +2513,7 @@ else >>>>>>>> # CCD end >>>>>>>> # Update collectd configuration and delete all >>>>>>>> RRD >>>>>>>> files of the removed connection >>>>>>>> &writecollectdconf(); >>>>>>>> - system ("/usr/local/bin/openvpnctrl -drrd >>>>>>>> $confighash{$cgiparams{'KEY'}}[1]"); >>>>>>>> + system ('/usr/local/bin/openvpnctrl', '-drrd', >>>>>>>> $confighash{$cgiparams{'KEY'}}[1]); >>>>>>>>=20 >>>>>>>> delete $confighash{$cgiparams{'KEY'}}; >>>>>>>> my $temp2 =3D `/usr/bin/openssl ca -gencrl -out >>>>>>>> ${General::swroot}/ovpn/crls/cacrl.pem -config >>>>>>>> ${General::swroot}/ovpn/openssl/ovpn.cnf`; --===============0475802339807020658==--