Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

borg mount default permissions #3628

Closed
anarcat opened this issue Feb 21, 2018 · 10 comments
Closed

borg mount default permissions #3628

anarcat opened this issue Feb 21, 2018 · 10 comments
Milestone

Comments

@anarcat
Copy link
Contributor

anarcat commented Feb 21, 2018

When mounting a repository (and probably an archive) with borg mount, careless sysadmins (like me, I guess) are likely to run the following command:

borg mount /media/disk/borg/ /mnt

... which will expose backup archives on /mnt. The problem is the /mnt permissions are pretty wide open:

drwxr-xr-x 1 root root 0 Jan 19 11:17 /mnt

This means other users on the system can immediately start looking in the archive, bypassing possible permission protections originally installed on the original archive (e.g. in my case /media/disk/borg, which is accessible only by the root user). It is impossible to correct that situation after the filesystem is mounted:

# chmod og-rxw /mnt
chmod: changing permissions of '/mnt': Read-only file system

Obviously, the filesystem mount permissions forbid such changes. Now it could be argued that the permissions inside the archive should be sufficient to protect its content. I would counter that this is highly environment-specific: UID 1000 on one system may be a very different person from UID 1000 on another, yet with the above approach, they might be able to read each other's contents, which is the core of the issue I see here.

Another argument is that backups shouldn't be stored on a shared system. I would counter that this could easily be overlooked by administrators. On smaller systems, it is common to share multiple
resources like this, or at least it is something I do here. On larger systems, designs may involve multiple machines checking into a central backup server which would isolate those machines based on unix user IDs. A recovery process from a careless admin might lead backups from
machine A to be exposed through such a mountpoint, which could, eventually, allow machine B to inspect the content of machine A through this vulnerability. While SSH authorized_keys restrictions should protect against such an attack, there could be vulnerabilities at that layer (or in borg itself) that could allow the user to bypass that security restriction.

It would seem better to assume defense in depth here and ensure better default permissions for the mountpoint, as we currently do for backups themselves.

A workaround for this issue is to mount the archive in a subdirectory that is protected. For example, on my system here, /root is only accessible by the root user, and mounting the repository on /root/mnt defeats the attack.

This problem is mitigated by the fact that, without allow_root or allow_other, FUSE mountpoints are unreadable to other users. But users may inadvertently set that as a default for other (valid) reasons and not expect borg to follow the same path. Furthermore, borg mount currently has the following option as well:

 --umask M             set umask to M (local and remote, default: 0077)

