[PATCH] s3: introduce new share parameter "open special files"

[Andrew Bartlett]

On Fri, 2013-05-03 at 13:31 +0200, Ralph Wuerthner wrote:
> On Fri, 3 May 2013 13:15:33 +0200
> Ralph Wuerthner <ralphw at de.ibm.com> wrote:
> 
> > Hi list,
> > 
> > attached patch introduces a new share parameter "open special files"
> > to control whether special files such as sockets, devices and fifo's
> > will be opened by the server or not. If set to "no" open requests to
> > special files will fail with "access denied". Default value for "open
> > special files" is "no".
> > 
> > Access to special files impose a security risk because it may for
> > example allow remote clients raw access to local hard drives or
> > kernel memory.
> > 
> > Regards
> > 
> > 	Ralph
> 
> I found a bug in above patch: the check for special files
> must be done after checking for directories. Otherwise opening a
> directory as a file will fail with "access denied" instead of "file is
> a directory".

So, does this pass a full 'make test' now?

I'm quite sceptical about this patch - special files come in many types
- fifos that can and are used by folks for communication between clients
and server processes, and character devices that could plausibly be used
in the same way, and block devices/socket that are unlikely to make
sense remotely.

Why set this to 'no' by default?  Remote access to kernel memory or
local hard drives would mean that either the remote user is root, or any
local user (or local process) on the system already has such access.  

Having Samba be the 'last line of defence' in this situation seems
wrong.  If these can somehow be created unsafely (backups of remote
sytstems for example), why not mount with 'nodev' and have the kernel
enforce this for everyone, and for all remote file servers, such as
http, afp and ftp?

My other worry is not that this isn't a possibly useful feature, but
while this patch is a feature request, once we promise it works, and if
we have any other path that opens a file (because this seems to be in
the smbd code, not in the VFS) and doesn't include this check, that the
next patch comes with 'security issue' attached, because we promised to
do this and didn't. 

Between these issues and concerns, I'm sceptical about the value and
merit of including this patch, and opposed to it's inclusion at this
time. 

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org