Re: [PATCH 1/2] ima: split ima_add_digest_entry() function

[Roberto Sassu <roberto.sassu@polito.it>]

On 12/05/2011 02:03 PM, Mimi Zohar wrote:
> On Mon, 2011-12-05 at 11:04 +0100, Roberto Sassu wrote:
>
>> Hi Mimi
>>
>> i think moving this logic to the TPM driver (or in general, delaying
>> the action after the list mutex is unlocked) is not safe, because in
>> this way you are relying on the kernel trustworthiness to protect
>> itself and IMA against unmeasured potential attacks. So, the verifier
>> is unable to detect a kernel tampering that removed the limitation
>> on the TPM Quote operation.
>>
>> What i'm proposing in the patch:
>>
>> https://lkml.org/lkml/2011/11/21/202
>>
>> is in fact a new extension, which is triggered by a new kernel
>> parameter, so that the behaviour of the base IMA is not modified.
>
> How/why the TPM fails is important.  If the TPM fails because of an
> intermittent problem, then your solution of denying read/execute could
> work, but what would happen if it was persistent?  Would you be able to
> quiesce the system?
>
> As there is no way of differentiating a persistent from intermittent
> failure, both need to be addressed in the same manor.  For persistent
> TPM failure, we can not access the TPM to modify the PCR.  So what
> options do we have left?  My suggestion, though not optimal, prevents
> the IMA PCR from being quoted.
>

Hi Mimi

the solution you are proposing is reasonable as the default
behaviour, because not all IMA users need the high confidence
in the measurements, as ensured by denying the execution of
system calls.

However, during the IMA initialization the TPM is tested
by issuing a PCR read (the test procedure may be extended
to better detect existing errors in advance). So, this means
that a TPM failure when the system is already powered on is
very unlikely and may cause serious issues as it could happen
if other devices are involved.

For this reason, also my extension seems helpful especially
in the situations where all events need to be measured properly.
In this case, IMA users are aware that a TPM failure could hang
their systems, because they need to manually insert the required
kernel parameter.


>  From ima_queue.c: ima_add_template_entry()
>
>          result = ima_pcr_extend(digest);
>          if (result != 0) {
>                  audit_cause = "TPM error";
>                  audit_info = 0;
>          }
> out:
>          mutex_unlock(&ima_extend_list_mutex);
>
> Either in ima_pcr_extend() itself or as a new call, set a flag of some
> sort to prevent the TPM dd from quoting the IMA PCR.
>
> I'm open to other suggestions.
>
>> Instead, regardless of this patch, we should fix the memory leaks and
>> the memory reference errors as it has been proposed in the patch 2/2 of
>> this set.
>>
>> Roberto Sassu
>
> Please separate bug fixes from any other changes.
>

Ok, i will modify the patch 2/2 to work without the 1/2.

Thanks

Roberto Sassu


> thanks,
>
> Mimi
>