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

[Roberto Sassu]

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@xxxxxxxxx>
> > > >
> > > > 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


> thanks,
>
> Mimi

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