From mboxrd@z Thu Jan  1 00:00:00 1970
From: Michael Tremer <michael.tremer@ipfire.org>
To: development@lists.ipfire.org
Subject: Re: [PATCH] OpenVPN: Delete RRD dir if connection is deleted
Date: Sat, 11 Apr 2020 13:24:53 +0100
Message-ID: <05978D35-0266-4D83-B3AA-A0EBE070D097@ipfire.org>
In-Reply-To: <3c4bb2e7dfffee58fdc332303a1eefd1ed0fcc77.camel@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============1788782878506873942=="
List-Id: <development.lists.ipfire.org>

--===============1788782878506873942==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

Hi,

> On 11 Apr 2020, at 12:59, ummeegge <ummeegge(a)ipfire.org> 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.
> 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?
> 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 -->
> 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...

If they belong to root, the web UI won=E2=80=99t have permissions to delete t=
hem.

That is something we will have to handle in openvpnctrl then.

> 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 =E2=80=9Crm=
=E2=80=9D?


-Michael

>=20
> Best,
>=20
>=20
> Erik
>=20
>>=20
>> -Michael
>>=20
>>> On 11 Apr 2020, at 09:06, ummeegge <ummeegge(a)ipfire.org> 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                 processes-
>>> 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,,,,,,,,,,,,,,,,,,,,,,,,,,,,dynam
>>> ic
>>> 2,on,rwtwoname,rwtwocert,host,cert,,,,,,,,,,,,,,,,,,,,,,,,,,,,dynam
>>> 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 <peter.mueller(a)ipfire.org>
>>>>>=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
> 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 <ummeegge(a)ipfire.org>
>>>>>> ---
>>>>>> 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`;
>>>>>>=20
>>>>=20
>>>>=20
>>=20
>>=20
>=20


--===============1788782878506873942==--