| 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
| ||
|
Date: Mon, 18 Nov 2013 17:30:19 +0200
From: Dmitry Kasatkin <dmitry.kasatkin@...il.com>
To: Roberto Sassu <roberto.sassu@...ito.it>
Cc: linux-security-module@...r.kernel.org,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
linux-ima-devel@...ts.sourceforge.net,
Mimi Zohar <zohar@...ibm.com>,
Dmitry Kasatkin <d.kasatkin@...sung.com>,
james.l.morris@...cle.com, Mimi Zohar <zohar@...ux.vnet.ibm.com>
Subject: Re: [PATCH 5/6] ima: do not include field length in template digest
calc for ima template
On Fri, Nov 15, 2013 at 3:45 PM, Roberto Sassu <roberto.sassu@...ito.it> wrote:
> To maintain compatibility with userspace tools, the field length must not
> be included in the template digest calculation for the 'ima' template.
>
> Signed-off-by: Roberto Sassu <roberto.sassu@...ito.it>
> Signed-off-by: Mimi Zohar <zohar@...ux.vnet.ibm.com>
> ---
> security/integrity/ima/ima.h | 3 ++-
> security/integrity/ima/ima_api.c | 1 +
> security/integrity/ima/ima_crypto.c | 20 ++++++++++++--------
> 3 files changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index bf03c6a..a21cf70 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -97,7 +97,8 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
> const char *op, struct inode *inode,
> const unsigned char *filename);
> int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash);
> -int ima_calc_field_array_hash(struct ima_field_data *field_data, int num_fields,
> +int ima_calc_field_array_hash(struct ima_field_data *field_data,
> + struct ima_template_desc *desc, int num_fields,
> struct ima_digest_data *hash);
> int __init ima_calc_boot_aggregate(struct ima_digest_data *hash);
> void ima_add_violation(struct file *file, const unsigned char *filename,
> diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> index 0e75408..8037484 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -94,6 +94,7 @@ int ima_store_template(struct ima_template_entry *entry,
> /* this function uses default algo */
> hash.hdr.algo = HASH_ALGO_SHA1;
> result = ima_calc_field_array_hash(&entry->template_data[0],
> + entry->template_desc,
> num_fields, &hash.hdr);
> if (result < 0) {
> integrity_audit_msg(AUDIT_INTEGRITY_PCR, inode,
> diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
> index e22708b..fdf60de 100644
> --- a/security/integrity/ima/ima_crypto.c
> +++ b/security/integrity/ima/ima_crypto.c
> @@ -140,6 +140,7 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash)
> * Calculate the hash of template data
> */
> static int ima_calc_field_array_hash_tfm(struct ima_field_data *field_data,
> + struct ima_template_desc *td,
> int num_fields,
> struct ima_digest_data *hash,
> struct crypto_shash *tfm)
> @@ -160,12 +161,13 @@ static int ima_calc_field_array_hash_tfm(struct ima_field_data *field_data,
> return rc;
>
> for (i = 0; i < num_fields; i++) {
> - rc = crypto_shash_update(&desc.shash,
> - (const u8 *) &field_data[i].len,
> - sizeof(field_data[i].len));
> - if (rc)
> - break;
> -
> + if (strcmp(td->name, IMA_TEMPLATE_IMA_NAME) != 0) {
> + rc = crypto_shash_update(&desc.shash,
> + (const u8 *) &field_data[i].len,
> + sizeof(field_data[i].len));
> + if (rc)
> + break;
> + }
What was actually the point in including field length in the hash calculation?
Does it really make it cryptographically stronger?
If not then remove it at all...
- Dmitry
> rc = crypto_shash_update(&desc.shash, field_data[i].data,
> field_data[i].len);
> if (rc)
> @@ -178,7 +180,8 @@ static int ima_calc_field_array_hash_tfm(struct ima_field_data *field_data,
> return rc;
> }
>
> -int ima_calc_field_array_hash(struct ima_field_data *field_data, int num_fields,
> +int ima_calc_field_array_hash(struct ima_field_data *field_data,
> + struct ima_template_desc *desc, int num_fields,
> struct ima_digest_data *hash)
> {
> struct crypto_shash *tfm;
> @@ -188,7 +191,8 @@ int ima_calc_field_array_hash(struct ima_field_data *field_data, int num_fields,
> if (IS_ERR(tfm))
> return PTR_ERR(tfm);
>
> - rc = ima_calc_field_array_hash_tfm(field_data, num_fields, hash, tfm);
> + rc = ima_calc_field_array_hash_tfm(field_data, desc, num_fields,
> + hash, tfm);
>
> ima_free_tfm(tfm);
>
> --
> 1.8.1.4
>
--
Thanks,
Dmitry
--
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