| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1393965405.10148.112.camel@dhcp-9-2-203-236.watson.ibm.com>
Date: Tue, 04 Mar 2014 15:36:45 -0500
From: Mimi Zohar <zohar@...ux.vnet.ibm.com>
To: Dmitry Kasatkin <dmitry.kasatkin@...il.com>
Cc: Casey Schaufler <casey@...aufler-ca.com>,
Dmitry Kasatkin <d.kasatkin@...sung.com>,
linux-security-module@...r.kernel.org,
James Morris <jmorris@...ei.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 8/8] evm: introduce EVM hmac xattr list
On Tue, 2014-03-04 at 16:18 +0200, Dmitry Kasatkin wrote:
> On Tue, Mar 4, 2014 at 5:21 AM, Mimi Zohar <zohar@...ux.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.kasatkin@...sung.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 majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists