Message-ID: <5daedb73-6c42-d556-2090-25d08f536ff1@linux.microsoft.com>
Date:   Thu, 27 May 2021 09:19:46 -0700
From:   Tushar Sugandhi <tusharsu@...ux.microsoft.com>
To:     Mimi Zohar <zohar@...ux.ibm.com>
Cc:     tyhicks@...ux.microsoft.com, sashal@...nel.org, jmorris@...ei.org,
        nramas@...ux.microsoft.com, linux-integrity@...r.kernel.org,
        linux-kernel@...r.kernel.org, pvorel@...e.cz
Subject: Re: [PATCH v4] IMA: support for duplicate measurement records

Hi Mimi,
Sorry for the late response.
I wanted to spend some time thinking what other scenarios I could test.
Responses below.

On 2021-05-20 1:35 p.m., Mimi Zohar wrote:
> Hi Tushar,
> 
> On Mon, 2021-05-10 at 12:09 -0700, Tushar Sugandhi wrote:
>> IMA measures contents of a given file/buffer/critical-data record,
>> and properly re-measures it on change.  However, IMA does not measure
>> the duplicate value for a given record, since TPM extend is a very
>> expensive operation.  For example, if the record changes from value
>> 'v#1' to 'v#2', and then back to 'v#1', IMA will not measure and log
>> the last change to 'v#1', since the hash of 'v#1' for that record is
>> already present in the IMA htable.  This limits the ability of an
>> external attestation service to accurately determine the current state
>> of the system.  The service would incorrectly conclude that the latest
>> value of the given record on the system is 'v#2', and act accordingly.
>>
>> Define and use a new Kconfig option IMA_DISABLE_HTABLE to permit
>> duplicate records in the IMA measurement list.
>>
>> In addition to the duplicate measurement records described above,
>> other duplicate file measurement records may be included in the log,
>> when CONFIG_IMA_DISABLE_HTABLE=y.
>> For example,
>>      - i_version is not enabled,
>>      - i_generation changed,
>>      - an inode is evicted from dcache etc.
> 
> Missing from this list are the same file, perhaps on different
> filesystmes, such as initramfs and real root.  These can be identified
> by the different i_ino.  Is there anything else?
> 
> thanks,
> 
> Mimi
> 
Sure, I can add the 4th line to the list:

For example,
     - i_version is not enabled,
     - i_generation changed,
     - an inode is evicted from dcache,
     - same file present on different filesystems, with different i_ino
       etc.

Should I spin up v5 of this patch with the updated patch description?

I was also thinking if I should cover any other scenarios, and
soft-links/hard-links seemed like a good scenario to test - so I went
ahead and tested it. And it looks like the patch is working as expected
in this scenario.

Here are the detailed findings:

If I have a file original.txt, and create a soft-link file -
softlink.txt,

(1) only original.txt is reported in IMA log, regardless if I touch
     original.txt or softlink.txt.
     This is true in both cases - when allow_dup is turned on or off.
     (i.e. with or without this patch)
(2) duplicate entries are measured only when allow_dup is turned on.

If I have a file original.txt, and create a hard-link file -
hardlink.txt,
(1) whichever file I touch first, either original.txt or hardlink.txt,
     gets measured in IMA log, both when allow_dup is turned on or off.
     (i.e. with or without this patch)
(2) duplicate entries are measured only when allow_dup is turned on.

Since the the observed behavior is identical in both cases,
soft-links and hard-links (1), and duplicates are measured only when
allow_dup is turned on as expected (2), I don't believe we should call
out soft-links/hard-links in the list above. It is not a special case
like ones mentioned in the list.

Please let me know if you think otherwise.

Thanks,
Tushar
>>
>> Signed-off-by: Tushar Sugandhi <tusharsu@...ux.microsoft.com>
>> Reviewed-by: Petr Vorel <pvorel@...e.cz>

Powered by blists - more mailing lists