[PATCH ima-evm-utils] Add sanity check for file parameter of ima_boot_aggregate

[PATCH ima-evm-utils] Add sanity check for file parameter of ima_boot_aggregate

[Petr Vorel]

Parameter expects to be a copy of
/sys/kernel/security/tpm0/binary_bios_measurements (i.e. regular file,
not a directory, block or character device, socket, ...)

Fixes: f49e982 ("ima-evm-utils: read the TPM 1.2 binary_bios_measurements")

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Hi Mimi,

feel free to modify this patchset to fits your needs (unless I'm wrong
and this should not be applied at all).

Kind regards,
Petr

 src/evmctl.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/evmctl.c b/src/evmctl.c
index 04dc2ad..3ad5039 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -2082,6 +2082,13 @@ static int read_binary_bios_measurements(char *file, struct tpm_bank_info *bank)
 	int len;
 	int i;
 
+	struct stat s;
+	stat(file, &s);
+	if (!S_ISREG(s.st_mode)) {
+		log_errno("Not a regular file or link to regular file.\n");
+		return 1;
+	}
+
 	fp = fopen(file, "r");
 	if (!fp) {
 		log_errno("Failed to open TPM 1.2 event log.\n");
-- 
2.27.0.rc0

Re: [PATCH ima-evm-utils] Add sanity check for file parameter of ima_boot_aggregate

[Mimi Zohar]

On Fri, 2020-07-17 at 14:04 +0200, Petr Vorel wrote:
> Parameter expects to be a copy of
> /sys/kernel/security/tpm0/binary_bios_measurements (i.e. regular file,
> not a directory, block or character device, socket, ...)
> 
> Fixes: f49e982 ("ima-evm-utils: read the TPM 1.2 binary_bios_measurements")
> 
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> Hi Mimi,
> 
> feel free to modify this patchset to fits your needs (unless I'm wrong
> and this should not be applied at all).
> 

Thanks!  I made minor changes as noted below.  A subsequent patch
makes a similar change for the new TPM 1.2 PCRs.

> 
>  src/evmctl.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/src/evmctl.c b/src/evmctl.c
> index 04dc2ad..3ad5039 100644
> --- a/src/evmctl.c
> +++ b/src/evmctl.c
> @@ -2082,6 +2082,13 @@ static int read_binary_bios_measurements(char *file, struct tpm_bank_info *bank)
>  	int len;
>  	int i;
> 
> +	struct stat s;
> +	stat(file, &s);

Checked stat return code.


> +	if (!S_ISREG(s.st_mode)) {
> +		log_errno("Not a regular file or link to regular file.\n");

Prefixed message with "Bios event log: not ..."

> +		return 1;
> +	}
> +
>  	fp = fopen(file, "r");
>  	if (!fp) {
>  		log_errno("Failed to open TPM 1.2 event log.\n");

Re: [PATCH ima-evm-utils] Add sanity check for file parameter of ima_boot_aggregate

[Petr Vorel]

Hi Mimi,

...
> Thanks!  I made minor changes as noted below.  A subsequent patch
> makes a similar change for the new TPM 1.2 PCRs.
+1 to all the changes. I guess you haven't pushed it yet.


Kind regards,
Petr

> >  src/evmctl.c | 7 +++++++
> >  1 file changed, 7 insertions(+)

> > diff --git a/src/evmctl.c b/src/evmctl.c
> > index 04dc2ad..3ad5039 100644
> > --- a/src/evmctl.c
> > +++ b/src/evmctl.c
> > @@ -2082,6 +2082,13 @@ static int read_binary_bios_measurements(char *file, struct tpm_bank_info *bank)
> >  	int len;
> >  	int i;

> > +	struct stat s;
> > +	stat(file, &s);

> Checked stat return code.


> > +	if (!S_ISREG(s.st_mode)) {
> > +		log_errno("Not a regular file or link to regular file.\n");

> Prefixed message with "Bios event log: not ..."
...

Re: [PATCH ima-evm-utils] Add sanity check for file parameter of ima_boot_aggregate

[Mimi Zohar]

On Mon, 2020-07-20 at 10:00 +0200, Petr Vorel wrote:
> Hi Mimi,
> 
> ...
> > Thanks!  I made minor changes as noted below.  A subsequent patch
> > makes a similar change for the new TPM 1.2 PCRs.
> +1 to all the changes. I guess you haven't pushed it yet.

Thank you for the reviews.  Everything is there now.

Mimi