Messages in this thread | | | Subject | Re: [PATCH v2 4/5] LSM: Define SELinux function to measure security state | From | Lakshmi Ramasubramanian <> | Date | Thu, 16 Jul 2020 12:13:15 -0700 |
| |
On 7/16/20 11:54 AM, Stephen Smalley wrote:
>> 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.
>> +/* 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.
> >> + >> + 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.
> >> + >> + ++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.
> >> + 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
| |