Re: [PATCH] selinux: include a consumer of the new IMA critical data hook

On 1/14/21 11:58 AM, Tyler Hicks wrote:

On 2021-01-14 14:29:09, Paul Moore wrote:

quoted

On Thu, Jan 14, 2021 at 2:15 PM Lakshmi Ramasubramanian
[off-list ref] wrote:

quoted

SELinux stores the active policy in memory, so the changes to this data
at runtime would have an impact on the security guarantees provided
by SELinux.  Measuring in-memory SELinux policy through IMA subsystem
provides a secure way for the attestation service to remotely validate
the policy contents at runtime.

Measure the hash of the loaded policy by calling the IMA hook
ima_measure_critical_data().  Since the size of the loaded policy
can be large (several MB), measure the hash of the policy instead of
the entire policy to avoid bloating the IMA log entry.

To enable SELinux data measurement, the following steps are required:

1, Add "ima_policy=critical_data" to the kernel command line arguments
    to enable measuring SELinux data at boot time.
For example,
   BOOT_IMAGE=/boot/vmlinuz-5.10.0-rc1+ root=UUID=fd643309-a5d2-4ed3-b10d-3c579a5fab2f ro nomodeset security=selinux ima_policy=critical_data

2, Add the following rule to /etc/ima/ima-policy
    measure func=CRITICAL_DATA label=selinux

Sample measurement of the hash of SELinux policy:

To verify the measured data with the current SELinux policy run
the following commands and verify the output hash values match.

   sha256sum /sys/fs/selinux/policy | cut -d' ' -f 1

   grep "selinux-policy-hash" /sys/kernel/security/integrity/ima/ascii_runtime_measurements | tail -1 | cut -d' ' -f 6

Note that the actual verification of SELinux policy would require loading
the expected policy into an identical kernel on a pristine/known-safe
system and run the sha256sum /sys/kernel/selinux/policy there to get
the expected hash.

Signed-off-by: Lakshmi Ramasubramanian <redacted>
Suggested-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Acked-by: Paul Moore <paul@paul-moore.com>
Reviewed-by: Tyler Hicks <redacted>
---
  Documentation/ABI/testing/ima_policy |  3 +-
  security/selinux/Makefile            |  2 +
  security/selinux/ima.c               | 44 +++++++++++++++++++
  security/selinux/include/ima.h       | 24 +++++++++++
  security/selinux/include/security.h  |  3 +-
  security/selinux/ss/services.c       | 64 ++++++++++++++++++++++++----
  6 files changed, 129 insertions(+), 11 deletions(-)
  create mode 100644 security/selinux/ima.c
  create mode 100644 security/selinux/include/ima.h

I think this has changed enough that keeping the "Acked-by" and
"Reviewed-by" tags is probably not a good choice.  I took a quick look
and this still looks okay from a SELinux perspective, I'll leave Mimi
to comment on it from a IMA perspective.

Thanks for reviewing the change Paul.

quoted

Unless Tyler has reviewed this version prior to your posting, it might
be a good idea to remove his "Reviewed-by" unless he has a chance to
look this over again before it is merged.

Thanks for calling this out. I hadn't reviewed it prior to the posting
but I was keeping an eye on the thread.

This new revision still looks good to me and I like the idea of
controlling re-measurements via policy. So,

  Reviewed-by: Tyler Hicks [off-list ref]

Thanks for the quick response Tyler.

  -lakshmi