[RFC] IMA: New IMA measurements for dm-crypt and selinux

[RFC] IMA: New IMA measurements for dm-crypt and selinux

[Tushar Sugandhi]

The goals of the kernel integrity subsystem are to detect if files have
been accidentally or maliciously altered, both remotely and locally,
appraise a file's measurement against a "good" value stored as an
extended attribute, and enforce local file integrity [1].

To achieve these goals, IMA subsystem measures several in-memory
constructs and files.

We propose to measure constructs in dm-crypt and selinux to further
enhance measuring capabilities of IMA.

If there is existing or planned work to measure dm-crypt and selinux
constructs, we would like to contribute to that.

dm-crypt is a subsystem used for encryption of the block device, which
is essential for ensuring protection of data and secrets at rest.

Measuring encryption status of the device will ensure the device is not
maliciously reporting false encryption status - thus, it can be
entrusted with sensitive data to be protected at rest.

SELinux is an implementation of mandatory access controls (MAC) on
Linux. Mandatory access controls allow an administrator of a system to
define how applications and users can access different resources - such
as files, devices, networks and inter-process communication. With
SELinux an administrator can differentiate a user from the applications
a user runs [2].

Measuring SELinux status and various SELinux policies can help ensure
mandatory access control of the system is not compromised.

Proposal:
---------
A. Measuring dmcrypt constructs:
     We can add an IMA hook in crypt_ctr() present in
     drivers/md/dm-crypt.c, so that IMA can start measuring the status of
     various dm-crypt targets (represented by crypt_target struct - also
     defined in dm-crypt.c).
     The mapping table[3] has information of devices being encrypted
     (start sector, size, target name, cypher, key, device path, and
     other optional parameters.)
     e.g.
     0 417792 crypt serpent-cbc-essiv:sha256
     a7f67ad520bd83b9725df6ebd76c3eee 0 /dev/sdb 0 1 allow_discards

     We can pass various attributes of mapping table to IMA through a key
     value pair of various dmcrypt constructs.

     Proposed Function Signature of the IMA hook:
     void ima_dmcrypt_status(void *dmcrypt_status, int len);

B. Measuring selinux constructs:
     We propose to add an IMA hook in enforcing_set() present under
     security/selinux/include/security.h.
     enforcing_set() sets the selinux state to enforcing/permissive etc.
     and is called from key places like selinux_init(),
     sel_write_enforce() etc.
     The hook will measure various attributes related to selinux status.
     Majority of the attributes are present in the struct selinux_state
     present in security/selinux/include/security.h
     e.g.
     $sestatus
            SELinux status:              enabled
            SELinuxfs mount:             /sys/fs/selinux
            SELinux root directory:      /etc/selinux
            Loaded policy name:          default
            Current mode:                permissive
            Mode from config file:       permissive
            Policy MLS status:           enabled
            Policy deny_unknown status:  allowed
            Memory protection checking:  requested (insecure)
            Max kernel policy version:   32

     The above attributes will be serialized into a set of key=value
     pairs when passed to IMA for measurement.

     Proposed Function Signature of the IMA hook:
     void ima_selinux_status(void *selinux_status, int len);

Please provide comments\feedback on the proposal.

Thanks,
Tushar

[1] https://sourceforge.net/p/linux-ima/wiki/Home/
[2] https://selinuxproject.org/page/FAQ
[3] https://gitlab.com/cryptsetup/cryptsetup/wikis/DMCrypt

Re: [RFC] IMA: New IMA measurements for dm-crypt and selinux

[Milan Broz]

On 08/04/2020 12:19, Tushar Sugandhi wrote:
> The goals of the kernel integrity subsystem are to detect if files have
> been accidentally or maliciously altered, both remotely and locally,
> appraise a file's measurement against a "good" value stored as an
> extended attribute, and enforce local file integrity [1].
> 
> To achieve these goals, IMA subsystem measures several in-memory
> constructs and files.
> 
> We propose to measure constructs in dm-crypt and selinux to further
> enhance measuring capabilities of IMA.

...

> Proposal:
> ---------
> A. Measuring dmcrypt constructs:
>      We can add an IMA hook in crypt_ctr() present in
>      drivers/md/dm-crypt.c, so that IMA can start measuring the status of
>      various dm-crypt targets (represented by crypt_target struct - also
>      defined in dm-crypt.c).

