Re: [PATCH] OpenVPN: Delete RRD dir if connection is deleted

[ummeegge <ummeegge@ipfire.org>]

[-- Attachment #1: Type: text/plain, Size: 8712 bytes --]

Hi Michael,
have seen this a little late sorry.

Am Donnerstag, den 23.04.2020, 21:03 +0100 schrieb Michael Tremer:
> Hi,
> 
> So where are we on this issue?
Currently no step further. We have the following problems.
1) The OpenVPN RRD directories do have permissions for root:root so it
might be problematic to delete them via ovpnmain.cgi.
2) The permissions seems to be set via openvpn.so from collectd ? If
so, we would need to patch this again.
3) What i could figured out was that the system command in ovpnmain.cgi
which deletes the the OpenVPN RRDs should search for [3] not for [2] in
ovpnconfig.
3) openvpnctrl should be able to delete them since it operates with
root permissions ?
4) There seems to be a problem with the rrd section in openvpnctrl.

Am not in the c++ part of openvpnctrl so it might be great if you can
take a look into it.

> 
> Is the patch ready to be accepted?
It do not works currently.

> 
> How do we delete the files that should already have been deleted?
The command from bugzilla which i posted can may be extended/optimized
?

> 
> -Michael
> 

Best,

Erik

> > On 11 Apr 2020, at 13:52, ummeegge <ummeegge(a)ipfire.org> wrote:
> > 
> > Hi Michael,
> > 
> > Am Samstag, den 11.04.2020, 13:24 +0100 schrieb Michael Tremer:
> > > Hi,
> > > 
> > > > On 11 Apr 2020, at 12:59, ummeegge <ummeegge(a)ipfire.org> wrote:
> > > > 
> > > > Hi Michael,
> > > > 
> > > > Am Samstag, den 11.04.2020, 11:46 +0100 schrieb Michael Tremer:
> > > > > Hi,
> > > > > 
> > > > > This is a good find.
> > > > > 
> > > > > Did you have a connection that had a space in the common
> > > > > name?
> > > > > Potentially it is that.
> > > > 
> > > > No, the connections doesn´t have spaces. 
> > > > 
> > > > > 
> > > > > 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 “nobody”?
> > > > 
> > > > 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
> > > > 
> > > > @@ -1288,6 +1277,9 @@
> > > >    while ($file = glob("${General::swroot}/ovpn/n2nconf/*")) {
> > > > 	system ("rm -rf $file");
> > > >    }
> > > > +    while ($file =
> > > > glob("/var/log/rrd/collectd/localhost/openvpn-
> > > > *")) {
> > > > +    	system ("rm -rf $file");
> > > > +    }
> > > > 
> > > > which would spare this code -->
> > > > 
> > 
> > 
https://git.ipfire.org/?p=ipfire-2.x.git;a=commitdiff;h=e1297cbb7659618c526fdc1ab07e97f57f55fd78
> > > > . Haven´t checked that yet for the deletion of only one
> > > > connection...
> > > 
> > > If they belong to root, the web UI won’t 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 
> > too which it currently do not.
> > 
> > > 
> > > That is something we will have to handle in openvpnctrl then.
> > 
> > Yes.
> > 
> > > 
> > > > Might it be possible that openvpnctrl handles there something
> > > > incorrect ?
> > > 
> > > Is there any code to handle it? And if so, why is the CGI calling
> > > “rm”?
> > 
> > It is held in the already existing coding style -->
> > 
https://git.ipfire.org/?p=ipfire-2.x.git;a=blob;f=html/cgi-bin/ovpnmain.cgi;h=e76a688fe7dcda0b77bf716eb2538342cd775b00;hb=refs/heads/core142#l1231
> > which should prevent the rmdir/unlink part for every connection i
> > think.
> > 
> > 
> > Best,
> > 
> > 
> > Erik
> > 
> > 
> > > 
> > > 
> > > -Michael
> > > 
> > > > 
> > > > Best,
> > > > 
> > > > 
> > > > Erik
> > > > 
> > > > > 
> > > > > -Michael
> > > > > 
> > > > > > On 11 Apr 2020, at 09:06, ummeegge <ummeegge(a)ipfire.org>
> > > > > > wrote:
> > > > > > 
> > > > > > 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
> > > > > > -->
> > > > > > 
> > > > > > # root @ ipfire-server in /var/log/rrd/collectd/localhost
> > > > > > [8:34:50] 
> > > > > > $ 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                 pr
> > > > > > oces
> > > > > > 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
> > > > > > 
> > > > > > $ cat /var/ipfire/ovpn/ovpnconfig 
> > > > > > 1,on,rwonename,rwonecert,host,cert,,,,,,,,,,,,,,,,,,,,,,,,,
> > > > > > ,,,d
> > > > > > ynam
> > > > > > ic
> > > > > > 2,on,rwtwoname,rwtwocert,host,cert,,,,,,,,,,,,,,,,,,,,,,,,,
> > > > > > ,,,d
> > > > > > ynam
> > > > > > ic,,,,,,,,,,,
> > > > > > 
> > > > > > strangely enough if i set the element index to [2] it
> > > > > > doesn´t
> > > > > > work.
> > > > > > Currently not sure why that´s happen.
> > > > > > 
> > > > > > It is better to revert this patch.
> > > > > > 
> > > > > > Best,
> > > > > > 
> > > > > > Erik
> > > > > > 
> > > > > > Am Samstag, den 28.03.2020, 10:45 +0100 schrieb ummeegge:
> > > > > > > Hi Peter,
> > > > > > > 
> > > > > > > Am Samstag, den 28.03.2020, 09:25 +0000 schrieb Peter
> > > > > > > Müller:
> > > > > > > > Reviewed-by: Peter Müller <peter.mueller(a)ipfire.org>
> > > > > > > > 
> > > > > > > > In my opinion, this fixes #11713.
> > > > > > > 
> > > > > > > Haven´t seen that one, yes i think so.
> > > > > > > Have found another one in here --> 
> > > > > > > 
> > > > > > 
> > > > > > 
> > > > 
> > > > 
> > 
> > 
https://git.ipfire.org/?p=ipfire-2.x.git;a=blob;f=html/cgi-bin/ovpnmain.cgi;h=e76a688fe7dcda0b77bf716eb2538342cd775b00;hb=HEAD#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".
> > > > > > > 
> > > > > > > Need a little more time.
> > > > > > > 
> > > > > > > Best,
> > > > > > > 
> > > > > > > Erik
> > > > > > > 
> > > > > > > > 
> > > > > > > > > Signed-off-by: Erik Kapfer <ummeegge(a)ipfire.org>
> > > > > > > > > ---
> > > > > > > > > html/cgi-bin/ovpnmain.cgi | 2 +-
> > > > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > > > > 
> > > > > > > > > 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]);
> > > > > > > > > 
> > > > > > > > > 		delete $confighash{$cgiparams{'KEY'}};
> > > > > > > > > 		my $temp2 = `/usr/bin/openssl ca
> > > > > > > > > -gencrl -out
> > > > > > > > > ${General::swroot}/ovpn/crls/cacrl.pem -config
> > > > > > > > > ${General::swroot}/ovpn/openssl/ovpn.cnf`;
> 
>