From: Mimi Zohar <zohar@linux.ibm.com>
To: Thiago Jung Bauermann <bauerman@linux.ibm.com>,
linux-integrity@vger.kernel.org
Cc: linux-security-module@vger.kernel.org, keyrings@vger.kernel.org,
linux-crypto@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
Dmitry Kasatkin <dmitry.kasatkin@gmail.com>,
James Morris <jmorris@namei.org>,
"Serge E. Hallyn" <serge@hallyn.com>,
David Howells <dhowells@redhat.com>,
David Woodhouse <dwmw2@infradead.org>,
Jessica Yu <jeyu@kernel.org>,
Herbert Xu <herbert@gondor.apana.org.au>,
"David S. Miller" <davem@davemloft.net>,
Jonathan Corbet <corbet@lwn.net>,
"AKASHI, Takahiro" <takahiro.akashi@linaro.org>
Subject: Re: [PATCH v8 04/14] integrity: Introduce struct evm_xattr
Date: Thu, 29 Nov 2018 15:59:12 -0500 [thread overview]
Message-ID: <1543525152.3902.411.camel@linux.ibm.com> (raw)
In-Reply-To: <20181116200712.14154-5-bauerman@linux.ibm.com>
On Fri, 2018-11-16 at 18:07 -0200, Thiago Jung Bauermann wrote:
> Even though struct evm_ima_xattr_data includes a fixed-size array to hold a
> SHA1 digest, most of the code ignores the array and uses the struct to mean
> "type indicator followed by data of unspecified size" and tracks the real
> size of what the struct represents in a separate length variable.
>
> The only exception to that is the EVM code, which correctly uses the
> definition of struct evm_ima_xattr_data.
>
> So make this explicit in the code by removing the length specification from
> the array in struct evm_ima_xattr_data. Also, change the name of the
> element from digest to data since in most places the array doesn't hold a
> digest.
>
> A separate struct evm_xattr is introduced, with the original definition of
> evm_ima_xattr_data to be used in the places that actually expect that
> definition.
, specifically the EVM HMAC code.
>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Other than commenting the evm_xattr usage is limited to HMAC before
the structure definition, this looks good.
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> ---
> security/integrity/evm/evm_main.c | 8 ++++----
> security/integrity/ima/ima_appraise.c | 7 ++++---
> security/integrity/integrity.h | 5 +++++
> 3 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index 7f3f54d89a6e..a1b42d10efc7 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -169,7 +169,7 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
> /* check value type */
> switch (xattr_data->type) {
> case EVM_XATTR_HMAC:
> - if (xattr_len != sizeof(struct evm_ima_xattr_data)) {
> + if (xattr_len != sizeof(struct evm_xattr)) {
> evm_status = INTEGRITY_FAIL;
> goto out;
> }
> @@ -179,7 +179,7 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
> xattr_value_len, &digest);
> if (rc)
> break;
> - rc = crypto_memneq(xattr_data->digest, digest.digest,
> + rc = crypto_memneq(xattr_data->data, digest.digest,
> SHA1_DIGEST_SIZE);
> if (rc)
> rc = -EINVAL;
> @@ -523,7 +523,7 @@ int evm_inode_init_security(struct inode *inode,
> const struct xattr *lsm_xattr,
> struct xattr *evm_xattr)
> {
> - struct evm_ima_xattr_data *xattr_data;
> + struct evm_xattr *xattr_data;
> int rc;
>
> if (!evm_key_loaded() || !evm_protected_xattr(lsm_xattr->name))
> @@ -533,7 +533,7 @@ int evm_inode_init_security(struct inode *inode,
> if (!xattr_data)
> return -ENOMEM;
>
> - xattr_data->type = EVM_XATTR_HMAC;
> + xattr_data->data.type = EVM_XATTR_HMAC;
> rc = evm_init_hmac(inode, lsm_xattr, xattr_data->digest);
> if (rc < 0)
> goto out;
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index deec1804a00a..8bcef90939f8 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -167,7 +167,8 @@ enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value,
> return sig->hash_algo;
> break;
> case IMA_XATTR_DIGEST_NG:
> - ret = xattr_value->digest[0];
> + /* first byte contains algorithm id */
> + ret = xattr_value->data[0];
> if (ret < HASH_ALGO__LAST)
> return ret;
> break;
> @@ -175,7 +176,7 @@ enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value,
> /* this is for backward compatibility */
> if (xattr_len == 21) {
> unsigned int zero = 0;
> - if (!memcmp(&xattr_value->digest[16], &zero, 4))
> + if (!memcmp(&xattr_value->data[16], &zero, 4))
> return HASH_ALGO_MD5;
> else
> return HASH_ALGO_SHA1;
> @@ -274,7 +275,7 @@ int ima_appraise_measurement(enum ima_hooks func,
> /* xattr length may be longer. md5 hash in previous
> version occupied 20 bytes in xattr, instead of 16
> */
> - rc = memcmp(&xattr_value->digest[hash_start],
> + rc = memcmp(&xattr_value->data[hash_start],
> iint->ima_hash->digest,
> iint->ima_hash->length);
> else
> diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
> index e60473b13a8d..20ac02bf1b84 100644
> --- a/security/integrity/integrity.h
> +++ b/security/integrity/integrity.h
> @@ -79,6 +79,11 @@ enum evm_ima_xattr_type {
>
> struct evm_ima_xattr_data {
> u8 type;
> + u8 data[];
> +} __packed;
> +
Please add a comment here saying that evm_xattr is limited to HMAC.
> +struct evm_xattr {
> + struct evm_ima_xattr_data data;
> u8 digest[SHA1_DIGEST_SIZE];
> } __packed;
>
next prev parent reply other threads:[~2018-11-29 20:59 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-16 20:06 [PATCH v8 00/14] Appended signatures support for IMA appraisal Thiago Jung Bauermann
2018-11-16 20:06 ` [PATCH v8 01/14] MODSIGN: Export module signature definitions Thiago Jung Bauermann
2018-11-16 20:07 ` [PATCH v8 02/14] PKCS#7: Refactor verify_pkcs7_signature() and add pkcs7_get_message_sig() Thiago Jung Bauermann
2018-11-16 20:07 ` [PATCH v8 03/14] PKCS#7: Introduce pkcs7_get_digest() Thiago Jung Bauermann
2018-11-16 20:07 ` [PATCH v8 04/14] integrity: Introduce struct evm_xattr Thiago Jung Bauermann
2018-11-29 20:59 ` Mimi Zohar [this message]
2018-11-16 20:07 ` [PATCH v8 05/14] integrity: Introduce integrity_keyring_from_id() Thiago Jung Bauermann
2018-11-16 20:07 ` [PATCH v8 06/14] integrity: Introduce asymmetric_sig_has_known_key() Thiago Jung Bauermann
2018-11-16 20:07 ` [PATCH v8 07/14] integrity: Select CONFIG_KEYS instead of depending on it Thiago Jung Bauermann
2018-11-16 20:07 ` [PATCH v8 08/14] ima: Introduce is_signed() Thiago Jung Bauermann
2018-11-16 20:07 ` [PATCH v8 09/14] ima: Export func_tokens Thiago Jung Bauermann
2018-11-16 20:07 ` [PATCH v8 10/14] ima: Add modsig appraise_type option for module-style appended signatures Thiago Jung Bauermann
2018-11-16 20:07 ` [PATCH v8 11/14] ima: Implement support " Thiago Jung Bauermann
2018-11-16 20:07 ` [PATCH v8 12/14] ima: Add new "d-sig" template field Thiago Jung Bauermann
2018-11-16 20:07 ` [PATCH v8 13/14] ima: Write modsig to the measurement list Thiago Jung Bauermann
2018-11-16 20:07 ` [PATCH v8 14/14] ima: Store the measurement again when appraising a modsig Thiago Jung Bauermann
2018-12-04 21:59 ` [PATCH v8 00/14] Appended signatures support for IMA appraisal James Morris
2018-12-04 23:35 ` Thiago Jung Bauermann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1543525152.3902.411.camel@linux.ibm.com \
--to=zohar@linux.ibm.com \
--cc=bauerman@linux.ibm.com \
--cc=corbet@lwn.net \
--cc=davem@davemloft.net \
--cc=dhowells@redhat.com \
--cc=dmitry.kasatkin@gmail.com \
--cc=dwmw2@infradead.org \
--cc=herbert@gondor.apana.org.au \
--cc=jeyu@kernel.org \
--cc=jmorris@namei.org \
--cc=keyrings@vger.kernel.org \
--cc=linux-crypto@vger.kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-integrity@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=serge@hallyn.com \
--cc=takahiro.akashi@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).