Skip to content

Commit

Permalink
Add nfsv4-specific permissions checks to kernel (#2)
Browse files Browse the repository at this point in the history
There are various places in which evaluation of permissions
in the presence of an NFSv4 ACL is more nuanced than what is
typical when evaluating traditional POSIX permissions. For
example, a user may be permitted to delete a file if he
has DELETE permissions on the file or DELETE_CHILD permissions
on the parent directory. Traditional POSIX permissions will
only check for MAY_WRITE | MAY_EXEC on parent directory.

Several new inode permissions masks have been added to facilitate these
NFSv4-specific checks corresponding to different NFSv4 permissions
that grant abilities to make changes to files. For the purpose of
this commit and the goal of providing rough a approximation of
NFSv4 access checks, only write (and not read) access checks have
been implemented. This is selectively done in a way to grant
minimal compliance with permissions as defined in RFC-5661.

The new permissions-related behavior is only applied when the
inode sb_flag SB_NFS4ACL is present. In this case, the onus of full
implementation of requisite features to satisfy the ACL behavior
specified in RFC-5661 is delegated to the filesystem's inode
permissions interface (i_op->permission). If possible we try to
check for the convention POSIX permission first before trying
the NFSv4-equivalent. For example, when writing an xattr, we
check for WRITE_DATA before WRITE_NAMED_ATTRS because in the case
of former with a trivial ACL we can avoid having to evaluate the
full ACL, and instead merely look at POSIX mode.
  • Loading branch information
anodos325 authored and usaleem-ix committed Nov 5, 2024
1 parent 80cc84a commit 15f5a04
Show file tree
Hide file tree
Showing 4 changed files with 173 additions and 3 deletions.
69 changes: 69 additions & 0 deletions fs/attr.c
Original file line number Diff line number Diff line change
Expand Up @@ -179,21 +179,69 @@ int setattr_prepare(struct mnt_idmap *idmap, struct dentry *dentry,
goto kill_priv;

/* Make sure a caller can chown. */
#if CONFIG_TRUENAS
/*
* Check for ACE4_WRITE_OWNER. RFC 5661 Section 6.2.1.3.1
* On UNIX systems, this is the ability to execute chown() and
* chgrp().
*/
if ((ia_valid & ATTR_UID) &&
!chown_ok(idmap, inode, attr->ia_vfsuid)) {
if (!IS_NFSV4ACL(inode)) {
return -EPERM;
}
else if (inode_permission(idmap, inode, MAY_WRITE_OWNER)) {
return -EPERM;
}
}
#else
if ((ia_valid & ATTR_UID) &&
!chown_ok(idmap, inode, attr->ia_vfsuid))
return -EPERM;
#endif

/* Make sure caller can chgrp. */
#if CONFIG_TRUENAS
if ((ia_valid & ATTR_GID) &&
!chgrp_ok(idmap, inode, attr->ia_vfsgid)) {
if (!IS_NFSV4ACL(inode)) {
return -EPERM;
}
else if (inode_permission(idmap, inode, MAY_WRITE_OWNER)) {
return -EPERM;
}
}
#else
if ((ia_valid & ATTR_GID) &&
!chgrp_ok(idmap, inode, attr->ia_vfsgid))
return -EPERM;
#endif

/* Make sure a caller can chmod. */
if (ia_valid & ATTR_MODE) {
vfsgid_t vfsgid;

#if CONFIG_TRUENAS
/*
* Check for ACE4_WRITE_ACL. RFC 5661 Section 6.2.1.3.1
* Permission to write the acl or mode attributes.
*/
if (IS_NFSV4ACL(inode)) {
if (!inode_owner_or_capable(idmap, inode)) {
if (inode_permission(idmap, inode,
MAY_WRITE_ACL)) {
return -EPERM;
}
}
}
else {
if (!inode_owner_or_capable(idmap, inode))
return -EPERM;
}
#else
if (!inode_owner_or_capable(idmap, inode))
return -EPERM;
#endif

if (ia_valid & ATTR_GID)
vfsgid = attr->ia_vfsgid;
Expand All @@ -207,6 +255,12 @@ int setattr_prepare(struct mnt_idmap *idmap, struct dentry *dentry,

/* Check for setting the inode time. */
if (ia_valid & (ATTR_MTIME_SET | ATTR_ATIME_SET | ATTR_TIMES_SET)) {
/*
* Check for ACE4_WRITE_ATTRIBUTES. RFC 5661 Section 6.2.1.3.1
* Users with ACE4_WRITE_ATTRIBUTES or ACE4_WRITE_DATA can
* change the times associated with a file to the _current_
* server time. This permissions check happens in notify_change().
*/
if (!inode_owner_or_capable(idmap, inode))
return -EPERM;
}
Expand Down Expand Up @@ -338,7 +392,22 @@ int may_setattr(struct mnt_idmap *idmap, struct inode *inode,
return -EPERM;

if (!inode_owner_or_capable(idmap, inode)) {
#if CONFIG_TRUENAS
if (IS_NFSV4ACL(inode)) {
error = inode_permission(idmap, inode,
MAY_WRITE);
if (error) {
error = inode_permission(idmap,
inode, MAY_WRITE_ATTRS);
}
}
else {
error = inode_permission(idmap, inode,
MAY_WRITE);
}
#else
error = inode_permission(idmap, inode, MAY_WRITE);
#endif
if (error)
return error;
}
Expand Down
47 changes: 46 additions & 1 deletion fs/namei.c
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,16 @@ static inline int do_inode_permission(struct mnt_idmap *idmap,
*/
static int sb_permission(struct super_block *sb, struct inode *inode, int mask)
{
#if CONFIG_TRUENAS
/*
* NFSv4 ACLs have more granular write permissions. Same logic
* should apply here as with generic MAY_WRITE. Specifically, protect
* against changes to a readonly filesystem.
*/
if (unlikely(mask & (MAY_WRITE | NFS41ACL_WRITE_ALL))) {
#else
if (unlikely(mask & MAY_WRITE)) {
#endif
umode_t mode = inode->i_mode;

/* Nobody gets write access to a read-only fs. */
Expand Down Expand Up @@ -516,7 +525,15 @@ int inode_permission(struct mnt_idmap *idmap,
if (retval)
return retval;

#if CONFIG_TRUENAS
/*
* NFSv4 ACLs have more granular write permissions. Same logic
* should apply here as with generic MAY_WRITE.
*/
if (unlikely(mask & (MAY_WRITE | NFS41ACL_WRITE_ALL))) {
#else
if (unlikely(mask & MAY_WRITE)) {
#endif
/*
* Nobody gets write access to an immutable file.
*/
Expand Down Expand Up @@ -3071,8 +3088,36 @@ static int may_delete(struct mnt_idmap *idmap, struct inode *dir,
return -EOVERFLOW;

audit_inode_child(dir, victim, AUDIT_TYPE_CHILD_DELETE);

#if CONFIG_TRUENAS
if (IS_NFSV4ACL(inode)) {
/*
* See RFC 5661 Section 6.2.1.3.2
* for implementation details of DELETE vs DELETE_CHILD.
*
* Since there may be a variety of ways to implement
* allow in VFS if MAY_DELETE is permitted on viction,
* MAY_DELETE_CHILD is permitted on directory, or MAY_WRITE
* and MAY_EXEC are permitted on directory. This allows
* filesystem to enforce stricter permissions if needed.
*
* MAY_WRITE|MAY_EXEC is checked first to give opportunity
* to perform check via generic_permission() first.
*/
error = inode_permission(idmap, dir, MAY_WRITE | MAY_EXEC);
if (error) {
error = inode_permission(idmap, inode, MAY_DELETE);
if (error) {
error = inode_permission(idmap, dir,
MAY_DELETE_CHILD);
}
}
}
else {
error = inode_permission(idmap, dir, MAY_WRITE | MAY_EXEC);
}
#else
error = inode_permission(idmap, dir, MAY_WRITE | MAY_EXEC);
#endif
if (error)
return error;
if (IS_APPEND(dir))
Expand Down
44 changes: 42 additions & 2 deletions fs/xattr.c
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,11 @@ static int
xattr_permission(struct mnt_idmap *idmap, struct inode *inode,
const char *name, int mask)
{
#if CONFIG_TRUENAS
if (mask & (MAY_WRITE | MAY_WRITE_NAMED_ATTRS)) {
#else
if (mask & MAY_WRITE) {
#endif
int ret;

ret = may_write_xattr(idmap, inode);
Expand All @@ -134,7 +138,11 @@ xattr_permission(struct mnt_idmap *idmap, struct inode *inode,
*/
if (!strncmp(name, XATTR_TRUSTED_PREFIX, XATTR_TRUSTED_PREFIX_LEN)) {
if (!capable(CAP_SYS_ADMIN))
#if CONFIG_TRUENAS
return (mask & (MAY_WRITE | MAY_WRITE_NAMED_ATTRS)) ? -EPERM : -ENODATA;
#else
return (mask & MAY_WRITE) ? -EPERM : -ENODATA;
#endif
return 0;
}

Expand All @@ -145,9 +153,17 @@ xattr_permission(struct mnt_idmap *idmap, struct inode *inode,
*/
if (!strncmp(name, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN)) {
if (!S_ISREG(inode->i_mode) && !S_ISDIR(inode->i_mode))
#if CONFIG_TRUENAS
return (mask & (MAY_WRITE | MAY_WRITE_NAMED_ATTRS)) ? -EPERM : -ENODATA;
#else
return (mask & MAY_WRITE) ? -EPERM : -ENODATA;
#endif
if (S_ISDIR(inode->i_mode) && (inode->i_mode & S_ISVTX) &&
#if CONFIG_TRUENAS
(mask & (MAY_WRITE | MAY_WRITE_NAMED_ATTRS)) &&
#else
(mask & MAY_WRITE) &&
#endif
!inode_owner_or_capable(idmap, inode))
return -EPERM;
}
Expand Down Expand Up @@ -278,8 +294,20 @@ __vfs_setxattr_locked(struct mnt_idmap *idmap, struct dentry *dentry,
{
struct inode *inode = dentry->d_inode;
int error;

#if CONFIG_TRUENAS
if (IS_NFSV4ACL(inode)) {
error = xattr_permission(idmap, inode, name, MAY_WRITE);
if (error) {
error = xattr_permission(idmap, inode, name,
MAY_WRITE_NAMED_ATTRS);
}
}
else {
error = xattr_permission(idmap, inode, name, MAY_WRITE);
}
#else
error = xattr_permission(idmap, inode, name, MAY_WRITE);
#endif
if (error)
return error;

Expand Down Expand Up @@ -537,8 +565,20 @@ __vfs_removexattr_locked(struct mnt_idmap *idmap,
{
struct inode *inode = dentry->d_inode;
int error;

#if CONFIG_TRUENAS
if (IS_NFSV4ACL(inode)) {
error = xattr_permission(idmap, inode, name, MAY_WRITE);
if (error) {
error = xattr_permission(idmap, inode, name,
MAY_WRITE_NAMED_ATTRS);
}
}
else {
error = xattr_permission(idmap, inode, name, MAY_WRITE);
}
#else
error = xattr_permission(idmap, inode, name, MAY_WRITE);
#endif
if (error)
return error;

Expand Down
16 changes: 16 additions & 0 deletions include/linux/fs.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,22 @@ typedef int (dio_iodone_t)(struct kiocb *iocb, loff_t offset,
/* called from RCU mode, don't block */
#define MAY_NOT_BLOCK 0x00000080

#if CONFIG_TRUENAS
/*
* Extended NFSv41 write permissions. These are used selectively
* for permissions checks where NFSv4 ACL handling is
* more nuanced than a simple POSIX permissions.
*/
#define MAY_WRITE_NAMED_ATTRS 0x00000100
#define MAY_DELETE_CHILD 0x00000400
#define MAY_WRITE_ATTRS 0x00001000
#define MAY_DELETE 0x00100000
#define MAY_WRITE_ACL 0x00400000
#define MAY_WRITE_OWNER 0x00800000
#define NFS41ACL_WRITE_ALL (MAY_DELETE_CHILD|MAY_WRITE_ATTRS|MAY_DELETE|\
MAY_WRITE_ACL|MAY_WRITE_OWNER|MAY_WRITE_NAMED_ATTRS)
#endif

/*
* flags in file.f_mode. Note that FMODE_READ and FMODE_WRITE must correspond
* to O_WRONLY and O_RDWR via the strange trick in do_dentry_open()
Expand Down

0 comments on commit 15f5a04

Please sign in to comment.