nfs4, special files, and set/listxattr asymmetry

Trond, I've discovered an asymmetry in how nfs4 (in v2.6.24-rc7) handles listxattr vs. setxattr for special files. I caught this with Unionfs when testing a copyup of a character device from a readonly nfs4 mount to a writable nfs4 mount.

nfs4_setxattr returns -EPERM if the inode isn't a regular file or a (sticky) dir. However, nfs4_listxattr returns XATTR_NAME_NFSV4_ACL (16 bytes, "system.nfs4_acl") regardless of the type of the inode. So during copyup, I can ->listxattr fine from the source inode, but I get EPERM trying to ->setxattr on the destination branch of of the copyup. (I already handle the case when copying-up from a f/s which supports xattr's to one which doesn't, but when both file systems are the same and mounted the same, there typically should not be difference).

Assuming that the original intent of the EPERM check in nfs4_setxattr was to prevent setting xattrs over nfs4 for non-files/dirs, then I "fixed" this by adding a similar check in nfs4_listxattr, which returns an empty xattr list for those. I've included a patch below for your consideration/review.

Quoted 14 lines · Erez Zadok

Trond, I've discovered an asymmetry in how nfs4 (in v2.6.24-rc7) handles listxattr vs. setxattr for special files. I caught this with Unionfs when testing a copyup of a character device from a readonly nfs4 mount to a writable nfs4 mount.

nfs4_setxattr returns -EPERM if the inode isn't a regular file or a (sticky) dir. However, nfs4_listxattr returns XATTR_NAME_NFSV4_ACL (16 bytes, "system.nfs4_acl") regardless of the type of the inode. So during copyup, I can ->listxattr fine from the source inode, but I get EPERM trying to ->setxattr on the destination branch of of the copyup. (I already handle the case when copying-up from a f/s which supports xattr's to one which doesn't, but when both file systems are the same and mounted the same, there typically should not be difference).

Quoted 4 lines

Assuming that the original intent of the EPERM check in nfs4_setxattr was to prevent setting xattrs over nfs4 for non-files/dirs, then I "fixed" this by adding a similar check in nfs4_listxattr, which returns an empty xattr list for those. I've included a patch below for your consideration/review.

I can see no reason in the RFC why you shouldn't be able to set an NFSv4 ACL on a special file. AFAICS this is a bug in the nfs4_setxattr code, which is probably due to some improper cut n' pasting from the posix acl code.

Bruce?

From 9fa16668ed09b212ec0325786d91ac65734336ea Mon Sep 17 00:00:00 2001

Quoted 21 lines · Trond Myklebust

Quoted 14 lines · Erez Zadok

Trond, I've discovered an asymmetry in how nfs4 (in v2.6.24-rc7) handles listxattr vs. setxattr for special files. I caught this with Unionfs when testing a copyup of a character device from a readonly nfs4 mount to a writable nfs4 mount.

nfs4_setxattr returns -EPERM if the inode isn't a regular file or a (sticky) dir. However, nfs4_listxattr returns XATTR_NAME_NFSV4_ACL (16 bytes, "system.nfs4_acl") regardless of the type of the inode. So during copyup, I can ->listxattr fine from the source inode, but I get EPERM trying to ->setxattr on the destination branch of of the copyup. (I already handle the case when copying-up from a f/s which supports xattr's to one which doesn't, but when both file systems are the same and mounted the same, there typically should not be difference).

Quoted 4 lines

Assuming that the original intent of the EPERM check in nfs4_setxattr was to prevent setting xattrs over nfs4 for non-files/dirs, then I "fixed" this by adding a similar check in nfs4_listxattr, which returns an empty xattr list for those. I've included a patch below for your consideration/review.

Quoted 6 lines

I can see no reason in the RFC why you shouldn't be able to set an NFSv4 ACL on a special file. AFAICS this is a bug in the nfs4_setxattr code, which is probably due to some improper cut n' pasting from the posix acl code.

Bruce?

I think you're right.

I'm also mystified by this rule that the sticky bit prevents non-owners from writing a directory's extended attributes. (See fs/xattr.c.) Any idea what the source of that is? Anyway, I don't see why the client should enforce such a rule in this case.

