From e6a536221cb30ae732c3b4e40beb458608264a3d Mon Sep 17 00:00:00 2001 From: rwatson Date: Sat, 2 Sep 2000 20:31:26 +0000 Subject: Modify extended attribute protection model to authorize based on attribute namespace and DAC protection on file: - Attribute names beginning with '$' are in the system namespace - The attribute name "$" is reserved - System namespace attributes may only be read/set by suser() or by kernel (cred == NULL) - Other attribute names are in the application namespace - The attribute name "" is reserved - Application namespace attributes are protected in the manner of the target file permission o Kernel changes - Add ufs_extattr_valid_attrname() to check whether the requested attribute "set" or "enable" is appropriate (i.e., non-reserved) - Modify ufs_extattr_credcheck() to accept target file vnode, not to take inode uid - Modify ufs_extattr_credcheck() to check namespace, then enforce either kernel/suser for system namespace, or vaccess() for application namespace o EA backing file format changes - Remove permission fields from extended attribute backing file header - Bump extended attribute backing file header version to 3 o Update extattrctl.c and extattrctl.8 - Remove now deprecated -r and -w arguments to initattr, as permissions are now implicit - (unrelated) fix error reporting and unlinking during failed initattr to remove duplicate/inaccurate error messages, and to only unlink if the failure wasn't in the backing file open() Obtained from: TrustedBSD Project --- sys/ufs/ufs/extattr.h | 4 +- sys/ufs/ufs/ufs_extattr.c | 99 ++++++++++++++++++++++++++++------------------- 2 files changed, 60 insertions(+), 43 deletions(-) (limited to 'sys/ufs/ufs') diff --git a/sys/ufs/ufs/extattr.h b/sys/ufs/ufs/extattr.h index f6a975e..834efd5 100644 --- a/sys/ufs/ufs/extattr.h +++ b/sys/ufs/ufs/extattr.h @@ -33,7 +33,7 @@ #define _UFS_UFS_EXTATTR_H_ #define UFS_EXTATTR_MAGIC 0x00b5d5ec -#define UFS_EXTATTR_VERSION 0x00000002 +#define UFS_EXTATTR_VERSION 0x00000003 #define UFS_EXTATTR_FSROOTSUBDIR ".attribute" #define UFS_EXTATTR_MAXEXTATTRNAME 65 /* including null */ @@ -55,8 +55,6 @@ struct ufs_extattr_fileheader { u_int uef_magic; /* magic number for sanity checking */ u_int uef_version; /* version of attribute file */ u_int uef_size; /* size of attributes, w/o header */ - u_int uef_read_perm; /* permissions to read attribute */ - u_int uef_write_perm; /* permissions to write attribute */ }; struct ufs_extattr_header { diff --git a/sys/ufs/ufs/ufs_extattr.c b/sys/ufs/ufs/ufs_extattr.c index 84932be..74ed01c 100644 --- a/sys/ufs/ufs/ufs_extattr.c +++ b/sys/ufs/ufs/ufs_extattr.c @@ -49,8 +49,10 @@ static MALLOC_DEFINE(M_UFS_EXTATTR, "ufs_extattr", "ufs extended attribute"); -static int ufs_extattr_credcheck(struct ufs_extattr_list_entry *uele, - u_int32_t fowner, struct ucred *cred, struct proc *p, int access); +static int ufs_extattr_valid_attrname(const char *attrname); +static int ufs_extattr_credcheck(struct vnode *vp, + struct ufs_extattr_list_entry *uele, struct ucred *cred, struct proc *p, + int access); static int ufs_extattr_enable(struct ufsmount *ump, const char *attrname, struct vnode *backing_vnode, struct proc *p); static int ufs_extattr_disable(struct ufsmount *ump, const char *attrname, @@ -84,6 +86,28 @@ ufs_extattr_uepm_unlock(struct ufsmount *ump, struct proc *p) } /* + * Determine whether the name passed is a valid name for an actual + * attribute. + * + * Invalid currently consists of: + * NULL pointer for attrname + * zero-length attrname (used to retrieve application attr list) + * attrname consisting of "$" (used to treive system attr list) + */ +static int +ufs_extattr_valid_attrname(const char *attrname) +{ + + if (attrname == NULL) + return (0); + if (strlen(attrname) == 0) + return (0); + if (strlen(attrname) == 1 && attrname[0] == '$') + return (0); + return (1); +} + +/* * Locate an attribute given a name and mountpoint. * Must be holding uepm lock for the mount point. */ @@ -199,6 +223,8 @@ ufs_extattr_enable(struct ufsmount *ump, const char *attrname, struct uio auio; int error = 0; + if (!ufs_extattr_valid_attrname(attrname)) + return (EINVAL); if (backing_vnode->v_type != VREG) return (EINVAL); @@ -280,6 +306,9 @@ ufs_extattr_disable(struct ufsmount *ump, const char *attrname, struct proc *p) struct ufs_extattr_list_entry *uele; int error = 0; + if (!ufs_extattr_valid_attrname(attrname)) + return (EINVAL); + uele = ufs_extattr_find_attr(ump, attrname); if (!uele) return (ENOENT); @@ -363,41 +392,27 @@ ufs_extattrctl(struct mount *mp, int cmd, const char *attrname, * permissions. */ static int -ufs_extattr_credcheck(struct ufs_extattr_list_entry *uele, u_int32_t fowner, - struct ucred *cred, struct proc *p, int access) +ufs_extattr_credcheck(struct vnode *vp, struct ufs_extattr_list_entry *uele, + struct ucred *cred, struct proc *p, int access) { - u_int uef_perm; - - switch(access) { - case IREAD: - uef_perm = uele->uele_fileheader.uef_read_perm; - break; - case IWRITE: - uef_perm = uele->uele_fileheader.uef_write_perm; - break; - default: - return (EACCES); - } - - /* Kernel sponsoring request does so without passing a cred */ - if (!cred) - return (0); - - /* XXX there might eventually be a capability check here */ + int system_namespace; - /* If it's set to root-only, check for suser(p) */ - if (uef_perm == UFS_EXTATTR_PERM_ROOT && !suser(p)) - return (0); - - /* Allow the owner if appropriate */ - if (uef_perm == UFS_EXTATTR_PERM_OWNER && cred->cr_uid == fowner) - return (0); + system_namespace = (strlen(uele->uele_attrname) >= 1 && + uele->uele_attrname[0] == '$'); - /* Allow anyone if appropriate */ - if (uef_perm == UFS_EXTATTR_PERM_ANYONE) + /* + * Kernel-invoked always succeeds + */ + if (cred == NULL) return (0); - return (EACCES); + /* + * XXX What capability should apply here? + */ + if (system_namespace) + return (suser_xxx(cred, p, PRISON_ROOT)); + else + return (VOP_ACCESS(vp, access, cred, p)); } /* @@ -451,12 +466,16 @@ ufs_extattr_get(struct vnode *vp, const char *name, struct uio *uio, if (!(ump->um_extattr.uepm_flags & UFS_EXTATTR_UEPM_STARTED)) return (EOPNOTSUPP); + if (strlen(name) == 0 || (strlen(name) == 1 && name[0] == '$')) { + /* XXX retrieve attribute lists */ + return (EINVAL); + } + attribute = ufs_extattr_find_attr(ump, name); if (!attribute) return (ENOENT); - if ((error = ufs_extattr_credcheck(attribute, ip->i_uid, cred, p, - IREAD))) + if ((error = ufs_extattr_credcheck(vp, attribute, cred, p, IREAD))) return (error); /* @@ -613,16 +632,16 @@ ufs_extattr_set(struct vnode *vp, const char *name, struct uio *uio, if (vp->v_mount->mnt_flag & MNT_RDONLY) return (EROFS); - if (!(ump->um_extattr.uepm_flags & UFS_EXTATTR_UEPM_STARTED)) return (EOPNOTSUPP); + if (!ufs_extattr_valid_attrname(name)) + return (EINVAL); attribute = ufs_extattr_find_attr(ump, name); if (!attribute) return (ENOENT); - if ((error = ufs_extattr_credcheck(attribute, ip->i_uid, cred, - p, IWRITE))) + if ((error = ufs_extattr_credcheck(vp, attribute, cred, p, IWRITE))) return (error); /* @@ -718,16 +737,16 @@ ufs_extattr_rm(struct vnode *vp, const char *name, struct ucred *cred, if (vp->v_mount->mnt_flag & MNT_RDONLY) return (EROFS); - if (!(ump->um_extattr.uepm_flags & UFS_EXTATTR_UEPM_STARTED)) return (EOPNOTSUPP); + if (!ufs_extattr_valid_attrname(name)) + return (EINVAL); attribute = ufs_extattr_find_attr(ump, name); if (!attribute) return (ENOENT); - if ((error = ufs_extattr_credcheck(attribute, ip->i_uid, cred, p, - IWRITE))) + if ((error = ufs_extattr_credcheck(vp, attribute, cred, p, IWRITE))) return (error); /* -- cgit v1.1