Re: [RFC] [PATCH] Add a "nolinks" mount option.

[Mattias Nissler]

On Tue, Oct 18, 2016 at 5:14 PM, Colin Walters <walters@xxxxxxxxxx> wrote:
>
> On Mon, Oct 17, 2016, at 09:02 AM, Mattias Nissler wrote:
> > OK, no more feedback thus far. Is there generally any interest in a
> > mount option to avoid path name aliasing resulting in target file
> > confusion? Perhaps a version that only disables symlinks instead of
> > also hard-disabling files hard-linked to multiple locations (those are
> > much lower risk for the situation I care about)?
>
> So the situation here is a (privileged) process that is trying to read/write
> to a filesystem tree writable by other processes that are in a separate
> security domain?

More or less. The scenario is that of an attacker gaining root access,
followed by a reboot. The dm-verity-protected root FS raises the bar
for re-exploiting the system after reboot, but the fact that we'll
consume data from the writable file system during boot can (and has
been) used to regain code execution.

You may argue that "all is lost" after the initial root exploit, but
we think there's value in hardening the system to the point where
getting a *persistent* exploit (i.e. retaining control across reboots)
is significantly harder. This increases our chances to successfully
auto-update devices even after a root exploit and thus retroactively
fix them.

>
> That's a classic situation that requires extreme care, and I am doubtful
> that symlinks are the only issue you're facing.  For example, if this
> process is also *parsing* any data there, there's another whole source
> of risk.

Agreed. Essentially, the entire writable file system has to be
regarded as untrusted input. Every process that stores or reads data
needs to do so cautiously. I'm aware this is quite a rabbit hole and
difficult to harden. In particular it'll require attention and changes
across a number of areas. The fact that we've seen multiple exploits
that rely on target file confusion by creating symlinks suggests that
there's value in blocking this as an attack vector though (in
particular given that we have only few legit uses of symlinks that are
relatively easy to clean up).

>
> I suspect for you it wouldn't be too hard to have a "follow untrusted
> path" helper function, it's possible to implement in userspace safely
> with O_NOFOLLOW etc.

Note that O_NOFOLLOW only affects the final path component. If there's
a symlink in any of the parent directories, that'll still be traversed
even with O_NOFOLLOW. This situation is less risky as an attacker will
have to deal with the restriction of a fixed filename in the last
component, but might still be exploitable.

>
> Regardless too, it sounds like what you want more is a
> "same filesystem" traversal (stat and compare devices).
>
> Or does it even need to handle full traversal?  Would it have mitigated
> the security issue to fstat() any files you opened and verified they
> were from the writable partition you expected?

These are all good ideas, and in fact I'm already looking into making
safe helpers to use when dealing with files from the writable file
system. There is an idea to even go a step further and mount the
writable file system in a separate mount namespace (to avoid
accidental access from the rest of the system). Then, the helper tools
would take care of performing file access in said mount namespace and
can prevent malicious symlinks by canonicalizing the requested path
and bailing out if it doesn't match the requested path.

The difficulty lies in applying these measures of precaution
system-wide. This affects most init scripts and daemons, and
everything else that keeps state on the writable file system. Some of
the affected code we have written ourselves so is relatively easy to
fix, but we're also running a number of third party packages for which
things are more complicated. Going through with a fine comb and
auditing all file access is possible in theory, but hardly in
practice. So we'll have to make do with partial auditing and
implementing mitigations that reduce risk for the rest of the code,
which led to the idea of killing the symlink attack vector
systematically.

FWIW, I plan to send an updated patch later that only disables
symlinks for consideration.