Erez, does just removing this check solve your problem?

--b.

From 9fa16668ed09b212ec0325786d91ac65734336ea Mon Sep 17 00:00:00 2001
From: J. Bruce Fields <[email protected]>
Date: Tue, 8 Jan 2008 12:41:41 -0500
Subject: [PATCH] nfs4: allow nfsv4 acls on non-regular-files

The rfc doesn't give any reason it shouldn't be possible to set an attribute on a non-regular file. And if the server supports it, then it shouldn't be up to us to prevent it.

Signed-off-by J. Bruce Fields <[email protected]>

---
 fs/nfs/nfs4proc.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

fs/nfs/nfs4proc.c +0 -4

index f03d9d5..e9b9903 100644
@@ -3625,10 +3625,6 @@ int nfs4_setxattr(struct dentry *dentry, const char *key, const void *buf,
 	if (strcmp(key, XATTR_NAME_NFSV4_ACL) != 0)
 		return -EOPNOTSUPP;
 
-	if (!S_ISREG(inode->i_mode) &&
-	    (!S_ISDIR(inode->i_mode) || inode->i_mode & S_ISVTX))
-		return -EPERM;
-
 	return nfs4_proc_set_acl(inode, buf, buflen);
 }
 

Signature · 1 line

1.5.4.rc2.60.gb2e62

Yes, it works equally well for me. Thanks.

Acked-by Erez Zadok <[email protected]>

Quoted 1 line · Erez Zadok

Yes, it works equally well for me. Thanks.

OK, thanks.

Quoted 1 line

Acked-by Erez Zadok <[email protected]>

Maybe that should be Reviewed-by? Tested-by? Up to Trond, I guess.

Trond, could you apply?

From: J. Bruce Fields <[email protected]>

The rfc doesn't give any reason it shouldn't be possible to set an attribute on a non-regular file. And if the server supports it, then it shouldn't be up to us to prevent it.

Quoted 5 lines · Erez Zadok

Trond, I've discovered an asymmetry in how nfs4 (in v2.6.24-rc7) handles listxattr vs. setxattr for special files. I caught this with Unionfs when testing a copyup of a character device from a readonly nfs4 mount to a writable nfs4 mount.

Quoted 8 lines

nfs4_setxattr returns -EPERM if the inode isn't a regular file or a (sticky) dir. However, nfs4_listxattr returns XATTR_NAME_NFSV4_ACL (16 bytes, "system.nfs4_acl") regardless of the type of the inode. So during copyup, I can ->listxattr fine from the source inode, but I get EPERM trying to ->setxattr on the destination branch of of the copyup. (I already handle the case when copying-up from a f/s which supports xattr's to one which doesn't, but when both file systems are the same and mounted the same, there typically should not be difference).

Quoted 7 lines · Trond Myklebust

Quoted 5 lines · Erez Zadok

Trond, I've discovered an asymmetry in how nfs4 (in v2.6.24-rc7) handles listxattr vs. setxattr for special files. I caught this with Unionfs when testing a copyup of a character device from a readonly nfs4 mount to a writable nfs4 mount.

Quoted 8 lines

Quoted 8 lines

nfs4_setxattr returns -EPERM if the inode isn't a regular file or a (sticky) dir. However, nfs4_listxattr returns XATTR_NAME_NFSV4_ACL (16 bytes, "system.nfs4_acl") regardless of the type of the inode. So during copyup, I can ->listxattr fine from the source inode, but I get EPERM trying to ->setxattr on the destination branch of of the copyup. (I already handle the case when copying-up from a f/s which supports xattr's to one which doesn't, but when both file systems are the same and mounted the same, there typically should not be difference).

I do wonder about relying on this, though. The user.* attributes may work that way, but I don't know if there are any rules you can count on for the system.* or other xattrs--is there any guarantee a particular system.* attribute couldn't be readonly, for example?

--b.

A few months ago I looked into the issue of xattrs and copyup in more detail, when I was tracking a problem for a user using an SE-linux enabled livecd with unionfs. I didn't realize before then that selinux made such a heavy use of xattrs. After tracking down the code maze I found out that the list of xattrs being defined/used depends on your overall security mode and global security ops.

