If the template field 'd' is chosen and the digest to be added to the
measurement entry was not calculated with SHA1 or MD5, it is
recalculated with SHA1, by using the passed file descriptor. However, this
cannot be done for boot_aggregate, because there is no file descriptor.
This patch adds a call to ima_calc_boot_aggregate() in
ima_eventdigest_init(), so that the digest can be recalculated also for the
boot_aggregate entry.
Cc: [email protected] # 3.13.x
Fixes: 3ce1217d6cd5d ("ima: define template fields library and new helpers")
Reported-by: Takashi Iwai <[email protected]>
Signed-off-by: Roberto Sassu <[email protected]>
---
security/integrity/ima/ima.h | 3 ++-
security/integrity/ima/ima_crypto.c | 6 +++---
security/integrity/ima/ima_init.c | 2 +-
security/integrity/ima/ima_template_lib.c | 18 ++++++++++++++++++
4 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 02796473238b..df93ac258e01 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -57,6 +57,7 @@ extern int ima_hash_algo_idx __ro_after_init;
extern int ima_extra_slots __ro_after_init;
extern int ima_appraise;
extern struct tpm_chip *ima_tpm_chip;
+extern const char boot_aggregate_name[];
/* IMA event related data */
struct ima_event_data {
@@ -144,7 +145,7 @@ int ima_calc_buffer_hash(const void *buf, loff_t len,
struct ima_digest_data *hash);
int ima_calc_field_array_hash(struct ima_field_data *field_data,
struct ima_template_entry *entry);
-int __init ima_calc_boot_aggregate(struct ima_digest_data *hash);
+int ima_calc_boot_aggregate(struct ima_digest_data *hash);
void ima_add_violation(struct file *file, const unsigned char *filename,
struct integrity_iint_cache *iint,
const char *op, const char *cause);
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index f3a7f4eb1fc1..ba5cc3264240 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -806,8 +806,8 @@ static void __init ima_pcrread(u32 idx, struct tpm_digest *d)
* hash algorithm for reading the TPM PCRs as for calculating the boot
* aggregate digest as stored in the measurement list.
*/
-static int __init ima_calc_boot_aggregate_tfm(char *digest, u16 alg_id,
- struct crypto_shash *tfm)
+static int ima_calc_boot_aggregate_tfm(char *digest, u16 alg_id,
+ struct crypto_shash *tfm)
{
struct tpm_digest d = { .alg_id = alg_id, .digest = {0} };
int rc;
@@ -835,7 +835,7 @@ static int __init ima_calc_boot_aggregate_tfm(char *digest, u16 alg_id,
return rc;
}
-int __init ima_calc_boot_aggregate(struct ima_digest_data *hash)
+int ima_calc_boot_aggregate(struct ima_digest_data *hash)
{
struct crypto_shash *tfm;
u16 crypto_id, alg_id;
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index fc1e1002b48d..4902fe7bd570 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -19,7 +19,7 @@
#include "ima.h"
/* name for boot aggregate entry */
-static const char boot_aggregate_name[] = "boot_aggregate";
+const char boot_aggregate_name[] = "boot_aggregate";
struct tpm_chip *ima_tpm_chip;
/* Add the boot aggregate to the IMA measurement list and extend
diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index 9cd1e50f3ccc..635c6ac05050 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -286,6 +286,24 @@ int ima_eventdigest_init(struct ima_event_data *event_data,
goto out;
}
+ if ((const char *)event_data->filename == boot_aggregate_name) {
+ if (ima_tpm_chip) {
+ hash.hdr.algo = HASH_ALGO_SHA1;
+ result = ima_calc_boot_aggregate(&hash.hdr);
+
+ /* algo can change depending on available PCR banks */
+ if (!result && hash.hdr.algo != HASH_ALGO_SHA1)
+ result = -EINVAL;
+
+ if (result < 0)
+ memset(&hash, 0, sizeof(hash));
+ }
+
+ cur_digest = hash.hdr.digest;
+ cur_digestsize = hash_digest_size[HASH_ALGO_SHA1];
+ goto out;
+ }
+
if (!event_data->file) /* missing info to re-calculate the digest */
return -EINVAL;
--
2.17.1
Hi Roberto,
On Wed, 2020-06-03 at 17:08 +0200, Roberto Sassu wrote:
> If the template field 'd' is chosen and the digest to be added to the
> measurement entry was not calculated with SHA1 or MD5, it is
> recalculated with SHA1, by using the passed file descriptor. However, this
> cannot be done for boot_aggregate, because there is no file descriptor.
>
> This patch adds a call to ima_calc_boot_aggregate() in
> ima_eventdigest_init(), so that the digest can be recalculated also for the
> boot_aggregate entry.
>
> Cc: [email protected] # 3.13.x
> Fixes: 3ce1217d6cd5d ("ima: define template fields library and new helpers")
> Reported-by: Takashi Iwai <[email protected]>
> Signed-off-by: Roberto Sassu <[email protected]>
Thanks, Roberto.
I've pushed both patches out to the next-integrity branch and would
appreciate some additional testing.
thanks,
Mimi
On Wed, Jun 03, 2020 at 06:03:35PM -0400, Mimi Zohar wrote:
> Hi Roberto,
>
> On Wed, 2020-06-03 at 17:08 +0200, Roberto Sassu wrote:
> > If the template field 'd' is chosen and the digest to be added to the
> > measurement entry was not calculated with SHA1 or MD5, it is
> > recalculated with SHA1, by using the passed file descriptor. However, this
> > cannot be done for boot_aggregate, because there is no file descriptor.
> >
> > This patch adds a call to ima_calc_boot_aggregate() in
> > ima_eventdigest_init(), so that the digest can be recalculated also for the
> > boot_aggregate entry.
> >
> > Cc: [email protected] # 3.13.x
> > Fixes: 3ce1217d6cd5d ("ima: define template fields library and new helpers")
> > Reported-by: Takashi Iwai <[email protected]>
> > Signed-off-by: Roberto Sassu <[email protected]>
>
> Thanks, Roberto.
>
> I've pushed both patches out to the next-integrity branch and would
> appreciate some additional testing.
>
> thanks,
>
> Mimi
>
Hi Mimi and Roberto,
FWIW, I've tested this patch manually and things went fine, with no
unexpected behavior or results.
However, wouldn't it be worth add a note in kmsg about the
ima_calc_boot_aggregate() being called with an algo different from the
system's default? Just to let the user know he won't find a sha256 when
check the measurement. But that's something we can add later too.
Thanks.
--
bmeneg
PGP Key: http://bmeneg.com/pubkey.txt
On Thu, 2020-06-04 at 16:12 -0300, Bruno Meneguele wrote:
> On Wed, Jun 03, 2020 at 06:03:35PM -0400, Mimi Zohar wrote:
> > Hi Roberto,
> >
> > On Wed, 2020-06-03 at 17:08 +0200, Roberto Sassu wrote:
> > > If the template field 'd' is chosen and the digest to be added to the
> > > measurement entry was not calculated with SHA1 or MD5, it is
> > > recalculated with SHA1, by using the passed file descriptor. However, this
> > > cannot be done for boot_aggregate, because there is no file descriptor.
> > >
> > > This patch adds a call to ima_calc_boot_aggregate() in
> > > ima_eventdigest_init(), so that the digest can be recalculated also for the
> > > boot_aggregate entry.
> > >
> > > Cc: [email protected] # 3.13.x
> > > Fixes: 3ce1217d6cd5d ("ima: define template fields library and new helpers")
> > > Reported-by: Takashi Iwai <[email protected]>
> > > Signed-off-by: Roberto Sassu <[email protected]>
> >
> > Thanks, Roberto.
> >
> > I've pushed both patches out to the next-integrity branch and would
> > appreciate some additional testing.
> >
> > thanks,
> >
> > Mimi
> >
>
> Hi Mimi and Roberto,
>
> FWIW, I've tested this patch manually and things went fine, with no
> unexpected behavior or results.
Thanks, Bruno!
> However, wouldn't it be worth add a note in kmsg about the
> ima_calc_boot_aggregate() being called with an algo different from the
> system's default? Just to let the user know he won't find a sha256 when
> check the measurement. But that's something we can add later too.
There's no guarantees that the IMA default crypto algorithm will be
used for calculating the boot_aggregate. The algorithm is dependent
on the TPM. For example, the default IMA algorithm could be sha256,
but on a system with TPM 1.2, the boot_aggregate would have to be
sha1.
This patch addresses a mismatch between the template format field ('d'
field) and the larger digest. We could require the "ima_template_fmt"
specified on the boot command line define an appropriate format, but
Roberto decided to support the situation where both "d" and "d-ng" are
defined.
Mimi
On Thu, Jun 04, 2020 at 03:35:20PM -0400, Mimi Zohar wrote:
> On Thu, 2020-06-04 at 16:12 -0300, Bruno Meneguele wrote:
> > On Wed, Jun 03, 2020 at 06:03:35PM -0400, Mimi Zohar wrote:
> > > Hi Roberto,
> > >
> > > On Wed, 2020-06-03 at 17:08 +0200, Roberto Sassu wrote:
> > > > If the template field 'd' is chosen and the digest to be added to the
> > > > measurement entry was not calculated with SHA1 or MD5, it is
> > > > recalculated with SHA1, by using the passed file descriptor. However, this
> > > > cannot be done for boot_aggregate, because there is no file descriptor.
> > > >
> > > > This patch adds a call to ima_calc_boot_aggregate() in
> > > > ima_eventdigest_init(), so that the digest can be recalculated also for the
> > > > boot_aggregate entry.
> > > >
> > > > Cc: [email protected] # 3.13.x
> > > > Fixes: 3ce1217d6cd5d ("ima: define template fields library and new helpers")
> > > > Reported-by: Takashi Iwai <[email protected]>
> > > > Signed-off-by: Roberto Sassu <[email protected]>
> > >
> > > Thanks, Roberto.
> > >
> > > I've pushed both patches out to the next-integrity branch and would
> > > appreciate some additional testing.
> > >
> > > thanks,
> > >
> > > Mimi
> > >
> >
> > Hi Mimi and Roberto,
> >
> > FWIW, I've tested this patch manually and things went fine, with no
> > unexpected behavior or results.
>
> Thanks, Bruno!
>
> > However, wouldn't it be worth add a note in kmsg about the
> > ima_calc_boot_aggregate() being called with an algo different from the
> > system's default? Just to let the user know he won't find a sha256 when
> > check the measurement. But that's something we can add later too.
>
> There's no guarantees that the IMA default crypto algorithm will be
> used for calculating the boot_aggregate. ?The algorithm is dependent
> on the TPM. ?For example, the default IMA algorithm could be sha256,
> but on a system with TPM 1.2, the boot_aggregate would have to be
> sha1.
Ah Indeed.. it makes sense.
>
> This patch addresses a mismatch between the template format field ('d'
> field) and the larger digest. ?We could require the "ima_template_fmt"
> specified on the boot command line define an appropriate format, but
> Roberto decided to support the situation where both "d" and "d-ng" are
> defined.
Yes, personally I also prefer to "fail earlier" or to be more stricter
on user definitions, but I also understand going the other way allowing
both d & d-ng.
>
> Mimi
>
thanks Mimi.
--
bmeneg
PGP Key: http://bmeneg.com/pubkey.txt