On 2020-10-22 2:15 p.m., Mimi Zohar wrote:

Hi Tushar,

The above Subject line should be truncated to "IMA: add policy to
measure critical data".

On Wed, 2020-09-23 at 12:20 -0700, Tushar Sugandhi wrote:

quoted

There would be several candidate kernel components suitable for IMA
measurement. Not all of them would have support for IMA measurement.

This intro is besides the point.  The patch description should describe
what is meant by "critical data".

Thanks. I will fix the description to address this.

quoted

Also, system administrators may not want to measure data for all of
them, even when they support IMA measurement.
An IMA policy option
specific to various kernel components is needed to measure their
respective critical data.

This policy option needs to be constrained to measure data for
specific kernel components that are specified as input values to the
policy option.

Add a new IMA policy option - "data_sources:=" to allow measuring
various critical kernel components. This policy option would enable the
system administrators to limit the measurement to the components
listed in "data_sources:=", if the components support IMA measurement.

The new policy option "data_sources:=" is different from the existing
policy option "keyrings:=".

In case of "keyrings:=", a policy may measure all keyrings (when
"keyrings:=" option is not provided for func KEY_CHECK), or may
constrain which keyrings need to be measured (when "keyrings:=" option
is provided for func KEY_CHECK).

But unlike "keyrings:=", the entries in "data_sources:=" would have
different data format. Further, the components listed in
"data_sources:=" need to be modified to call IMA to measure their
data. Therefore, unlike "keyrings:=", IMA shouldn't measure all of the
components by default, when "data_sources:=" is not specified. Because
measuring non-vetted components just by specifying them as a policy
option value may impact the overall reliability of the system.

To address this, "data_sources:=" should be a mandatory policy option
for func=CRITICAL_DATA. This func is introduced in the 5th patch in this
series). And the compile-time vetting functionality described above is
introduced in the 6th patch in this series.

Signed-off-by: Tushar Sugandhi <redacted>

I don't understand what you mean by "non-vetted" components, nor how
measuring critical data may impact the overall reliability of the
system.

Tushar: Before we introduced the mechanism to check supported
data-sources at compile time (patch 6/6 of this series), there was a
back-and-forth on whether “data_sources:=” should be a mandatory policy
option, or optional like “keyrings:=”. And we decided to make the
“data_sources:=” mandatory. But now that we have the compile time check
(patch 6/6 of this series), we can switch to make “data_sources:=”
optional (with the default to allow measuring all critical data – just 
like what you suggested below). I will make the code and description 
changes accordingly.

The system owner or adminstrator defines what they want to measure,
including "critical data", based on the policy rules.  They might
decide that they want to constrain which "critical data" is measured by
specifying "data_sources:=", but that decision is their perogative.
The default should allow measuring all critical data.

Makes sense.
To summarize, we will make the decision which sources to measure- based
on the sources defined in the allow list (in patch 6) and the sources
defined in “data_sources:=”. If “data_sources:=” is not present, we will
measure all sources defined in the allow list.
Hope my this understanding is correct based on your feedback.

  A simple example of "critical data" could be some in memory structure,
along the lines of __ro_after_init, or hash of the memory structure.
Once the data structure is initialized, the "critical data" measurement
shouldn't change.    From the attestation server perspective, the IMA
measurement list would contain a single record unless the critical data
changes.  The attestation server doesn't need to know anything about
the initial value, just that it has changed in order to trigger some
sort alert.

Yes agreed. After the updates (based on your feedback) I stated above,
the behavior should remain consistent with what you described here.

thanks,

Mimi