On 8/2/22 4:33 PM, Eric W. Biederman wrote:
Paul Moore <paul@xxxxxxxxxxxxxx> writes:
On July 22, 2022 2:12:03 AM Martin KaFai Lau <kafai@xxxxxx> wrote:
On Thu, Jul 21, 2022 at 12:28:04PM -0500, Frederick Lawler wrote:
While creating a LSM BPF MAC policy to block user namespace creation, we
used the LSM cred_prepare hook because that is the closest hook to prevent
a call to create_user_ns().
The calls look something like this:
cred = prepare_creds()
security_prepare_creds()
call_int_hook(cred_prepare, ...
if (cred)
create_user_ns(cred)
We noticed that error codes were not propagated from this hook and
introduced a patch [1] to propagate those errors.
The discussion notes that security_prepare_creds()
is not appropriate for MAC policies, and instead the hook is
meant for LSM authors to prepare credentials for mutation. [2]
Ultimately, we concluded that a better course of action is to introduce
a new security hook for LSM authors. [3]
This patch set first introduces a new security_create_user_ns() function
and userns_create LSM hook, then marks the hook as sleepable in BPF.
Patch 1 and 4 still need review from the lsm/security side.
This patchset is in my review queue and assuming everything checks
out, I expect to merge it after the upcoming merge window closes.
It doesn't even address my issues with the last patchset.
Are you referring to [1], and with regards to [2], is the issue that the
wording could be improved for both the cover letter and patch 1/4?
Ultimately, the goal of CF is to leverage and use user namespaces and
block tasks whose meta information do not align with our allow list
criteria. Yes, there is a higher goal of restricting our attack surface.
Yes, people will find ways around security. The point is to have
multiple levels of security, and this patch series allows people to add
another level.
Calling this hook a regression is not true since there's no actual
regression in the code. What would constitute a perceived regression is
an admin imposing such a SELinux or BPF restriction within their
company, but developers in that company ideally would try to work with
the admin to enable user namespaces for certain use cases, or
alternatively do what you don't want given current tooling: always run
code as root. That's where this hook comes in: let people observe and
enforce how they see fit. The average enthusiasts would see no impact.
I was requested to add _some_ test to BPF and to add a SELinux
implementation. The low hanging fruit for a test to prove that the hook
is capable of doing _something_ was to simply just block outright, and
provide _some example_ of use. It doesn't make sense for us to write a
test that outlines specifically what CF or others are doing because that
would put too much emphasis on an implementation detail that doesn't
matter to prove that the hook works.
Without Djalal's comment, I can't defend an observability use case that
we're not currently leveraging. We have it now, so therefore I'll defend
it per KP's suggestion[3] in v5.
By not responding to the email discussions, we can't accurately gauge
what should or should not be in the descriptions. No one here
necessarily disagrees with some of the points you made, and others have
appropriately responded. As others have also wrote, you're not proposing
alternatives. How do you expect us to work with that?
Please, let us know which bits and pieces ought to be included in the
descriptions, and let us know what things we should call out caveats to
that would satisfy your concerns.
Links:
1.
https://lore.kernel.org/all/01368386-521f-230b-1d49-de19377c27d1@xxxxxxxxxxxxxx/
2.
https://lore.kernel.org/all/877d45kri4.fsf@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/#t
3.
https://lore.kernel.org/all/CACYkzJ4x90DamdN4dRCn1gZuAHLqJNy4MoP=qTX+44Bqx1uxSQ@xxxxxxxxxxxxxx/
4.
https://lore.kernel.org/all/CAEiveUdPhEPAk7Y0ZXjPsD=Vb5hn453CHzS9aG-tkyRa8bf_eg@xxxxxxxxxxxxxx/#t
So it has my NACK.
Eric
