From mboxrd@z Thu Jan  1 00:00:00 1970
From: Michael Tremer <michael.tremer@ipfire.org>
To: development@lists.ipfire.org
Subject: Re: [RFC PATCH] backup: Set owner of /var/ipfire/backup/{in, ex}clude
 to "root"
Date: Sun, 18 Sep 2022 11:17:15 +0200
Message-ID: <128FB790-14D0-47E0-9E14-B00E9F2FF166@ipfire.org>
In-Reply-To: <5563a19f-545d-c6c3-2634-c49e426276c1@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============3044361758843798859=="
List-Id: <development.lists.ipfire.org>

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

Hello,

> On 17 Sep 2022, at 12:17, Peter M=C3=BCller <peter.mueller(a)ipfire.org> wr=
ote:
>=20
> Hello Michael,
>=20
> thanks for your reply. Indeed, glad you caught that.
>=20
> Before I submit a second version: Shouldn't the {in,ex}clude.user files als=
o be owned
> by root? I was unable to find any instance in the source code where these a=
re modified
> by an unprivileged user.

Yes.

> On that note, is it intended/desired that many subfolders of /var/ipfire/ a=
re owned
> by "nobody"? While I of course see the need for "nobody" to write _files_, =
do not quite
> get why the parent folders (such as /var/ipfire/auth/, /var/ipfire/ca/, etc=
. pp.) have
> to be owned by that user as well.

Sometimes, programs create temporary files which is why those directories nee=
d to have those ownerships.

It would be much more preferable to create any temporary files in /tmp and th=
en move then, but the code is written that way, and I would prefer to not tou=
ch it any more.

-Michael

>=20
> Thanks, and best regards,
> Peter M=C3=BCller
>=20
>=20
>> Hello Peter,
>>=20
>> I agree that the files should be owned by root. However, your patch doesn=
=E2=80=99t fix that.
>>=20
>>> On 15 Sep 2022, at 21:15, Peter M=C3=BCller <peter.mueller(a)ipfire.org> =
wrote:
>>>=20
>>> Since these files are static, there is no legitimate reason why they
>>> should be owned (hence writable) by "nobody".
>>>=20
>>> Signed-off-by: Peter M=C3=BCller <peter.mueller(a)ipfire.org>
>>> ---
>>> lfs/backup | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>=20
>>> diff --git a/lfs/backup b/lfs/backup
>>> index 6f686bf22..adbf16e65 100644
>>> --- a/lfs/backup
>>> +++ b/lfs/backup
>>> @@ -1,7 +1,7 @@
>>> #########################################################################=
######
>>> #                                                                        =
     #
>>> # IPFire.org - A linux based firewall                                    =
     #
>>> -# Copyright (C) 2007-2021  IPFire Team  <info(a)ipfire.org>             =
        #
>>> +# Copyright (C) 2007-2022  IPFire Team  <info(a)ipfire.org>             =
        #
>>> #                                                                        =
     #
>>> # This program is free software: you can redistribute it and/or modify   =
     #
>>> # it under the terms of the GNU General Public License as published by   =
     #
>>> @@ -61,8 +61,8 @@ $(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects))
>>> 	@$(PREBUILD)
>>> 	-mkdir -p /var/ipfire/backup/bin
>>> 	install -v -m 755 -o root $(DIR_SRC)/config/backup/backup.pl /var/ipfire=
/backup/bin
>>> -	install -v -m 644 $(DIR_SRC)/config/backup/include /var/ipfire/backup/
>>> -	install -v -m 644 $(DIR_SRC)/config/backup/exclude /var/ipfire/backup/
>>> +	install -v -m 644 -o root $(DIR_SRC)/config/backup/include /var/ipfire/=
backup/
>>> +	install -v -m 644 -o root $(DIR_SRC)/config/backup/exclude /var/ipfire/=
backup/
>>=20
>> They have been created as root before. That is the default.
>>=20
>>> 	chown nobody:nobody -R /var/ipfire/backup/
>>=20
>> And here is where they will be changed. Still.
>>=20
>>> 	chown root:root -R /var/ipfire/backup/bin/
>>> 	-mkdir -p /var/ipfire/backup/addons
>>> --=20
>>> 2.35.3
>>=20
>> -Michael


--===============3044361758843798859==--