[PATCH 0/3] ima-evm-utils: miscellanous bug fixes

[PATCH 0/3] ima-evm-utils: miscellanous bug fixes

[Bruno Meneguele]

While testing in RHEL7 the latest 'next-testing' branch changes the build
failed due to an "out" label being placed at the end of the function
calc_bootaggr() with no instructions for systems with OpenSSL version less
then 1.1. Corrected it by putting a simple no-op 'return' there (the
function returns nothing).

The other bugs are a simple memory leak, also on calc_bootaggr(), when
_DigestUpdate() returns error; and an overflow while reading the
boot_aggregate buffer due to the lack of the null char at the end.

Bruno Meneguele (3):
  ima-evm-utils: fix empty label at end of function.
  ima-evm-utils: fix memory leak in case of error
  ima-evm-utils: fix overflow on printing boot_aggregate

 src/evmctl.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

-- 
2.26.2

[PATCH 1/3] ima-evm-utils: fix empty label at end of function.

[Bruno Meneguele]

Distros running older OpenSSL versions (<= 1.1) fail to build due to the
empty label at the end of calc_bootaggr(). For these, that label is no-op.

Signed-off-by: Bruno Meneguele <bmeneg@redhat.com>
---
 src/evmctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/evmctl.c b/src/evmctl.c
index 90a3eeb..d974ba6 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -2167,7 +2167,7 @@ out:
 #if OPENSSL_VERSION_NUMBER >= 0x10100000
 	EVP_MD_CTX_free(pctx);
 #endif
-
+	return;
 }
 
 /*
-- 
2.26.2

[PATCH 2/3] ima-evm-utils: fix memory leak in case of error

[Bruno Meneguele]

OpenSSL context should be freed in case of versions >= 1.1 before leaving
the function in case EVP_DigestUpdate() returns any error.

Signed-off-by: Bruno Meneguele <bmeneg@redhat.com>
---
 src/evmctl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/evmctl.c b/src/evmctl.c
index d974ba6..2f5bd52 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -2143,7 +2143,7 @@ static void calc_bootaggr(struct tpm_bank_info *bank)
 		err = EVP_DigestUpdate(pctx, bank->pcr[i], bank->digest_size);
 		if (!err) {
 			log_err("EVP_DigestUpdate() failed\n");
-			return;
+			goto out;
 		}
 	}
 
@@ -2152,7 +2152,7 @@ static void calc_bootaggr(struct tpm_bank_info *bank)
 			err = EVP_DigestUpdate(pctx, bank->pcr[i], bank->digest_size);
 			if (!err) {
 				log_err("EVP_DigestUpdate() failed\n");
-				return;
+				goto out;
 			}
 		}
 	}
-- 
2.26.2

[PATCH 3/3] ima-evm-utils: fix overflow on printing boot_aggregate

[Bruno Meneguele]

There was no room for placing the '\0' at the end of boot_aggregate value,
thus printf() was reading 1 byte beyond the array limit.

Signed-off-by: Bruno Meneguele <bmeneg@redhat.com>
---
 src/evmctl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/evmctl.c b/src/evmctl.c
index 2f5bd52..2bd37c2 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -2252,7 +2252,8 @@ static int cmd_ima_bootaggr(struct command *cmd)
 		bootaggr_len += strlen(tpm_banks[i].algo_name) + 1;
 		bootaggr_len += (tpm_banks[i].digest_size * 2) + 1;
 	}
-	bootaggr = malloc(bootaggr_len);
+	/* Make room for the leading \0 */
+	bootaggr = malloc(bootaggr_len + 1);
 
 	/*
 	 * Calculate and convert the per TPM 2.0 PCR bank algorithm
@@ -2266,6 +2267,7 @@ static int cmd_ima_bootaggr(struct command *cmd)
 		calc_bootaggr(&tpm_banks[i]);
 		offset += append_bootaggr(bootaggr + offset, tpm_banks + i);
 	}
+	bootaggr[bootaggr_len] = '\0';
 	printf("%s", bootaggr);
 	free(bootaggr);
 	return 0;
-- 
2.26.2

Re: [PATCH 3/3] ima-evm-utils: fix overflow on printing boot_aggregate

[Mimi Zohar]

On Wed, 2020-07-15 at 18:39 -0300, Bruno Meneguele wrote:
> diff --git a/src/evmctl.c b/src/evmctl.c
> index 2f5bd52..2bd37c2 100644
> --- a/src/evmctl.c
> +++ b/src/evmctl.c
> @@ -2252,7 +2252,8 @@ static int cmd_ima_bootaggr(struct command *cmd)
>  		bootaggr_len += strlen(tpm_banks[i].algo_name) + 1;
>  		bootaggr_len += (tpm_banks[i].digest_size * 2) + 1;
>  	}
> -	bootaggr = malloc(bootaggr_len);
> +	/* Make room for the leading \0 */

^Trailing null

Mimi

> +	bootaggr = malloc(bootaggr_len + 1);
>  
>  	/*
>  	 * Calculate and convert the per TPM 2.0 PCR bank algorithm

Re: [PATCH 3/3] ima-evm-utils: fix overflow on printing boot_aggregate

[Bruno Meneguele]

[-- Attachment #1: Type: text/plain, Size: 800 bytes --]

On Wed, Jul 15, 2020 at 06:30:07PM -0400, Mimi Zohar wrote:
> On Wed, 2020-07-15 at 18:39 -0300, Bruno Meneguele wrote:
> > diff --git a/src/evmctl.c b/src/evmctl.c
> > index 2f5bd52..2bd37c2 100644
> > --- a/src/evmctl.c
> > +++ b/src/evmctl.c
> > @@ -2252,7 +2252,8 @@ static int cmd_ima_bootaggr(struct command *cmd)
> >  		bootaggr_len += strlen(tpm_banks[i].algo_name) + 1;
> >  		bootaggr_len += (tpm_banks[i].digest_size * 2) + 1;
> >  	}
> > -	bootaggr = malloc(bootaggr_len);
> > +	/* Make room for the leading \0 */
> 
> ^Trailing null
> 

hahah.. of course.

Thanks :)

> Mimi
> 
> > +	bootaggr = malloc(bootaggr_len + 1);
> >  
> >  	/*
> >  	 * Calculate and convert the per TPM 2.0 PCR bank algorithm
> 

-- 
bmeneg 
PGP Key: http://bmeneg.com/pubkey.txt

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 0/3] ima-evm-utils: miscellanous bug fixes

[Mimi Zohar]

On Wed, 2020-07-15 at 18:39 -0300, Bruno Meneguele wrote:
> While testing in RHEL7 the latest 'next-testing' branch changes the build
> failed due to an "out" label being placed at the end of the function
> calc_bootaggr() with no instructions for systems with OpenSSL version less
> then 1.1. Corrected it by putting a simple no-op 'return' there (the
> function returns nothing).
> 
> The other bugs are a simple memory leak, also on calc_bootaggr(), when
> _DigestUpdate() returns error; and an overflow while reading the
> boot_aggregate buffer due to the lack of the null char at the end.

Applied, thanks!

Mimi