Re: [PATCH v2 4/5] LSM: Define SELinux function to measure security state

On 7/16/20 11:54 AM, Stephen Smalley wrote:

quoted

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?

Sure - I'll provide an example.

quoted

+/* 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.

That's a good point - will do.

quoted

+
+       if (selinux_state_string_len < 0)
+               return;

How can this happen legitimately (i.e. as a result of something other
than a kernel bug)?

Since snprintf can return an error I wanted to handle that. But I agree 
this should not happen for the input data to snprintf used here.

quoted

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

I am not sure if failing (kernel panic/BUG) to "measure" LSM data under 
memory pressure conditions is the right thing. But I am open to treating 
this error as a fatal error. Please let me know.

quoted

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

Will do.

thanks,
  -lakshmi