[PATCH] Recent coverity changes added directory_create_or_exist() checks to many directories.

[Andrew Bartlett]

On Wed, 2012-12-26 at 08:53 +1100, Andrew Bartlett wrote:
> On Mon, 2012-12-24 at 08:58 +0100, Andreas Schneider wrote:
> > On Saturday, December 22, 2012 10:22:50 Andrew Bartlett wrote:
> > > On Fri, 2012-12-21 at 15:18 -0800, Jeremy Allison wrote:
> > > > Fix to make "make test" work again in master, no matter what the umask.
> > 
> > Thanks for the fix.

It turns out we still have issues with make test in the AD DC tests, in
the top level make test, so we are not out of the woods yet. 

The issue is that the "state dir" (in particular) is created by
provision, and is umask dependent. 

> > > I'm not entirely happy with the original change (we should only enforce
> > > directory permissions when not doing so would be a security disaster),
> > > but for this change:
> > 
> > I think the questions is if this function should enforce uid and permissions 
> > at all, or if we should add an argument to disable enforcement.
> 
> The uid and permission checks are critical, for the original use case
> around winbind (and similar) pipes.  These don't work or create security
> issues if they are not owned by root, and for example the privileged
> pipe should not be world-readable.  
> 
> Part of the reason for enforcing the permissions is to avoid users who
> are having trouble with (eg) squid just setting the permissions to 777
> and compromising the security of the whole server.  
> 
> I think the admin can and should have more discretion on if the pid and
> lock directories have broader or more restricted permissions. 

We have a particular issue with the state and lock directory permission
enforcement this 'fix' brought.  Depending on the umask of the system
involved, we now have a situation where Samba refuses to start due to
the permissions on a directory that Samba itself created.  As I'm sure
you appreciate, this is a nightmare for the upgrade case, were it to
stay this way.

Why did we need these changes in the first place?  I'm inclined to
suggest the Coverity report was a false positive for the system path
case.   I've not seen the actual details however, but if somehow we did
fail to create the directory, that we would just fail later when we
decided to use that directory to hold a file. 

Either way, the current situation needs to be fixed as soon as folks are
back on board after the holidays, and we need to stop enforcing
permissions except where we did at the time of the 4.0 release. 

If we do want strict checking on the mkdir() (and so can't just revert),
perhaps 'directory_create_or_exist() and
directory_create_or_exist_perms()'?

Thanks,

Andrew Bartlett

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