Re: [Linux-ima-user] [PATCH] ima: platform-independent hash value

[Mimi Z. <zo...@li...>]

On Thu, 2014-05-15 at 15:23 +0300, Dmitry Kasatkin wrote: 
> On 15 May 2014 14:18, Mimi Zohar <zo...@li...> wrote:
> > On Thu, 2014-05-15 at 09:11 +0200, Roberto Sassu wrote:
> >> On 05/15/2014 08:55 AM, Dmitry Kasatkin wrote:
> >> > On 15 May 2014 00:32, Mimi Zohar <zo...@li...> wrote:
> >> >> On Wed, 2014-05-07 at 18:13 +0200, Andreas Steffen wrote:
> >> >>> For remote attestion it is important for the ima measurement values
> >> >>> to be platform-independent. Therefore integer fields to be hashed
> >> >>> must be converted to network order first.
> >> >>>
> >> >>> Signed-off-by: Andreas Steffen <and...@st...>
> >> >>> ---
> >> >>>   security/integrity/ima/ima_crypto.c | 5 +++--
> >> >>>   1 file changed, 3 insertions(+), 2 deletions(-)
> >> >>>
> >> >>> diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
> >> >>> index 1bde8e6..8725bff 100644
> >> >>> --- a/security/integrity/ima/ima_crypto.c
> >> >>> +++ b/security/integrity/ima/ima_crypto.c
> >> >>> @@ -170,11 +170,12 @@ static int ima_calc_field_array_hash_tfm(struct ima_field_data *field_data,
> >> >>>                u8 buffer[IMA_EVENT_NAME_LEN_MAX + 1] = { 0 };
> >> >>>                u8 *data_to_hash = field_data[i].data;
> >> >>>                u32 datalen = field_data[i].len;
> >> >>> +             u32 datalen_to_hash = htonl(datalen);
> >> >>>
> >> >>>                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));
> >> >>> +                                             (const u8 *) &datalen_to_hash,
> >> >>> +                                             sizeof(datalen_to_hash));
> >> >>>                        if (rc)
> >> >>>                                break;
> >> >>>                } else if (strcmp(td->fields[i]->field_id, "n") == 0) {
> >> >>
> >> >> After thinking about this some more, and discussing it with Roberto and
> >> >> Dave, this change is probably unnecessary.  Assuming the measurement
> >> >> list is sent in host native format, a hash of the template data will
> >> >> match the value contained in the measurement list.  Only if the template
> >> >> data is converted to network byte order, prior to sending it, would
> >> >> there be a problem.
> >> >>
> >> >> The PCR value provides a good heuristic for determining the data
> >> >> endianness.
> >> >>
> >> >
> >> > So you suggestion is try to change endianness if PCR reconstruction fails?
> >
> > No, from the raw data the endianness can be heuristically determined. So
> > if the raw data is sent to the attestation server, there is no problem.
> > As Roberto pointed out, we need a method of indicating this in the
> > integrity reports.
> >
> 
> Yes. It is very easy to check length field and understand what endianness is.
> It will be very big number if it is incorrect.
> 
> But what about PCR. I have not understood that.

The first element in the measurement list is the PCR number, normally 10
for IMA.  Currently there are only 23 PCR registers. So it should be
simple to determine the endianness.

At this point, I don't see a need for this patch to be upstreamed.

thanks,

Mimi