[Acl-Devel] No extended attributes for sticky directories? (and samba)

[Gerard Neil]

Hello,

I have some queries about permissions for extended attributes in the
user.* namespace on sticky directories.

The documented behaviour (from attr(5)) is that "access to extended
user attributes is restricted  to the owner and to users with
appropriate capabilities for directories with the sticky bit set".

I did some digging and I understand why write access to extended
attributes needs to be restricted on mode 1777 directories like /tmp.
Fair enough, there's a potential DoS otherwise.

What I don't understand is the current behaviour under linux (I'm
looking at current stable tree 2.6.18.1). The vfs code in fs/xattr.c
prevents *all* read or write access to extended attributes for sticky
directories, for *all* users (including root). This contradicts the
documentation. The vfs logic overrides any filesystem-specific
implementation (such as xfs, which implements the documented
behaviour). The jfs code implements the same logic as vfs; and
according to my reading and testing ext2/3 don't appear to implement
their own logic, they seem to rely on the vfs code.

Some context: I've come across this via problems with samba. When
samba is configured with "store dos attributes = yes", it uses the
extended attribute "user.DOSATTRIB" to store the DOS filesystem
permissions "hidden" and "system". This works well until samba
encounters a directory with the sticky bit set and gets EPERM when
trying to read attributes. After that, the smbd process gives up on
reading extended attributes altogether. The net effect is that you
can't use "store dos attributes = yes" and sticky directories at the
same time with Samba; the DOS attributes mysteriously disappear as
soon as an smbd process encouters a sticky bit. I haven't been able to
find a reference to this problem or it's cause anywhere else, except
here: http://lists.samba.org/archive/samba-technical/2006-September/049259.html

It seems clear to me samba behaviour is "buggy" in that it doesn't
take sticky directories into account very well, regardless of how the
kernel behaves. I'm an experienced developer, but I'm new at
contributing to open source; I've been a Gentoo user for about 18
months and this is the first time I've attempted to contribute by
trying to fix something. I guess the next thing to do on this front
would be to submit a bug/patch for samba via Gentoo.

As far as the kernel goes, it isn't clear to me whether there's a bug,
or, if there is, how far to go in fixing it. It seems trivial to patch
vfs so that it conforms to the documented behaviour, but this worries
me in itself; so maybe the current behaviour is intended or my
implementation is no good because I don't know enough. It seems only
slightly more difficult to patch vfs/jfs/xfs to go a bit further, and
deny write access to all but owner and privileged users, but *allow*
read access to everyone... which seems like a better solution to me,
in my very humble opinion. From some code snippets (patches in posts)
I've seen, this latter behaviour was present at some stage. I've
attached the vfs part of such patch as an example, I tested this on
ext2/3, it seems to work.

I apologise if this isn't the right forum for these sorts of
questions, but I guess I just wanted to discuss the issue and ask how
to proceed.

Regards,

Gerard Neil (patch follows)

--- fs/old/xattr.c      2006-10-24 16:24:57.591166000 +1000
+++ fs/xattr.c  2006-10-25 23:14:24.653205000 +1000
@@ -53,10 +53,19 @@
        if (!strncmp(name, XATTR_TRUSTED_PREFIX, XATTR_TRUSTED_PREFIX_LEN))
                return (capable(CAP_SYS_ADMIN) ? 0 : -EPERM);

-       if (!strncmp(name, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN)) {
-               if (!S_ISREG(inode->i_mode) &&
-                   (!S_ISDIR(inode->i_mode) || inode->i_mode & S_ISVTX))
-                       return -EPERM;
+        /*
+         * In user.* namespace, only regular files and directories can have
+         * extended attributes. For sticky directories, only the owner or a
+         * privileged user can write attributes.
+         */
+
+        if (!strncmp(name, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN)) {
+               if (!S_ISREG(inode->i_mode) && !S_ISDIR(inode->i_mode))
+                        return -EPERM;
+                if (S_ISDIR(inode->i_mode) && (inode->i_mode & S_ISVTX) &&
+                    (mask & MAY_WRITE) && (current->fsuid != inode->i_uid)
+                    && !capable(CAP_FOWNER))
+                        return -EPERM;
        }

        return permission(inode, mask, NULL);