Verify permissions when adding a new source file or folder

[rugk]

Describe the bug A clear and concise description of what the bug is.

To Reproduce Steps to reproduce the behavior:

1. Create a new repo with local path.
2. Go to "Sources" and add a directory. Now choose a path you do not have access with the current user, like a different user home dir (/home/someonelese). You can select that by pressing enter.

What happens: The whole UI freezes, I cannot click anymore. Note that the path also is not added in the UI. What should happen: Show an error message.

Environment (please complete the following information):

• OS: Fedora 33
• Vorta version: v0.7.5
• Installed from: Flathub

Additional context

2021-03-06 14:23:09,875 - vorta.i18n - DEBUG - Loading translation succeeded for ['de', 'de-DE', 'de-Latn-DE'].
2021-03-06 14:23:09,893 - apscheduler.scheduler - INFO - Scheduler started
2021-03-06 14:23:12,606 - root - INFO - Using NetworkManagerMonitor NetworkStatusMonitor implementation.
2021-03-06 14:23:12,722 - vorta.borg.borg_thread - INFO - Running command /app/bin/borg --version
2021-03-06 14:23:47,689 - asyncio - DEBUG - Using selector: EpollSelector
2021-03-06 14:24:37,910 - asyncio - DEBUG - Using selector: EpollSelector
2021-03-06 14:24:37,913 - asyncio - DEBUG - Using selector: EpollSelector
2021-03-06 14:24:37,914 - root - DEBUG - Found 0 passwords matching repo URL.
2021-03-06 14:25:51,776 - asyncio - DEBUG - Using selector: EpollSelector
2021-03-06 14:25:51,777 - vorta.borg.borg_thread - DEBUG - Using VortaSecretStorageKeyring keyring to store passwords.
2021-03-06 14:25:51,780 - asyncio - DEBUG - Using selector: EpollSelector
2021-03-06 14:25:51,782 - root - DEBUG - Found 0 passwords matching repo URL.
[…]
2021-03-06 14:25:53,395 - vorta.borg.borg_thread - INFO - Done.
2021-03-06 14:25:53,652 - vorta.borg.borg_thread - WARNING - 
By default repositories initialized with this version will produce security
errors if written to with an older version (up to and including Borg 1.0.8).

If you want to use these older versions, you can disable the check by running:
borg upgrade --disable-tam *************************

See https://borgbackup.readthedocs.io/en/stable/changes.html#pre-1-0-9-manifest-spoofing-vulnerability for details about the security implications.
2021-03-06 14:25:53,655 - vorta.borg.borg_thread - WARNING - 
IMPORTANT: you will need both KEY AND PASSPHRASE to access this repo!
Use "borg key export" to export the key, optionally in printable format.
Write down the passphrase. Store both at safe place(s).

2021-03-06 14:25:53,883 - asyncio - DEBUG - Using selector: EpollSelector
2021-03-06 14:25:54,234 - asyncio - DEBUG - Using selector: EpollSelector
[…]
2021-03-06 14:25:54,245 - asyncio - DEBUG - Using selector: EpollSelector
2021-03-06 14:25:54,252 - root - DEBUG - Found 1 passwords matching repo URL.
2021-03-06 14:25:54,351 - vorta.borg.borg_thread - INFO - Running command /app/bin/borg list --info --log-json --json *************************

$ flatpak info com.borgbase.Vorta 

Vorta - Backup client

         Kennung: com.borgbase.Vorta
             Ref: app/com.borgbase.Vorta/x86_64/stable
     Architektur: x86_64
           Zweig: stable
         Version: v0.7.5
         License: GPL-3.0
        Ursprung: flathub
        Sammlung: org.flathub.Stable
    Installation: system
     Installiert: 53,7 MB
Laufzeitumgebung: org.kde.Platform/x86_64/5.15
             Sdk: org.kde.Sdk/x86_64/5.15

          Commit: 42c0ff004ec431c3d454072e4311f79fa65e3ba09abc98d07bc1bcca38740…
          Parent: 7bd3aa88cf115575981e8a1e6c947d16f5987b6f8720b6c57470bb1901efa…
         Subject: Update v0.7.5 (b324ae38)
            Date: 2021-03-03 17:42:57 +0000

Nothing logged to the Stdout:

flatpak run com.borgbase.Vorta                         
QSocketNotifier: Can only be used with threads started with QThread
2021-03-06 14:40:09,791 - vorta.i18n - DEBUG - Loading translation succeeded for ['de', 'de-DE', 'de-Latn-DE'].
2021-03-06 14:40:09,807 - apscheduler.scheduler - INFO - Scheduler started
QObject::connect: No such signal QPlatformNativeInterface::systemTrayWindowChanged(QScreen*)
2021-03-06 14:40:10,003 - root - INFO - Using NetworkManagerMonitor NetworkStatusMonitor implementation.
qt.qpa.wayland: Wayland does not support QWindow::requestActivate()
2021-03-06 14:40:10,095 - vorta.borg.borg_thread - INFO - Running command /app/bin/borg --version

[m3nu]

Thanks for reporting! Does this also happen, when you disable "Get statistic of file/folder when added" from the Misc tab? Apart from that we don't do anything with the folder you add there. So the issue is probably there.

[rugk]

No it does not happen then. However, it also:

• does not show any error
• does not add the path to the source menu

Again, this is not what I'd expect. IMHO it should check that the path is writable/can be used and show an error hen, if it is not.