..which may confuse the user in believing this will apply to the contents of the mountpoint (it doesn't). So maybe this should affect the mountpoint's permissions, or there should be a way to do so.

This is reported publicly after going through a private disclosure process with the author, who does not consider this to be a security issue within borg.

@ThomasWaldmann
Copy link
Member

As @anarcat just re-posted his very first email here, without considering any feedback from the private discussion we had, I'll also just add my reponse "as is" below.

@ThomasWaldmann
Copy link
Member

Hi Anarcat,

> When mounting a repository (and probably an archive) with borg mount,
> careless sysadmins (like me, I guess) are likely to run the following
> command:
>
>     borg mount /media/disk/borg/ /mnt
>
> ... which will expose backup archives on /mnt. The problem is the /mnt
> permissions are pretty wide open:
>
>     drwxr-xr-x 1 root root 0 Jan 19 11:17 /mnt

Well, isn't that always an issue, when you mount some stuff somewhere?

This does not sound like a borg issue, you can also have this with a usb
disk or any other filesystem.

For FUSE, there are allow_root and allow_other options, so FUSE stuff
might be even a bit better protected than other stuff, depending on your
fuse configuration. Maybe actually try these before assuming that there
is a security issue.

BTW, the usual issue seen with allow_root and allow_other configuration
is rather that people can NOT access stuff which they thought they can
access, which is a bit embarassing when you think along the usual "root
can do everything" lines.

>     # chmod og-rxw /mnt
>     chmod: changing permissions of '/mnt': Read-only file system
>
> Obviously, the filesystem permissions forbid such changes.

It's not the permissions, it is just a read-only fs.

> I haven't
> tested this, but I suspect changing the mountpoint permissions before
> mounting the filesystem will have no effect either, as the directory
> should be shadowed by the fuse filesystem.

I would also assume this.

> Now it could be argued that the permissions inside the archive should be
> sufficient to protect its content. I would counter that this is highly
> environment-specific: UID 1000 on one system may be a very different
> person from UID 1000 on another,

Sure.

> Another argument is that backups shouldn't be stored on a shared
> system.

No, you just have to be more careful when there actually are people
looking in contrary to when there are none anyway.

> It would seem better to assume defense in depth here and ensure better
> default permissions for the mountpoint, as we currentlydo for backups
> themselves.

Well, you are assuming a specific intention of the admin here and you
would like to enforce that. Not sure if that is a good idea, your
assumption could be just wrong sometimes and then you enforce something
unwanted and need to add workarounds for that...

It sounds a bit like trying to solve a not-in-borg problem in borg.

> A workaround for this issue is to mount the archive in a subdirectory
> that is protected.

I'ld even say that is not a workaround but THE usual approach.

Cheers, Thomas

@ThomasWaldmann
Copy link
Member

> Actually, now that I think about it, it's true that a modified
> /etc/fuse.conf *could* cause significant problems here

No news here: if you configure stuff with less security, it gets less
secure. 

> and it would be
> useful to have a way for users to ensure reliable and safe permissions
> without having to create subdirectories.
>
> It seems to me the `borg mount --umask` option should do
> something.

borg just does a os.umask() call early, either with the --umask value or
its default (both client and server side). So fs calls done by borg,
creating new files and dirs will be affected by it. Has nothing to do
with the "virtual" fs constructed for borg mount.

> yet I do not believe it applies to the mountpoint at all.

Yes, because it is for something else.

> Why not pass
> that down into the fuse subsystem so users have at least some control
> over this?

The would be rather a new mount option, discuss it as a feature request
on the issue tracker.

> I would file this as an issue if you don't mind, provided you do not see
> this as a security issue requiring a more elaborate disclosure process.

I currently do not see a security issue in borg here.

@anarcat
Copy link
Contributor Author

anarcat commented Feb 23, 2018

As @anarcat just re-posted his very first email here, without considering any feedback from the private discussion we had, I'll also just add my reponse "as is" below.

You obviously didn't reread the message I posted here. I have tried to edit the message to follow and summarize the conversation. Some of the quotes you reply to, in your comments, are from a private conversation I did not actually include here.

But whatever, I wasted enough time on this, again. I did consider feedback you sent, otherwise this would have a CVE number.

@ThomasWaldmann
Copy link
Member

I did. It somehow read the same... (from memory, didn't actually compare it byte-by-byte to original, though).

Also, I didn't feel like it considered my replies well enough, that's why I added them.

@bwurst
Copy link

bwurst commented Apr 11, 2018

Thank you @anarcat for pointing this situation out. So there must be three things coming together for this to be a security issue: 1. mounting on a public accessible location (nobody mounts to /mnt directly!), 2. mounting on a different system with mismatching UIDs and 3. using "-o allow_other" accidentially.

I came to this the other way round. I was looking for a solution to give a specific user access to his backups. In borg mount, there is the path specification and there is --strip-components but I forgot about allow_others so the user could not access the backups.

So perhaps both issues could be addressed by extending the documentation for the possible mount options for borg mount and making a statement about -o allow_other. Currently, there is only a reference to "man fuse".
Is there a public repo for the borg online docs that I can clone and make a pull request?

@anarcat
Copy link
Contributor Author

anarcat commented Apr 11, 2018

@bwurst the docs are in this very repo here, see https://github.com/borgbackup/borg/tree/master/docs

@bwurst
Copy link

bwurst commented Apr 13, 2018

All files I could find haver a mark that they are autocreated from another source. :)
I could track it down to this file for the source of the help text: /src/borg/archiver.py

@ThomasWaldmann do you think mentioning allow_other would be a good thing? Is the file I fould the right place to edit?

@ThomasWaldmann
Copy link
Member

ThomasWaldmann commented Apr 13, 2018

"For mount options, see the fuse(8) manual page."

We already have that. Could be extended to "... and configuration, see ...".

I don't want to duplicate fuse documentation, but we could maybe mention the names of some interesting configuration items.

@ThomasWaldmann ThomasWaldmann added this to the 1.1.9 milestone Feb 4, 2019
@ThomasWaldmann
Copy link
Member

I'll fix this with a docs update in #3903.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants