[v2] ipblocklist: Both "settings" and "modify" need to be writable for "nobody"

Message ID 06825b38-ec53-38c5-c8ce-12d70c1acb5b@ipfire.org
State Superseded
Headers
Series [v2] ipblocklist: Both "settings" and "modify" need to be writable for "nobody" |

Commit Message

Peter Müller Aug. 22, 2022, 8:11 p.m. UTC
  The second version of this patch avoids being generous with file
permissions, as Stefan pointed out that /var/ipfire/ipblocklist/sources
must not be writable to "nobody".

Therefore, the needed files ("settings" and "modify") are prepared
during the Core Upgrade and LFS file, and equipped with appropriate
permissions.

Fixes: #12917
Cc: Stefan Schantl <stefan.schantl@ipfire.org>
Signed-off-by: Peter Müller <peter.mueller@ipfire.org>
---
 config/rootfiles/core/170/update.sh | 4 ++++
 lfs/ipblocklist-sources             | 2 ++
 2 files changed, 6 insertions(+)
  

Comments

Michael Tremer Aug. 25, 2022, 3:49 p.m. UTC | #1
Hello,

I was told that this patch isn’t solving the problem it is supposed to solve.

However, I do not see why. Could someone explain to my little brain why?

-Michael

> On 22 Aug 2022, at 21:11, Peter Müller <peter.mueller@ipfire.org> wrote:
> 
> The second version of this patch avoids being generous with file
> permissions, as Stefan pointed out that /var/ipfire/ipblocklist/sources
> must not be writable to "nobody".
> 
> Therefore, the needed files ("settings" and "modify") are prepared
> during the Core Upgrade and LFS file, and equipped with appropriate
> permissions.
> 
> Fixes: #12917
> Cc: Stefan Schantl <stefan.schantl@ipfire.org>
> Signed-off-by: Peter Müller <peter.mueller@ipfire.org>
> ---
> config/rootfiles/core/170/update.sh | 4 ++++
> lfs/ipblocklist-sources             | 2 ++
> 2 files changed, 6 insertions(+)
> 
> diff --git a/config/rootfiles/core/170/update.sh b/config/rootfiles/core/170/update.sh
> index b6b66f3f1..9d16f4a32 100644
> --- a/config/rootfiles/core/170/update.sh
> +++ b/config/rootfiles/core/170/update.sh
> @@ -164,6 +164,10 @@ ldconfig
> mkdir -pv /var/lib/ipblocklist
> chown nobody:nobody /var/lib/ipblocklist
> 
> +# Create necessary files for IPBlocklist and set their ownership accordingly (#12917)
> +touch /var/ipfire/ipblocklist/{settings,modified}
> +chown nobody:nobody /var/ipfire/ipblocklist/{settings,modified}
> +
> # Rebuild fcrontab from scratch
> /usr/bin/fcrontab -z
> 
> diff --git a/lfs/ipblocklist-sources b/lfs/ipblocklist-sources
> index 30b9e94a4..d0ce30350 100644
> --- a/lfs/ipblocklist-sources
> +++ b/lfs/ipblocklist-sources
> @@ -49,5 +49,7 @@ $(TARGET) :
> 	@$(PREBUILD)
> 	mkdir -p /var/ipfire/ipblocklist
> 	install -v -m 0644 $(DIR_SRC)/config/ipblocklist/sources /var/ipfire/ipblocklist
> +	touch /var/ipfire/ipblocklist/{settings,modified}
> +	chown nobody:nobody /var/ipfire/ipblocklist/{settings,modified}
> 
> 	@$(POSTBUILD)
> -- 
> 2.35.3
  
Peter Müller Sept. 1, 2022, 8:34 p.m. UTC | #2
Hello Michael,

thanks for your reply and apologies for my belated response.

Stefan pointed out to me that if we would create these files in ipblocklist itself,
they would have became part of the component's rootfile (which was also not updated
in the patch). This would have caused user settings for ipblocklist to be overwritten,
if ipblocklist is updated in a future Core Update.

configroot is the better place, since we must never ship this, and this is where
all the other settings files are created already. Also, file permissions are already
taken care of there.

Version 3 should _finally_ solve the issue. Please let me know if it doesn't.

