Re: [RFC][PATCH] ima: add measurement for first unverified write on ima policy file

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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?

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.

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
> > > > 
> > 
> 





[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux Kernel]     [Linux Kernel Hardening]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux SCSI]

  Powered by Linux