Messages in this thread | | | Subject | Re: [PATCH 2/2] SELinux: Measure state and hash of policy using IMA | From | Lakshmi Ramasubramanian <> | Date | Mon, 17 Aug 2020 16:20:24 -0700 |
| |
On 8/17/20 4:11 PM, Mimi Zohar wrote: > On Mon, 2020-08-17 at 15:33 -0700, Lakshmi Ramasubramanian wrote: >> On 8/17/20 3:00 PM, Casey Schaufler wrote: >>> On 8/17/2020 2:31 PM, Mimi Zohar wrote: >>>> On Thu, 2020-08-13 at 14:13 -0400, Stephen Smalley wrote: >>>>> On Thu, Aug 13, 2020 at 2:03 PM Lakshmi Ramasubramanian >>>>> <nramas@linux.microsoft.com> wrote: >>>>>> On 8/13/20 10:58 AM, Stephen Smalley wrote: >>>>>>> On Thu, Aug 13, 2020 at 1:52 PM Lakshmi Ramasubramanian >>>>>>> <nramas@linux.microsoft.com> wrote: >>>>>>>> On 8/13/20 10:42 AM, Stephen Smalley wrote: >>>>>>>> >>>>>>>>>> diff --git a/security/selinux/measure.c b/security/selinux/measure.c >>>>>>>>>> new file mode 100644 >>>>>>>>>> index 000000000000..f21b7de4e2ae >>>>>>>>>> --- /dev/null >>>>>>>>>> +++ b/security/selinux/measure.c >>>>>>>>>> @@ -0,0 +1,204 @@ >>>>>>>>>> +static int selinux_hash_buffer(void *buf, size_t buf_len, >>>>>>>>>> + void **buf_hash, int *buf_hash_len) >>>>>>>>>> +{ >>>>>>>>>> + struct crypto_shash *tfm; >>>>>>>>>> + struct shash_desc *desc = NULL; >>>>>>>>>> + void *digest = NULL; >>>>>>>>>> + int desc_size; >>>>>>>>>> + int digest_size; >>>>>>>>>> + int ret = 0; >>>>>>>>>> + >>>>>>>>>> + tfm = crypto_alloc_shash("sha256", 0, 0); >>>>>>>>>> + if (IS_ERR(tfm)) >>>>>>>>>> + return PTR_ERR(tfm); >>>>>>>>> Can we make the algorithm selectable via kernel parameter and/or writing >>>>>>>>> to a new selinuxfs node? >>>>>>>> I can add a kernel parameter to select this hash algorithm. >>>>>>> Also can we provide a Kconfig option for the default value like IMA does? >>>>>>> >>>>>> Would we need both - Kconfig and kernel param? >>>>>> >>>>>> The other option is to provide an IMA function to return the current >>>>>> hash algorithm used for measurement. That way a consistent hash >>>>>> algorithm can be employed by both IMA and the callers. Would that be better? >>>>> This is why I preferred just passing the serialized policy buffer to >>>>> IMA and letting it handle the hashing. But apparently that approach >>>>> wouldn't fly. IMA appears to support both a Kconfig option for >>>>> selecting a default algorithm and a kernel parameter for overriding >>>>> it. I assume the idea is that the distros can pick a reasonable >>>>> default and then the end users can override that if they have specific >>>>> requirements. I'd want the same for SELinux. If IMA is willing to >>>>> export its hash algorithm to external components, then I'm willing to >>>>> reuse that but not sure if that's a layering violation. >>>> With the new ima_measure_critical_data() hook, I agree with you and >>>> Casey it doesn't make sense for each caller to have to write their own >>>> function. Casey suggested exporting IMA's hash function or defining a >>>> new common hash function. There's nothing specific to IMA. >>> >>> Except that no one is going to use the function unless they're >>> doing an IMA operation. >> >> Can we do the following instead: >> >> In ima_measure_critical_data() IMA hook, we can add another param for >> the caller to indicate whether >> >> => The contents of "buf" needs to be measured >> OR >> => Hash of the contents of "buf" needs to be measured. >> >> This way IMA doesn't need to export any new function to meet the hashing >> requirement. > > I'm not sure overloading the parameters is a good idea, but extending > ima_measure_critical_data() to calculate a simple buffer hash should be > fine. >
Sorry I wasn't clear - I didn't mean to say overload existing parameters, but extending the IMA hook to calculate the hash of the buffer - like the following:
int ima_measure_critical_data(const char *event_name, const char *event_data_source, const void *buf, int buf_len, bool measure_buf_hash);
If measure_buf_hash is true, IMA will calculate the hash of contents of "buf" and measure the hash. Else, IMA will measure the contents of "buf".
-lakshmi
| |