All the best,
Peter Müller

> Hello,
> 
> I was told that this patch isn’t solving the problem it is supposed to solve.
> 
> However, I do not see why. Could someone explain to my little brain why?
> 
> -Michael
> 
>> On 22 Aug 2022, at 21:11, Peter Müller <peter.mueller@ipfire.org> wrote:
>>
>> The second version of this patch avoids being generous with file
>> permissions, as Stefan pointed out that /var/ipfire/ipblocklist/sources
>> must not be writable to "nobody".
>>
>> Therefore, the needed files ("settings" and "modify") are prepared
>> during the Core Upgrade and LFS file, and equipped with appropriate
>> permissions.
>>
>> Fixes: #12917
>> Cc: Stefan Schantl <stefan.schantl@ipfire.org>
>> Signed-off-by: Peter Müller <peter.mueller@ipfire.org>
>> ---
>> config/rootfiles/core/170/update.sh | 4 ++++
>> lfs/ipblocklist-sources             | 2 ++
>> 2 files changed, 6 insertions(+)
>>
>> diff --git a/config/rootfiles/core/170/update.sh b/config/rootfiles/core/170/update.sh
>> index b6b66f3f1..9d16f4a32 100644
>> --- a/config/rootfiles/core/170/update.sh
>> +++ b/config/rootfiles/core/170/update.sh
>> @@ -164,6 +164,10 @@ ldconfig
>> mkdir -pv /var/lib/ipblocklist
>> chown nobody:nobody /var/lib/ipblocklist
>>
>> +# Create necessary files for IPBlocklist and set their ownership accordingly (#12917)
>> +touch /var/ipfire/ipblocklist/{settings,modified}
>> +chown nobody:nobody /var/ipfire/ipblocklist/{settings,modified}
>> +
>> # Rebuild fcrontab from scratch
>> /usr/bin/fcrontab -z
>>
>> diff --git a/lfs/ipblocklist-sources b/lfs/ipblocklist-sources
>> index 30b9e94a4..d0ce30350 100644
>> --- a/lfs/ipblocklist-sources
>> +++ b/lfs/ipblocklist-sources
>> @@ -49,5 +49,7 @@ $(TARGET) :
>> 	@$(PREBUILD)
>> 	mkdir -p /var/ipfire/ipblocklist
>> 	install -v -m 0644 $(DIR_SRC)/config/ipblocklist/sources /var/ipfire/ipblocklist
>> +	touch /var/ipfire/ipblocklist/{settings,modified}
>> +	chown nobody:nobody /var/ipfire/ipblocklist/{settings,modified}
>>
>> 	@$(POSTBUILD)
>> -- 
>> 2.35.3
>
  

Patch

diff --git a/config/rootfiles/core/170/update.sh b/config/rootfiles/core/170/update.sh
index b6b66f3f1..9d16f4a32 100644
--- a/config/rootfiles/core/170/update.sh
+++ b/config/rootfiles/core/170/update.sh
@@ -164,6 +164,10 @@  ldconfig
 mkdir -pv /var/lib/ipblocklist
 chown nobody:nobody /var/lib/ipblocklist
 
+# Create necessary files for IPBlocklist and set their ownership accordingly (#12917)
+touch /var/ipfire/ipblocklist/{settings,modified}
+chown nobody:nobody /var/ipfire/ipblocklist/{settings,modified}
+
 # Rebuild fcrontab from scratch
 /usr/bin/fcrontab -z
 
diff --git a/lfs/ipblocklist-sources b/lfs/ipblocklist-sources
index 30b9e94a4..d0ce30350 100644
--- a/lfs/ipblocklist-sources
+++ b/lfs/ipblocklist-sources
@@ -49,5 +49,7 @@  $(TARGET) :
 	@$(PREBUILD)
 	mkdir -p /var/ipfire/ipblocklist
 	install -v -m 0644 $(DIR_SRC)/config/ipblocklist/sources /var/ipfire/ipblocklist
+	touch /var/ipfire/ipblocklist/{settings,modified}
+	chown nobody:nobody /var/ipfire/ipblocklist/{settings,modified}
 
 	@$(POSTBUILD)