From bbf060770040a1a98aeae506e34ccc6d349830ac Mon Sep 17 00:00:00 2001 From: Robert Watson Date: Sat, 2 Sep 2000 20:31:26 +0000 Subject: [PATCH] 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 +++++++++++++++++++------------- usr.sbin/extattrctl/extattrctl.8 | 30 +--------- usr.sbin/extattrctl/extattrctl.c | 64 +++++---------------- 4 files changed, 78 insertions(+), 119 deletions(-) diff --git a/sys/ufs/ufs/extattr.h b/sys/ufs/ufs/extattr.h index f6a975ea5ee5..834efd561a95 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 84932be64fc3..74ed01c8292f 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, @@ -83,6 +85,28 @@ ufs_extattr_uepm_unlock(struct ufsmount *ump, struct proc *p) lockmgr(&ump->um_extattr.uepm_lock, LK_RELEASE, 0, 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; + int system_namespace; - 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); - } + system_namespace = (strlen(uele->uele_attrname) >= 1 && + uele->uele_attrname[0] == '$'); - /* Kernel sponsoring request does so without passing a cred */ - if (!cred) + /* + * Kernel-invoked always succeeds + */ + if (cred == NULL) return (0); - /* XXX there might eventually be a capability check here */ - - /* 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); - - /* Allow anyone if appropriate */ - if (uef_perm == UFS_EXTATTR_PERM_ANYONE) - 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); /* diff --git a/usr.sbin/extattrctl/extattrctl.8 b/usr.sbin/extattrctl/extattrctl.8 index 223118cfb5de..f65fe1aabc67 100644 --- a/usr.sbin/extattrctl/extattrctl.8 +++ b/usr.sbin/extattrctl/extattrctl.8 @@ -41,8 +41,6 @@ .Nm extattrctl .Cm initattr .Op Fl p Ar path -.Op Fl r Ar kroa -.Op Fl w Ar kroa .Ar attrsize .Ar attrfile .Nm extattrctl @@ -63,7 +61,7 @@ as well as initialization of attribute backing files, and enabling and disabling of specific extended attributes on a file system. .Pp The first argument on the command line indicates the operation to be -performend. Operation must be one of the following: +performed. Operation must be one of the following: .Bl -tag -width indent .It Cm start Ar path Start extended attribute support on the file system named using @@ -77,8 +75,6 @@ Extended attribute support must previously have been started. .It Xo .Cm initattr .Op Fl p Ar path -.Op Fl r Ar kroa -.Op Fl w Ar kroa .Ar attrsize attrfile .Xc Create and initialize a file to use as an attribute backing file. @@ -95,25 +91,6 @@ This has the advantage of guaranteeing that space will be available for attributes when they are written, preventing low disk space conditions from denying attribute service. .Pp -The -.Fl r -and -.Fl w -options can be used to set the read and write permissions on the named -attribute, respectively. -There are four levels possible for both read and write: -.Dq k -limits reading or writing to the kernel, -.Dq r -limits activities to root, -.Dq o -limits activities to root and the owner of the file having the attribute -read or written, and -.Dq q -allows any user to perform the attribute operation. -The default is to limit activities to the root user, or -.Dq r . -.Pp This file should not exist before running .Cm initattr. .It Cm enable Ar path Ar attrname Ar attrfile @@ -145,9 +122,8 @@ Start extended attributes on the root file system. .Dl extattrctl initattr 17 /.attribute/md5 .Pp Create an attribute backing file in /.attribute/md5, and set the maximum -size of each attribute to 17 bytes. Sparse files are used for storing the -attributes, and the default permissions limiting access to the root user -are implied. +size of each attribute to 17 bytes, with a sparse file used for storing +the attributes. .Pp .Dl extattrctl enable / md5 /.attribute/md5 .Pp diff --git a/usr.sbin/extattrctl/extattrctl.c b/usr.sbin/extattrctl/extattrctl.c index 6b8c5d164b1d..c5c250855197 100644 --- a/usr.sbin/extattrctl/extattrctl.c +++ b/usr.sbin/extattrctl/extattrctl.c @@ -54,41 +54,12 @@ usage(void) "usage:\n" " extattrctl start [path]\n" " extattrctl stop [path]\n" - " extattrctl initattr [-p path] [-r [kroa]] [-w [kroa]] " - "[attrsize] [attrfile]\n" + " extattrctl initattr [-p path] [attrsize] [attrfile]\n" " extattrctl enable [path] [attrname] [attrfile]\n" " extattrctl disable [path] [attrname]\n"); exit(-1); } -/* - * Return a level, or -1 - */ -int -extattr_level_from_string(char *string) -{ - - if (strlen(string) != 1) - return (-1); - - switch(string[0]) { - case 'k': - case 'K': - return (UFS_EXTATTR_PERM_KERNEL); - case 'r': - case 'R': - return (UFS_EXTATTR_PERM_ROOT); - case 'o': - case 'O': - return (UFS_EXTATTR_PERM_OWNER); - case 'a': - case 'A': - return (UFS_EXTATTR_PERM_ANYONE); - default: - return (-1); - } -} - long num_inodes_by_path(char *path) { @@ -111,8 +82,6 @@ initattr(int argc, char *argv[]) char *fs_path = NULL; char *zero_buf = NULL; long loop, num_inodes; - int initattr_rlevel = UFS_EXTATTR_PERM_ROOT; - int initattr_wlevel = UFS_EXTATTR_PERM_ROOT; int ch, i, error; optind = 0; @@ -121,16 +90,6 @@ initattr(int argc, char *argv[]) case 'p': fs_path = strdup(optarg); break; - case 'r': - initattr_rlevel = extattr_level_from_string(optarg); - if (initattr_rlevel == -1) - usage(); - break; - case 'w': - initattr_wlevel = extattr_level_from_string(optarg); - if (initattr_wlevel == -1) - usage(); - break; case '?': default: usage(); @@ -146,8 +105,6 @@ initattr(int argc, char *argv[]) if ((i = open(argv[1], O_CREAT | O_EXCL | O_WRONLY, 0600)) != -1) { uef.uef_magic = UFS_EXTATTR_MAGIC; uef.uef_version = UFS_EXTATTR_VERSION; - uef.uef_write_perm = initattr_wlevel; - uef.uef_read_perm = initattr_rlevel; uef.uef_size = atoi(argv[0]); if (write(i, &uef, sizeof(uef)) == -1) error = -1; @@ -170,7 +127,12 @@ initattr(int argc, char *argv[]) } } } - if (i == -1 || error == -1) { + if (i == -1) { + /* unable to open file */ + perror(argv[1]); + return (-1); + } + if (error == -1) { perror(argv[1]); unlink(argv[1]); return (-1); @@ -191,29 +153,33 @@ main(int argc, char *argv[]) if (argc != 3) usage(); error = extattrctl(argv[2], UFS_EXTATTR_CMD_START, 0, 0); + if (error) + perror("extattrctl start"); } else if (!strcmp(argv[1], "stop")) { if (argc != 3) usage(); error = extattrctl(argv[2], UFS_EXTATTR_CMD_STOP, 0, 0); + if (error) + perror("extattrctl stop"); } else if (!strcmp(argv[1], "enable")) { if (argc != 5) usage(); error = extattrctl(argv[2], UFS_EXTATTR_CMD_ENABLE, argv[3], argv[4]); + if (error) + perror("extattrctl enable"); } else if (!strcmp(argv[1], "disable")) { if (argc != 4) usage(); error = extattrctl(argv[2], UFS_EXTATTR_CMD_DISABLE, argv[3], NULL); + if (error) + perror("extattrctl disable"); } else if (!strcmp(argv[1], "initattr")) { argc -= 2; argv += 2; error = initattr(argc, argv); } else usage(); - - if (error) - perror(argv[1]); - return(error); }