2.0.7: inherit permissions = yes breaks setting read-only on files

[Robert Dahlem]

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 at 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 at gmx.net
Radio Bornheim - 2:2461/332 at fidonet +49-69-4930830  (ZyX, V34)
                 2:2461/326 at fidonet +49-69-94414444 (ISDN X.75)
---------------------------------------------------------------