Make libpreloadpatchmanager.so and other /tmp/patchmanager* paths accessible from sailjailed apps

[b100dian]

As in https://forum.sailfishos.org/t/patchmanager-patches-in-koli-4-0-1/4925/114

I see a couple or more options here:

1. Patch /etc/firejail/whitelist-common.local automatically at install time - this need to take into accounts ports that already have entries in there (e.g. volla https://piggz.co.uk/sailfishos-porters-archive/index.php?log=2021-07-10.txt#line43 )
2. Add an 'option' to enable patchmanager for sailjailed apps
3. Just make a patch with this and not ship it with patchmanager

The latter seems harder, considering the contents (or absence/presence) of the whitelist-common.local file differs from one device to another.

[nephros]

That is going to be challenging to do reliably across apps.

Because other apps may need to modify it as well - while we might find a solution to do it reliably, others may not, or not be able to cooperate with ours.

Still my thinking would be to do something like

    ## @@BEGIN harbour-patchmanager @@
    whitelist foo
    whitelist bar
    ...
    ## @@END harbour-patchmanager @@

Now, how to modify?

• shipping a patch (I think that is out, that can not work reliably with other users of that file)
• sed script in RPM %post/%postun/%preun (not ideal, requires reinstall/update if something else changes the file)
• systemd oneshot service (to be triggered from QML, so this can be switched on or off)
• custom function in the main (C++) app, (to be triggered from QML, so this can be switched on or off)

All these need root access of course.

I think for really intelligent file mangling the last two are the only ones feasible. Systemd service need not be a binary, can also be done in any scripting language, similar to pm_apply/unapply

Also to consider:

We could ship a /etc/firejail/whitelist-common-patchmanager.local file and modify /etc/firejail/whitelist-common.inc to include it instead - but that is a system file that won't survive package updates.

[CODeRUS]

hi. isn't whitelist-common contains any of shared directories you can reusee? you can mount tmpfs dir to this location and use it instead of /tmp/patchmanager-root one

[nephros]

hi. isn't whitelist-common contains any of shared directories you can reusee? you can mount tmpfs dir to this location and use it instead of /tmp/patchmanager-root one

Good idea, but by default this file is empty, so no common dir is defined in there.

There is whitelist-var-common.inc however, which provides e.g. /var/run and /var/tmp which definitely would be suitable locations:

https://github.com/sailfishos-mirror/firejail/blob/a67bb37b0ddac080008cd5cf494aaaf8531f45c0/etc/inc/whitelist-var-common.inc

I don't know however if whitelist-var-common.inc is actually used by sailjailed applications.

[CODeRUS]

iirc /var/tmp should be mounted to tmpfs as well, should be enouth ot jus tchange locations

[b100dian]

Interesting idea, finding an already supported folder would save us a bunch of lines (only private-etc would be needed).

But I don't see a special /var mount (least /var/tmp) on my device, I'll dig further into the sailjail settings.

We could ship a /etc/firejail/whitelist-common-patchmanager.local file and modify /etc/firejail/whitelist-common.inc

Why not include that from whitelist-common.local?

[b100dian]

@nephros do you have /var/run on tmpfs? I missed that it was a link :trophy:

[nephros]

Yes, /var/run is linked to /run while /var/tmp is a regular directory. Which is why I chose to try the former, doesn't need cleanup after boot.

[nephros]

I missed that /run is mounted nodev though, that might make the socket file not work, not sure ATM.

[Olf0]

Maybe the second variant is not that bad, and it is simple: An sed script in the RPM spec file (%post and %postun / %preun) I do not fully agree with @nephros' comment "not ideal, requires reinstall/update if something else changes the file", because nothing should alter extant lines in that file (/etc/firejail/whitelist-common.local). We already have a pattern for that in the spec file. I.e., stubbornly remove the entries (lines) we are going to add (if they are there) and set them anew. Did I miss some aspect, why applying this pattern would not work for /etc/firejail/whitelist-common.local?

Edit: O.K., first thing I missed, is that uninstalling PM shall only remove entries a PM-installation made. This can be solved per section markers, as nephros suggested and commanding sed to remove everything between the start and end marker (i.e., two sed-"addresses", each of which can be defined by a Regex). If other apps do not play nicely (and remove entries created by PM), this is not PM's fault.

[b100dian]

Yes, the section would help to add/remove only patchmanager needs. There is also the fact that one of the lines may be already there - the private-etc ld.so.preload You don't want to remove that from a device that employs it.

I've tested today:

1. the /var/run commit by @nephros This does not seem to load the patched files in sailjailed apps. I didn't firejail --trace it, but what I did is I noticed that: a. adding to our local whitelist /var/run makes it work and b. adding instead whitelist /var/run/patchmanager etc does not. The errors I encountered are Error: invalid whitelist path /var/run/patchmanager and Error: invalid whitelist path /run (when trying the link target).

2. adding private-etc ld.so.preload twice Works

3. adding a comment at the end of the line, e.g. private-etc ld.so.preload # pm3 Doesn't work.

So we're left with the initial proposal of adding/removing a whole section IMO.

[nephros]

Thanks a lot for testing this!

Looking a bit more into it, the reason is that:

1. Sailjailed Apps use /etc/sailjail/permissions/Base.permission, and that only includes include whitelist-common.inc, not the var version. So we're back to using that.
2. However, it seems that one can actually use a include statement also in a .local file, so the following works:

   
   nemo@PGXperia10:/etc/firejail $ cat whitelist-common.local
   include whitelist-common-pm.local
   nemo@PGXperia10:/etc/firejail $ cat whitelist-common-pm.local
   ### patchmanager for sailjailed apps
   whitelist /tmp/patchmanager-socket
   whitelist /tmp/patchmanager
   whitelist /tmp/patchmanager3
   private-etc ld.so.preload

Which means we might be able to ship a `/etc/firejail/whitelist-common-pm.local` or similar file with all the stuff we need, and only need to patch one line in `whitelist-common.local`, which can be done the same way we do it for ld.so.preload in the rpm, or one of the other methods I proposed above.

Now, about `/var/run` - I would like to keep that change actually - primarily out of "taste", `/var` seems to be a better place. Files in `/tmp` can be deleted by any cleanuptools or users at any time.
Especially a socket file IMO should not be placed in `/tmp`.

Linux FHS on `/tmp`:
> Programs must not assume that any files or directories in /tmp are preserved between invocations of the program.

Linux FHS on `/run`:
> System programs that maintain transient UNIX-domain sockets must place them in this directory or an appropriate subdirectory as outlined above.

Linux FHX on `/var/run`:

> This directory was once intended for system information data describing the system since it was booted. These functions have been moved to /run; this directory exists to ensure compatibility with systems and software using an older version of this specification.
> In general, the requirements for /run shall also apply to /var/run. It is valid to implement /var/run as a symlink to /run.
> [...]
> Programs should not access both /var/run and /run directly, except to access /var/run/utmp.

So yeah, that boils down to a change from `/tmp` to `/run`.

I will update my local branch with these ideas so we can see how that works.

(rebased against master, sorry ;) )