Hi,

I do not think you should just cherry-pick dm-crypt here. What about other
device-mapper targets? Apparently, dm-verity or dm-integrity are obvious
candidates too.

But device-mapper logic is based on stacking devices, so in generic case
(not just in some very special embedded configuration) you need to measure
the whole stack of devices.
(Just imagine a target stacked below dm-crypt that decrypts the device or so. :-)

Moreover, dm-crypt allows some specific actions like wiping and reloading
of the encryption key through device-mapper dm-crypt message.
If you check parameter only in crypt_ctr, this message path must be disabled,
basically crippling dm-crypt functionality (it is intended to wipe key in-memory
during hw suspend).


IMO if you want implement something like IMA measurement, I think you should
implement it in device-mapper core, and provide support for all targets.

I guess some new target specific callback is needed and some flags that
could enforce/disable stacking if a IMA measurement is in place etc.

Milan

Re: [RFC] IMA: New IMA measurements for dm-crypt and selinux

[Casey Schaufler]

On 4/8/2020 3:19 AM, Tushar Sugandhi wrote:
> The goals of the kernel integrity subsystem are to detect if files have
> been accidentally or maliciously altered, both remotely and locally,
> appraise a file's measurement against a "good" value stored as an
> extended attribute, and enforce local file integrity [1].
>
> To achieve these goals, IMA subsystem measures several in-memory
> constructs and files.
>
> We propose to measure constructs in dm-crypt and selinux to further
> enhance measuring capabilities of IMA.
>
> If there is existing or planned work to measure dm-crypt and selinux
> constructs, we would like to contribute to that.
>
> dm-crypt is a subsystem used for encryption of the block device, which
> is essential for ensuring protection of data and secrets at rest.
>
> Measuring encryption status of the device will ensure the device is not
> maliciously reporting false encryption status - thus, it can be
> entrusted with sensitive data to be protected at rest.
>
> SELinux is an implementation of mandatory access controls (MAC) on
> Linux. Mandatory access controls allow an administrator of a system to
> define how applications and users can access different resources - such
> as files, devices, networks and inter-process communication. With
> SELinux an administrator can differentiate a user from the applications
> a user runs [2].
>
> Measuring SELinux status and various SELinux policies can help ensure
> mandatory access control of the system is not compromised.
>
> Proposal:
> ---------
> A. Measuring dmcrypt constructs:
>     We can add an IMA hook in crypt_ctr() present in
>     drivers/md/dm-crypt.c, so that IMA can start measuring the status of
>     various dm-crypt targets (represented by crypt_target struct - also
>     defined in dm-crypt.c).
>     The mapping table[3] has information of devices being encrypted
>     (start sector, size, target name, cypher, key, device path, and
>     other optional parameters.)
>     e.g.
>     0 417792 crypt serpent-cbc-essiv:sha256
>     a7f67ad520bd83b9725df6ebd76c3eee 0 /dev/sdb 0 1 allow_discards
>
>     We can pass various attributes of mapping table to IMA through a key
>     value pair of various dmcrypt constructs.
>
>     Proposed Function Signature of the IMA hook:
>     void ima_dmcrypt_status(void *dmcrypt_status, int len);
>
> B. Measuring selinux constructs:
>     We propose to add an IMA hook in enforcing_set() present under
>     security/selinux/include/security.h.
>     enforcing_set() sets the selinux state to enforcing/permissive etc.
>     and is called from key places like selinux_init(),
>     sel_write_enforce() etc.
>     The hook will measure various attributes related to selinux status.
>     Majority of the attributes are present in the struct selinux_state
>     present in security/selinux/include/security.h
>     e.g.
>     $sestatus
>            SELinux status:              enabled
>            SELinuxfs mount:             /sys/fs/selinux
>            SELinux root directory:      /etc/selinux
>            Loaded policy name:          default
>            Current mode:                permissive
>            Mode from config file:       permissive
>            Policy MLS status:           enabled
>            Policy deny_unknown status:  allowed
>            Memory protection checking:  requested (insecure)
>            Max kernel policy version:   32
>
>     The above attributes will be serialized into a set of key=value
>     pairs when passed to IMA for measurement.
>
>     Proposed Function Signature of the IMA hook:
>     void ima_selinux_status(void *selinux_status, int len);
>
> Please provide comments\feedback on the proposal.

