realpath() and setfsuid programs

realpath() and setfsuid programs

[Timo Teras]

Hi,

It seems realpath() does not work in binaries using setfsuid(). (At
least on grsec kernels, vanilla kernel might be affected too.)

The problem is that realpath() opens the file, and then
uses just readlink on /proc/self/fd/<fd> to read the canonicalized
path.

However, /proc/self/fd is not accessible if setfsuid() has been used to
drop privileges.

The problem I'm looking at in this case is fuse. fusermount, the
suid wrapper to do user fuse mounts, seems to basically do:
 oldfsuid = setfsuid(getuid())
 oldfsgid = setfsgid(getgid())
 take realpath of mountpoint
 chdir("/")
 setfsuid(oldfsuid)
 setfsgid(oldfsgid)

I believe they want to drop privileges so it works as also access check
to the mount point directory. As realpath() in practice checks that the
user has access to the entry too.

This works glibc, as realpath() canonicalizes the path
component-by-component in userland. But musl breaks due to the /proc
not being accessible while privileges dropped.

Any suggestions?

Thanks,
Timo

Re: realpath() and setfsuid programs

[Rich Felker]

On Sat, Feb 07, 2015 at 09:53:54AM +0200, Timo Teras wrote:
> Hi,
> 
> It seems realpath() does not work in binaries using setfsuid(). (At
> least on grsec kernels, vanilla kernel might be affected too.)
> 
> The problem is that realpath() opens the file, and then
> uses just readlink on /proc/self/fd/<fd> to read the canonicalized
> path.
> 
> However, /proc/self/fd is not accessible if setfsuid() has been used to
> drop privileges.
> 
> The problem I'm looking at in this case is fuse. fusermount, the
> suid wrapper to do user fuse mounts, seems to basically do:
>  oldfsuid = setfsuid(getuid())
>  oldfsgid = setfsgid(getgid())
>  take realpath of mountpoint
>  chdir("/")
>  setfsuid(oldfsuid)
>  setfsgid(oldfsgid)
> 
> I believe they want to drop privileges so it works as also access check
> to the mount point directory. As realpath() in practice checks that the
> user has access to the entry too.

Could you clarify what you think the security intent of this code is?
As far as I can tell it's nonsense. realpath is not usable for much of
anything security-related; in particular, it's non-atomic and subject
to all sorts of trickery involving renaming/moving directories during
its operation, even moreso when it's done component-by-component in
userspace.

Why is the check not simply an ownership check for the mount point? I
suspect it has to do with the need to pass a pathname rather than fd
to mount, which is subject to renaming/moving races, but the realpath
call would be subject to the same and worse. Presumably the correct
way to do this is to open a fd to the mountpoint then pass
/proc/self/fd/%d to the mount function after checking ownership.

> This works glibc, as realpath() canonicalizes the path
> component-by-component in userland. But musl breaks due to the /proc
> not being accessible while privileges dropped.
> 
> Any suggestions?

I think first we should figure out if the code even makes sense. I
suspect it's a bug.

Rich

Re: realpath() and setfsuid programs

[Rich Felker]

On Sat, Feb 07, 2015 at 07:26:03AM -0500, Rich Felker wrote:
> On Sat, Feb 07, 2015 at 09:53:54AM +0200, Timo Teras wrote:
> > Hi,
> > 
> > It seems realpath() does not work in binaries using setfsuid(). (At
> > least on grsec kernels, vanilla kernel might be affected too.)
> > 
> > The problem is that realpath() opens the file, and then
> > uses just readlink on /proc/self/fd/<fd> to read the canonicalized
> > path.
> > 
> > However, /proc/self/fd is not accessible if setfsuid() has been used to
> > drop privileges.
> > 
> > The problem I'm looking at in this case is fuse. fusermount, the
> > suid wrapper to do user fuse mounts, seems to basically do:
> >  oldfsuid = setfsuid(getuid())
> >  oldfsgid = setfsgid(getgid())
> >  take realpath of mountpoint
> >  chdir("/")
> >  setfsuid(oldfsuid)
> >  setfsgid(oldfsgid)
> > 
> > I believe they want to drop privileges so it works as also access check
> > to the mount point directory. As realpath() in practice checks that the
> > user has access to the entry too.
> 
> Could you clarify what you think the security intent of this code is?
> As far as I can tell it's nonsense. realpath is not usable for much of
> anything security-related; in particular, it's non-atomic and subject
> to all sorts of trickery involving renaming/moving directories during
> its operation, even moreso when it's done component-by-component in
> userspace.
> 
> Why is the check not simply an ownership check for the mount point? I
> suspect it has to do with the need to pass a pathname rather than fd
> to mount, which is subject to renaming/moving races, but the realpath
> call would be subject to the same and worse. Presumably the correct
> way to do this is to open a fd to the mountpoint then pass
> /proc/self/fd/%d to the mount function after checking ownership.

Or of course just using chdir and checking ownership of ".".

Rich

Re: realpath() and setfsuid programs

[Timo Teras]

On Sat, 7 Feb 2015 07:32:43 -0500
Rich Felker <dalias@libc.org> wrote:

> On Sat, Feb 07, 2015 at 07:26:03AM -0500, Rich Felker wrote:
> > On Sat, Feb 07, 2015 at 09:53:54AM +0200, Timo Teras wrote:
> > > I believe they want to drop privileges so it works as also access
> > > check to the mount point directory. As realpath() in practice
> > > checks that the user has access to the entry too.
> > 
> > Could you clarify what you think the security intent of this code
> > is? As far as I can tell it's nonsense. realpath is not usable for
> > much of anything security-related; in particular, it's non-atomic
> > and subject to all sorts of trickery involving renaming/moving
> > directories during its operation, even moreso when it's done
> > component-by-component in userspace.
> > 
> > Why is the check not simply an ownership check for the mount point?
> > I suspect it has to do with the need to pass a pathname rather than
> > fd to mount, which is subject to renaming/moving races, but the
> > realpath call would be subject to the same and worse. Presumably
> > the correct way to do this is to open a fd to the mountpoint then
> > pass /proc/self/fd/%d to the mount function after checking
> > ownership.
> 
> Or of course just using chdir and checking ownership of ".".

Agreed. In this case fuse seems to be the place needing fix. Dropping
privileges just for realpath() does not sound like the right approach.

Though, I'm wondering if the issue showing up in other places -- that
is realpath() failing if fs uid is set to something that cannot
read /proc/self/fd/...

/Timo

Re: realpath() and setfsuid programs

[Rich Felker]

On Sat, Feb 07, 2015 at 04:28:29PM +0200, Timo Teras wrote:
> > > Why is the check not simply an ownership check for the mount point?
> > > I suspect it has to do with the need to pass a pathname rather than
> > > fd to mount, which is subject to renaming/moving races, but the
> > > realpath call would be subject to the same and worse. Presumably
> > > the correct way to do this is to open a fd to the mountpoint then
> > > pass /proc/self/fd/%d to the mount function after checking
> > > ownership.
> > 
> > Or of course just using chdir and checking ownership of ".".
> 
> Agreed. In this case fuse seems to be the place needing fix. Dropping
> privileges just for realpath() does not sound like the right approach.
> 
> Though, I'm wondering if the issue showing up in other places -- that
> is realpath() failing if fs uid is set to something that cannot
> read /proc/self/fd/...

realpath can fail for various reasons, like lacking permissions to a
path component, fd exhaustion or other resource issues, etc. so I
don't think it's a big deal for it to fail when /proc/self/fd/%d is
not accessible. It's not a robust operation to begin with.

Rich