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-03-06 at 08:20 +0000, Enrico  Bravi wrote:
> 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.

CRITICAL_DATA and POLICY_CHECK are separate hooks.

They can do measurement differently, CRITICAL_DATA can be for measuring
the current state of the full policy, while POLICY_CHECK could be for
policies sent to the kernel.


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

Welcome.

Roberto

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






[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