On Mon, Jun 27, 2022 at 11:51 AM Frederick Lawler [off-list ref] wrote:

On 6/27/22 7:11 AM, Christian Brauner wrote:

quoted

On Thu, Jun 23, 2022 at 11:21:37PM -0400, Paul Moore wrote:

quoted

On Wed, Jun 22, 2022 at 10:24 AM Frederick Lawler [off-list ref] wrote:

quoted

On 6/21/22 7:19 PM, Casey Schaufler wrote:

quoted

On 6/21/2022 4:39 PM, Frederick Lawler wrote:

...

quoted

quoted

quoted

quoted

quoted

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 create_user_ns LSM hook, then marks the hook as sleepable in BPF.

Why restrict this hook to user namespaces? It seems that an LSM that
chooses to preform controls on user namespaces may want to do so for
network namespaces as well.

IIRC, CLONE_NEWUSER is the only namespace flag that does not require
CAP_SYS_ADMIN. There is a security use case to prevent this namespace
from being created within an unprivileged environment. I'm not opposed
to a more generic hook, but I don't currently have a use case to block
any others. We can also say the same is true for the other namespaces:
add this generic security function to these too.

I'm curious what others think about this too.

While user namespaces are obviously one of the more significant
namespaces from a security perspective, I do think it seems reasonable
that the LSMs could benefit from additional namespace creation hooks.
However, I don't think we need to do all of them at once, starting
with a userns hook seems okay to me.

I also think that using the same LSM hook as an access control point
for all of the different namespaces would be a mistake.  At the very

Agreed. >

quoted

least we would need to pass a flag or some form of context to the hook
to indicate which new namespace(s) are being requested and I fear that
is a problem waiting to happen.  That isn't to say someone couldn't
mistakenly call the security_create_user_ns(...) from the mount
namespace code, but I suspect that is much easier to identify as wrong
than the equivalent security_create_ns(USER, ...).

Yeah, I think that's a pretty unlikely scenario.

quoted

We also should acknowledge that while in most cases the current task's
credentials are probably sufficient to make any LSM access control
decisions around namespace creation, it's possible that for some
namespaces we would need to pass additional, namespace specific info
to the LSM.  With a shared LSM hook this could become rather awkward.

Agreed.

quoted

quoted

quoted

Also, the hook seems backwards. You should
decide if the creation of the namespace is allowed before you create it.
Passing the new namespace to a function that checks to see creating a
namespace is allowed doesn't make a lot off sense.

I think having more context to a security hook is a good thing.

This is one of the reasons why I usually like to see at least one LSM
implementation to go along with every new/modified hook.  The
implementation forces you to think about what information is necessary
to perform a basic access control decision; sometimes it isn't always
obvious until you have to write the access control :)

I spoke to Frederick at length during LSS and as I've been given to
understand there's a eBPF program that would immediately use this new
hook. Now I don't want to get into the whole "Is the eBPF LSM hook
infrastructure an LSM" but I think we can let this count as a legitimate
first user of this hook/code.

quoted

[aside: If you would like to explore the SELinux implementation let me
know, I'm happy to work with you on this.  I suspect Casey and the
other LSM maintainers would also be willing to do the same for their
LSMs.]

I can take a shot at making a SELinux implementation, but the question
becomes: is that for v2 or a later patch? I don't think the
implementation for SELinux would be too complicated (i.e. make a call to
avc_has_perm()?) but, testing and revisions might take a bit longer.

Isn't that the truth, writing code is easy(ish); testing it well is
the hard part ;)  Joking aside, I would suggest starting with v2.

As an example, the code below might be a good place to start - we would need to
discuss this on the SELinux list as there are some design decisions
I'm glossing over[1].

  int selinux_userns_create(struct cred *new)
  {
    u32 sid = current_sid();

    return avc_has_perm(&selinux_state, sid, sid,
                        SECCLASS_NAMESPACE,
                        NAMESPACE__USERNS_CREATE);
  }

You would also need to add the "namespace" class and the
"userns_create" permission in security/selinux/include/classmap.h.

  const struct security_class_mapping secclass_map[] = {
    ...
    { "namespace",
      { "userns_create", NULL } },
  }

As I mentioned earlier, if you find yourself getting stuck, or needing
some help, please feel free to send mail.

[1] This code snippet uses a new object class and permission for this
(namespace:userns_create).  I made that choice as object classes are
limited to 32 unique permissions and I expect the number of namespaces
to continue to grow.

-- 
paul-moore.com