Re: [PATCH v3 1/5] lsm: Add hook security_unix_find

["Günther Noack" <gnoack@google.com>]

Hello!


John:

Friendly ping; as Paul said in [1], we would appreciate a look from
the AppArmor side whether this path-based LSM hook makes sense for
you.


Everyone:

In [2], we are currently discussing how the UNIX restriction feature
would work in the bigger scheme in Landlock, and the current plan is
that long-term we would like to support semantics where a UNIX
connection attempt is allowed if EITHER:

 (a) the path is allow-listed in the policy, OR
 (b) the server side we connect to is part of the same Landlock
     sandbox ("domain")


With the currently proposed hook, (a) can be checked in the
security_unix_find() hook, and (b) can be checked in the
security_hook_socket_connect() hook.  Unfortunately, it also would
mean that if the (a) check fails, we would have to store that
information on the side (struct sock LSM blob?), return 0 from (a) and
then later use that information in hook (b), so that we can check
whether maybe the second possible condition is met.

Q: The passing of information across multiple LSM hooks is slightly
more complicated than I had hoped; is this an approach that is
recommended?

Therefore, in [2], Tingmao is suggesting that we change the
security_unix_find() hook and pass the "other" struct sock instead of
the type.

There is obviously a balance between hooks that are very generic and
usable across multiple LSMs and hooks that are convenient to use for
every LSM.  

Paul:

You have previously said that you would like hooks to be generic and
ideally reflect the arguments of the same function that they are
called from [3].

Q: Would it be acceptable to change the hook arguments, if we can then
avoid passing additional data between hooks through that side-storage?
You can see Tingmao's proposal for that in [2].  TL;DR: It moves the
call to security_unix_find() just after the place where the sk
variable ("other"-side socket) is looked up and then calls the hook
with the sk as argument instead of with the type.  That way, we can do
both check (a) and (b) from above in the same hook and do not need to
store data on the side.  Is that an acceptable trade-off for the LSM
interface?

Thanks,
—Günther

[1] https://lore.kernel.org/all/CAHC9VhQZ_J9316Us0squV_f-MjYXPcex34BnJ14vEBxS9Jyjbg@mail.gmail.com/
[2] https://lore.kernel.org/all/e6b6b069-384c-4c45-a56b-fa54b26bc72a@maowtm.org/
[3] https://lore.kernel.org/all/CAHC9VhQ234xihpndTs4e5ToNJ3tGCsP7AVtXuz8GajG-_jn3Ow@mail.gmail.com/