TL;DR - Why make this SELinux specific?

Integrating IMA and SELinux is a layering violation at best.
Why isn't this ima_lsm_status(void *lsm_status, int len)?
Or, better yet, how about ima_lsm_status(char *name, void *value, int len),
and you pass each name/value pair separately? That makes the
interface generally useful.

Believe it or not, there *ARE* security modules that
are not SELinux. 

>
> Thanks,
> Tushar
>
> [1] https://sourceforge.net/p/linux-ima/wiki/Home/
> [2] https://selinuxproject.org/page/FAQ
> [3] https://gitlab.com/cryptsetup/cryptsetup/wikis/DMCrypt

Re: [RFC] IMA: New IMA measurements for dm-crypt and selinux

[Stephen Smalley]

On Wed, Apr 8, 2020 at 6:28 AM Tushar Sugandhi
<tusharsu@linux.microsoft.com> wrote:
> Measuring SELinux status and various SELinux policies can help ensure
> mandatory access control of the system is not compromised.
<snip>
> B. Measuring selinux constructs:
>      We propose to add an IMA hook in enforcing_set() present under
>      security/selinux/include/security.h.
>      enforcing_set() sets the selinux state to enforcing/permissive etc.
>      and is called from key places like selinux_init(),
>      sel_write_enforce() etc.
>      The hook will measure various attributes related to selinux status.
>      Majority of the attributes are present in the struct selinux_state
>      present in security/selinux/include/security.h
>      e.g.
>      $sestatus
>             SELinux status:              enabled
>             SELinuxfs mount:             /sys/fs/selinux
>             SELinux root directory:      /etc/selinux
>             Loaded policy name:          default
>             Current mode:                permissive
>             Mode from config file:       permissive
>             Policy MLS status:           enabled
>             Policy deny_unknown status:  allowed
>             Memory protection checking:  requested (insecure)
>             Max kernel policy version:   32
>
>      The above attributes will be serialized into a set of key=value
>      pairs when passed to IMA for measurement.
>
>      Proposed Function Signature of the IMA hook:
>      void ima_selinux_status(void *selinux_status, int len);

This won't detect changes to any of these state variables via a kernel
write vulnerability,
so it would be good to provide a way to trigger measurement of the
current values on
demand.
You'll also likely want to measure parts of the child structures of
selinux_state, e.g. selinux_ss,
especially selinux_map and policydb.  You can simplify measurement of
the policydb by
serializing it first via policydb_write() and hashing the result. I
suppose one question is whether you can do all of this
already from userspace by just having userspace read
/sys/fs/selinux/enforce, /sys/fs/selinux/policy, etc.

Re: [RFC] IMA: New IMA measurements for dm-crypt and selinux

[Lev R. Oshvang .]

On Sat, Apr 11, 2020 at 10:07 PM Stephen Smalley
<stephen.smalley@gmail.com> wrote:
>
> On Wed, Apr 8, 2020 at 6:28 AM Tushar Sugandhi
> <tusharsu@linux.microsoft.com> wrote:
> > Measuring SELinux status and various SELinux policies can help ensure
> > mandatory access control of the system is not compromised.
> <snip>
> > B. Measuring selinux constructs:
> >      We propose to add an IMA hook in enforcing_set() present under
> >      security/selinux/include/security.h.
> >      enforcing_set() sets the selinux state to enforcing/permissive etc.
> >      and is called from key places like selinux_init(),
> >      sel_write_enforce() etc.
> >      The hook will measure various attributes related to selinux status.
> >      Majority of the attributes are present in the struct selinux_state
> >      present in security/selinux/include/security.h
> >      e.g.
> >      $sestatus
> >             SELinux status:              enabled
> >             SELinuxfs mount:             /sys/fs/selinux
> >             SELinux root directory:      /etc/selinux
> >             Loaded policy name:          default
> >             Current mode:                permissive
> >             Mode from config file:       permissive
> >             Policy MLS status:           enabled
> >             Policy deny_unknown status:  allowed
> >             Memory protection checking:  requested (insecure)
> >             Max kernel policy version:   32
> >
> >      The above attributes will be serialized into a set of key=value
> >      pairs when passed to IMA for measurement.
> >
> >      Proposed Function Signature of the IMA hook:
> >      void ima_selinux_status(void *selinux_status, int len);
>
> This won't detect changes to any of these state variables via a kernel
> write vulnerability,
> so it would be good to provide a way to trigger measurement of the
> current values on
> demand.
> You'll also likely want to measure parts of the child structures of
> selinux_state, e.g. selinux_ss,
> especially selinux_map and policydb.  You can simplify measurement of
> the policydb by
> serializing it first via policydb_write() and hashing the result. I
> suppose one question is whether you can do all of this
> already from userspace by just having userspace read
> /sys/fs/selinux/enforce, /sys/fs/selinux/policy, etc.

