[Samba] still ACL bug in 3.0.14a

[Jeremy Allison]

On Fri, Apr 15, 2005 at 04:18:59PM -0400, Yannick Bergeron wrote:
> I still have the bug after upgrading to 3.0.14a
> 
> logfile
> 
> [2005/04/15 16:18:28, 10] smbd/statcache.c:stat_cache_lookup(243)
>  stat_cache_lookup: lookup succeeded for name [CBBSP/CBBSP6/NEW TEXT 
> DOCUMENT.TXT] -> [CBBSP/CBBSP6/New Text Document.txt]
> [2005/04/15 16:18:28, 10] smbd/reply.c:can_delete(1502)
>  can_delete: CBBSP/CBBSP6/New Text Document.txt, dirtype = 0
> [2005/04/15 16:18:28, 8] smbd/dosmode.c:dos_mode(283)
>  dos_mode: CBBSP/CBBSP6/New Text Document.txt
> [2005/04/15 16:18:28, 8] smbd/dosmode.c:dos_mode_from_sbuf(151)
>  dos_mode_from_sbuf returning a
> [2005/04/15 16:18:28, 8] smbd/dosmode.c:dos_mode(315)
>  dos_mode returning a
> [2005/04/15 16:18:28, 10] 
> smbd/posix_acls.c:check_posix_acl_group_write(3912)
>  check_posix_acl_group_write: file CBBSP/CBBSP6 failed to match on user or 
> group in token (ret = -1).
> [2005/04/15 16:18:28, 10] 
> smbd/posix_acls.c:check_posix_acl_group_write(3919)
>  check_posix_acl_group_write: file CBBSP/CBBSP6 returning (ret = -1).

Wait a minute. Did you configure with --with-acl-support ?

>From this log I don't see the debug line :

DEBUG(10,("check_posix_acl_group_write: ret = %d before check_stat:\n", ret));

which should always be written if you're getting to the line :

 DEBUG(10,("check_posix_acl_group_write: file %s \
failed to match on user or group in token (ret = %d).\n", fname, ret ));

That means it failed to read the ACL (ie. this line :

        if ((posix_acl = SMB_VFS_SYS_ACL_GET_FILE(conn, fname, SMB_ACL_TYPE_ACCESS)) == NULL) {
                goto check_stat;
        }

failed and so you went directly to check_stat, do not pass Go, do
not collect your $200.....

If you're running in an ACL aware environment, in order for smbd to
accurately check if you have write access to a directory it must be
compiled with acl support so it can actually read the ACL entries.

I'm starting to think this is the cause of the problems for people.
I can check this by compiling without acl support and seeing if I
can reproduce the bug.

The reason we need ACL support in Samba is that to determine
*before deletion* that file can be deleted in a ACL environment 
we must read the full directory ACL. If we don't do this then
the delete access can sometimes silently fail (ie. no error return
to the client) as the "open for delete" request will succeed,
but then the "delete open file" request fails - we return the
error but the client ignores it. So we must detect failure to
delete at *open* time - which means checking the directory ACL.

The "delete silently failing" bug was the reason this ACL
check was added in the first place - it improves corretness
w.r.t. delete semantics.

Jeremy.

Jeremy.