On Mon, 2025-03-03 at 10:26 +0000, Enrico Bravi wrote: > On Thu, 2025-02-27 at 15:49 +0100, Roberto Sassu wrote: > > On Thu, 2025-02-27 at 11:36 +0000, Enrico Bravi wrote: > > > On Wed, 2025-02-26 at 22:05 -0500, Mimi Zohar wrote: > > > > On Wed, 2025-02-26 at 22:53 +0000, Enrico Bravi wrote: > > > > > On Tue, 2025-02-25 at 20:53 -0500, Mimi Zohar wrote: > > > > > > On Tue, 2025-02-25 at 14:12 +0100, Enrico Bravi wrote: > > > > > > > The first write on the ima policy file permits to override the > > > > > > > default > > > > > > > policy defined with the ima_policy= boot parameter. This can be done > > > > > > > by adding the /etc/ima/ima-policy which allows loading the custom > > > > > > > policy > > > > > > > during boot. It is also possible to load custom policy at runtime > > > > > > > through > > > > > > > file operations: > > > > > > > > > > > > > > cp custom_ima_policy /sys/kernel/security/ima/policy > > > > > > > cat custom_ima_policy > /sys/kernel/security/ima/policy > > > > > > > > > > > > > > or by writing the absolute path of the file containing the custom > > > > > > > policy: > > > > > > > > > > > > > > echo /path/of/custom_ima_policy > /sys/kernel/security/ima/policy > > > > > > > > > > > > > > In these cases, file signature can be necessary to load the policy > > > > > > > (func=POLICY_CHECK). Custom policy can also be set at runtime by > > > > > > > directly > > > > > > > writing the policy stream on the ima policy file: > > > > > > > > > > > > > > echo -e "measure func=BPRM_CHECK mask=MAY_EXEC\n" \ > > > > > > > "audit func=BPRM_CHECK mask=MAY_EXEC\n" \ > > > > > > > > /sys/kernel/security/ima/policy > > > > > > > > > > > > > > In this case, there is no mechanism to verify the integrity of the > > > > > > > new > > > > > > > policy. > > > > > > > > > > > > > > Add a new entry in the ima measurements list containing the ascii > > > > > > > custom > > > > > > > ima policy buffer when not verified at load time. > > > > > > > > > > > > > > Signed-off-by: Enrico Bravi <enrico.bravi@xxxxxxxxx> > > > > > > > > > > > > Hi Enrico, > > > > > > > > > > Hi Mimi, > > > > > > > > > > thank you for the quick response. > > > > > > > > > > > This patch set hard codes measuring the initial custom IMA policy > > > > > > rules > > > > > > that > > > > > > replace the builtin policies specified on the boot command line. IMA > > > > > > shouldn't hard code policy. > > > > > > > > > > My first approach was to define a new critical-data record, > > > > > > Hi Mimi, > > > > > > > Hopefully the new critical-data will be of the entire IMA policy. > > > > > > yes, absolutely. > > > > > > > > but performing the > > > > > measurement after the custom policy becomes effective, the measurement > > > > > could > > > > > be > > > > > bypassed omitting func=CRITICAL_DATA in the custom policy. > > > > > > > > > > > I'm not quite sure why you're differentiating between > > > > > > measuring the initial and subsequent custom IMA policy rules. > > > > > > > > > > My intention is to measure the first direct write (line by line) on the > > > > > policy > > > > > file, without loading the initial custom policy from a file. This case, > > > > > if > > > > > I'm > > > > > not wrong, is not covered by func=POLICY_CHECK. > > > > > > > > When secure boot is enabled, the arch specific policy rules require the > > > > IMA > > > > policy to be signed. Without secure boot enabled, you're correct. The > > > > custom > > > > policy rules may directly be loaded without being measured. > > > > > > > > Why only measure "the first direct write"? Additional custom policy rules > > > > may > > > > be directly appended without being measured. > > > > > > Yes, you right. The aim was to measure (at least) the first one, because it > > > substitutes the boot policy, but if you are ok with adding a critical-data > > > record, it would be definitely better. > > > > Hi Enrico > > > > in addition to what Mimi suggested, I also like to idea that the > > POLICY_CHECK hook catches the direct policy loading. That would mean > > that those updates would be seen if the 'tcb' IMA policy is selected. > > Hi Roberto, > > in this case, wouldn't be used the current template? Wouldn't be better to use > the ima-buf in order to include the textual policy representation? Hi Enrico I would use the current template, I don't find any particular issues for it. Sure, we don't have a file to measure but there are other cases where in process_measurement() we measure a buffer instead of a file (when it is called by ima_post_read_file()). We can have both critical data and POLICY_CHECK measurement. > In addition, there would be a new record for each line of the input buffer, and > measuring the input buffer would produce different measurements for the same > resulting policy entry, because different or multiple separators can be used. > > I opted to perform the measurement in ima_release_policy() because is where the > new policy becomes effective after ima_update_policy() and can be done a single > measurement of the new running policy. I would simply measure what is passed to ima_write_policy() regardless of whether the policy will be accepted or not. This is more in line with the trusted computing paradigm of measure & load. If potentially there is a bug in the policy code, measuring the policy before with a vulnerable kernel would allow you to see the measurement. After, it depends on the seriousness of the vulnerability. Roberto > The measurement could be done a bit earlier, working on ima_policy_rules and > ima_temp_rules (which basically contains the input buffer) before the splicing, > so it would be considered the current policy and not the new one. In this case, > it would work also when ima_policy=tcb is set, and it could be called > process_buffer_measurement() with POLICY_CHECK, to get a record with the entire > IMA policy. > What do you think about it? > > BR, > > Enrico > > > I would have recommended to try to add a process_measurement() call in > > ima_write_policy(), where the buffer to be processed is. > > > > However, I guess you need to have a valid file descriptor in order to > > use that function (maybe an anonymous inode?). > > ima_collect_measurement() should be already able to handle buffers, > > passed by ima_post_read_file(). > > > > Thanks > > > > Roberto > > > > > Thank you, > > > > > > Enrico > > > > > > > > > > > > > > Consider defining a new critical-data record to measure the current > > > > > > IMA > > > > > > policy > > > > > > rules. Also consider including the new critical-data rule in the arch > > > > > > specific policy rules. > > > > > > > > > > I think that your suggestion, to add the critical-data rule in the arch > > > > > policy > > > > > rules, solves the problems of bypassing the measurement and hard coding > > > > > policy. > > > > > > > > > > Thank you very much for your feedback. > > > > > > > > You're welcome. > > > > > > > > Mimi > > > > > > > > > > >