It sees to me that  LKRG (kernel run time guard)  takes the role of
measuring kernel structures.  Perhaps you need to consult with LKRG
guys.
Lev.

Re: [RFC] IMA: New IMA measurements for dm-crypt and selinux

[Mimi Zohar]

On Sun, 2020-04-12 at 11:15 +0300, Lev R. Oshvang . wrote:
> On Sat, Apr 11, 2020 at 10:07 PM Stephen Smalley
> It sees to me that  LKRG (kernel run time guard)  takes the role of
> measuring kernel structures.  Perhaps you need to consult with LKRG
> guys.

There definitely sounds like there is some overlap.  LKRG seems to be
measuring kernel structures for enforcing local integrity.  In the
context of IMA, measurements are included in the IMA measurement list
and used to extend a TPM PCR so that it can be quoted.

A generic method for measuring structures and including them in the
IMA measurement list sounds interesting.

Mimi

Re: [RFC] IMA: New IMA measurements for dm-crypt and selinux

[Lev R. Oshvang .]

On Tue, Apr 14, 2020 at 4:11 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Sun, 2020-04-12 at 11:15 +0300, Lev R. Oshvang . wrote:
> > On Sat, Apr 11, 2020 at 10:07 PM Stephen Smalley
> > It sees to me that  LKRG (kernel run time guard)  takes the role of
> > measuring kernel structures.  Perhaps you need to consult with LKRG
> > guys.
>
> There definitely sounds like there is some overlap.  LKRG seems to be
> measuring kernel structures for enforcing local integrity.  In the
> context of IMA, measurements are included in the IMA measurement list
> and used to extend a TPM PCR so that it can be quoted.
>
> A generic method for measuring structures and including them in the
> IMA measurement list sounds interesting.
>
> Mimi
>
I frankly do not understand the threat model.
Secure boot or TPM provides trust in encryption/decryption keys
dm-crypt/dm-verify use.
When dm-verify discovers that the disk image is modified it will just
do not allow the system to work ( mount roots, etc).
So imagine that adversary took control of TPM  and changed the keys
dm-verify work with in order to sign malicious content on disk. In
this case, remote attestation should alert of compromised TPM, no
matter whether dmvery keys or other keys were forged.

SELinux is another story and I think a run-time check of SElinux
structures fits well into LKRG. IMA only provide guarantees that
SELinux (or any other LSM) control files and attributes were intact.

Re: [RFC] IMA: New IMA measurements for dm-crypt and selinux

[Tushar Sugandhi]

On 2020-04-08 9:28 a.m., Milan Broz wrote:
> On 08/04/2020 12:19, Tushar Sugandhi wrote:
<snip>
>> Proposal:
>> ---------
>> A. Measuring dmcrypt constructs:
>>       We can add an IMA hook in crypt_ctr() present in
>>       drivers/md/dm-crypt.c, so that IMA can start measuring the status of
>>       various dm-crypt targets (represented by crypt_target struct - also
>>       defined in dm-crypt.c).
> 
> Hi,
> 
> I do not think you should just cherry-pick dm-crypt here. What about other
> device-mapper targets? Apparently, dm-verity or dm-integrity are obvious
> candidates too.
> 
> But device-mapper logic is based on stacking devices, so in generic case
> (not just in some very special embedded configuration) you need to measure
> the whole stack of devices.
> (Just imagine a target stacked below dm-crypt that decrypts the device or so. :-)
> 
> Moreover, dm-crypt allows some specific actions like wiping and reloading
> of the encryption key through device-mapper dm-crypt message.
> If you check parameter only in crypt_ctr, this message path must be disabled,
> basically crippling dm-crypt functionality (it is intended to wipe key in-memory
> during hw suspend).
> 
> 
> IMO if you want implement something like IMA measurement, I think you should
> implement it in device-mapper core, and provide support for all targets.
I agree that this needs to be implemented in device-mapper core,
rather than highter applications like  dm-crypt, dm-verity, or dm-integrity.
Functions like dm_table_create(), dm_table_destroy(), 
dm_table_verify_integrity(),
dm_table_complete(), dm_table_add_target() etc. in drivers/md/dm-table.c 
look like good
candidates to add hooks for IMA.
Please let me know if you have any other recommendations.
> I guess some new target specific callback is needed and some flags that
> could enforce/disable stacking if a IMA measurement is in place etc.
> 
> Milan
> 

