Robert Dahlem
2000-Aug-21 16:49 UTC
2.0.7: inherit permissions = yes breaks setting read-only on files
Hi,
While testing the upgrade from 2.0.6 to 2.0.7 I found some kind of
misbehaviour of inherit permissions.
It was already known that the SGID bit on directories is only useable
with "inherit permissions = yes". Most of my shares depend heavily on
this (unix) feature.
Today I found out I cannot change a file attribute to read-only from NT
while "inherit permissions" is set to yes.
Now I'm stuck in the dilemma that I cannot upgrade to 2.0.7 because I
need "inherit permissions = yes" for the SGID bit but also need
"inherit permissions = no" for the read-only attribute.
smb.conf.5 states:
The permissions on new files and directories are normally
^^^
governed by "create mask", "directory mask", "force
create
mode"
and "force directory mode" but the boolean inherit permissions
parameter overrides this.
At every position smb.conf.5 speaks about "inherit permissions" this
is
mentioned in the context of NEW files, so later changes of file
attributes should work as a user would expect them to.
I consider this either a code bug or a documentation bug and I would
like to state that I would prefer to look at this as a code bug to be
fixed soon, preferably before the next main release.
As far as I'm personally concerned I also consider the suppression of
the SGID bit under "inherit permissions = no" as a bug: This is
clearly
not what the unix adminstrator wanted when (s)he originally set the
SGID bit on the directory.
The situation gets really fatal under view of the circumstance that
Samba 2.0.7 represents a necessary W2K bugfix release.
To summarize ...
2.0.6: known WIN2K bugs
2.0.7, inherit permissions = no: no SGID on directories
2.0.7, inherit permissions = yes: no read-only attributes
So I would really prefer something like a 2.0.7a or at least some
"official" patch against 2.0.7.
After some code inspection I found out that two functions are
responsible for the changed behaviour: dos_mkdir() in lib/doscalls.c
and unix_mode() in smbd/dosmode.c, both have been modified between
2.0.6 and 2.0.7
I tried to reverse those modifications and noted the behaviour in each
case (ip=n means "inherit permissions = no", ip=y means "inherit
permissions = yes"):
SGID read-only
-----------------------------------------------------------------
2.0.7 release
ip=y ok (inherit+SGID) NOK
ip=n NOK (dir mask-SGID) ok
w/o mkdir-modification
ip=y ok (inherit+SGID) NOK
ip=n ok (dir mask+SGID) ok
w/o inherit-patch
NOK (dir mask-SGID) ok
w/o both
ok (dir mask+SGID) ok
So the behaviour of SGID handling seems not to depend on the "inherit
permissions" modification, but on the dos_mkdir() modification.
And really: the patch from 2.0.6 to 2.0.7 is:
/*******************************************************************
Mkdir() that calls dos_to_unix.
+ Cope with UNIXes that don't allow high order mode bits on mkdir.
+ Patch from gcarter@lanier.com.
********************************************************************/
int dos_mkdir(char *dname,mode_t mode)
{
- return(mkdir(dos_to_unix(dname,False),mode));
+ int ret = mkdir(dos_to_unix(dname,False),mode);
+ if(!ret)
+ return(dos_chmod(dname,mode));
+ else
+ return ret;
}
I'm quite sure this does not do what was intended: the part behind
if(!ret)
gets called when mkdir() does NOT fail, but it should be called when
mkdir() DOES fail.
So I removed the "!" and now my table looks like this:
SGID read-only
-----------------------------------------------------------------
2.0.7 release with my mkdir-modification
ip=y ok (inherit+s) NOK
ip=n ok (dir mask+s) ok
which is somewhat better then the release code.
As far as I'm concerned ip=y is not what I would use anyway and the
problem seems to be solved.
But anybody using "inherit permissions = yes" should be aware of the
fact that his users cannot set any file to read-only as long as the
parent directory has the w-Bit set!
Regards,
Robert
---------------------------------------------------------------
Robert.Dahlem@gmx.net
Radio Bornheim - 2:2461/332@fidonet +49-69-4930830 (ZyX, V34)
2:2461/326@fidonet +49-69-94414444 (ISDN X.75)
---------------------------------------------------------------
Gerald Carter
2000-Aug-21 16:59 UTC
2.0.7: inherit permissions = yes breaks setting read-only on files
Robert Dahlem wrote:> > Today I found out I cannot change a file attribute > to read-only from NT while "inherit permissions" is set to yes.Check to make sure you are being thrown off by the security mask parameter which defaults to the same value as the create mask parameter. Cheers, jerry ---------------------------------------------------------------------- /\ Gerald (Jerry) Carter Professional Services \/ http://www.valinux.com VA Linux Systems gcarter@valinux.com http://www.samba.org SAMBA Team jerry@samba.org http://www.eng.auburn.edu/~cartegw "...a hundred billion castaways looking for a home." - Sting "Message in a Bottle" ( 1979 )
Michael Tokarev
2000-Aug-22 11:45 UTC
2.0.7: inherit permissions = yes breaks setting read-only on files
Robert Dahlem wrote:> > Hi, > > While testing the upgrade from 2.0.6 to 2.0.7 I found some kind of > misbehaviour of inherit permissions. >[]> And really: the patch from 2.0.6 to 2.0.7 is: > > /******************************************************************* > Mkdir() that calls dos_to_unix. > + Cope with UNIXes that don't allow high order mode bits on mkdir. > + Patch from gcarter@lanier.com. > ********************************************************************/ > > int dos_mkdir(char *dname,mode_t mode) > { > - return(mkdir(dos_to_unix(dname,False),mode)); > + int ret = mkdir(dos_to_unix(dname,False),mode); > + if(!ret) > + return(dos_chmod(dname,mode)); > + else > + return ret; > }[] This is a known bug that of Samba Team still can't decide if it is a bug or not. I reported this many times, some team members agreed that this is a bug, but without any further work. Just remove all the new lines introduced in 2.0.7 (do this as 2.0.6 did), and you're in buisiness. This is a crap by gcarter@lanier.com that have over side effects also. BTW, I just can't realize how many times people should report about the same thing and how many times should people spend searching for the same problem to get it fixed in the first place! :((( Maybe one should start some sort of petition for each bug found, and the problem will be cured only after some millions of users will sign that petition ?! :^8> I'm quite sure this does not do what was intended: the part behind > > if(!ret) > > gets called when mkdir() does NOT fail, but it should be called when > mkdir() DOES fail.Nope, by that bad design chmod should be called only if mkdir succed, or chmod will fail also. Regards, Michael.
Cole, Timothy D.
2000-Aug-22 20:57 UTC
2.0.7: inherit permissions = yes breaks setting read-only on files
> -----Original Message----- > From: Robert Dahlem [SMTP:Robert.Dahlem@gmx.net] > Sent: Tuesday, August 22, 2000 16:36 > To: Michael Tokarev > Cc: samba@samba.org; samba-technical@samba.org > Subject: Re: 2.0.7: inherit permissions = yes breaks setting > read-only on files > > Michael, > > On Tue, 22 Aug 2000 20:37:59 +0400, Michael Tokarev wrote: > > >BTW, this will be better written as (just polishing, > >same logic): > > > >int dos_mkdir(char *dname,mode_t mode) > >{ > > int ret = mkdir(dos_to_unix(dname,False),mode); > >#ifdef HAVE_BROKEN_MKDIR > > SMB_STRUCT_STAT sbuf; > > if(!ret && !dos_stat(dname,&sbuf) && mode & ~sbuf.st_mode) > > dos_chmod(dname,sbuf.st_mode | mode & ~sbuf.st_mode); > >#endif > > return ret; > >} > > > >(note that sbuf will be unused in your case if !HAVE_BROKEN_MKDIR). > > It won't compile (at least on my system). It needs some additional > curly braces. > > int dos_mkdir(char *dname,mode_t mode) > { > int ret = mkdir(dos_to_unix(dname,False),mode); > #ifdef HAVE_BROKEN_MKDIR > { > SMB_STRUCT_STAT sbuf; > if(!ret && !dos_stat(dname,&sbuf) && mode & ~sbuf.st_mode) > dos_chmod(dname,sbuf.st_mode | mode & ~sbuf.st_mode); > } > #endif > return ret; > } >I believe non-constant initializers are a GNU C extension.