Re: [PATCH v2 4/5] LSM: Define SELinux function to measure security state
flat view
From: Stephen Smalley <stephen.smalley.work@gmail.com>
Date: 2020-07-16 18:54:20
Also in:
linux-integrity, lkml, selinux
On Thu, Jul 16, 2020 at 1:44 PM Lakshmi Ramasubramanian [off-list ref] wrote:
SELinux configuration and policy are some of the critical data for this security module that needs to be measured. To enable this measurement SELinux needs to implement the interface function, security_measure_data(), that the LSM can call. Define the security_state() function in SELinux to measure SELinux configuration and policy. Call this function to measure SELinux data when there is a change in the security module's state. Sample measurement of SELinux state and hash of the policy: 10 e32e...5ac3 ima-buf sha256:86e8...4594 selinux-state 656e61626c65643d313b656e666f7263696e673d303b636865636b72657170726f743d313b6e6574706565723d313b6f70656e7065726d3d313b657874736f636b636c6173733d313b616c776179736e6574776f726b3d303b6367726f75707365636c6162656c3d313b6e6e706e6f737569647472616e736974696f6e3d313b67656e66737365636c6162656c73796d6c696e6b3d303b 10 f4a7...9408 ima-buf sha256:4941...68fc selinux-policy-hash 8d1d...1834 The data for selinux-state in the above measurement is: enabled=1;enforcing=0;checkreqprot=1;network_peer_controls=1;open_perms=1;extended_socket_class=1;always_check_network=0;cgroup_seclabel=1;nnp_nosuid_transition=1;genfs_seclabel_symlinks=0; The data for selinux-policy-hash in the above measurement is the SHA256 hash of the SELinux policy.
Can you show an example of how to verify that the above measurement matches a given state and policy, e.g. the sha256sum commands and inputs to reproduce the same from an expected state and policy?
Signed-off-by: Lakshmi Ramasubramanian <redacted> Suggested-by: Stephen Smalley <stephen.smalley.work@gmail.com> ---
quoted hunk ↗ jump to hunk
diff --git a/security/selinux/measure.c b/security/selinux/measure.c new file mode 100644 index 000000000000..27cbb309e926 --- /dev/null +++ b/security/selinux/measure.c@@ -0,0 +1,158 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Measure SELinux state using IMA subsystem. + */ +#include <linux/ima.h> +#include "security.h" + +/* Pre-allocated buffer used for measuring state */ +static char *selinux_state_string; +static size_t selinux_state_string_len; +static char *selinux_state_string_fmt = + "%s=%d;%s=%d;%s=%d;%s=%d;%s=%d;%s=%d;%s=%d;%s=%d;%s=%d;%s=%d;"; + +void __init selinux_init_measurement(void) +{ + selinux_state_string_len = + snprintf(NULL, 0, selinux_state_string_fmt, + "enabled", 0, + "enforcing", 0, + "checkreqprot", 0, + selinux_policycap_names[POLICYDB_CAPABILITY_NETPEER], 0, + selinux_policycap_names[POLICYDB_CAPABILITY_OPENPERM], 0, + selinux_policycap_names[POLICYDB_CAPABILITY_EXTSOCKCLASS], 0, + selinux_policycap_names[POLICYDB_CAPABILITY_ALWAYSNETWORK], 0, + selinux_policycap_names[POLICYDB_CAPABILITY_CGROUPSECLABEL], 0, + selinux_policycap_names[POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION], 0, + selinux_policycap_names[POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS], + 0);
I was thinking you'd dynamically construct the format string with a for loop from 0 to POLICYDB_CAPABILITY_MAX and likewise for the values so that we wouldn't have to patch this code every time we add a new one.
quoted hunk ↗ jump to hunk
+ + if (selinux_state_string_len < 0) + return;
How can this happen legitimately (i.e. as a result of something other than a kernel bug)?
quoted hunk ↗ jump to hunk
+ + ++selinux_state_string_len; + + selinux_state_string = kzalloc(selinux_state_string_len, GFP_KERNEL); + if (!selinux_state_string) + selinux_state_string_len = 0; +}
Not sure about this error handling approach (silent, proceeding as if the length was zero and then later failing with ENOMEM on every attempt?). I'd be more inclined to panic/BUG here but I know Linus doesn't like that.
+ if (ret)
+ pr_err("%s: error %d\n", __func__, ret);This doesn't seem terribly useful as an error message; I'd be inclined to drop it.