On Mon, Nov 27, 2023 at 12:08 PM Mimi Zohar <zohar@xxxxxxxxxxxxx> wrote: > On Wed, 2023-11-22 at 09:22 -0500, Paul Moore wrote: ... > > 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. Fair enough. I personally think it is nice to have a brief discussion of how userspace might use a kernel feature, but if you prefer to drop that part of the design doc I doubt anyone will object very strongly. > ----------------------------------------------------------------------- > | 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. That looks fine to me. > 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. I imagine that should be trivial to implement and I can't imagine there being any objection to that. If we are going to have a record count, I imagine it would also be helpful to maintain a securityfs file with the total size (in bytes) of the in-memory measurement log. In fact, I suspect this will probably be more useful for those who wish to manage the size of the measurement log. > Defining other IMA securityfs files like > how many times the measurement list has been trimmed might be > beneficial as well. I have no objection to that. Would a total record count, i.e. a value that doesn't reset on a snapshot event, be more useful here? > Of course properly document the integrity > implications and repercussions of the new Kconfig that allows trimming > the measurement list. Of course. > 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. ... > 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. What if the snapshot_aggregate was a hash of the measurement log starting with either the boot_aggregate or the latest snapshot_aggregate and ending on the record before the new snapshot_aggregate? The performance impact at snapshot time should be minimal as the hash can be incrementally updated as new records are added to the measurement list. While the hash wouldn't capture the TPM state, it would allow some crude verification when reassembling the log. If one could bear the cost of a TPM signing operation, the log digest could be signed by the TPM. > 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. I disagree about the snapshot_aggregate record being disjoint from the measurement log, but I suspect Tushar and Sush are willing to forgo the snapshot_aggregate if that is a blocker from your perspective. Once again, the main goal is the ability to manage the size of the measurement log; while having a snapshot_aggregate that can be used to establish a root of trust similar to the boot_aggregate is nice, it is not a MUST have. > 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/ You will note that Sush and I replied to Stefan two weeks ago. -- paul-moore.com