Oh forget it, it does happen anyway! (I did not notice, because you don't notice it unless you click somewhere :sweat_smile:) So it freezes and you cannot do anything. It could also be that the path is not accessible from the flatpak, i.e. /home/….

[sten0]

I took a look at reproducing this using the Debian package, to test to see if identical behaviour occurred without Flatpak. "Get statistic of file/folder when added" is enabled.

1. As an unprivileged user, add a source (/root) to an existing repo, click "ok" -> error dialogue with text "Could not enter folder /root".
2. But then it gets weird. If I repeat (1) then there's no error! "Size" is of course "0", as is "File Count". "Recalculate sizes" does not cause a freeze/crash.
3. I created a new profile and went back to repeat (1). After clicking on "/root" I saw the "Could not enter folder /root" error. Click "OK" or pushing "Enter" again added it without an error on the second try.

It looks to me like there is a bug in Vorta's permission checks done when adding sources. I speculate that this may result in a hang when using Flatpak, most likely due to how permissions are blocked through container namespaces and possibly also how apparmor and SELinux (which Fedora uses by default seems to have stronger Mandatory Access Controls than apparmor) extend this to block even more. P.S. IMHO this is a good thing, since Linux namespace based containerisation is weak compared to Solaris Zones or FreeBSD Jails ;-)

I'm curious if solving the simple case (permission check, and refuse to add source if it's inaccessible, thus eliminating the incorrect behaviour at (2 and 3)) will prevent the branch leading to the more complicated Flatpak case from being taken, thus preventing this bug (UI freeze) from being triggered.

@m3nu, in other news: now that 0.7.5 has finally migrated to Debian testing, we can make a targeted bug fix upload (I can cherry pick from master and ship patches on top of 0.7.5) :-) I'd like to cherry pick the future fix for this issue, plus 064ba7c, plus the future fix to another permissions issue reported in Debian; I hope to forward this report to you later today. The adoption rate stats for Vorta (in Debian) are unlike any I've ever seen. It looks like it will be popular :-)

[m3nu]

I dont recall any permission checks that are done when adding a folder. 🙈

But we should clearly have some, if it causes issues. os.access or os.stat should do the job.

Also great news on the Debian package update! Your work on it already lead to many improvements in the project itself. So non-Debian users profit too.

[sten0]

Hi Manu!

Manu @.***> writes:

I dont recall any permission checks that are done when adding a folder. 🙈

Yikes! Oops :-p

But we should clearly have some, if it causes issues. os.access or os.stat should do the job.

Yes, thank you, I'm happy you agree. I've upgraded the priority of the corresponding Debian bug (https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=985424) to "important", with rationale--imho the rationale is worth reading.

Could we see this check added soon, before HEAD diverges too much from 0.7.5?

Also great news on the Debian package update! Your work on it already lead to many improvements in the project itself. So non-Debian users profit too.

Thanks :-) Also, that's great to hear, truly the ideal, and one of the reasons working on Vorta for Debian has been some of the most inspiring of last six months!

Kind regards, Nicholas

[m3nu]

I've addressed those two issues in #951.

I agree with the arguments in the Debian bug on the possibility of producing incomplete backups. The reason why we don't fail the whole backup when Borg reports access errors is that this is very common on macOS. The user will get a popup to grant permissions to the Documents or Downloads folder. And there are some folders that are always restricted, like calendar, contacts, etc. So it wouldn't be wise to fully fail in such a case. It's still good to add a notice. So I changed the return message for backups with inaccessible files to point this out.

[m3nu]

(I'm keeping all the changes minimal for now to make them easy to pick.)

[sten0]

Manu @.***> writes:

I've addressed those two issues in #951.

I agree with the arguments in the Debian bug on the possibility of producing incomplete backups. The reason why we don't fail the whole backup when Borg reports access errors is that this is very common on macOS. The user will get a popup to grant permissions to the Documents or Downloads folder. And there are some folders that are always restricted, like calendar, contacts, etc. So it wouldn't be wise to fully fail in such a case. It's still good to add a notice. So I changed the return message for backups with inaccessible files to point this out.

Ohh, I see. I hadn't realised that these weren't accessible. Does this mean that only Time Machine can produce a complete backup of a user's data on a macOS system?

Yes, this is the sort of popup and behaviour I had in mind :-) See my review on your PR two issues with the proposed solution. I hadn't considered that this was normal and expected on a macOS system, so users might also appreciate an "ignore inaccessible paths" toggle under the Miscellaneous configuration tab. As ever, if I made any mistakes due to ignorance, please point them out so I can learn.

[sten0]

Manu @.***> writes:

(I'm keeping all the changes minimal for now to make them easy to pick.)

Thank you so much, this accommodation is truly appreciated. I also think it will increase the chances of finally seeing Vorta in the "Top Ten Linux Backup Solutions" lists in 2021 ;-)

[m3nu]

Ohh, I see. I hadn't realised that these weren't accessible. Does this mean that only Time Machine can produce a complete backup of a user's data on a macOS system?

On macOS, the user will either get a few prompts and then Vorta can access most folders, like Documents. Some Apple-internal settings folders are still not accessible.

To get a full backup without access errors, Full Disk Access needs to be granted. Maybe we should check for this on startup, in case the user doesn't change the setting himself. Would be just os.access('~/Library/Cookies', os.R_OK) (since cookies are usually restricted)

[m3nu]

Addressed the accessibility issue on macOS in a separate PR #952. This will reduce the possibility of incomplete backups. Even if it's mostly Apple auxiliary files.

[m3nu]

Fixed this via #951. Vorta will now refuse to add folders without read permissions and give a warning, if Borg has an unclean exit code.