Messages in this thread | | | Date | Mon, 18 Nov 2013 17:30:19 +0200 | Subject | Re: [PATCH 5/6] ima: do not include field length in template digest calc for ima template | From | Dmitry Kasatkin <> |
| |
On Fri, Nov 15, 2013 at 3:45 PM, Roberto Sassu <roberto.sassu@polito.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@polito.it> > Signed-off-by: Mimi Zohar <zohar@linux.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
| |