Re: [RFC PATCH v2 1/2] security: add fault injection capability

[Casey Schaufler <casey@schaufler-ca.com>]

On 10/27/2020 10:29 AM, Aleksandr Nogikh wrote:
> (resending the previous message in a plain/text mode)
>
> On Mon, Oct 26, 2020 at 7:20 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> [...]
>>> -     int RC = IRC;                                           \
>>> -     do {                                                    \
>>> +     int RC = lsm_hooks_inject_fail();                       \
>>> +     if (RC == 0) {                                                          \
>> Injecting the failure here will prevent the loaded LSM hooks from
>> being called.
> In this RFC, fault injection was intentionally placed before the code that
> invokes LSM hooks. The reasoning was that it would simultaneously check
> how the kernel code reacts to LSM denials and the effect of fault injections
> on LSM modules.
>
>>>               struct security_hook_list *P;                   \
>>> +             RC = IRC;                                                               \
>>>                                                               \
>>>               hlist_for_each_entry(P, &security_hook_heads.FUNC, list) { \
>>>                       RC = P->hook.FUNC(__VA_ARGS__);         \
>>>                       if (RC != 0)                            \
>>>                               break;                          \
>>>               }                                               \
>>> -     } while (0);                                            \
>>> +     }                                                       \
>> Injecting the failure here would allow the loaded LSM hooks to
>> be called. It shouldn't make a difference, but hooks with side-effects
>> are always possible. I don't have an issue either way.
>>
>>>       RC;                                                     \
>>>  })
>>>
> Should we expect LSM modules to properly handle the cases when their
> hooks with side effects were not invoked (unlike the selinux crash that
> is described in the cover letter)? From the source code it seems that a
> failure/denial from one module prevents the execution of the subsequent
> hooks, so this looks like a realistic scenario.

Yes. Security modules have to accept the possibility that something
ahead of them in the stack will fail. This may be a DAC check, a
capability check or another security module.

> If that is not true in general and depends on the specific active modules,
> then it probably makes sense to introduce an option to control whether to
> inject faults at the beginning of call_int_hook() or after the hooks have
> been invoked.

If you want to do that you could implement it as an LSM. You could place it
anywhere in the stack that way. Based on what I see with the BPF lsm that might
be more work than it is worth.