On Tue, 2014-03-04 at 16:18 +0200, Dmitry Kasatkin wrote: > On Tue, Mar 4, 2014 at 5:21 AM, Mimi Zohar <zo...@linux.vnet.ibm.com> wrote: > > On Mon, 2014-03-03 at 19:00 -0800, Casey Schaufler wrote: > >> On 3/3/2014 6:39 PM, Mimi Zohar wrote: > >> > On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote: > >> >> EVM currently uses source hard coded list of xattrs which needs to be > >> >> included into the HMAC calculation. This is very unflexible. > >> >> Adding new attributes requires modifcation of the source code and > >> >> prevents building the kernel which works with previously labeled > >> >> filesystems. > >> >> > >> >> Early versions of Smack used only one xattr security.SMACK64, > >> >> which is protected by EVM. Now Smack has a few more attributes and > >> >> they are not protected. Adding support to the source code makes it > >> >> impossible to use new kernel with previousely labeled filesystems. > >> > I think this patch will break any existing filesystems labeled with > >> > 'security.smack64'. Details inline. > >> > > >> >> This patch replaces hardcoded xattr array with dynamic list which is > >> >> initialized from CONFIG_EVM_HMAC_XATTRS variable. It allows to build > >> >> kernel with with support of older and newer EVM HMAC formats.
So instead of having a single kernel, this allows you to build different kernels with different xattr labels included in the HMAC. Wouldn't you want a migration mode, similar to 'fix' mode, that only updates the HMAC, if the existing HMAC verified based on the prior set of xattrs? > >> >> > >> >> Possible future extension will be to read xattr list from the kernel > >> >> command line or from securityfs entry. > >> >> > >> >> Signed-off-by: Dmitry Kasatkin <d.kasat...@samsung.com> > >> >> --- > >> >> security/integrity/evm/Kconfig | 10 ++++++ > >> >> security/integrity/evm/evm.h | 7 +++- > >> >> security/integrity/evm/evm_crypto.c | 8 ++--- > >> >> security/integrity/evm/evm_main.c | 69 > >> >> +++++++++++++++++++------------------ > >> >> 4 files changed, 55 insertions(+), 39 deletions(-) > >> >> > >> >> diff --git a/security/integrity/evm/Kconfig > >> >> b/security/integrity/evm/Kconfig > >> >> index 2be51fa..06237b8 100644 > >> >> --- a/security/integrity/evm/Kconfig > >> >> +++ b/security/integrity/evm/Kconfig > >> >> @@ -25,3 +25,13 @@ config EVM_HMAC_ATTRS > >> >> WARNING: changing the HMAC calculation method or adding > >> >> additional info to the calculation, requires existing EVM > >> >> labeled file systems to be relabeled. > >> >> + > >> >> +config EVM_HMAC_XATTRS > >> >> + string "HMAC xattrs" > >> >> + default "security.selinux security.SMACK64 security.ima > >> >> security.capability" > >> >> + help > >> >> + This options allows to specify list of extended attributes > >> >> included into HMAC > >> >> + calculation. It makes it possible easily upgrade to newer kernels. > >> >> + > >> >> + Default value: > >> >> + security.selinux, security.SMACK64, security.ima, > >> >> security.capability > >> >> diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h > >> >> index c8fa0aa..4d1c51e 100644 > >> >> --- a/security/integrity/evm/evm.h > >> >> +++ b/security/integrity/evm/evm.h > >> >> @@ -31,8 +31,13 @@ extern struct crypto_shash *hash_tfm; > >> >> > >> >> #define EVM_HMAC_ATTR_FSUUID 0x0001 > >> >> > >> >> +struct evm_hmac_xattr { > >> >> + struct list_head list; > >> >> + char *xattr; > >> >> +}; > >> >> + > >> >> /* List of EVM protected security xattrs */ > >> >> -extern char *evm_config_xattrnames[]; > >> >> +extern struct list_head evm_hmac_xattrs; > >> >> > >> >> int evm_init_key(void); > >> >> int evm_update_evmxattr(struct dentry *dentry, > >> >> diff --git a/security/integrity/evm/evm_crypto.c > >> >> b/security/integrity/evm/evm_crypto.c > >> >> index ab034e5..7e5bfb7 100644 > >> >> --- a/security/integrity/evm/evm_crypto.c > >> >> +++ b/security/integrity/evm/evm_crypto.c > >> >> @@ -133,7 +133,7 @@ static int evm_calc_hmac_or_hash(struct dentry > >> >> *dentry, > >> >> { > >> >> struct inode *inode = dentry->d_inode; > >> >> struct shash_desc *desc; > >> >> - char **xattrname; > >> >> + struct evm_hmac_xattr *entry; > >> >> size_t xattr_size = 0; > >> >> char *xattr_value = NULL; > >> >> int error; > >> >> @@ -146,15 +146,15 @@ static int evm_calc_hmac_or_hash(struct dentry > >> >> *dentry, > >> >> return PTR_ERR(desc); > >> >> > >> >> error = -ENODATA; > >> >> - for (xattrname = evm_config_xattrnames; *xattrname != NULL; > >> >> xattrname++) { > >> >> + list_for_each_entry(entry, &evm_hmac_xattrs, list) { > >> >> if ((req_xattr_name && req_xattr_value) > >> >> - && !strcmp(*xattrname, req_xattr_name)) { > >> >> + && !strcmp(entry->xattr, req_xattr_name)) { > >> >> error = 0; > >> >> crypto_shash_update(desc, (const u8 > >> >> *)req_xattr_value, > >> >> req_xattr_value_len); > >> >> continue; > >> >> } > >> >> - size = vfs_getxattr_alloc(dentry, *xattrname, > >> >> + size = vfs_getxattr_alloc(dentry, entry->xattr, > >> >> &xattr_value, xattr_size, > >> >> GFP_NOFS); > >> >> if (size == -ENOMEM) { > >> >> error = -ENOMEM; > >> >> diff --git a/security/integrity/evm/evm_main.c > >> >> b/security/integrity/evm/evm_main.c > >> >> index 9c05929..13e03ad 100644 > >> >> --- a/security/integrity/evm/evm_main.c > >> >> +++ b/security/integrity/evm/evm_main.c > >> >> @@ -34,19 +34,7 @@ char *evm_hmac = "hmac(sha1)"; > >> >> char *evm_hash = "sha1"; > >> >> int evm_hmac_attrs; > >> >> > >> >> -char *evm_config_xattrnames[] = { > >> >> -#ifdef CONFIG_SECURITY_SELINUX > >> >> - XATTR_NAME_SELINUX, > >> >> -#endif > >> >> -#ifdef CONFIG_SECURITY_SMACK > >> >> - XATTR_NAME_SMACK, > >> >> -#endif > >> >> -#ifdef CONFIG_IMA_APPRAISE > >> >> - XATTR_NAME_IMA, > >> >> -#endif > >> >> - XATTR_NAME_CAPS, > >> >> - NULL > >> >> -}; > >> >> +LIST_HEAD(evm_hmac_xattrs); > >> >> > >> >> static int evm_fixmode; > >> >> static int __init evm_set_fixmode(char *str) > >> >> @@ -61,27 +49,53 @@ static int __init evm_init_config(void) > >> >> { > >> >> char *attrs = CONFIG_EVM_HMAC_ATTRS; > >> >> char *p; > >> >> + char *xattrs = CONFIG_EVM_HMAC_XATTRS; > >> >> + struct evm_hmac_xattr *entry; > >> >> > >> >> while ((p = strsep(&attrs, ", \t"))) { > >> >> if (!strcmp(p, "fsuuid")) > >> >> evm_hmac_attrs |= EVM_HMAC_ATTR_FSUUID; > >> >> } > >> >> pr_info("HMAC attrs: 0x%x\n", evm_hmac_attrs); > >> >> + while ((p = strsep(&xattrs, ", \t"))) { > >> >> +#ifndef CONFIG_SECURITY_SELINUX > >> >> + if (!strcmp(p, XATTR_NAME_SELINUX)) > >> >> + continue; > >> >> +#endif > >> >> +#ifndef CONFIG_SECURITY_SMACK > >> >> + if (strstr(p, "SMACK64")) > >> >> + continue; > >> >> +#endif > >> > As you mentioned, filesystems previously only included > >> > 'security.smack64' in the HMAC calculation. This patch includes all > >> > xattrs prefixed with smack64. All previously labeled filesystems would > >> > need to be relabeled. > >> > > >> > Mimi > >> > >> Only the SMACK64 attribute is assigned to all files. The SMACK64EXEC > >> and SMACK64TRANSMUTE attributes are optional. You do not want these > >> attributes in most cases. Few files should actually have them. They > >> should only be used if they are there, and ignored otherwise. If that > >> is not possible it is better to ignore them completely. > > > > Going forward the code would work just fine. All smack64 prefixed > > labels would be included in the HMAC calculation. My concern is for > > existing labeled filesystems, which only included the SMACK64 label in > > the HMAC calculation, but have other SMACK64 labels. The HMAC > > verification would fail for these files. > > > > Hello, > > There is no problem for existing labeled filesystems. > > Patch does not work with SMACK64 prefix, but with exact name. > > Only xattrs listed in the configuration options are included > By default "security.selinux, security.SMACK64, security.ima, > security.capability" > which correspond to EVM current functionality... > > Existing systems using new kernel may stick to original EVM xattr set. > > But new systems might prefer to add "more" xattrs.. > > This patch makes it flexible... Ok, the list of xattrs included in the HMAC calculation is build time configurable and runtime verified. thanks, Mimi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/