borgbase / vorta

Desktop Backup Client for Borg Backup
https://vorta.borgbase.com
GNU General Public License v3.0
1.93k stars 128 forks source link

UMASK variable has no effect #1378

Open homandr opened 1 year ago

homandr commented 1 year ago

The UMASK container variable doesn't have any effect on Vorta application. When restoring files and directories, they always have 600/700 permissions. This can be confirmed by connecting to the container shell, running ps to identify PID of the Vorta process, then checking its umask in /proc/(pid)/status. It's always set to 0077. While I'm all for secure defaults, it should be configurable.

Upon further investigation, I found that umask is hard coded in https://github.com/borgbase/vorta/blob/master/src/vorta/store/connection.py. Perhaps the code could be changed to check environment variable and set the umask accordingly and use 0077 if no variable is set. I'm not sure if Python inherits shell umask at all, so this check would need to be done in Vorta's code.

https://github.com/borgbase/vorta/blob/d8e4a93cdd527b692869da0969c0fbf43f08305d/src/vorta/store/connection.py#L30-L34

I'm not sure if this issue belongs here or in the Vorta repository. But since overriding umask is part of the vorta-docker functionality, I'm posting it here.

Ranbato commented 1 year ago

That will have to be fixed in Vorta itself so open a ticket there. Once you do that, I'll update the documentation with a link to the ticket so users are aware that it isn't working and why.

m3nu commented 1 year ago

Transferred this issue to the relevant repo.

Once again, what's the issue with having limited permissions on our DB file (which may container the repo password, if no system keychain is available)?

What mechanism would you want to change it?

homandr commented 1 year ago

Thank you for looking into this issue.

There is no issue with DB file permission. The issue is that Vorta main process runs with umask 0077 without any way to change it short of logging in to Vorta container and modifying Vorta's Python code above. Standard Linux applications should honor OS umask setting, and if they override it, at least they should make it a configurable setting.

I believe the intent of the code above was strictly for DB file permissions. If that is correct, then it overreaches by setting umask on the main process.

Because of existing borgbackup issue (https://github.com/borgbackup/borg/issues/1751), any file restored from Vorta creates the restored directory hierarchy with 700 permissions, making it inaccessible in many scenarios. Especially when Docker containers run as a limited user like "nobody".

Running chmod after the restore is the only fix right now, but it implies root shell access to the server where Vorta container runs and knowledge to do that. Vorta is a friendly GUI front-end to borgbackup, but this issue makes it a lot less friendly. If I set it up for non-technical users and allow VNC access only, they effectively won't be able to restore files.

The easiest way to address it would be to preserve the OS umask in a variable and restore it back to the original value after the DB file is initialized. This would produce the expected normal Linux application behavior.

Another way would be to read the OS environment variable UMASK and if it's set, use that, defaulting to 0077 otherwise. Although that is specific to Vorta Docker container where they use UMASK as one of the customization variables.

real-yfprojects commented 1 year ago

Shouldn't this be as easy as storing the old mask when overriding and restoring it after creating the DB file?

old_mask = os.umask(0o0077)  # control permissions of DB file
DB.initialize(con)
DB.connect()
os.umask(old_mask)  # restore umask