linux-kernel - Re: [PATCH v2 resend] vfs: new O

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 5 Nov 2009 18:25:48 +0000
From:	Alan Cox <alan@...rguk.ukuu.org.uk>
To:	Miklos Szeredi <miklos@...redi.hu>
Cc:	miklos@...redi.hu, akpm@...ux-foundation.org,
	viro@...IV.linux.org.uk, dhowells@...hat.com, hch@...radead.org,
	adilger@....com, mtk.manpages@...il.com,
	torvalds@...ux-foundation.org, drepper@...il.com,
	jamie@...reable.org, linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 resend] vfs: new O_NODE open flag

> Take a step back and look at what would be required for this to be
> exploitable:

Oh I realise its very minor but care is needed, and its the cases that
haven't been considered that I worry about far more - eg the way you
missed the device stuff.

>  1) a file which is readable and writable by some user
>  2) but is not reachable by said user (because of permissions on path)
>  3) a privileged process opening this file with O_RDONLY
>  4) sending the fd it to an unprivileged process owned by user
>  5) assuming that user won't be able to write it (even though the file
>     has write permission)

The case I found examples of is this

	normal uid process
	exec setuid process
	chdir into locked away directory
	setreuid back

That case is safe because the process is undumpable/unptraceable (which
also keeps /proc/*/fd covered)

The case where it then execs unpriviledged code is interesting. It would
then open up the proc hole but its also true that dumpable would be false
so ptrace could also be used. So its not adding a hole.

> If I was a system designer, I'd think of that as a very fragile
> assumption.

Agreed. Read app code for an afternoon ;)

> Right, that's a valid point.
> 
> So re-opening a device node opened with O_NODE is not safe, I agree.
> Which means, I'll either have to remove the possibilty of re-opening
> O_NODE files through proc, or limit it to non-device nodes.
> 
> I'd really prefer just limiting it, since that would leave re-opening
> a useful feature, while having minimal risk (especially if documented)
> of it causing trouble.

It's certainly a better starting point. See why I said it should default
to unsupported and get added as you review each case ?

> But I can be convinced either way with sufficiently good arguments :)

Basically you need revoke(). That isn't new news either

> > open O_NODE
> > remote file moves
> > new one appears
> > reopen
> 
> Consider
> 
>  open O_RDONLY
>  remote file moves
>  new one appears
>  fstat
> 
> What's the difference?

CIFS in the second case gets a handle to the remote object I believe. NFS
certainly gets it right.

> > Now what should happen and what does happen ?

You stat the wrong file, the O_RDONLY case should stat the right file.
Similar for NFS as NFS will acquire a handle to the file. There isn't any
reason any of these shouldn't work but the moment you start going into
the fs you disappear back into the murky world of unmount

Possibly Christoph is broken as designed ;)

The unix worldview is essentially that a name is an unreliable reference
- it's a lookup directory that changes all the time. A file handle is an
object reference. It persists with the object, its effectively even
refcounted. The handle *is* the thing itself, the name is a transient
momentary thing.

> > That doesn't seem to be the case testing it, but its fixable trivially if
> > so and its fixable without API breakage.
> 
> No it's not.  While a node is looked up, it will pin the mount, albeit
> for just a short time.  And permission on the inode itself are not
> required for lookup, only permissions on the parent.
> 
> I think you are being paranoid here.  If the user has access to the
> path leading up the mountpoint, he might as well pin that mount.
> Permissions on the mount itself shouldn't really make a difference.

Which gets us back to revoke() strangely enough. I am being paranoid. But
security is about being paranoid because you have to out-think all the
bad guys all the time.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/