[nephros]

Thanks a lot for testing this!

Looking a bit more into it, the reason is that:

1. Sailjailed Apps use /etc/sailjail/permissions/Base.permission, and that only includes include whitelist-common.inc, not the var version. So we're back to using that.
2. However, it seems that one can actually use a include statement also in a .local file, so the following works:

nemo@PGXperia10:/etc/firejail $ cat whitelist-common.local
include whitelist-common-pm.local
nemo@PGXperia10:/etc/firejail $ cat whitelist-common-pm.local
### patchmanager for sailjailed apps
whitelist /tmp/patchmanager-socket
whitelist /tmp/patchmanager
whitelist /tmp/patchmanager3
private-etc ld.so.preload

Which means we might be able to ship a /etc/firejail/whitelist-common-pm.local or similar file with all the stuff we need, and only need to patch one line in whitelist-common.local, which can be done the same way we do it for ld.so.preload in the rpm, or one of the other methods I proposed above.

BTW, I am basing this on looking around on my 4.0 device. I now have also a 4.2 Gemini to test on, and there it looks like a lot has changed, including the fact that Base.permission isn't used by most Sailjail permission configurations. Further research ongoing...

[nephros]

Now, about /var/run - I would like to keep that change actually - primarily out of "taste", /var seems to be a better place. Files in /tmp can be deleted by any cleanuptools or users at any time. Especially a socket file IMO should not be placed in /tmp.

Okay, implemented and tested, doesn't work, with much the same results as @b100dian gives above.

[nephros]

Right, so it turns out none of the path moving worked.

I have dropped the commits doing that and now https://github.com/nephros/patchmanager/tree/issue-55 contains only the new whitelist changes, which I submit a PR for (#58).

The history of my attempts are archived in https://github.com/nephros/patchmanager/tree/issue-55-test

[b100dian]

I think this is the explanation: you start with /var, but fail to be exactly /var/run -> you fail. https://github.com/netblue30/firejail/blob/release-0.9.64/src/firejail/fs_whitelist.c#L617

I think we are good with /tmp for the purpose of this bug. Thanks @nephros!

[b100dian]

Also, I tested the CI build on 4.2 aarch64. I even moved out my whitelist-common.local for a test. This is a clean solution, I like it

[b100dian]

Fixed with https://github.com/sailfishos-patches/patchmanager/pull/58