Message-ID: <4ECA6595.7010406@polito.it>
Date:	Mon, 21 Nov 2011 15:52:05 +0100
From:	Roberto Sassu <roberto.sassu@...ito.it>
To:	Mimi Zohar <zohar@...ux.vnet.ibm.com>
CC:	Rajiv Andrade <srajiv@...ux.vnet.ibm.com>,
	linux-security-module@...r.kernel.org,
	linux-kernel@...r.kernel.org, jmorris@...ei.org
Subject: Re: [PATCH 1/2] ima: split ima_add_digest_entry() function

On 11/18/2011 06:31 PM, Mimi Zohar wrote:
> On Fri, 2011-11-18 at 11:27 +0100, Roberto Sassu wrote:
>> On 11/17/2011 10:15 PM, Mimi Zohar wrote:
>>> On Thu, 2011-11-17 at 11:57 +0100, Roberto Sassu wrote:
>>>> On 11/16/2011 07:52 PM, Rajiv Andrade wrote:
>>>>>
>>>>> Thanks, Rajiv Andrade Security Development IBM Linux Technology Center
>>>>>
>>>>> On 16-11-2011 12:37, Roberto Sassu wrote:
>>>>>> On 11/16/2011 02:38 PM, Mimi Zohar wrote:
>>>>>>> On Wed, 2011-11-16 at 11:10 +0100, Roberto Sassu wrote:
>>>>>>>> The ima_add_digest_entry() function has been split in order to avoid
>>>>>>>> adding an entry in the measurements list for which the PCR extend
>>>>>>>> operation subsequently fails. Required memory is allocated earlier
>>>>>>>> in the
>>>>>>>> new function ima_prepare_template_entry() and the template entry is
>>>>>>>> added
>>>>>>>> after ima_pcr_extend().
>>>>>>>>
>>>>>>>> Signed-off-by: Roberto Sassu<roberto.sassu@...ito.it>
>>>>>>>
>>>>>>
>>>>>> Hi Mimi
>>>>>>
>>>>>> i don't know if this condition can happen, but suppose that
>>>>>> for whatever reason the PCR extend fails. In this case, since
>>>>>> the PCR is not extended, the measurements list can be modified,
>>>>>> by removing the non-measured entry, without this fact being
>>>>>> detected by the verifier. So, probably we can avoid to display
>>>>>> the entry.
>>>>>>
>>>>>>
>>>>> Hi Roberto,
>>>>>
>>>>> IMA's trustworthiness is built on the assumption that the TPM underneath
>>>>> can
>>>>> be trusted. If that can't be, the eventlog alone doesn't provide us any
>>>>> security.
>>>>> It's the TPM device driver's job though to workaround any HW bug so that
>>>>> in the
>>>>> end all its stakeholders have their commands processed successfully, as
>>>>> we've
>>>>> pursued in some changes here:
>>>>>
>>>>
>>>> Hi Rajiv
>>>>
>>>> thanks for your comments.
>>>>
>>>> I absolutely agree that we have to trust the TPM for the correct
>>>> execution of IMA.
>>>>
>>>> I think the principle that has been used to build IMA (according
>>>> to the TGC specifications) is that we can trust the eventlog
>>>> as long as the measurement infrastructure is reliable or it is
>>>> possible to detect a threat from previous measurements.
>>>>
>>>> For this reason, a system call is never executed before the
>>>> inode measurement is inserted in the eventlog and the PCR
>>>> is extended. Since these operations must be considered as
>>>> atomic, their execution is protected by a mutex, that is
>>>> released only after all tasks have been performed. This
>>>> ensures that we begin with a measured kernel and we can
>>>> reliably measure all further interactions. This also explains,
>>>> in my view, why delaying the PCR extend operation may lead
>>>> to security risks.
>>>>
>>>> About my patch, i did not move out the protected region any
>>>> of the above described operations. Instead, i'm preventing
>>>> measurements for which the PCR extend failed to be added to
>>>> the measurements list, because in any case it is impossible
>>>> for a verifier to detect their removal from the list.
>>>>
>>>> As i mentioned in the previous mail, one solution to overcome
>>>> this issue is to deny, on the platform running IMA, the execution
>>>> of those system calls for which the measurement process ended
>>>> with an error.
>>>>
>>>> Regards
>>>>
>>>> Roberto Sassu
>>>
>>> True, if the TPM failed to extend the PCR, a malicious user would be
>>> able to remove the measurement from the measurement list without it
>>> being detected.  However, according to the TPM specs, the PCR extend
>>> operation is always suppose to succeed, even in the case when the TPM is
>>> not enabled.
>>>
>>
>> Hi Mimi
>>
>> i don't know if this can happen, but, since the TPM always returns
>> a result in its response, there may be particular conditions under
>> which the PCR extend fails.
>>
>> After quickly looking at the code in 'drivers/char/tpm.c' there are
>> some additional cases where the operation may fail. For instance,
>> an operation may have been cancelled or it may have just reached the
>> defined time limit.
>>
>>
>>> More importantly we need to be able to detect when the PCR has not been
>>> extended appropriately in order to address it.  Otherwise we're just
>>> covering it up.
>>>
>>
>> Probably i did not understand this point, but do the return code of
>> tpm_pcr_extend() is not sufficient to determine if the operation was
>> completed successfully?
>>
>> Roberto Sassu
>
> Your concern, that if the PCR extend fails, the PCR value will not match
> the measurement list, is a valid concern. I'm not disagreeing with you
> on this point.  At the same time, the IMA measurement policy defines
> which files to include in the measurement list.  This policy is
> independent of the TPM, whether it is enabled or not, or even if the TPM
> is failing for some reason.  IMA goes into TPM by-pass mode and
> maintains the measurement list on systems without a TPM.
>

Hi Mimi

i think that if at some point IMA goes in TPM by-pass mode, this allows
an attacker to hide malicious programs if they are measured after the
switch.

So, i agree that an IMA user wants the complete measurements list for
the corresponding policy, but i think it is also necessary to give
verifiers the proof that the list contains exactly all events occurred
in the platform to be attested, not only those for which the
measurement process ended successfully.

For this reason, i implemented a feature that denies the execution of
a requested system call if the measurement cannot be taken. I think
this will not affect users in their normal work, since the events that
cause this behaviour are very rare. Further, even if some measurements
were not taken, the Administrator can become aware of occurred errors by 
checking the audit log.

By default, this feature is deactivated so that IMA will behave as
usual. Instead, if a verifier needs the proof mentioned above, he can
check whether the newly introduced parameter that enables the feature
is present in the kernel command line.

Roberto Sassu


> Removing measurements from the measurement list, so that we can verify
> the PCR value, to me at least, is the wrong approach.  Maybe a better
> approach would be to go into TPM by-pass mode.
>
> Mimi
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists