On Wed, Jul 15, 2020 at 11:48 AM Lakshmi Ramasubramanian <nramas@xxxxxxxxxxxxxxxxxxx> 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_state(), 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;netpeer=1;openperm=1;extsockclass=1;alwaysnetwork=0;cgroupseclabel=1;nnpnosuidtransition=1;genfsseclabelsymlink=0; > > The data for selinux-policy-hash in the above measurement is > the SHA256 hash of the SELinux policy. > > Signed-off-by: Lakshmi Ramasubramanian <nramas@xxxxxxxxxxxxxxxxxxx> > Suggested-by: Stephen Smalley <stephen.smalley.work@xxxxxxxxx> > --- > diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h > index b0e02cfe3ce1..691c7e35f82a 100644 > --- a/security/selinux/include/security.h > +++ b/security/selinux/include/security.h > @@ -222,16 +222,35 @@ static inline bool selinux_policycap_genfs_seclabel_symlinks(void) > return state->policycap[POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS]; > } > > +static inline bool selinux_checkreqprot(void) > +{ > + struct selinux_state *state = &selinux_state; > + > + return state->checkreqprot; > +} Probably should use READ_ONCE(). > diff --git a/security/selinux/measure.c b/security/selinux/measure.c > new file mode 100644 > index 000000000000..b909e8e61542 > --- /dev/null > +++ b/security/selinux/measure.c > @@ -0,0 +1,122 @@ > +int selinux_security_state(void) Let's call this selinux_measure_state() or similar. Needs a verb. And pass it a struct selinux_state * pointer argument to be measured, even though initially it will always be passed &selinux_state. The encapsulation of selinux state within selinux_state was to support multiple selinux namespaces in the future, each with their own state. > +{ > + int rc = 0; > + int count; > + size_t buflen; > + int policy_hash_len; > + char *state = NULL; > + void *policy = NULL; > + void *policy_hash = NULL; > + static char *security_state_string = > + "enabled=%d;enforcing=%d;checkreqprot=%d;" \ > + "netpeer=%d;openperm=%d;extsockclass=%d;" \ > + "alwaysnetwork=%d;cgroupseclabel=%d;" \ > + "nnpnosuidtransition=%d;genfsseclabelsymlink=%d;"; Rather than hardcoding policy capability names here, I would recommend using the selinux_policycap_names[] array for the names and the selinux_state.policycap[] array for the values. Also recommend passing in a struct selinux_state * here to allow for future case where there are multiple selinux states, one per selinux namespace. > diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c > index ef0afd878bfc..0c289d13ef6a 100644 > --- a/security/selinux/ss/services.c > +++ b/security/selinux/ss/services.c > @@ -3724,10 +3724,11 @@ int security_netlbl_sid_to_secattr(struct selinux_state *state, > * security_read_policy - read the policy. > * @data: binary policy data > * @len: length of data in bytes > - * > + * @alloc_kernel_memory: flag to indicate memory allocation > */ > -int security_read_policy(struct selinux_state *state, > - void **data, size_t *len) > +int security_read_selinux_policy(struct selinux_state *state, > + void **data, size_t *len, > + bool alloc_kernel_memory) Instead of passing in a boolean to control how the memory is allocated, split this into a helper function that takes an already-allocated buffer and two different front-end wrappers, one for kernel-internal use and one for userspace use. > @@ -3738,7 +3739,10 @@ int security_read_policy(struct selinux_state *state, > > *len = security_policydb_len(state); > > - *data = vmalloc_user(*len); > + if (alloc_kernel_memory) > + *data = kzalloc(*len, GFP_KERNEL); You need vmalloc() since policy image size may exceed kmalloc max (or at least that used to be the case).