Re: [PATCH 1/7] dm: measure data on table load

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Mimi,

On 7/21/21 2:17 PM, Mimi Zohar wrote:
On Wed, 2021-07-21 at 12:07 -0400, Mimi Zohar wrote:
On Wed, 2021-07-21 at 11:42 -0400, Mike Snitzer wrote:
On Tue, Jul 20 2021 at 10:12P -0400,
Mimi Zohar <zohar@xxxxxxxxxxxxx> wrote:

Hi Tushar, Mike,

On Mon, 2021-07-12 at 17:48 -0700, Tushar Sugandhi wrote:
+struct dm_ima_device_table_metadata {
+       /*
+        * Contains data specific to the device which is common across
+        * all the targets in the table.e.g. name, uuid, major, minor etc.
+        * The values are stored in comma separated list of key1=val1,key2=val2; pairs
+        * delimited by a semicolon at the end of the list.
+        */
+       char *device_metadata;
+       unsigned int device_metadata_len;
+       unsigned int num_targets;
+
+       /*
+        * Contains the sha256 hashs of the IMA measurements of the
+        * target attributes key-value pairs from the active/inactive tables.
+        */

 From past experience hard coding the hash algorithm is really not a
good idea.

Mimi

+       char *hash;
+       unsigned int hash_len;
+
+};

Hi Mimi,

The dm-ima.c code is using SHASH_DESC_ON_STACK and then storing the
more opaque result via 'hash' and 'hash_len'.

So if/when the dm-ima.c hash algorithm were to change this detail
won't change the dm_ima_device_table_metadata structure at all right?
But even if changes were needed this is purely an implementation
detail correct?  Why might users care which algorithm is used by
dm-ima to generate the hashes?

Assuming there is a valid reason for users to care about this, we can
improve this aspect as follow-on work.. so I don't consider this a
blocker for this patchset at this point.  Please clarify if you feel
it should be a blocker.

This goes back to my question as to if or how the template data in
these DM critical data records are to be validated by the attestation
server.   Asumming the hash/hash_len is being stored in the IMA
measurement list, the less the attestation should need to know about
the specific kernel version the better.

Hi Mike, Tushar,  Laskshmi,

Perhaps, when defining a new IMA "critical data" record, especially if
you know it's going to change, the critical data should contain a
version number.

Just to close the loop on this thread:

As I explained on the other thread in this patch series -

@the hash verification:
the clear-text for the active/inactive table hashes is already logged in
the 'table_load' event. And we will prefix the active/inactive table
hashes with hash_algo.  (e.g. sha256:<hash>) in the remaining events.
Together it should be sufficient for the attestation server to verify
the active/inactive table hashes without maintaining any list of
expected hashes or referring to kernel version etc.

@versioning:
We are already logging versions for individual targets. What was missing
was some versioning at device level. So thanks again for the suggestion.
We will add a version at device level in each of the events. Together
that should help attestation server to determine what attributes to expect in the event data - without relying on specific kernel version.

Thanks again for your feedback.

Regards,
Tushar

thanks,

Mimi





[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux Kernel]     [Linux Kernel Hardening]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux SCSI]

  Powered by Linux