On Wed, Jan 13, 2021 at 2:13 PM Mimi Zohar [off-list ref] wrote:

On Tue, 2021-01-12 at 11:27 -0500, Paul Moore wrote:

quoted

On Thu, Jan 7, 2021 at 11:07 PM Tushar Sugandhi
[off-list ref] wrote:

quoted

From: Lakshmi Ramasubramanian <redacted>

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>
Reviewed-by: Tyler Hicks <redacted>
---
 Documentation/ABI/testing/ima_policy |  3 +-
 security/selinux/Makefile            |  2 +
 security/selinux/ima.c               | 64 ++++++++++++++++++++++++++++
 security/selinux/include/ima.h       | 24 +++++++++++
 security/selinux/include/security.h  |  3 +-
 security/selinux/ss/services.c       | 64 ++++++++++++++++++++++++----
 6 files changed, 149 insertions(+), 11 deletions(-)
 create mode 100644 security/selinux/ima.c
 create mode 100644 security/selinux/include/ima.h

I remain concerned about the possibility of bypassing a measurement by
tampering with the time, but I appear to be the only one who is
worried about this so I'm not going to block this patch on those
grounds.

Acked-by: Paul Moore <paul@paul-moore.com>

Thanks, Paul.

Including any unique string would cause the buffer hash to change,
forcing a new measurement.  Perhaps they were concerned with
overflowing a counter.

My understanding is that Lakshmi wanted to force a new measurement
each time and felt using a timestamp would be the best way to do that.
A counter, even if it wraps, would have a different value each time
whereas a timestamp is vulnerable to time adjustments.  While a
properly controlled and audited system could be configured and
monitored to detect such an event (I *think*), why rely on that if it
isn't necessary?

-- 
paul moore
www.paul-moore.com