Messages in this thread | | | Subject | Re: [PATCH v5 2/7] IMA: update process_buffer_measurement to measure buffer hash | From | Tushar Sugandhi <> | Date | Thu, 12 Nov 2020 13:47:27 -0800 |
| |
Hello Mimi,
On 2020-11-05 6:30 a.m., Mimi Zohar wrote: > Hi Tushar, > > Please don't include the filename in the Subject line[1]. The Subject > line should be a summary phrase describing the patch. In this case, > it is adding support for measuring the buffer data hash. > Thanks. Will update the subject line accordingly.
> On Sun, 2020-11-01 at 14:26 -0800, Tushar Sugandhi wrote: >> process_buffer_measurement() currently only measures the input buffer. >> In case of SeLinux policy measurement, the policy being measured could >> be large (several MB). This may result in a large entry in IMA >> measurement log. > > SELinux is an example of measuring large buffer data. Please rewrite > this patch description (and the other patch descriptions in this patch > set) without using the example to describe its purpose [1]. > > In this case, you might say, > > The original IMA buffer data measurement sizes were small (e.g. boot > command line), but new buffer data measurement use cases are a lot > larger. Just as IMA measures the file data hash, not the file data, > IMA should similarly support measuring the buffer data hash. > Sure. Thanks a lot for giving an example wording for us. Will update. >> >> Introduce a boolean parameter measure_buf_hash to support measuring >> hash of a buffer, which would be much smaller, instead of the buffer >> itself. > >> To use the functionality introduced in this patch, the attestation >> client and the server changes need to go hand in hand. The >> client/kernel would know what data is being measured as-is >> (e.g. KEXEC_CMDLINE), and what data has it’s hash measured (e.g. SeLinux >> Policy). And the attestation server should verify data/hash accordingly. >> >> Just like the data being measured in other cases, the attestation server >> will know what are possible values of the large buffers being measured. >> e.g. the possible valid SeLinux policy values that are being pushed to >> the client. The attestation server will have to maintain the hash of >> those buffer values. > > Each patch in the patch set builds upon the previous one. (Think of > it as a story, where each chapter builds upon the previous ones.) > With rare exceptions, should patches reference subsequent patches. [2] > > [1] Refer to Documentation/process/submitting-patches.rst > [2] Refer to the section "8) Commenting" in > Documentation/process/coding-style.rst > > thanks, > > Mimi > I am not sure if you have any concerns about the last two paragraphs. The description about the attestation client and server (the last two paragraphs) was added for information/clarification purpose only, as per your feedback on previous iterations. The subsequent patches don’t have any code pertaining to attestation client/server.
*Question* Maybe the last two paragraphs are confusing/redundant. Could you please let me know if I should remove the above two paragraphs altogether? (starting with “To use the functionality introduced in this patch ...”)
If we decide to keep the paragraphs, I will remove the specific examples (KEXEC_CMDLINE, SeLinux etc.) as you mentioned elsewhere.
| |