Messages in this thread | | | Date | Tue, 19 Jul 2022 14:59:44 -0500 | Subject | Re: [PATCH v2 0/4] Introduce security_create_user_ns() | From | Frederick Lawler <> |
| |
On 7/14/22 9:27 AM, Serge E. Hallyn wrote: > On Fri, Jul 08, 2022 at 09:11:15AM -0700, Casey Schaufler wrote: >> On 7/8/2022 7:01 AM, Frederick Lawler wrote: >>> On 7/8/22 7:10 AM, Christian Göttsche wrote: >>>> ,On Fri, 8 Jul 2022 at 00:32, Frederick Lawler <fred@cloudflare.com> >>>> 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 create_user_ns LSM hook, then marks the hook as sleepable in BPF. >>>> >>>> Some thoughts: >>>> >>>> I. >>>> >>>> Why not make the hook more generic, e.g. support all other existing >>>> and potential future namespaces? >>> >>> The main issue with a generic hook is that different namespaces have >>> different calling contexts. We decided in a previous discussion to >>> opt-out of a generic hook for this reason. [1] >>> >>>> Also I think the naming scheme is <object>_<verb>. >>> >>> That's a good call out. I was originally hoping to keep the >>> security_*() match with the hook name matched with the caller function >>> to keep things all aligned. If no one objects to renaming the hook, I >>> can rename the hook for v3. >>> >>>> >>>> LSM_HOOK(int, 0, namespace_create, const struct cred *cred, >>>> unsigned int flags) >>>> >>>> where flags is a bitmap of CLONE flags from include/uapi/linux/sched.h >>>> (like CLONE_NEWUSER). >>>> >>>> II. >>>> >>>> While adding policing for namespaces maybe also add a new hook for >>>> setns(2) >>>> >>>> LSM_HOOK(int, 0, namespace_join, const struct cred *subj, const >>>> struct cred *obj, unsigned int flags) >>>> >>> >>> IIUC, setns() will create a new namespace for the other namespaces >>> except for user namespace. If we add a security hook for the other >>> create_*_ns() functions, then we can catch setns() at that point. >>> >>>> III. >>>> >>>> Maybe even attach a security context to namespaces so they can be >>>> further governed? >> >> That would likely add confusion to the existing security module namespace >> efforts. SELinux, Smack and AppArmor have all developed namespace models. >> That, or it could replace the various independent efforts with a single, > > I feel like you're attaching more meaning to this than there needs to be. > I *think* he's just talking about a user_namespace->u_security void*. > So that for instance while deciding whether to allow some transition, > selinux could check whether the caller's user namespace was created by > a task in an selinux context authorized to create user namespaces. > > The "user namespaces are DAC and orthogonal to MAC" is of course true > (where the LSM does not itself tie them together), except that we all > know that a process running as root in a user namespace gains access to > often-less-trustworthy code gated under CAP_SYS_ADMIN. > >> unified security module namespace effort. There's more work to that than >> adding a context to a namespace. Treating namespaces as objects is almost, >> but not quite, solidifying containers as a kernel construct. We know we >> can't do that. > > What we "can't do" (imo) is to create a "full container" construct which > ties together the various namespaces and other concepts in a restrictive > way. >
Is this the direction we want to go with the SELinux implementation? If so, where can I find a similar implementation to make the userns_create work with this? If not, I have a v3 with the hook name change ready to post.
>>>> SELinux example: >>>> >>>> type domainA_userns_t; >>>> type_transition domainA_t domainA_t : namespace domainA_userns_t >>>> "user"; >>>> allow domainA_t domainA_userns_t:namespace create; >>>> >>>> # domainB calling setns(2) with domainA as target >>>> allow domainB_t domainA_userns_t:namespace join; >> >> While I'm not an expert on SELinux policy, I'd bet a refreshing beverage >> that there's already a way to achieve this with existing constructs. >> Smack, which is subject+object MAC couldn't care less about the user >> namespace configuration. User namespaces are DAC constructs. >> >>>> >>> >>> Links: >>> 1. >>> https://lore.kernel.org/all/CAHC9VhSTkEMT90Tk+=iTyp3npWEm+3imrkFVX2qb=XsOPp9F=A@mail.gmail.com/ >>> >>>>> >>>>> Links: >>>>> 1. >>>>> https://lore.kernel.org/all/20220608150942.776446-1-fred@cloudflare.com/ >>>>> >>>>> 2. >>>>> https://lore.kernel.org/all/87y1xzyhub.fsf@email.froward.int.ebiederm.org/ >>>>> 3. >>>>> https://lore.kernel.org/all/9fe9cd9f-1ded-a179-8ded-5fde8960a586@cloudflare.com/ >>>>> >>>>> Changes since v1: >>>>> - Add selftests/bpf: Add tests verifying bpf lsm create_user_ns hook >>>>> patch >>>>> - Add selinux: Implement create_user_ns hook patch >>>>> - Change function signature of security_create_user_ns() to only take >>>>> struct cred >>>>> - Move security_create_user_ns() call after id mapping check in >>>>> create_user_ns() >>>>> - Update documentation to reflect changes >>>>> >>>>> Frederick Lawler (4): >>>>> security, lsm: Introduce security_create_user_ns() >>>>> bpf-lsm: Make bpf_lsm_create_user_ns() sleepable >>>>> selftests/bpf: Add tests verifying bpf lsm create_user_ns hook >>>>> selinux: Implement create_user_ns hook >>>>> >>>>> include/linux/lsm_hook_defs.h | 1 + >>>>> include/linux/lsm_hooks.h | 4 + >>>>> include/linux/security.h | 6 ++ >>>>> kernel/bpf/bpf_lsm.c | 1 + >>>>> kernel/user_namespace.c | 5 ++ >>>>> security/security.c | 5 ++ >>>>> security/selinux/hooks.c | 9 ++ >>>>> security/selinux/include/classmap.h | 2 + >>>>> .../selftests/bpf/prog_tests/deny_namespace.c | 88 >>>>> +++++++++++++++++++ >>>>> .../selftests/bpf/progs/test_deny_namespace.c | 39 ++++++++ >>>>> 10 files changed, 160 insertions(+) >>>>> create mode 100644 >>>>> tools/testing/selftests/bpf/prog_tests/deny_namespace.c >>>>> create mode 100644 >>>>> tools/testing/selftests/bpf/progs/test_deny_namespace.c >>>>> >>>>> -- >>>>> 2.30.2 >>>>> >>>
| |