On Wed, 2023-11-22 at 09:22 -0500, Paul Moore wrote:
> On Wed, Nov 22, 2023 at 8:18 AM Mimi Zohar <zohar@xxxxxxxxxxxxx> wrote:
> > On Tue, 2023-11-21 at 23:27 -0500, Paul Moore wrote:
> > > On Thu, Nov 16, 2023 at 5:28 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote:
> > > > On Tue, Oct 31, 2023 at 3:15 PM Mimi Zohar <zohar@xxxxxxxxxxxxx> wrote:
> > >
> > > ...
> > >
> > > > > Userspace can already export the IMA measurement list(s) via the
> > > > > securityfs {ascii,binary}_runtime_measurements file(s) and do whatever
> > > > > it wants with it. All that is missing in the kernel is the ability to
> > > > > trim the measurement list, which doesn't seem all that complicated.
> > > >
> > > > From my perspective what has been presented is basically just trimming
> > > > the in-memory measurement log, the additional complexity (which really
> > > > doesn't look that bad IMO) is there to ensure robustness in the face
> > > > of an unreliable userspace (processes die, get killed, etc.) and to
> > > > establish a new, transitive root of trust in the newly trimmed
> > > > in-memory log.
> > > >
> > > > I suppose one could simplify things greatly by having a design where
> > > > userspace captures the measurement log and then writes the number of
> > > > measurement records to trim from the start of the measurement log to a
> > > > sysfs file and the kernel acts on that. You could do this with, or
> > > > without, the snapshot_aggregate entry concept; in fact that could be
> > > > something that was controlled by userspace, e.g. write the number of
> > > > lines and a flag to indicate if a snapshot_aggregate was desired to
> > > > the sysfs file. I can't say I've thought it all the way through to
> > > > make sure there are no gotchas, but I'm guessing that is about as
> > > > simple as one can get.
> >
> > > > If there is something else you had in mind, Mimi, please share the
> > > > details. This is a very real problem we are facing and we want to
> > > > work to get a solution upstream.
> > >
> > > Any thoughts on this Mimi? We have a real interest in working with
> > > you to solve this problem upstream, but we need more detailed feedback
> > > than "too complicated". If you don't like the solutions presented
> > > thus far, what type of solution would you like to see?
> >
> > Paul, the design copies the measurement list to a temporary "snapshot"
> > file, before trimming the measurement list, which according to the
> > design document locks the existing measurement list. And further
> > pauses extending the measurement list to calculate the
> > "snapshot_aggregate".
>
> I believe the intent is to only pause the measurements while the
> snapshot_aggregate is generated, not for the duration of the entire
> snapshot process. The purpose of the snapshot_aggregate is to
> establish a new root of trust, similar to the boot_aggregate, to help
> improve attestation performance.
>
> > Userspace can export the measurement list already, so why this
> > complicated design?
>
> The current code has no provision for trimming the measurement log,
> that's the primary reason.
>
> > As I mentioned previously and repeated yesterday, the
> > "snapshot_aggregate" is a new type of critical data and should be
> > upstreamed independently of this patch set that trims the measurement
> > list. Trimming the measurement list could be based, as you suggested
> > on the number of records to remove, or it could be up to the next/last
> > "snapshot_aggregate" record.
>
> Okay, we are starting to get closer, but I'm still missing the part
> where you say "if you do X, Y, and Z, I'll accept and merge the
> solution." Can you be more explicit about what approach(es) you would
> be willing to accept upstream?
Included with what is wanted/needed is an explanation as to my concerns
with the existing proposal.
First we need to differentiate between kernel and uhserspace
requirements. (The "snapshotting" design proposal intermixes them.)
>From the kernel persective, the Log Snapshotting Design proposal "B.1
Goals" is very nice, but once the measurement list can be trimmed it is
really irrelevant. Userspace can do whatever it wants with the
measurement list records. So instead of paying lip service to what
should be done, just call it as it is - trimming the measurement list.
-----------------------------------------------------------------------
| B.1 Goals |
-----------------------------------------------------------------------
To address the issues described in the section above, we propose
enhancements to the IMA subsystem to achieve the following goals:
a. Reduce memory pressure on the Kernel caused by larger in-memory
IMA logs.
b. Preserve the system's ability to get remotely attested using the
IMA log, even after implementing the enhancements to reduce memory
pressure caused by the IMA log. IMA's Integrity guarantees should
be maintained.
c. Provide mechanisms from Kernel side to the remote attestation
service to make service-side processing more efficient.
>From the kernel perspective there needs to be a method of trimming N
number of records from the head of the measurement list. In addition
to the existing securityfs "runtime measurement list", defining a new
securityfs file containing the current count of in memory measurement
records would be beneficial. Defining other IMA securityfs files like
how many times the measurement list has been trimmed might be
beneficial as well. Of course properly document the integrity
implications and repercussions of the new Kconfig that allows trimming
the measurement list.
Defining a simple "trim" marker measurement record would be a visual
indication that the measurement list has been trimmed. I might even
have compared it to the "boot_aggregate". However, the proposed marker
based on TPM PCRs requires pausing extending the measurement list.
Although the TCG TPM spec allows reading multiple PCRs, it may fail due
to the output buffer size. To avoid TPM read multiple PCRs failure,
reading one TPM PCR value at a time is safer. The more TPM banks and
PCRs needed the longer it will take. Remember this critical-data
record won't be limited to just software TPMs, but could be used with
physical ones as well. For a physical TPM, this could be on the orderof 240 ms per TPM bank (24 PCRs).
Before defining a new critical-data record, we need to decide whether
it is really necessary or if it is redundant. If we define a new
"critical-data" record, can it be defined such that it doesn't require
pausing extending the measurement list? For example, a new simple
visual critical-data record could contain the number of records (e.g.
<securityfs>/ima/runtime_measurements_count) up to that point.
The new critical-data record and trimming the measurement list should
be disjoint features. If the first record after trimming the
measurement list should be the critical-data record, then trim the
measurement list up to that point.
>From a userspace perspective, trimming the measurement list is a major
change and will break existing attestation requests, unless the change
is transparent. Removing "snapshots"/"shards" will of course break
attestation requests. Refer to Stefan's suggestions:
https://lore.kernel.org/linux-integrity/1ed2d72c-4cb2-48b3-bb0f-b0877fc1e9ca@xxxxxxxxxxxxx/
--
thanks,
Mimi
