On Wed, 2025-03-05 at 09:59 +0100, Roberto Sassu wrote: > 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. Hi Roberto, sorry, I didn't get this point. What do you mean? > > 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. Ok perfect, I get your point. Thank you for the explanation. Enrico > 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 > > > > > > > > > > > > > > > >
