From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Serge E. Hallyn" Subject: Re: [PATCH v6 04/20] evm: add support for different security.evm data types Date: Thu, 2 Jun 2011 17:50:06 -0500 Message-ID: <20110602225005.GB23700@mail.hallyn.com> References: <1307017423-15093-1-git-send-email-zohar@linux.vnet.ibm.com> <1307017423-15093-5-git-send-email-zohar@linux.vnet.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: linux-security-module@vger.kernel.org, Dmitry Kasatkin , linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, James Morris , David Safford , Andrew Morton , Greg KH , Dmitry Kasatkin , Mimi Zohar To: Mimi Zohar Return-path: Content-Disposition: inline In-Reply-To: <1307017423-15093-5-git-send-email-zohar@linux.vnet.ibm.com> Sender: linux-kernel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org Quoting Mimi Zohar (zohar@linux.vnet.ibm.com): > From: Dmitry Kasatkin > > EVM protects a file's security extended attributes(xattrs) against integrity > attacks. The current patchset maintains an HMAC-sha1 value across the security > xattrs, storing the value as the extended attribute 'security.evm'. We > anticipate other methods for protecting the security extended attributes. > This patch reserves the first byte of 'security.evm' as a place holder for > the type of method. > > Changelog v6: > - move evm_ima_xattr_type definition to security/integrity/integrity.h > - defined a structure for the EVM xattr called evm_ima_xattr_data > (based on Serge Hallyn's suggestion) > > Signed-off-by: Dmitry Kasatkin > Signed-off-by: Mimi Zohar > --- > include/linux/integrity.h | 1 + > security/integrity/evm/evm_crypto.c | 11 +++++++---- > security/integrity/evm/evm_main.c | 10 +++++----- > security/integrity/integrity.h | 11 +++++++++++ > 4 files changed, 24 insertions(+), 9 deletions(-) > > diff --git a/include/linux/integrity.h b/include/linux/integrity.h > index e715a2a..9684433 100644 > --- a/include/linux/integrity.h > +++ b/include/linux/integrity.h > @@ -19,6 +19,7 @@ enum integrity_status { > INTEGRITY_UNKNOWN, > }; > > +/* List of EVM protected security xattrs */ > #ifdef CONFIG_INTEGRITY > extern int integrity_inode_alloc(struct inode *inode); > extern void integrity_inode_free(struct inode *inode); > diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c > index d49bb00..c631b99 100644 > --- a/security/integrity/evm/evm_crypto.c > +++ b/security/integrity/evm/evm_crypto.c > @@ -141,14 +141,17 @@ int evm_update_evmxattr(struct dentry *dentry, const char *xattr_name, > const char *xattr_value, size_t xattr_value_len) > { > struct inode *inode = dentry->d_inode; > - u8 hmac[SHA1_DIGEST_SIZE]; > + struct evm_ima_xattr_data xattr_data; > int rc = 0; > > rc = evm_calc_hmac(dentry, xattr_name, xattr_value, > - xattr_value_len, hmac); > - if (rc == 0) > + xattr_value_len, xattr_data.digest); > + if (rc == 0) { > + xattr_data.type = EVM_XATTR_HMAC; > rc = __vfs_setxattr_noperm(dentry, XATTR_NAME_EVM, > - hmac, SHA1_DIGEST_SIZE, 0); > + &xattr_data, > + sizeof(xattr_data), 0); > + } > else if (rc == -ENODATA) > rc = inode->i_op->removexattr(dentry, XATTR_NAME_EVM); > return rc; > diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c > index a8fa45f..c0580dd1 100644 > --- a/security/integrity/evm/evm_main.c > +++ b/security/integrity/evm/evm_main.c > @@ -51,20 +51,20 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry, > size_t xattr_value_len, > struct integrity_iint_cache *iint) > { > - char hmac_val[SHA1_DIGEST_SIZE]; > + struct evm_ima_xattr_data xattr_data; > int rc; > > if (iint->hmac_status != INTEGRITY_UNKNOWN) > return iint->hmac_status; > > - memset(hmac_val, 0, sizeof hmac_val); Why did you drop the memset here? (You didn't in the previous version of this patch) Otherwise, looks good. Acked-by: Serge Hallyn > rc = evm_calc_hmac(dentry, xattr_name, xattr_value, > - xattr_value_len, hmac_val); > + xattr_value_len, xattr_data.digest); > if (rc < 0) > return INTEGRITY_UNKNOWN; > > - rc = vfs_xattr_cmp(dentry, XATTR_NAME_EVM, hmac_val, sizeof hmac_val, > - GFP_NOFS); > + xattr_data.type = EVM_XATTR_HMAC; > + rc = vfs_xattr_cmp(dentry, XATTR_NAME_EVM, (u8 *)&xattr_data, > + sizeof xattr_data, GFP_NOFS); > if (rc < 0) > goto err_out; > iint->hmac_status = INTEGRITY_PASS; > diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h > index 397a46b..7efbf56 100644 > --- a/security/integrity/integrity.h > +++ b/security/integrity/integrity.h > @@ -18,6 +18,17 @@ > /* iint cache flags */ > #define IMA_MEASURED 0x01 > > +enum evm_ima_xattr_type { > + IMA_XATTR_DIGEST = 0x01, > + EVM_XATTR_HMAC, > + EVM_IMA_XATTR_DIGSIG, > +}; > + > +struct evm_ima_xattr_data { > + u8 type; > + u8 digest[SHA1_DIGEST_SIZE]; > +} __attribute__((packed)); > + > /* integrity data associated with an inode */ > struct integrity_iint_cache { > struct rb_node rb_node; /* rooted in integrity_iint_tree */ > -- > 1.7.3.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/