Re: [RFC PATCH] backup: Set owner of /var/ipfire/backup/{in, ex}clude to "root"

[Michael Tremer <michael.tremer@ipfire.org>]

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

Hello,

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

Yes.

> On that note, is it intended/desired that many subfolders of /var/ipfire/ are 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 need to have those ownerships.

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

-Michael

> 
> Thanks, and best regards,
> Peter Müller
> 
> 
>> Hello Peter,
>> 
>> I agree that the files should be owned by root. However, your patch doesn’t fix that.
>> 
>>> On 15 Sep 2022, at 21:15, Peter Müller <peter.mueller(a)ipfire.org> wrote:
>>> 
>>> Since these files are static, there is no legitimate reason why they
>>> should be owned (hence writable) by "nobody".
>>> 
>>> Signed-off-by: Peter Müller <peter.mueller(a)ipfire.org>
>>> ---
>>> lfs/backup | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>> 
>>> 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/
>> 
>> They have been created as root before. That is the default.
>> 
>>> 	chown nobody:nobody -R /var/ipfire/backup/
>> 
>> And here is where they will be changed. Still.
>> 
>>> 	chown root:root -R /var/ipfire/backup/bin/
>>> 	-mkdir -p /var/ipfire/backup/addons
>>> -- 
>>> 2.35.3
>> 
>> -Michael