Re: [PATCH v5 03/21] evm: re-release

Mimi Zohar
2011-05-20T12:29:16Z

On Thu, 2011-05-19 at 16:37 -0500, Serge E. Hallyn wrote:
> Quoting Mimi Zohar (zohar@linux.vnet.ibm.com):
> ...
> > +extern int evm_hmac_size;
> ...
> > +int evm_hmac_size = SHA1_DIGEST_SIZE;
>
> I think I object to having both MAX_DIGEST_SIZE and evm_hmac_size, both
> of which are set to SHA1_DIGEST_SIZE throughout this patchset. Especially
> because of the comment I was about to make on patch 4/21, where you
> then prepend the hmac with a 'type' byte, and start passing around
> MAX_DIGEST_SIZE+1 and evm_hmac_size+1.
>
> Even if you're going to be using those differently in a later patchset,
> let's focus on this set for now and keep things simpler. One constant
> for the hmac size, and then please define a new one (in patch 4) for
> the annotated digest size. I can't think think of a good name. Which
> suggests that perhaps you should define a nicely typed struct to contain
> the header+hmac...
>
> I see no other problems, so presuming that these are nicely addressed
> I expect to happily ack.
>
> thanks,
> -serge

Ok, MAX_DIGEST_SIZE was defined in the first patch of this patchset,
which moves the iint from IMA to integrity, but it seems to be
unnecessary for any of the additional EVM or IMA extensions, including
support for additional IMA hash sizes. I'll remove MAX_DIGEST_SIZE.

The reason for introducing the extra byte at this point in the patch
set, as opposed to waiting to do so in the digital signature patches, is
to permit existing labeled systems to continue to run properly (and be
bisect safe). Defining a structure is a good idea.

thanks,

Mimi