Re: [RFC] IMA: New IMA measurements for dm-crypt and selinux

[Tushar Sugandhi]

On 2020-04-08 9:34 a.m., Casey Schaufler wrote:
> On 4/8/2020 3:19 AM, Tushar Sugandhi wrote:
<snip>
>>
>> B. Measuring selinux constructs:
>>      We propose to add an IMA hook in enforcing_set() present under
>>      security/selinux/include/security.h.
>>      enforcing_set() sets the selinux state to enforcing/permissive etc.
>>      and is called from key places like selinux_init(),
>>      sel_write_enforce() etc.
>>      The hook will measure various attributes related to selinux status.
>>      Majority of the attributes are present in the struct selinux_state
>>      present in security/selinux/include/security.h
>>      e.g.
>>      $sestatus
>>             SELinux status:              enabled
>>             SELinuxfs mount:             /sys/fs/selinux
>>             SELinux root directory:      /etc/selinux
>>             Loaded policy name:          default
>>             Current mode:                permissive
>>             Mode from config file:       permissive
>>             Policy MLS status:           enabled
>>             Policy deny_unknown status:  allowed
>>             Memory protection checking:  requested (insecure)
>>             Max kernel policy version:   32
>>
>>      The above attributes will be serialized into a set of key=value
>>      pairs when passed to IMA for measurement.
>>
>>      Proposed Function Signature of the IMA hook:
>>      void ima_selinux_status(void *selinux_status, int len);
>>
>> Please provide comments\feedback on the proposal.
> 
> TL;DR - Why make this SELinux specific?
> 
> Integrating IMA and SELinux is a layering violation at best.
> Why isn't this ima_lsm_status(void *lsm_status, int len)?
That seems like a good idea.
I will investigate where can I place the hook for LSM.
Please let me know if you have any recommendations.
> Or, better yet, how about ima_lsm_status(char *name, void *value, int len),
> and you pass each name/value pair separately? That makes the
> interface generally useful.
> 
> Believe it or not, there *ARE* security modules that
> are not SELinux.
> 
>>
>> Thanks,
>> Tushar
>>
>> [1] https://sourceforge.net/p/linux-ima/wiki/Home/
>> [2] https://selinuxproject.org/page/FAQ
>> [3] https://gitlab.com/cryptsetup/cryptsetup/wikis/DMCrypt

Re: [RFC] IMA: New IMA measurements for dm-crypt and selinux

[Tushar Sugandhi]

On 2020-04-12 1:15 a.m., Lev R. Oshvang . wrote:
<snip>
> 
> It sees to me that  LKRG (kernel run time guard)  takes the role of
> measuring kernel structures.  Perhaps you need to consult with LKRG
Thanks Lev for the feedback. I will investigate more into it.
> guys.
> Lev.
> 

Re: [RFC] IMA: New IMA measurements for dm-crypt and selinux

[Tushar Sugandhi]

On 2020-04-13 6:11 p.m., Mimi Zohar wrote:
> On Sun, 2020-04-12 at 11:15 +0300, Lev R. Oshvang . wrote:
>> On Sat, Apr 11, 2020 at 10:07 PM Stephen Smalley
>> It sees to me that  LKRG (kernel run time guard)  takes the role of
>> measuring kernel structures.  Perhaps you need to consult with LKRG
>> guys.
> 
> There definitely sounds like there is some overlap.  LKRG seems to be
> measuring kernel structures for enforcing local integrity.  In the
> context of IMA, measurements are included in the IMA measurement list
> and used to extend a TPM PCR so that it can be quoted.
> 
> A generic method for measuring structures and including them in the
> IMA measurement list sounds interesting.
Thanks for the feedback Mimi.
We were also thinking along the same lines of generic method
for measuring structures.
We will take this feedback into account while implementing.
> 
> Mimi
>