Anyway, xattrs can be used for security reasons, and really any semantics could be attached to them. So in Unionfs I take the conservative approach:
if unionfs is compiled with xattr support, then during copyup I try to copy the xattrs too (if any exist). If unionfs fails to copyup the xattrs, then I abort the copyup. I figured it's safer to abort the copyup (which is typically initiated when trying to modify a file on a readonly branch/media), than to potentially open up a security hole by giving a copied-up file more permissions than its source file may have had. This policy has so far worked for unionfs, at least for those users who use xattrs/selinux, and in my limited testing. But maybe I've got to rethink it?

NFS in some sense shares some common traits with a stackable file system, in that it has three "layers": client -> server -> backing-store f/s. I'm curious how does nfs/d handle xattrs (and selinux's use of them)? Does the client depend on having xattr support of any sort? Does the server depend on having xattr support in the export f/s? How do you handle mixes of those when one of the three layers has xattr support and another doesn't?

Quoted 19 lines · Erez Zadok

A few months ago I looked into the issue of xattrs and copyup in more detail, when I was tracking a problem for a user using an SE-linux enabled livecd with unionfs. I didn't realize before then that selinux made such a heavy use of xattrs. After tracking down the code maze I found out that the list of xattrs being defined/used depends on your overall security mode and global security ops.

Anyway, xattrs can be used for security reasons, and really any semantics could be attached to them. So in Unionfs I take the conservative approach:
if unionfs is compiled with xattr support, then during copyup I try to copy the xattrs too (if any exist). If unionfs fails to copyup the xattrs, then I abort the copyup. I figured it's safer to abort the copyup (which is typically initiated when trying to modify a file on a readonly branch/media), than to potentially open up a security hole by giving a copied-up file more permissions than its source file may have had. This policy has so far worked for unionfs, at least for those users who use xattrs/selinux, and in my limited testing. But maybe I've got to rethink it?

Quoted 6 lines

NFS in some sense shares some common traits with a stackable file system, in that it has three "layers": client -> server -> backing-store f/s. I'm curious how does nfs/d handle xattrs (and selinux's use of them)? Does the client depend on having xattr support of any sort? Does the server depend on having xattr support in the export f/s? How do you handle mixes of those when one of the three layers has xattr support and another doesn't?

We ignore xattr's entirely for now. The labeled nfs people (mailing list at [email protected]) have plans to address the selinux case, but I haven't really followed that.

--b.

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
_______________________________________________
Please note that [email protected] is being discontinued. Please subscribe to [email protected] instead.
http://vger.kernel.org/vger-lists.html#linux-nfs

xattrs are only the local Linux representation of a MAC attribute. For the purpose of interoperability we can't assume xattrs considering NFSv4 doesn't contain them in the specification. We considered the named attributes but they fall short concerning file labeling. Instead we are working on adding another recommended attribute to the NFSv4

specification to handle an opaque security label. For your purposes you can continue to think of them as extended attributes since that is the interface that Linux has chosen for security attributes. However, I don't see this functionality making its way into Linux any time soon since it will require a change to the NFSv4 standard and they are wrapping up the NFSv4.1 specification. I can't say how long it will be before NFSv4.2 starts up. Maybe future events will cause that to happen sooner than later.

Quoted 5 lines · Dave Quigley

xattrs are only the local Linux representation of a MAC attribute. For the purpose of interoperability we can't assume xattrs considering NFSv4 doesn't contain them in the specification. We considered the named attributes but they fall short concerning file labeling. Instead we are working on adding another recommended attribute to the NFSv4

Quoted 8 lines

specification to handle an opaque security label. For your purposes you can continue to think of them as extended attributes since that is the interface that Linux has chosen for security attributes. However, I don't see this functionality making its way into Linux any time soon since it will require a change to the NFSv4 standard and they are wrapping up the NFSv4.1 specification. I can't say how long it will be before NFSv4.2 starts up. Maybe future events will cause that to happen sooner than later.

I would hope that the existance of a good, well-documented implementation is the important thing, and would make things go more quickly.

--b.