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

[Michael Ju. Tokarev]

Robert Dahlem wrote:
> 
[]
> Well, I sniffed around a bit. The original patch was from
> gcarter at lanier.com. I sent a mail over there asking for some
> explanation, bid did not receive anything yet.

Last time I tried to contact to that address I got a bounce.

[]
> My own archive of the samba mailing list reaches back until 1996 and
> what I found there was really interesting:
> 
>     >I've set "force directory mode = 1775" in smb.conf, but when
>     >I make directories in a share through network neighborhood, I
>     >end up with 0775, i.e. user has rwx, group rwx, others rx.
>     >The sticky bit is not getting set.
> 
>     You need to use "directory mode" also.
>     Then, there's a problem in samba <= 2.0.6 regarding the sticky
>     bit.
>     Jeremy Allison posted a patch that will be in 2-0-7:
> 
>     --- source/lib/doscalls.c  Thu Apr  8 14:13:01 1999
>     +++ source/lib/doscalls.c      Tue Dec  7 19:08:41 1999
>     [...]
>     + Cope with UNIXes that don't allow high order mode bits on mkdir.
>     + Patch from gcarter at lanier.com.
> 
> In fact, at least Linux does not handle mkdir("dir", 0n777) with n!=0
> as expected: the mode seems to be and-ed with 0777.
> 
> I read you have access to some lot of operating systems. Could you
> give mkdir("dir", 01777) a try?
> 
> From what I expect from such tests at the moment I see no need for
> HAVE_BROKEN_MKDIR: if most operating systems need the additional
> stat/chmod sequence anyway (which seems to be not too expensive
> because dos_mkdir() won't be called too often in a production
> environment) we can simply drop HAVE_BROKEN_MKDIR.

Interesting.  I tried this on solaris also, with the same results.
(BTW, I don't have all that systems around, I _was_ have them someday).
Very interesting.   Ok, so maybe we should test for (mode & ~0777)
before trying to do stat() and chmod()?  And probably should use
regular unix stat() and chmod() instead of dos_ ones...  Like this:

int dos_mkdir(char *dname,mode_t mode)
{
  struct stat sbuf;
  int ret = mkdir(dos_to_unix(dname,False),mode);
  if(!ret && (mode & ~0777) && stat(dname,&sbuf) == 0 && (mode &
~sbuf.st_mode))
    chmod(dname, sbuf.st_mode | mode & ~sbuf.st_mode);
  return ret;
}

But note note note note.  All that tricks with chmod() have tiny
security hole.  This is very small hole, as chances to break it is
very limited in time, but it _is_ exists.  Especially if high bits in
mode involved.  If intruder can substitute (using just symlink) that
newly created directory _before samba will call_ chmod with his file,
it will be able to use samba's permissions.  Consider:

  User joe have a set-uid file that can't be executed by scott.
  Scott have access to shell and wants to execute that file.
  Joe at this moment copies a bunch of files (with dirs) from his
  machine using samba.  Scott knows that joe will create directory
  "sd" in share /tmp.  So he (scott) can wait until this directory
  will be created, and at this moment (very small timeslice) he can
  remove that directory and replace it with a symlink to that file.
  So, when samba calls chmod, it will change mode for a joe's file,
  not for his newly directory.  High-bits exists in mode, so file
  _can_ be made set-uid, and can be executable by scott.

Again, chances are very small, but exists.  Uhh.
With this in mind, I'd prefer to test mode flag only for sticky bit
(01000), not for whole high bits.  This will prevent to have setgid
semantics on systems that lacks that, but it will be safer, since
sticky bit is not so dangerous than setuid and setgid, and in many
systems we can't set it for a regular file unless we are root
(and on many systems it is ignored, like on linux) (I don't remember
what's purpose serves 0100 bit for non-executable files -- maybe
locking enforcement on read/write? -- but for executables it tells
kernel to leave exe image in memory when program will finish, to
speed up next startup time, thus this can be set only by root).

So, as a summary:

= Almost all mkdirs are "broken" (or, maybe better, my understanding of
this was broken).

= Supporting setting high-order bits with chmod have security troubles;
sticky-bit is only safe one to set here using chmod.

= There is no need to chmod if mode does not contain high-order bits,
moreover, it will fail on some situations (like my example with
msdos fs on linux).

= Maybe there are other systems that have really broken mkdir (that
maybe ignores mode completely?), and for that systems we probably
can have some workaround.  I can't agree with this myself, since
that broken mkdir if it exists should be fixed at the first place.


Someone have some notes on this?

Regards,
 Michael.

P.S.  I deleted samba at samba.org address from recipients list --
this (long) discussion belongs to samba-technical.  And I can
delete your (Robert's) address also, so that you will not receive
two copies of replies, but I don't know if you subscribed to
samba-technical.