* Re: [PATCH bpf-next v3 04/10] bpf: lsm: Add mutable hooks list for the BPF LSM
From: KP Singh @ 2020-02-11 12:43 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: KP Singh, linux-kernel, bpf, linux-security-module,
Brendan Jackman, Florent Revest, Thomas Garnier,
Alexei Starovoitov, Daniel Borkmann, James Morris, Kees Cook,
Thomas Garnier, Michael Halcrow, Paul Turner, Brendan Gregg,
Jann Horn, Matthew Garrett, Christian Brauner,
Mickaël Salaün, Florent Revest, Brendan Jackman,
Serge E. Hallyn, Mauro Carvalho Chehab, David S. Miller,
Greg Kroah-Hartman, kernel-team
In-Reply-To: <20200211031208.e6osrcathampoog7@ast-mbp>
On 10-Feb 19:12, Alexei Starovoitov wrote:
> On Thu, Jan 23, 2020 at 07:24:34AM -0800, KP Singh wrote:
> > +#define CALL_BPF_LSM_INT_HOOKS(FUNC, ...) ({ \
> > + int _ret = 0; \
> > + do { \
> > + struct security_hook_list *P; \
> > + int _idx; \
> > + \
> > + if (hlist_empty(&bpf_lsm_hook_heads.FUNC)) \
> > + break; \
> > + \
> > + _idx = bpf_lsm_srcu_read_lock(); \
> > + \
> > + hlist_for_each_entry(P, \
> > + &bpf_lsm_hook_heads.FUNC, list) { \
> > + _ret = P->hook.FUNC(__VA_ARGS__); \
> > + if (_ret && IS_ENABLED(CONFIG_SECURITY_BPF_ENFORCE)) \
> > + break; \
> > + } \
> > + bpf_lsm_srcu_read_unlock(_idx); \
> > + } while (0); \
> > + IS_ENABLED(CONFIG_SECURITY_BPF_ENFORCE) ? _ret : 0; \
> > +})
>
> This extra CONFIG_SECURITY_BPF_ENFORCE doesn't make sense to me.
We found it easier to debug the programs if enforcement is disabled.
But we can remove this option if you feel strongly about it.
> Why do all the work for bpf-lsm and ignore return code? Such framework already
> exists. For audit only case the user could have kprobed security_*() in
> security/security.c and had access to exactly the same data. There is no need
> in any of these patches if audit the only use case.
> Obviously bpf-lsm has to be capable of making go/no-go decision, so
> my preference is to drop this extra kconfig knob.
> I think the agreement seen in earlier comments in this thread that the prefered
> choice is to always have bpf-based lsm to be equivalent to LSM_ORDER_LAST. In
> such case how about using bpf-trampoline fexit logic instead?
Even if the BPF LSM is LSM_ORDER_LAST this is still different from
adding a trampoline to the exit of the security hooks for a few other
reasons.
> Pros:
> - no changes to security/ directory
* We do need to initialize the BPF LSM as a proper LSM (i.e. in
security/bpf) because it needs access to security blobs. This is
only possible statically for now as they should be set after boot
time to provide guarantees to any helper that uses information in
security blobs. I have proposed how we could make this dynamic as a
discussion topic for the BPF conference:
https://lore.kernel.org/bpf/20200127171516.GA2710@chromium.org
As you can see from some of the prototype use cases e.g:
https://github.com/sinkap/linux-krsi/blob/patch/v1/examples/samples/bpf/lsm_detect_exec_unlink.c
that they do rely on security blobs and that they are key in doing
meaningful security analysis.
* When using the semantic provided by fexit, the BPF LSM program will
always be executed and will be able to override / clobber the
decision of LSMs which appear before it in the ordered list. This
semantic is very different from what we currently have (i.e. the BPF
LSM hook is only called if all the other LSMs allow the action) and
seems to be bypassing the LSM framework.
* Not all security_* wrappers simply call the attached hooks and return
their exit code and not all of them pass the same arguments to the
hook e.g. security_bprm_check, security_file_open,
security_task_alloc to just name a few. Illustrating this further
using security_task_alloc as an example:
rc = call_int_hook(task_alloc, 0, task, clone_flags);
if (unlikely(rc))
security_task_free(task);
return rc;
Which means we would leak task_structs in this case. While
call_int_hook is sort of equivalent to the fexit trampoline for most
hooks, it's not really the case for some (quite important) LSM hooks.
- KP
> - no changes to call_int_hook() macro
> - patches 4, 5, 6 no longer necessary
> - when security is off all security_*() hooks do single
> if (hlist_empty(&bpf_lsm_hook_heads.FUNC)) check.
> With patch 4 there will two such checks. Tiny perf penalty.
> With fexit approach there will be no extra check.
> - fexit approach is fast even on kernels compiled with retpoline, since
> its using direct calls
> Cons:
> - bpf trampoline so far is x86 only and arm64 support is wip
>
> By plugging into fexit I'm proposing to let bpf-lsm prog type modify return
> value. Currently bpf-fexit prog type has read-only access to it. Adding write
> access is a straightforward verifier change. The bpf progs from patch 9 will
> still look exactly the same way:
> SEC("lsm/file_mprotect")
> int BPF_PROG(mprotect_audit, struct vm_area_struct *vma,
> unsigned long reqprot, unsigned long prot) { ... }
> The difference that libbpf will be finding btf_id of security_file_mprotect()
> function and adding fexit trampoline to it instead of going via
> security_list_options and its own lsm_hook_idx uapi. I think reusing existing
> tracing facilities to attach and multiplex multiple programs is cleaner. More
> code reuse. Unified testing of lsm and tracing, etc.
^ permalink raw reply
* Re: [PATCH bpf-next v3 03/10] bpf: lsm: Introduce types for eBPF based LSM
From: KP Singh @ 2020-02-11 12:44 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: KP Singh, linux-kernel, bpf, linux-security-module,
Brendan Jackman, Florent Revest, Thomas Garnier,
Alexei Starovoitov, Daniel Borkmann, James Morris, Kees Cook,
Thomas Garnier, Michael Halcrow, Paul Turner, Brendan Gregg,
Jann Horn, Matthew Garrett, Christian Brauner,
Mickaël Salaün, Florent Revest, Brendan Jackman,
Martin KaFai Lau, Song Liu, Yonghong Song, Serge E. Hallyn,
Mauro Carvalho Chehab, David S. Miller, Greg Kroah-Hartman,
Nicolas Ferre, Stanislav Fomichev, Quentin Monnet, Andrey Ignatov,
Joe Stringer
In-Reply-To: <20200210235811.pbzvlok6rin7lctd@ast-mbp>
On 10-Feb 15:58, Alexei Starovoitov wrote:
> On Thu, Jan 23, 2020 at 07:24:33AM -0800, KP Singh wrote:
> > +
> > +static const struct bpf_func_proto *get_bpf_func_proto(
> > + enum bpf_func_id func_id, const struct bpf_prog *prog)
> > +{
> > + switch (func_id) {
> > + case BPF_FUNC_map_lookup_elem:
> > + return &bpf_map_lookup_elem_proto;
> > + case BPF_FUNC_get_current_pid_tgid:
> > + return &bpf_get_current_pid_tgid_proto;
> > + default:
> > + return NULL;
> > + }
> > +}
> > +
> > +const struct bpf_verifier_ops lsm_verifier_ops = {
> > + .get_func_proto = get_bpf_func_proto,
> > +};
>
> Why artificially limit it like this?
> It will cause a lot of churn in the future. Like allowing map update and
> delete, in addition to lookup, will be an obvious next step.
> I think allowing tracing_func_proto() from the start is cleaner.
Sure, I will replace it to use tracing_func_proto in the next
revision.
- KP
^ permalink raw reply
* Re: [PATCH bpf-next v3 02/10] bpf: lsm: Add a skeleton and config options
From: KP Singh @ 2020-02-11 12:45 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: KP Singh, linux-kernel, bpf, linux-security-module,
Brendan Jackman, Florent Revest, Thomas Garnier,
Alexei Starovoitov, Daniel Borkmann, James Morris, Kees Cook,
Thomas Garnier, Michael Halcrow, Paul Turner, Brendan Gregg,
Jann Horn, Matthew Garrett, Christian Brauner,
Mickaël Salaün, Florent Revest, Brendan Jackman,
Martin KaFai Lau, Song Liu, Yonghong Song, Serge E. Hallyn,
Mauro Carvalho Chehab, David S. Miller, Greg Kroah-Hartman,
Nicolas Ferre, Stanislav Fomichev, Quentin Monnet, Andrey Ignatov,
Joe Stringer
In-Reply-To: <20200210235214.ypb56vrkvzol3qdu@ast-mbp>
On 10-Feb 15:52, Alexei Starovoitov wrote:
> On Thu, Jan 23, 2020 at 07:24:32AM -0800, KP Singh wrote:
> >
> > +BPF SECURITY MODULE
> > +M: KP Singh <kpsingh@chromium.org>
> > +L: linux-security-module@vger.kernel.org
> > +L: bpf@vger.kernel.org
> > +S: Maintained
> > +F: security/bpf/
>
> Instead of creating new entry I think it's more appropriate
> to add this to main BPF entry like:
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c74e4ea714a5..f656ddec0722 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3147,6 +3147,7 @@ R: Martin KaFai Lau <kafai@fb.com>
> R: Song Liu <songliubraving@fb.com>
> R: Yonghong Song <yhs@fb.com>
> R: Andrii Nakryiko <andriin@fb.com>
> +R: KP Singh <kpsingh@chromium.org>
I am okay with this too :) Will update it in the next revision.
- KP
> L: netdev@vger.kernel.org
> L: bpf@vger.kernel.org
> T: git git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git
> @@ -3172,6 +3173,7 @@ F: samples/bpf/
> F: tools/bpf/
> F: tools/lib/bpf/
> F: tools/testing/selftests/bpf/
> +F: security/bpf/
> K: bpf
> N: bpf
^ permalink raw reply
* Re: [PATCH v14 22/23] LSM: Add /proc attr entry for full LSM context
From: Stephen Smalley @ 2020-02-11 15:59 UTC (permalink / raw)
To: John Johansen, Casey Schaufler, Simon McVittie
Cc: casey.schaufler, jmorris, linux-security-module, selinux,
keescook, penguin-kernel, paul
In-Reply-To: <422e5db4-1b61-0048-b608-78881f5fa4b4@canonical.com>
On 2/10/20 2:00 PM, John Johansen wrote:
> On 2/10/20 10:32 AM, Casey Schaufler wrote:
>> Because attr/context (and later, SO_PEERCONTEXT) are new interfaces
>> there is no need to exactly duplicate what is in attr/current (later
>> SO_PEERSEC). I already plan to omit the "mode" component of the
>> AppArmor data in the AppArmor hook, as was discussed earlier. I would
>> prefer ASCII, but if AppArmor needs bytestrings, that's what we'll
>> have to do.
>>
>
> sadly, to not break userspace its a byte string because that is what the
> path based profile names are. AppArmor does support a more limited non
> path based profile name but I can't guarantee that is what userspace is
> using in policy.
Ok, so /proc/pid/attr/context and SO_PEERCONTEXT have to be defined as
returning bytestrings.
So far we've talked about having AppArmor drop the trailing newline, be
consistent with regard to whether the terminating NUL is included or
excluded, and drop the mode field from what it returns for
/proc/pid/attr/context and SO_PEERCONTEXT versus the current
/proc/pid/attr/current and SO_PEERSEC. Is that correct?
How do we envision a liblsm processing the value returned from
/proc/pid/attr/context or SO_PEEERCONTEXT? It can certainly split it up
per LSM. But can it then take the apparmor portion of the context and
pass that to existing libapparmor interfaces without breakage? Or will
the changes to the format described above create issues there?
^ permalink raw reply
* Re: [PATCH v2] ima: export the measurement list when needed
From: david.safford @ 2020-02-11 16:10 UTC (permalink / raw)
To: Mimi Zohar, Janne Karhunen, linux-integrity,
linux-security-module
Cc: Ken Goldman, monty.wiseman, Amir Goldstein, linux-fsdevel
In-Reply-To: <1581366258.5585.891.camel@linux.ibm.com>
On Mon, 2020-02-10 at 15:24 -0500, Mimi Zohar wrote:
> On Mon, 2020-02-10 at 13:18 -0500, david.safford@gmail.com wrote:
> > On Thu, 2020-02-06 at 09:13 -0500, Mimi Zohar wrote:
> > > Hi Janne,
> > >
> > > On Fri, 2020-01-10 at 10:48 +0200, Janne Karhunen wrote:
> > > > On Wed, Jan 8, 2020 at 1:18 PM Janne Karhunen <janne.karhunen@gmail.com> wrote:
> > > > > Some systems can end up carrying lots of entries in the ima
> > > > > measurement list. Since every entry is using a bit of kernel
> > > > > memory, allow the sysadmin to export the measurement list to
> > > > > the filesystem to free up some memory.
> > > >
> > > > Hopefully this addressed comments from everyone. The flush event can
> > > > now be triggered by the admin anytime and unique file names can be
> > > > used for each flush (log.1, log.2, ...) etc, so getting to the correct
> > > > item should be easy.
> > > >
> > > > While it can now be argued that since this is an admin-driven event,
> > > > kernel does not need to write the file. However, the intention is to
> > > > bring out a second patch a bit later that adds a variable to define
> > > > the max number of entries to be kept in the kernel memory and
> > > > workqueue based automatic flushing. In those cases the kernel has to
> > > > be able to write the file without any help from the admin..
> > >
> > > The implications of exporting and removing records from the IMA-
> > > measurement list needs to be considered carefully. Verifying a TPM
> > > quote will become dependent on knowing where the measurements are
> > > stored. The existing measurement list is stored in kernel memory and,
> > > barring a kernel memory attack, is protected from modification.
> > > Before upstreaming this or a similar patch, there needs to be a
> > > discussion as to how the measurement list will be protected once is it
> > > exported to userspace.
> >
> > "Protected" here can mean two different aspects: cryptographically
> > protected from tampering, which is covered with the TPM_QUOTE, and
> > availability protected from even accidental deletion, which is what
> > I suspect you are concerned about. Certainly my original TLV patches
> > were too flippant about this, as userspace had to be trusted not to
> > drop any records. In this patch, the kernel writes the data in an
> > atomic fashion. Either all records are successfully written, or none
> > are, and an error is returned.
>
> A third aspect, which I'm concerned about, is removing records from
> the measurement list. This changes the existing userspace
> expectations of returning the entire measurement list. Now userspace
> will need some out of band method of knowing where to look for the
> measurements.
This is a feature, not a bug. :-)
There is no reason to resend the same data for every attestation,
nor is there any reason to store already attested measurements anywhere
on the client. By versioning the log file names, userspace gets a
simple way to know what has and has not been attested, and for small
embedded devices we don't need to waste memory or filesystem space
on the data already attested.
> When Thiago and I added support for carrying the measurement list
> across kexec, there were a number of additional measurements after the
> kexec load. These additional measurements will need to be safely
> written out to file in order to validate the TPM quote.
>
> > I have been testing this patch on all of these scenarios, and it
> > provides a simple, powerful approach for all of them.
>
> Were you able to walk the measurement list and validate the TPM quote
> after a kexec?
I'm still working on this. (I've mainly been making sure this works
for normal template and TLV lists.) I should be able to write out the
remaining kexec measurements, but haven't actually validated that
yet...
> > > The kernel already exports the IMA measurement list to userspace via a
> > > securityfs file. From a userspace perspective, missing is the ability
> > > of removing N number of records. In this scenario, userspace would be
> > > responsible for safely storing the measurements (e.g. blockchain).
> > > The kernel would only be responsible for limiting permission, perhaps
> > > based on a capability, before removing records from the measurement
> > > list.
> >
> > I don't think we want to export 'N' records, as this would
> > be really hard to understand and coordinate with userspace.
> > Exporting all or none seems simpler.
>
> Userspace already has the ability of exporting the measurement list.
> However, beetween saving the measurement list to a file and telling
> the kernel to delete the records from the kernel, additional
> measurement could have been added.
This method of exporting is atomic, so only those items exported
get deleted.
> > > In the kernel usecase, somehow the kernel would need to safely export
> > > the measurement list, or some portion of the measurement list, to a
> > > file and then delete that portion. What protects the exported records
> > > stored in a file from modification?
> >
> > Tampering is prevented with the TPM_QUOTE. Accidental deletion is
> > protected with CAP_SYS_ADMIN. If CAP_SYS_ADMIN is untrusted, you
> > have bigger problems, and even then it will be detected.
>
> Agreed, attestation will detect any tampering, but up to now we didn't
> have to rely on DAC/MAC to prevent tampering of the measurement list.
The userspace attestation process has always been able to tamper or
delete the list data during its attestation, but we can detect this
remotely.
> > > Instead of exporting the measurement records, one option as suggested
> > > by Amir Goldstein, would be to use a vfs_tmpfile() to get an anonymous
> > > file for backing store. The existing securityfs measurement lists
> > > would then read from this private copy of the anonymous file.
> >
> > This doesn't help in use cases where we really do want to
> > export to a persistent file, without userspace help.
>
> Is to prevent needing to carry the measurement list across kexec the
> only reason for the kernel needing to write to a persistent file?
Well, that and the other reasons mentioned, such as completely freeing
the data from the client after attestation, and simplicity of the
mechanism.
dave
> Mimi
>
^ permalink raw reply
* [PATCH 05/24] proc: task_state(): use from_kfs{g,u}id_munged
From: Christian Brauner @ 2020-02-11 16:57 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Alexander Viro, Alexey Dobriyan, Serge Hallyn,
James Morris, Kees Cook, Jonathan Corbet, linux-kernel,
linux-fsdevel, containers, linux-security-module, linux-api,
Christian Brauner
In-Reply-To: <20200211165753.356508-1-christian.brauner@ubuntu.com>
If fsid mappings have been written, this will cause proc to look at fsid
mappings for the user namespace. If no fsid mappings have been written the
behavior is as before.
Here is part of the output from /proc/<pid>/status from the initial user
namespace for systemd running in an unprivileged container as user namespace
root with id mapping 0 100000 100000 and fsid mapping 0 300000 100000:
Name: systemd
Umask: 0000
State: S (sleeping)
Tgid: 13023
Ngid: 0
Pid: 13023
PPid: 13008
TracerPid: 0
Uid: 100000 100000 100000 300000
Gid: 100000 100000 100000 300000
FDSize: 64
Groups:
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
fs/proc/array.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/proc/array.c b/fs/proc/array.c
index 5efaf3708ec6..d4a04f85a67e 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -91,6 +91,7 @@
#include <linux/string_helpers.h>
#include <linux/user_namespace.h>
#include <linux/fs_struct.h>
+#include <linux/fsuidgid.h>
#include <asm/pgtable.h>
#include <asm/processor.h>
@@ -193,11 +194,11 @@ static inline void task_state(struct seq_file *m, struct pid_namespace *ns,
seq_put_decimal_ull(m, "\nUid:\t", from_kuid_munged(user_ns, cred->uid));
seq_put_decimal_ull(m, "\t", from_kuid_munged(user_ns, cred->euid));
seq_put_decimal_ull(m, "\t", from_kuid_munged(user_ns, cred->suid));
- seq_put_decimal_ull(m, "\t", from_kuid_munged(user_ns, cred->fsuid));
+ seq_put_decimal_ull(m, "\t", from_kfsuid_munged(user_ns, cred->fsuid));
seq_put_decimal_ull(m, "\nGid:\t", from_kgid_munged(user_ns, cred->gid));
seq_put_decimal_ull(m, "\t", from_kgid_munged(user_ns, cred->egid));
seq_put_decimal_ull(m, "\t", from_kgid_munged(user_ns, cred->sgid));
- seq_put_decimal_ull(m, "\t", from_kgid_munged(user_ns, cred->fsgid));
+ seq_put_decimal_ull(m, "\t", from_kfsgid_munged(user_ns, cred->fsgid));
seq_put_decimal_ull(m, "\nFDSize:\t", max_fds);
seq_puts(m, "\nGroups:\t");
--
2.25.0
^ permalink raw reply related
* [PATCH 06/24] fs: add is_userns_visible() helper
From: Christian Brauner @ 2020-02-11 16:57 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Alexander Viro, Alexey Dobriyan, Serge Hallyn,
James Morris, Kees Cook, Jonathan Corbet, linux-kernel,
linux-fsdevel, containers, linux-security-module, linux-api,
Christian Brauner
In-Reply-To: <20200211165753.356508-1-christian.brauner@ubuntu.com>
Introduce a helper which makes it possible to detect fileystems whose
superblock is visible in multiple user namespace. This currently only
means proc and sys. Such filesystems usually have special semantics so their
behavior will not be changed with the introduction of fsid mappings.
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
include/linux/fs.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 98e0349adb52..1449cd363fb6 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3627,4 +3627,9 @@ static inline int inode_drain_writes(struct inode *inode)
return filemap_write_and_wait(inode->i_mapping);
}
+static inline bool is_userns_visible(unsigned long iflags)
+{
+ return (iflags & SB_I_USERNS_VISIBLE);
+}
+
#endif /* _LINUX_FS_H */
--
2.25.0
^ permalink raw reply related
* [PATCH 01/24] user_namespace: introduce fsid mappings infrastructure
From: Christian Brauner @ 2020-02-11 16:57 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Alexander Viro, Alexey Dobriyan, Serge Hallyn,
James Morris, Kees Cook, Jonathan Corbet, linux-kernel,
linux-fsdevel, containers, linux-security-module, linux-api,
Christian Brauner
In-Reply-To: <20200211165753.356508-1-christian.brauner@ubuntu.com>
This introduces the infrastructure to setup fsid mappings which will be used in
later patches.
All new code depends on CONFIG_USER_NS_FSID=y. It currently defaults to "N".
If CONFIG_USER_NS_FSID is not set, no new code is added.
In this patch fsuid_m_show() and fsgid_m_show() are introduced. They are
identical to uid_m_show() and gid_m_show() until we introduce from_kfsuid() and
from_kfsgid() in a follow-up patch.
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
include/linux/user_namespace.h | 10 +++
init/Kconfig | 11 +++
kernel/user.c | 22 ++++++
kernel/user_namespace.c | 122 +++++++++++++++++++++++++++++++++
4 files changed, 165 insertions(+)
diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
index fb9f4f799554..ffbd5e7e5ec7 100644
--- a/include/linux/user_namespace.h
+++ b/include/linux/user_namespace.h
@@ -55,6 +55,10 @@ enum ucount_type {
struct user_namespace {
struct uid_gid_map uid_map;
struct uid_gid_map gid_map;
+#ifdef CONFIG_USER_NS_FSID
+ struct uid_gid_map fsuid_map;
+ struct uid_gid_map fsgid_map;
+#endif
struct uid_gid_map projid_map;
atomic_t count;
struct user_namespace *parent;
@@ -126,6 +130,12 @@ struct seq_operations;
extern const struct seq_operations proc_uid_seq_operations;
extern const struct seq_operations proc_gid_seq_operations;
extern const struct seq_operations proc_projid_seq_operations;
+#ifdef CONFIG_USER_NS_FSID
+extern const struct seq_operations proc_fsuid_seq_operations;
+extern const struct seq_operations proc_fsgid_seq_operations;
+extern ssize_t proc_fsuid_map_write(struct file *, const char __user *, size_t, loff_t *);
+extern ssize_t proc_fsgid_map_write(struct file *, const char __user *, size_t, loff_t *);
+#endif
extern ssize_t proc_uid_map_write(struct file *, const char __user *, size_t, loff_t *);
extern ssize_t proc_gid_map_write(struct file *, const char __user *, size_t, loff_t *);
extern ssize_t proc_projid_map_write(struct file *, const char __user *, size_t, loff_t *);
diff --git a/init/Kconfig b/init/Kconfig
index a34064a031a5..4da082e4f787 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1102,6 +1102,17 @@ config USER_NS
If unsure, say N.
+config USER_NS_FSID
+ bool "User namespace fsid mappings"
+ depends on USER_NS
+ default n
+ help
+ This allows containers, to alter their filesystem id mappings.
+ With this containers with different id mappings can still share
+ the same filesystem.
+
+ If unsure, say N.
+
config PID_NS
bool "PID Namespaces"
default y
diff --git a/kernel/user.c b/kernel/user.c
index 5235d7f49982..2ccaea9b810b 100644
--- a/kernel/user.c
+++ b/kernel/user.c
@@ -55,6 +55,28 @@ struct user_namespace init_user_ns = {
},
},
},
+#ifdef CONFIG_USER_NS_FSID
+ .fsuid_map = {
+ .nr_extents = 1,
+ {
+ .extent[0] = {
+ .first = 0,
+ .lower_first = 0,
+ .count = 4294967295U,
+ },
+ },
+ },
+ .fsgid_map = {
+ .nr_extents = 1,
+ {
+ .extent[0] = {
+ .first = 0,
+ .lower_first = 0,
+ .count = 4294967295U,
+ },
+ },
+ },
+#endif
.count = ATOMIC_INIT(3),
.owner = GLOBAL_ROOT_UID,
.group = GLOBAL_ROOT_GID,
diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index 8eadadc478f9..cbdf456f95f0 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -191,6 +191,16 @@ static void free_user_ns(struct work_struct *work)
kfree(ns->projid_map.forward);
kfree(ns->projid_map.reverse);
}
+#ifdef CONFIG_USER_NS_FSID
+ if (ns->fsgid_map.nr_extents > UID_GID_MAP_MAX_BASE_EXTENTS) {
+ kfree(ns->fsgid_map.forward);
+ kfree(ns->fsgid_map.reverse);
+ }
+ if (ns->fsuid_map.nr_extents > UID_GID_MAP_MAX_BASE_EXTENTS) {
+ kfree(ns->fsuid_map.forward);
+ kfree(ns->fsuid_map.reverse);
+ }
+#endif
retire_userns_sysctls(ns);
key_free_user_ns(ns);
ns_free_inum(&ns->ns);
@@ -637,6 +647,50 @@ static int projid_m_show(struct seq_file *seq, void *v)
return 0;
}
+#ifdef CONFIG_USER_NS_FSID
+static int fsuid_m_show(struct seq_file *seq, void *v)
+{
+ struct user_namespace *ns = seq->private;
+ struct uid_gid_extent *extent = v;
+ struct user_namespace *lower_ns;
+ uid_t lower;
+
+ lower_ns = seq_user_ns(seq);
+ if ((lower_ns == ns) && lower_ns->parent)
+ lower_ns = lower_ns->parent;
+
+ lower = from_kuid(lower_ns, KUIDT_INIT(extent->lower_first));
+
+ seq_printf(seq, "%10u %10u %10u\n",
+ extent->first,
+ lower,
+ extent->count);
+
+ return 0;
+}
+
+static int fsgid_m_show(struct seq_file *seq, void *v)
+{
+ struct user_namespace *ns = seq->private;
+ struct uid_gid_extent *extent = v;
+ struct user_namespace *lower_ns;
+ gid_t lower;
+
+ lower_ns = seq_user_ns(seq);
+ if ((lower_ns == ns) && lower_ns->parent)
+ lower_ns = lower_ns->parent;
+
+ lower = from_kgid(lower_ns, KGIDT_INIT(extent->lower_first));
+
+ seq_printf(seq, "%10u %10u %10u\n",
+ extent->first,
+ lower,
+ extent->count);
+
+ return 0;
+}
+#endif
+
static void *m_start(struct seq_file *seq, loff_t *ppos,
struct uid_gid_map *map)
{
@@ -674,6 +728,22 @@ static void *projid_m_start(struct seq_file *seq, loff_t *ppos)
return m_start(seq, ppos, &ns->projid_map);
}
+#ifdef CONFIG_USER_NS_FSID
+static void *fsuid_m_start(struct seq_file *seq, loff_t *ppos)
+{
+ struct user_namespace *ns = seq->private;
+
+ return m_start(seq, ppos, &ns->fsuid_map);
+}
+
+static void *fsgid_m_start(struct seq_file *seq, loff_t *ppos)
+{
+ struct user_namespace *ns = seq->private;
+
+ return m_start(seq, ppos, &ns->fsgid_map);
+}
+#endif
+
static void *m_next(struct seq_file *seq, void *v, loff_t *pos)
{
(*pos)++;
@@ -706,6 +776,22 @@ const struct seq_operations proc_projid_seq_operations = {
.show = projid_m_show,
};
+#ifdef CONFIG_USER_NS_FSID
+const struct seq_operations proc_fsuid_seq_operations = {
+ .start = fsuid_m_start,
+ .stop = m_stop,
+ .next = m_next,
+ .show = fsuid_m_show,
+};
+
+const struct seq_operations proc_fsgid_seq_operations = {
+ .start = fsgid_m_start,
+ .stop = m_stop,
+ .next = m_next,
+ .show = fsgid_m_show,
+};
+#endif
+
static bool mappings_overlap(struct uid_gid_map *new_map,
struct uid_gid_extent *extent)
{
@@ -1081,6 +1167,42 @@ ssize_t proc_projid_map_write(struct file *file, const char __user *buf,
&ns->projid_map, &ns->parent->projid_map);
}
+#ifdef CONFIG_USER_NS_FSID
+ssize_t proc_fsuid_map_write(struct file *file, const char __user *buf,
+ size_t size, loff_t *ppos)
+{
+ struct seq_file *seq = file->private_data;
+ struct user_namespace *ns = seq->private;
+ struct user_namespace *seq_ns = seq_user_ns(seq);
+
+ if (!ns->parent)
+ return -EPERM;
+
+ if ((seq_ns != ns) && (seq_ns != ns->parent))
+ return -EPERM;
+
+ return map_write(file, buf, size, ppos, CAP_SETUID, &ns->fsuid_map,
+ &ns->parent->fsuid_map);
+}
+
+ssize_t proc_fsgid_map_write(struct file *file, const char __user *buf,
+ size_t size, loff_t *ppos)
+{
+ struct seq_file *seq = file->private_data;
+ struct user_namespace *ns = seq->private;
+ struct user_namespace *seq_ns = seq_user_ns(seq);
+
+ if (!ns->parent)
+ return -EPERM;
+
+ if ((seq_ns != ns) && (seq_ns != ns->parent))
+ return -EPERM;
+
+ return map_write(file, buf, size, ppos, CAP_SETGID, &ns->fsgid_map,
+ &ns->parent->fsgid_map);
+}
+#endif
+
static bool new_idmap_permitted(const struct file *file,
struct user_namespace *ns, int cap_setid,
struct uid_gid_map *new_map)
--
2.25.0
^ permalink raw reply related
* [PATCH 15/24] commoncap:cap_bprm_set_creds(): handle fsid mappings
From: Christian Brauner @ 2020-02-11 16:57 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Alexander Viro, Alexey Dobriyan, Serge Hallyn,
James Morris, Kees Cook, Jonathan Corbet, linux-kernel,
linux-fsdevel, containers, linux-security-module, linux-api,
Christian Brauner
In-Reply-To: <20200211165753.356508-1-christian.brauner@ubuntu.com>
During exec the kfsids are currently reset to the effective kids. To retain the
same semantics with the introduction of fsid mappings, we lookup the userspace
effective id in the id mappings and translate the effective id into the
corresponding kfsid in the fsidmapping. This means, the behavior is unchanged
when no fsid mappings are setup and the semantics stay the same even when fsid
mappings are setup.
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
security/commoncap.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/security/commoncap.c b/security/commoncap.c
index ecfa0d0c250e..8d1a81e98610 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -811,7 +811,10 @@ int cap_bprm_set_creds(struct linux_binprm *bprm)
struct cred *new = bprm->cred;
bool effective = false, has_fcap = false, is_setid;
int ret;
- kuid_t root_uid;
+ kuid_t root_uid, kfsuid;
+ kgid_t kfsgid;
+ uid_t fsuid;
+ gid_t fsgid;
if (WARN_ON(!cap_ambient_invariant_ok(old)))
return -EPERM;
@@ -848,8 +851,15 @@ int cap_bprm_set_creds(struct linux_binprm *bprm)
old->cap_permitted);
}
- new->suid = new->fsuid = new->euid;
- new->sgid = new->fsgid = new->egid;
+ fsuid = from_kuid_munged(new->user_ns, new->euid);
+ kfsuid = make_kfsuid(new->user_ns, fsuid);
+ new->suid = new->euid;
+ new->fsuid = kfsuid;
+
+ fsgid = from_kgid_munged(new->user_ns, new->egid);
+ kfsgid = make_kfsgid(new->user_ns, fsgid);
+ new->sgid = new->egid;
+ new->fsgid = kfsgid;
/* File caps or setid cancels ambient. */
if (has_fcap || is_setid)
--
2.25.0
^ permalink raw reply related
* [PATCH 19/24] sys:__sys_setgid(): handle fsid mappings
From: Christian Brauner @ 2020-02-11 16:57 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Alexander Viro, Alexey Dobriyan, Serge Hallyn,
James Morris, Kees Cook, Jonathan Corbet, linux-kernel,
linux-fsdevel, containers, linux-security-module, linux-api,
Christian Brauner
In-Reply-To: <20200211165753.356508-1-christian.brauner@ubuntu.com>
Switch setgid() to lookup fsids in the fsid mappings. If no fsid mappings are
setup the behavior is unchanged, i.e. fsids are looked up in the id mappings.
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
kernel/sys.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/kernel/sys.c b/kernel/sys.c
index afaec8d46bc5..11f41e0a4974 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -416,24 +416,31 @@ long __sys_setgid(gid_t gid)
const struct cred *old;
struct cred *new;
int retval;
- kgid_t kgid;
+ kgid_t kgid, kfsgid;
kgid = make_kgid(ns, gid);
if (!gid_valid(kgid))
return -EINVAL;
+ kfsgid = make_kfsgid(ns, gid);
+ if (!gid_valid(kfsgid))
+ return -EINVAL;
+
new = prepare_creds();
if (!new)
return -ENOMEM;
old = current_cred();
retval = -EPERM;
- if (ns_capable(old->user_ns, CAP_SETGID))
- new->gid = new->egid = new->sgid = new->fsgid = kgid;
- else if (gid_eq(kgid, old->gid) || gid_eq(kgid, old->sgid))
- new->egid = new->fsgid = kgid;
- else
+ if (ns_capable(old->user_ns, CAP_SETGID)) {
+ new->gid = new->egid = new->sgid = kgid;
+ new->fsgid = kfsgid;
+ } else if (gid_eq(kgid, old->gid) || gid_eq(kgid, old->sgid)) {
+ new->egid = kgid;
+ new->fsgid = kfsgid;
+ } else {
goto error;
+ }
return commit_creds(new);
--
2.25.0
^ permalink raw reply related
* [PATCH 17/24] sys: __sys_setfsgid(): handle fsid mappings
From: Christian Brauner @ 2020-02-11 16:57 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Alexander Viro, Alexey Dobriyan, Serge Hallyn,
James Morris, Kees Cook, Jonathan Corbet, linux-kernel,
linux-fsdevel, containers, linux-security-module, linux-api,
Christian Brauner
In-Reply-To: <20200211165753.356508-1-christian.brauner@ubuntu.com>
Switch setfsgid() to lookup fsids in the fsid mappings. If no fsid mappings are
setup the behavior is unchanged, i.e. fsids are looked up in the id mappings.
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
kernel/sys.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/sys.c b/kernel/sys.c
index ae376d32c1e3..b89334ad0908 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -847,9 +847,9 @@ long __sys_setfsgid(gid_t gid)
kgid_t kgid;
old = current_cred();
- old_fsgid = from_kgid_munged(old->user_ns, old->fsgid);
+ old_fsgid = from_kfsgid_munged(old->user_ns, old->fsgid);
- kgid = make_kgid(old->user_ns, gid);
+ kgid = make_kfsgid(old->user_ns, gid);
if (!gid_valid(kgid))
return old_fsgid;
--
2.25.0
^ permalink raw reply related
* [PATCH 16/24] sys: __sys_setfsuid(): handle fsid mappings
From: Christian Brauner @ 2020-02-11 16:57 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Alexander Viro, Alexey Dobriyan, Serge Hallyn,
James Morris, Kees Cook, Jonathan Corbet, linux-kernel,
linux-fsdevel, containers, linux-security-module, linux-api,
Christian Brauner
In-Reply-To: <20200211165753.356508-1-christian.brauner@ubuntu.com>
Switch setfsuid() to lookup fsids in the fsid mappings. If no fsid mappings are
setup the behavior is unchanged, i.e. fsids are looked up in the id mappings.
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
kernel/sys.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/kernel/sys.c b/kernel/sys.c
index a9331f101883..ae376d32c1e3 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -59,6 +59,7 @@
#include <linux/sched/cputime.h>
#include <linux/rcupdate.h>
#include <linux/uidgid.h>
+#include <linux/fsuidgid.h>
#include <linux/cred.h>
#include <linux/nospec.h>
@@ -802,9 +803,9 @@ long __sys_setfsuid(uid_t uid)
kuid_t kuid;
old = current_cred();
- old_fsuid = from_kuid_munged(old->user_ns, old->fsuid);
+ old_fsuid = from_kfsuid_munged(old->user_ns, old->fsuid);
- kuid = make_kuid(old->user_ns, uid);
+ kuid = make_kfsuid(old->user_ns, uid);
if (!uid_valid(kuid))
return old_fsuid;
--
2.25.0
^ permalink raw reply related
* [PATCH 18/24] sys:__sys_setuid(): handle fsid mappings
From: Christian Brauner @ 2020-02-11 16:57 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Alexander Viro, Alexey Dobriyan, Serge Hallyn,
James Morris, Kees Cook, Jonathan Corbet, linux-kernel,
linux-fsdevel, containers, linux-security-module, linux-api,
Christian Brauner
In-Reply-To: <20200211165753.356508-1-christian.brauner@ubuntu.com>
Switch setuid() to lookup fsids in the fsid mappings. If no fsid mappings are
setup the behavior is unchanged, i.e. fsids are looked up in the id mappings.
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
kernel/sys.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/kernel/sys.c b/kernel/sys.c
index b89334ad0908..afaec8d46bc5 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -574,11 +574,16 @@ long __sys_setuid(uid_t uid)
struct cred *new;
int retval;
kuid_t kuid;
+ kuid_t kfsuid;
kuid = make_kuid(ns, uid);
if (!uid_valid(kuid))
return -EINVAL;
+ kfsuid = make_kfsuid(ns, uid);
+ if (!uid_valid(kfsuid))
+ return -EINVAL;
+
new = prepare_creds();
if (!new)
return -ENOMEM;
@@ -596,7 +601,8 @@ long __sys_setuid(uid_t uid)
goto error;
}
- new->fsuid = new->euid = kuid;
+ new->euid = kuid;
+ new->fsuid = kfsuid;
retval = security_task_fix_setuid(new, old, LSM_SETID_ID);
if (retval < 0)
--
2.25.0
^ permalink raw reply related
* [PATCH 14/24] commoncap: cap_task_fix_setuid(): handle fsid mappings
From: Christian Brauner @ 2020-02-11 16:57 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Alexander Viro, Alexey Dobriyan, Serge Hallyn,
James Morris, Kees Cook, Jonathan Corbet, linux-kernel,
linux-fsdevel, containers, linux-security-module, linux-api,
Christian Brauner
In-Reply-To: <20200211165753.356508-1-christian.brauner@ubuntu.com>
Switch cap_task_fix_setuid() to lookup fsids in the fsid mappings. If no fsid
mappings are setup the behavior is unchanged, i.e. fsids are looked up in the
id mappings.
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
security/commoncap.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/security/commoncap.c b/security/commoncap.c
index f4ee0ae106b2..ecfa0d0c250e 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -24,6 +24,7 @@
#include <linux/user_namespace.h>
#include <linux/binfmts.h>
#include <linux/personality.h>
+#include <linux/fsuidgid.h>
/*
* If a non-root user executes a setuid-root binary in
@@ -1051,7 +1052,7 @@ int cap_task_fix_setuid(struct cred *new, const struct cred *old, int flags)
* if not, we might be a bit too harsh here.
*/
if (!issecure(SECURE_NO_SETUID_FIXUP)) {
- kuid_t root_uid = make_kuid(old->user_ns, 0);
+ kuid_t root_uid = make_kfsuid(old->user_ns, 0);
if (uid_eq(old->fsuid, root_uid) && !uid_eq(new->fsuid, root_uid))
new->cap_effective =
cap_drop_fs_set(new->cap_effective);
--
2.25.0
^ permalink raw reply related
* [PATCH 13/24] attr: notify_change(): handle fsid mappings
From: Christian Brauner @ 2020-02-11 16:57 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Alexander Viro, Alexey Dobriyan, Serge Hallyn,
James Morris, Kees Cook, Jonathan Corbet, linux-kernel,
linux-fsdevel, containers, linux-security-module, linux-api,
Christian Brauner
In-Reply-To: <20200211165753.356508-1-christian.brauner@ubuntu.com>
Switch notify_change() to lookup fsids in the fsid mappings. If no fsid
mappings are setup the behavior is unchanged, i.e. fsids are looked up in the
id mappings.
Filesystems that share a superblock in all user namespaces they are mounted in
will retain their old semantics even with the introduction of fsidmappings.
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
fs/attr.c | 23 +++++++++++++++++------
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/fs/attr.c b/fs/attr.c
index df28035aa23e..3aa65165fb06 100644
--- a/fs/attr.c
+++ b/fs/attr.c
@@ -17,6 +17,8 @@
#include <linux/security.h>
#include <linux/evm.h>
#include <linux/ima.h>
+#include <linux/fsuidgid.h>
+#include <linux/fs.h>
static bool chown_ok(const struct inode *inode, kuid_t uid)
{
@@ -311,12 +313,21 @@ int notify_change(struct dentry * dentry, struct iattr * attr, struct inode **de
* Verify that uid/gid changes are valid in the target
* namespace of the superblock.
*/
- if (ia_valid & ATTR_UID &&
- !kuid_has_mapping(inode->i_sb->s_user_ns, attr->ia_uid))
- return -EOVERFLOW;
- if (ia_valid & ATTR_GID &&
- !kgid_has_mapping(inode->i_sb->s_user_ns, attr->ia_gid))
- return -EOVERFLOW;
+ if (is_userns_visible(inode->i_sb->s_iflags)) {
+ if (ia_valid & ATTR_UID &&
+ !kuid_has_mapping(inode->i_sb->s_user_ns, attr->ia_uid))
+ return -EOVERFLOW;
+ if (ia_valid & ATTR_GID &&
+ !kgid_has_mapping(inode->i_sb->s_user_ns, attr->ia_gid))
+ return -EOVERFLOW;
+ } else {
+ if (ia_valid & ATTR_UID &&
+ !kfsuid_has_mapping(inode->i_sb->s_user_ns, attr->ia_uid))
+ return -EOVERFLOW;
+ if (ia_valid & ATTR_GID &&
+ !kfsgid_has_mapping(inode->i_sb->s_user_ns, attr->ia_gid))
+ return -EOVERFLOW;
+ }
/* Don't allow modifications of files with invalid uids or
* gids unless those uids & gids are being made valid.
--
2.25.0
^ permalink raw reply related
* [PATCH 12/24] posix_acl: handle fsid mappings
From: Christian Brauner @ 2020-02-11 16:57 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Alexander Viro, Alexey Dobriyan, Serge Hallyn,
James Morris, Kees Cook, Jonathan Corbet, linux-kernel,
linux-fsdevel, containers, linux-security-module, linux-api,
Christian Brauner
In-Reply-To: <20200211165753.356508-1-christian.brauner@ubuntu.com>
Switch posix_acls() to lookup fsids in the fsid mappings. If no fsid
mappings are setup the behavior is unchanged, i.e. fsids are looked up in the
id mappings.
Afaict, all filesystems that share a superblock in all user namespaces
currently do not support acls so this change should be safe to do
unconditionally.
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
fs/posix_acl.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/fs/posix_acl.c b/fs/posix_acl.c
index 249672bf54fe..763bba24f380 100644
--- a/fs/posix_acl.c
+++ b/fs/posix_acl.c
@@ -22,6 +22,7 @@
#include <linux/xattr.h>
#include <linux/export.h>
#include <linux/user_namespace.h>
+#include <linux/fsuidgid.h>
static struct posix_acl **acl_by_type(struct inode *inode, int type)
{
@@ -692,12 +693,12 @@ static void posix_acl_fix_xattr_userns(
for (end = entry + count; entry != end; entry++) {
switch(le16_to_cpu(entry->e_tag)) {
case ACL_USER:
- uid = make_kuid(from, le32_to_cpu(entry->e_id));
- entry->e_id = cpu_to_le32(from_kuid(to, uid));
+ uid = make_kfsuid(from, le32_to_cpu(entry->e_id));
+ entry->e_id = cpu_to_le32(from_kfsuid(to, uid));
break;
case ACL_GROUP:
- gid = make_kgid(from, le32_to_cpu(entry->e_id));
- entry->e_id = cpu_to_le32(from_kgid(to, gid));
+ gid = make_kfsgid(from, le32_to_cpu(entry->e_id));
+ entry->e_id = cpu_to_le32(from_kfsgid(to, gid));
break;
default:
break;
@@ -746,12 +747,12 @@ posix_acl_from_xattr(struct user_namespace *user_ns,
return ERR_PTR(-EINVAL);
if (count == 0)
return NULL;
-
+
acl = posix_acl_alloc(count, GFP_NOFS);
if (!acl)
return ERR_PTR(-ENOMEM);
acl_e = acl->a_entries;
-
+
for (end = entry + count; entry != end; acl_e++, entry++) {
acl_e->e_tag = le16_to_cpu(entry->e_tag);
acl_e->e_perm = le16_to_cpu(entry->e_perm);
@@ -765,14 +766,14 @@ posix_acl_from_xattr(struct user_namespace *user_ns,
case ACL_USER:
acl_e->e_uid =
- make_kuid(user_ns,
+ make_kfsuid(user_ns,
le32_to_cpu(entry->e_id));
if (!uid_valid(acl_e->e_uid))
goto fail;
break;
case ACL_GROUP:
acl_e->e_gid =
- make_kgid(user_ns,
+ make_kfsgid(user_ns,
le32_to_cpu(entry->e_id));
if (!gid_valid(acl_e->e_gid))
goto fail;
@@ -817,11 +818,11 @@ posix_acl_to_xattr(struct user_namespace *user_ns, const struct posix_acl *acl,
switch(acl_e->e_tag) {
case ACL_USER:
ext_entry->e_id =
- cpu_to_le32(from_kuid(user_ns, acl_e->e_uid));
+ cpu_to_le32(from_kfsuid(user_ns, acl_e->e_uid));
break;
case ACL_GROUP:
ext_entry->e_id =
- cpu_to_le32(from_kgid(user_ns, acl_e->e_gid));
+ cpu_to_le32(from_kfsgid(user_ns, acl_e->e_gid));
break;
default:
ext_entry->e_id = cpu_to_le32(ACL_UNDEFINED_ID);
--
2.25.0
^ permalink raw reply related
* [PATCH 11/24] open: chown_common(): handle fsid mappings
From: Christian Brauner @ 2020-02-11 16:57 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Alexander Viro, Alexey Dobriyan, Serge Hallyn,
James Morris, Kees Cook, Jonathan Corbet, linux-kernel,
linux-fsdevel, containers, linux-security-module, linux-api,
Christian Brauner
In-Reply-To: <20200211165753.356508-1-christian.brauner@ubuntu.com>
Switch chown_common() to lookup fsids in the fsid mappings. If no fsid
mappings are setup the behavior is unchanged, i.e. fsids are looked up in the
id mappings.
Filesystems that share a superblock in all user namespaces they are mounted in
will retain their old semantics even with the introduction of fsidmappings.
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
fs/open.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/fs/open.c b/fs/open.c
index b62f5c0923a8..e5154841152c 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -32,6 +32,7 @@
#include <linux/ima.h>
#include <linux/dnotify.h>
#include <linux/compat.h>
+#include <linux/fsuidgid.h>
#include "internal.h"
@@ -626,8 +627,13 @@ static int chown_common(const struct path *path, uid_t user, gid_t group)
kuid_t uid;
kgid_t gid;
- uid = make_kuid(current_user_ns(), user);
- gid = make_kgid(current_user_ns(), group);
+ if (is_userns_visible(inode->i_sb->s_iflags)) {
+ uid = make_kuid(current_user_ns(), user);
+ gid = make_kgid(current_user_ns(), group);
+ } else {
+ uid = make_kfsuid(current_user_ns(), user);
+ gid = make_kfsgid(current_user_ns(), group);
+ }
retry_deleg:
newattrs.ia_valid = ATTR_CTIME;
--
2.25.0
^ permalink raw reply related
* [PATCH 20/24] sys:__sys_setreuid(): handle fsid mappings
From: Christian Brauner @ 2020-02-11 16:57 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Alexander Viro, Alexey Dobriyan, Serge Hallyn,
James Morris, Kees Cook, Jonathan Corbet, linux-kernel,
linux-fsdevel, containers, linux-security-module, linux-api,
Christian Brauner
In-Reply-To: <20200211165753.356508-1-christian.brauner@ubuntu.com>
Switch setreuid() to lookup fsids in the fsid mappings. If no fsid mappings are
setup the behavior is unchanged, i.e. fsids are looked up in the id mappings.
During setreuid() the kfsuid is set to the keuid corresponding the euid that is
requested by userspace. If the requested euid is -1 the kfsuid is reset to the
current keuid. For the latter case this means we need to lookup the
corresponding userspace euid corresponding to the current keuid in the id
mappings and translate this euid into the corresponding kfsuid in the fsid
mappings.
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
kernel/sys.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/kernel/sys.c b/kernel/sys.c
index 11f41e0a4974..ef1104c9df56 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -504,15 +504,18 @@ long __sys_setreuid(uid_t ruid, uid_t euid)
const struct cred *old;
struct cred *new;
int retval;
- kuid_t kruid, keuid;
+ kuid_t kruid, keuid, kfsuid;
kruid = make_kuid(ns, ruid);
keuid = make_kuid(ns, euid);
+ kfsuid = make_kfsuid(ns, euid);
if ((ruid != (uid_t) -1) && !uid_valid(kruid))
return -EINVAL;
if ((euid != (uid_t) -1) && !uid_valid(keuid))
return -EINVAL;
+ if ((euid != (uid_t) -1) && !uid_valid(kfsuid))
+ return -EINVAL;
new = prepare_creds();
if (!new)
@@ -535,6 +538,9 @@ long __sys_setreuid(uid_t ruid, uid_t euid)
!uid_eq(old->suid, keuid) &&
!ns_capable_setid(old->user_ns, CAP_SETUID))
goto error;
+ } else {
+ uid_t fsuid = from_kuid_munged(new->user_ns, new->euid);
+ kfsuid = make_kfsuid(ns, fsuid);
}
if (!uid_eq(new->uid, old->uid)) {
@@ -545,7 +551,7 @@ long __sys_setreuid(uid_t ruid, uid_t euid)
if (ruid != (uid_t) -1 ||
(euid != (uid_t) -1 && !uid_eq(keuid, old->uid)))
new->suid = new->euid;
- new->fsuid = new->euid;
+ new->fsuid = kfsuid;
retval = security_task_fix_setuid(new, old, LSM_SETID_RE);
if (retval < 0)
--
2.25.0
^ permalink raw reply related
* [PATCH 00/24] user_namespace: introduce fsid mappings
From: Christian Brauner @ 2020-02-11 16:57 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Alexander Viro, Alexey Dobriyan, Serge Hallyn,
James Morris, Kees Cook, Jonathan Corbet, linux-kernel,
linux-fsdevel, containers, linux-security-module, linux-api,
Christian Brauner
Hey everyone,
This is the implementation of shiftfs which was cooked up during lunch at
Linux Plumbers 2019 the day after the container's microconference. The
idea is a design-stew from Stéphane, Aleksa, Eric, and myself. Back then
we all were quite busy with other work and couldn't really sit down and
implement it. But I took a few days last week to do this work, including
demos and performance testing.
This implementation does not require us to touch the vfs substantially
at all. Instead, we implement shiftfs via fsid mappings.
With this patch, it took me 20 mins to port both LXD and LXC to support
shiftfs via fsid mappings.
For anyone wanting to play with this the branch can be pulled from:
https://github.com/brauner/linux/tree/fsid_mappings
https://gitlab.com/brauner/linux/-/tree/fsid_mappings
https://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux.git/log/?h=fsid_mappings
The main use case for shiftfs for us is in allowing shared writable
storage to multiple containers using non-overlapping id mappings.
In such a scenario you want the fsids to be valid and identical in both
containers for the shared mount. A demo for this exists in [3].
If you don't want to read on, go straight to the other demos below in
[1] and [2].
People not as familiar with user namespaces might not be aware that fsid
mappings already exist. Right now, fsid mappings are always identical to
id mappings. Specifically, the kernel will lookup fsuids in the uid
mappings and fsgids in the gid mappings of the relevant user namespace.
With this patch series we simply introduce the ability to create fsid
mappings that are different from the id mappings of a user namespace.
In the usual case of running an unprivileged container we will have
setup an id mapping, e.g. 0 100000 100000. The on-disk mapping will
correspond to this id mapping, i.e. all files which we want to appear as
0:0 inside the user namespace will be chowned to 100000:100000 on the
host. This works, because whenever the kernel needs to do a filesystem
access it will lookup the corresponding uid and gid in the idmapping
tables of the container.
Now think about the case where we want to have an id mapping of 0 100000
100000 but an on-disk mapping of 0 300000 100000 which is needed to e.g.
share a single on-disk mapping with multiple containers that all have
different id mappings.
This will be problematic. Whenever a filesystem access is requested, the
kernel will now try to lookup a mapping for 300000 in the id mapping
tables of the user namespace but since there is none the files will
appear to be owned by the overflow id, i.e. usually 65534:65534 or
nobody:nogroup.
With fsid mappings we can solve this by writing an id mapping of 0
100000 100000 and an fsid mapping of 0 300000 100000. On filesystem
access the kernel will now lookup the mapping for 300000 in the fsid
mapping tables of the user namespace. And since such a mapping exists,
the corresponding files will have correct ownership.
A note on proc (and sys), the proc filesystem is special in sofar as it
only has a single superblock that is (currently but might be about to
change) visible in all user namespaces (same goes for sys). This means
it has special semantics in many ways, including how file ownership and
access works. The fsid mapping implementation does not alter how proc
(and sys) ownership works. proc and sys will both continue to lookup
filesystem access in id mapping tables.
When Writing fsid mappings the same rules apply as when writing id
mappings so I won't reiterate them here. The limit of fs id mappings is
the same as for id mappings, i.e. 340 lines.
# Performance
Back when I extended the range of possible id mappings to 340 I did
performance testing by booting into single user mode, creating 1,000,000
files to fstat()ing them and calculated the mean fstat() time per file.
(Back when Linux was still fast. I won't mention that the stat
numbers have (thanks microcode!) doubled since then...)
I did the same test for this patchset: one vanilla kernel, one kernel
with my fsid mapping patches but CONFIG_USER_NS_FSID set to n and one
with fsid mappings patches enabled. I then ran the same test on all
three kernels and compared the numbers. The implementation does not
introduce overhead. That's all I can say. Here are the numbers:
| vanilla v5.5 | fsid mappings | fsid mappings | fsid mappings |
| | disabled in Kconfig | enabled in Kconfig | enabled in Kconfig |
| | | and unset for all | and set for all |
| | | test cases | test cases |
-------------|--------------|---------------------|--------------------|--------------------|
0 mappings | 367 ns | 365 ns | 365 ns | N/A |
1 mappings | 362 ns | 367 ns | 363 ns | 363 ns |
2 mappings | 361 ns | 369 ns | 363 ns | 364 ns |
3 mappings | 361 ns | 368 ns | 366 ns | 365 ns |
5 mappings | 365 ns | 368 ns | 363 ns | 365 ns |
10 mappings | 391 ns | 388 ns | 387 ns | 389 ns |
50 mappings | 395 ns | 398 ns | 401 ns | 397 ns |
100 mappings | 400 ns | 405 ns | 399 ns | 399 ns |
200 mappings | 404 ns | 407 ns | 430 ns | 404 ns |
300 mappings | 492 ns | 494 ns | 432 ns | 413 ns |
340 mappings | 495 ns | 497 ns | 500 ns | 484 ns |
# Demos
[1]: Create a container with different id and fsid mappings.
https://asciinema.org/a/300233
[2]: Create a container with id mappings but without fsid mappings.
https://asciinema.org/a/300234
[3]: Share storage between multiple containers with non-overlapping id
mappings.
https://asciinema.org/a/300235
Thanks!
Christian
Christian Brauner (24):
user_namespace: introduce fsid mappings infrastructure
proc: add /proc/<pid>/fsuid_map
proc: add /proc/<pid>/fsgid_map
fsuidgid: add fsid mapping helpers
proc: task_state(): use from_kfs{g,u}id_munged
fs: add is_userns_visible() helper
namei: may_{o_}create(): handle fsid mappings
inode: inode_owner_or_capable(): handle fsid mappings
capability: privileged_wrt_inode_uidgid(): handle fsid mappings
stat: handle fsid mappings
open: chown_common(): handle fsid mappings
posix_acl: handle fsid mappings
attr: notify_change(): handle fsid mappings
commoncap: cap_task_fix_setuid(): handle fsid mappings
commoncap:cap_bprm_set_creds(): handle fsid mappings
sys: __sys_setfsuid(): handle fsid mappings
sys: __sys_setfsgid(): handle fsid mappings
sys:__sys_setuid(): handle fsid mappings
sys:__sys_setgid(): handle fsid mappings
sys:__sys_setreuid(): handle fsid mappings
sys:__sys_setregid(): handle fsid mappings
sys:__sys_setresuid(): handle fsid mappings
sys:__sys_setresgid(): handle fsid mappings
devpts: handle fsid mappings
fs/attr.c | 23 ++-
fs/devpts/inode.c | 7 +-
fs/inode.c | 7 +-
fs/namei.c | 21 ++-
fs/open.c | 10 +-
fs/posix_acl.c | 21 +--
fs/proc/array.c | 5 +-
fs/proc/base.c | 34 ++++
fs/stat.c | 48 ++++--
include/linux/fs.h | 5 +
include/linux/fsuidgid.h | 70 ++++++++
include/linux/stat.h | 1 +
include/linux/user_namespace.h | 10 ++
init/Kconfig | 11 ++
kernel/capability.c | 13 +-
kernel/sys.c | 83 ++++++---
kernel/user.c | 22 +++
kernel/user_namespace.c | 303 ++++++++++++++++++++++++++++++++-
security/commoncap.c | 19 ++-
19 files changed, 638 insertions(+), 75 deletions(-)
create mode 100644 include/linux/fsuidgid.h
base-commit: d5226fa6dbae0569ee43ecfc08bdcd6770fc4755
--
2.25.0
^ permalink raw reply
* [PATCH 08/24] inode: inode_owner_or_capable(): handle fsid mappings
From: Christian Brauner @ 2020-02-11 16:57 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Alexander Viro, Alexey Dobriyan, Serge Hallyn,
James Morris, Kees Cook, Jonathan Corbet, linux-kernel,
linux-fsdevel, containers, linux-security-module, linux-api,
Christian Brauner
In-Reply-To: <20200211165753.356508-1-christian.brauner@ubuntu.com>
Switch inode_owner_or_capable() to lookup fsids in the fsid mappings. If no
fsid mappings are setup the behavior is unchanged, i.e. fsids are looked up in
the id mappings.
Filesystems that share a superblock in all user namespaces they are mounted in
will retain their old semantics even with the introduction of fsidmappings.
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
fs/inode.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/fs/inode.c b/fs/inode.c
index 96d62d97694e..c1ed43c5054c 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -20,6 +20,7 @@
#include <linux/ratelimit.h>
#include <linux/list_lru.h>
#include <linux/iversion.h>
+#include <linux/fsuidgid.h>
#include <trace/events/writeback.h>
#include "internal.h"
@@ -2083,8 +2084,12 @@ bool inode_owner_or_capable(const struct inode *inode)
return true;
ns = current_user_ns();
- if (kuid_has_mapping(ns, inode->i_uid) && ns_capable(ns, CAP_FOWNER))
+ if (is_userns_visible(inode->i_sb->s_iflags)) {
+ if (kuid_has_mapping(ns, inode->i_uid) && ns_capable(ns, CAP_FOWNER))
+ return true;
+ } else if (kfsuid_has_mapping(ns, inode->i_uid) && ns_capable(ns, CAP_FOWNER)) {
return true;
+ }
return false;
}
EXPORT_SYMBOL(inode_owner_or_capable);
--
2.25.0
^ permalink raw reply related
* [PATCH 10/24] stat: handle fsid mappings
From: Christian Brauner @ 2020-02-11 16:57 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Alexander Viro, Alexey Dobriyan, Serge Hallyn,
James Morris, Kees Cook, Jonathan Corbet, linux-kernel,
linux-fsdevel, containers, linux-security-module, linux-api,
Christian Brauner
In-Reply-To: <20200211165753.356508-1-christian.brauner@ubuntu.com>
Switch attribute functions looking up fsids to them up in the fsid mappings. If
no fsid mappings are setup the behavior is unchanged, i.e. fsids are looked up
in the id mappings.
Filesystems that share a superblock in all user namespaces they are mounted in
will retain their old semantics even with the introduction of fsidmappings.
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
fs/stat.c | 48 +++++++++++++++++++++++++++++++++++---------
include/linux/stat.h | 1 +
2 files changed, 39 insertions(+), 10 deletions(-)
diff --git a/fs/stat.c b/fs/stat.c
index c38e4c2e1221..1cced54e79d4 100644
--- a/fs/stat.c
+++ b/fs/stat.c
@@ -10,6 +10,7 @@
#include <linux/errno.h>
#include <linux/file.h>
#include <linux/highuid.h>
+#include <linux/fsuidgid.h>
#include <linux/fs.h>
#include <linux/namei.h>
#include <linux/security.h>
@@ -77,6 +78,8 @@ int vfs_getattr_nosec(const struct path *path, struct kstat *stat,
if (IS_AUTOMOUNT(inode))
stat->attributes |= STATX_ATTR_AUTOMOUNT;
+ stat->userns_visible = is_userns_visible(inode->i_sb->s_iflags);
+
if (inode->i_op->getattr)
return inode->i_op->getattr(path, stat, request_mask,
query_flags);
@@ -229,8 +232,13 @@ static int cp_old_stat(struct kstat *stat, struct __old_kernel_stat __user * sta
tmp.st_nlink = stat->nlink;
if (tmp.st_nlink != stat->nlink)
return -EOVERFLOW;
- SET_UID(tmp.st_uid, from_kuid_munged(current_user_ns(), stat->uid));
- SET_GID(tmp.st_gid, from_kgid_munged(current_user_ns(), stat->gid));
+ if (stat->userns_visible) {
+ SET_UID(tmp.st_uid, from_kuid_munged(current_user_ns(), stat->uid));
+ SET_GID(tmp.st_gid, from_kgid_munged(current_user_ns(), stat->gid));
+ } else {
+ SET_UID(tmp.st_uid, from_kfsuid_munged(current_user_ns(), stat->uid));
+ SET_GID(tmp.st_gid, from_kfsgid_munged(current_user_ns(), stat->gid));
+ }
tmp.st_rdev = old_encode_dev(stat->rdev);
#if BITS_PER_LONG == 32
if (stat->size > MAX_NON_LFS)
@@ -317,8 +325,13 @@ static int cp_new_stat(struct kstat *stat, struct stat __user *statbuf)
tmp.st_nlink = stat->nlink;
if (tmp.st_nlink != stat->nlink)
return -EOVERFLOW;
- SET_UID(tmp.st_uid, from_kuid_munged(current_user_ns(), stat->uid));
- SET_GID(tmp.st_gid, from_kgid_munged(current_user_ns(), stat->gid));
+ if (stat->userns_visible) {
+ SET_UID(tmp.st_uid, from_kuid_munged(current_user_ns(), stat->uid));
+ SET_GID(tmp.st_gid, from_kgid_munged(current_user_ns(), stat->gid));
+ } else {
+ SET_UID(tmp.st_uid, from_kfsuid_munged(current_user_ns(), stat->uid));
+ SET_GID(tmp.st_gid, from_kfsgid_munged(current_user_ns(), stat->gid));
+ }
tmp.st_rdev = encode_dev(stat->rdev);
tmp.st_size = stat->size;
tmp.st_atime = stat->atime.tv_sec;
@@ -461,8 +474,13 @@ static long cp_new_stat64(struct kstat *stat, struct stat64 __user *statbuf)
#endif
tmp.st_mode = stat->mode;
tmp.st_nlink = stat->nlink;
- tmp.st_uid = from_kuid_munged(current_user_ns(), stat->uid);
- tmp.st_gid = from_kgid_munged(current_user_ns(), stat->gid);
+ if (stat->userns_visible) {
+ tmp.st_uid, from_kuid_munged(current_user_ns(), stat->uid);
+ tmp.st_gid, from_kgid_munged(current_user_ns(), stat->gid);
+ } else {
+ tmp.st_uid, from_kfsuid_munged(current_user_ns(), stat->uid);
+ tmp.st_gid, from_kfsgid_munged(current_user_ns(), stat->gid);
+ }
tmp.st_atime = stat->atime.tv_sec;
tmp.st_atime_nsec = stat->atime.tv_nsec;
tmp.st_mtime = stat->mtime.tv_sec;
@@ -534,8 +552,13 @@ cp_statx(const struct kstat *stat, struct statx __user *buffer)
tmp.stx_blksize = stat->blksize;
tmp.stx_attributes = stat->attributes;
tmp.stx_nlink = stat->nlink;
- tmp.stx_uid = from_kuid_munged(current_user_ns(), stat->uid);
- tmp.stx_gid = from_kgid_munged(current_user_ns(), stat->gid);
+ if (stat->userns_visible) {
+ tmp.stx_uid = from_kuid_munged(current_user_ns(), stat->uid);
+ tmp.stx_gid = from_kgid_munged(current_user_ns(), stat->gid);
+ } else {
+ tmp.stx_uid = from_kfsuid_munged(current_user_ns(), stat->uid);
+ tmp.stx_gid = from_kfsgid_munged(current_user_ns(), stat->gid);
+ }
tmp.stx_mode = stat->mode;
tmp.stx_ino = stat->ino;
tmp.stx_size = stat->size;
@@ -605,8 +628,13 @@ static int cp_compat_stat(struct kstat *stat, struct compat_stat __user *ubuf)
tmp.st_nlink = stat->nlink;
if (tmp.st_nlink != stat->nlink)
return -EOVERFLOW;
- SET_UID(tmp.st_uid, from_kuid_munged(current_user_ns(), stat->uid));
- SET_GID(tmp.st_gid, from_kgid_munged(current_user_ns(), stat->gid));
+ if (stat->userns_visible) {
+ SET_UID(tmp.st_uid, from_kuid_munged(current_user_ns(), stat->uid));
+ SET_GID(tmp.st_gid, from_kgid_munged(current_user_ns(), stat->gid));
+ } else {
+ SET_UID(tmp.st_uid, from_kfsuid_munged(current_user_ns(), stat->uid));
+ SET_GID(tmp.st_gid, from_kfsgid_munged(current_user_ns(), stat->gid));
+ }
tmp.st_rdev = old_encode_dev(stat->rdev);
if ((u64) stat->size > MAX_NON_LFS)
return -EOVERFLOW;
diff --git a/include/linux/stat.h b/include/linux/stat.h
index 528c4baad091..e6d4ba73a970 100644
--- a/include/linux/stat.h
+++ b/include/linux/stat.h
@@ -47,6 +47,7 @@ struct kstat {
struct timespec64 ctime;
struct timespec64 btime; /* File creation time */
u64 blocks;
+ bool userns_visible;
};
#endif
--
2.25.0
^ permalink raw reply related
* [PATCH 07/24] namei: may_{o_}create(): handle fsid mappings
From: Christian Brauner @ 2020-02-11 16:57 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Alexander Viro, Alexey Dobriyan, Serge Hallyn,
James Morris, Kees Cook, Jonathan Corbet, linux-kernel,
linux-fsdevel, containers, linux-security-module, linux-api,
Christian Brauner
In-Reply-To: <20200211165753.356508-1-christian.brauner@ubuntu.com>
Switch may_{o_}create() to lookup fsids in the fsid mappings. If no fsid
mappings are setup the behavior is unchanged, i.e. fsids are looked up in the
id mappings.
Filesystems that share a superblock in all user namespaces they are mounted in
will retain their old semantics even with the introduction of fsidmappings.
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
fs/namei.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index 4fb61e0754ed..c85c65adfa9d 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -39,6 +39,7 @@
#include <linux/bitops.h>
#include <linux/init_task.h>
#include <linux/uaccess.h>
+#include <linux/fsuidgid.h>
#include "internal.h"
#include "mount.h"
@@ -2771,6 +2772,20 @@ static int may_delete(struct inode *dir, struct dentry *victim, bool isdir)
return 0;
}
+static bool fsid_has_mapping(struct user_namespace *ns, struct super_block *sb)
+{
+ if (is_userns_visible(sb->s_iflags)) {
+ if (!kuid_has_mapping(ns, current_fsuid()) ||
+ !kgid_has_mapping(ns, current_fsgid()))
+ return false;
+ } else if (!kfsuid_has_mapping(ns, current_fsuid()) ||
+ !kfsgid_has_mapping(ns, current_fsgid())) {
+ return false;
+ }
+
+ return true;
+}
+
/* Check whether we can create an object with dentry child in directory
* dir.
* 1. We can't do it if child already exists (open has special treatment for
@@ -2789,8 +2804,7 @@ static inline int may_create(struct inode *dir, struct dentry *child)
if (IS_DEADDIR(dir))
return -ENOENT;
s_user_ns = dir->i_sb->s_user_ns;
- if (!kuid_has_mapping(s_user_ns, current_fsuid()) ||
- !kgid_has_mapping(s_user_ns, current_fsgid()))
+ if (!fsid_has_mapping(s_user_ns, dir->i_sb))
return -EOVERFLOW;
return inode_permission(dir, MAY_WRITE | MAY_EXEC);
}
@@ -2972,8 +2986,7 @@ static int may_o_create(const struct path *dir, struct dentry *dentry, umode_t m
return error;
s_user_ns = dir->dentry->d_sb->s_user_ns;
- if (!kuid_has_mapping(s_user_ns, current_fsuid()) ||
- !kgid_has_mapping(s_user_ns, current_fsgid()))
+ if (!fsid_has_mapping(s_user_ns, dir->dentry->d_sb))
return -EOVERFLOW;
error = inode_permission(dir->dentry->d_inode, MAY_WRITE | MAY_EXEC);
--
2.25.0
^ permalink raw reply related
* [PATCH 03/24] proc: add /proc/<pid>/fsgid_map
From: Christian Brauner @ 2020-02-11 16:57 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Alexander Viro, Alexey Dobriyan, Serge Hallyn,
James Morris, Kees Cook, Jonathan Corbet, linux-kernel,
linux-fsdevel, containers, linux-security-module, linux-api,
Christian Brauner
In-Reply-To: <20200211165753.356508-1-christian.brauner@ubuntu.com>
The /proc/<pid>/fsgid_map file can be written to once to setup an fsgid mapping
for a user namespace. Writing to this file has the same restrictions as writing
to /proc/<pid>/fsgid_map.
root@e1-vm:/# cat /proc/13023/fsgid_map
0 300000 100000
Fsid mappings have always been around. They are currently always identical to
the id mappings for a user namespace. This means, currently whenever an fsid
needs to be looked up the kernel will use the id mapping of the user namespace.
With the introduction of fsid mappings the kernel will now lookup fsids in the
fsid mappings of the user namespace. If no fsid mapping exists the kernel will
continue looking up fsids in the id mappings of the user namespace. Hence, if a
system supports fsid mappings through /proc/<pid>/fs*id_map and a container
runtime is not aware of fsid mappings it or does not use them it will it will
continue to work just as before.
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
fs/proc/base.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index ad5f6adc9344..e085ad579604 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -2884,6 +2884,11 @@ static int proc_fsuid_map_open(struct inode *inode, struct file *file)
{
return proc_id_map_open(inode, file, &proc_fsuid_seq_operations);
}
+
+static int proc_fsgid_map_open(struct inode *inode, struct file *file)
+{
+ return proc_id_map_open(inode, file, &proc_fsgid_seq_operations);
+}
#endif
static const struct file_operations proc_uid_map_operations = {
@@ -2918,6 +2923,14 @@ static const struct file_operations proc_fsuid_map_operations = {
.llseek = seq_lseek,
.release = proc_id_map_release,
};
+
+static const struct file_operations proc_fsgid_map_operations = {
+ .open = proc_fsgid_map_open,
+ .write = proc_fsgid_map_write,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = proc_id_map_release,
+};
#endif
static int proc_setgroups_open(struct inode *inode, struct file *file)
@@ -3098,6 +3111,7 @@ static const struct pid_entry tgid_base_stuff[] = {
#ifdef CONFIG_USER_NS
#ifdef CONFIG_USER_NS_FSID
REG("fsuid_map", S_IRUGO|S_IWUSR, proc_fsuid_map_operations),
+ REG("fsgid_map", S_IRUGO|S_IWUSR, proc_fsgid_map_operations),
#endif
REG("uid_map", S_IRUGO|S_IWUSR, proc_uid_map_operations),
REG("gid_map", S_IRUGO|S_IWUSR, proc_gid_map_operations),
--
2.25.0
^ permalink raw reply related
* [PATCH 02/24] proc: add /proc/<pid>/fsuid_map
From: Christian Brauner @ 2020-02-11 16:57 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Alexander Viro, Alexey Dobriyan, Serge Hallyn,
James Morris, Kees Cook, Jonathan Corbet, linux-kernel,
linux-fsdevel, containers, linux-security-module, linux-api,
Christian Brauner
In-Reply-To: <20200211165753.356508-1-christian.brauner@ubuntu.com>
The /proc/<pid>/fsuid_map file can be written to once to setup an fsuid mapping
for a user namespace. Writing to this file has the same restrictions as writing
to /proc/<pid>/fsuid_map:
root@e1-vm:/# cat /proc/13023/fsuid_map
0 300000 100000
Fsid mappings have always been around. They are currently always identical to
the id mappings for a user namespace. This means, currently whenever an fsid
needs to be looked up the kernel will use the id mapping of the user namespace.
With the introduction of fsid mappings the kernel will now lookup fsids in the
fsid mappings of the user namespace. If no fsid mapping exists the kernel will
continue looking up fsids in the id mappings of the user namespace. Hence, if a
system supports fsid mappings through /proc/<pid>/fs*id_map and a container
runtime is not aware of fsid mappings it or does not use them it will it will
continue to work just as before.
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
fs/proc/base.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index ebea9501afb8..ad5f6adc9344 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -2879,6 +2879,13 @@ static int proc_projid_map_open(struct inode *inode, struct file *file)
return proc_id_map_open(inode, file, &proc_projid_seq_operations);
}
+#ifdef CONFIG_USER_NS_FSID
+static int proc_fsuid_map_open(struct inode *inode, struct file *file)
+{
+ return proc_id_map_open(inode, file, &proc_fsuid_seq_operations);
+}
+#endif
+
static const struct file_operations proc_uid_map_operations = {
.open = proc_uid_map_open,
.write = proc_uid_map_write,
@@ -2903,6 +2910,16 @@ static const struct file_operations proc_projid_map_operations = {
.release = proc_id_map_release,
};
+#ifdef CONFIG_USER_NS_FSID
+static const struct file_operations proc_fsuid_map_operations = {
+ .open = proc_fsuid_map_open,
+ .write = proc_fsuid_map_write,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = proc_id_map_release,
+};
+#endif
+
static int proc_setgroups_open(struct inode *inode, struct file *file)
{
struct user_namespace *ns = NULL;
@@ -3079,6 +3096,9 @@ static const struct pid_entry tgid_base_stuff[] = {
ONE("io", S_IRUSR, proc_tgid_io_accounting),
#endif
#ifdef CONFIG_USER_NS
+#ifdef CONFIG_USER_NS_FSID
+ REG("fsuid_map", S_IRUGO|S_IWUSR, proc_fsuid_map_operations),
+#endif
REG("uid_map", S_IRUGO|S_IWUSR, proc_uid_map_operations),
REG("gid_map", S_IRUGO|S_IWUSR, proc_gid_map_operations),
REG("projid_map", S_IRUGO|S_IWUSR, proc_projid_map_operations),
--
2.25.0
^ permalink raw reply related
* [PATCH 24/24] devpts: handle fsid mappings
From: Christian Brauner @ 2020-02-11 16:57 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Alexander Viro, Alexey Dobriyan, Serge Hallyn,
James Morris, Kees Cook, Jonathan Corbet, linux-kernel,
linux-fsdevel, containers, linux-security-module, linux-api,
Christian Brauner
In-Reply-To: <20200211165753.356508-1-christian.brauner@ubuntu.com>
When a uid or gid mount option is specified with devpts have it lookup the
corresponding kfsids in the fsid mappings. If no fsid mappings are setup the
behavior is unchanged, i.e. fsids are looked up in the id mappings.
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
fs/devpts/inode.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/fs/devpts/inode.c b/fs/devpts/inode.c
index 42e5a766d33c..139958892572 100644
--- a/fs/devpts/inode.c
+++ b/fs/devpts/inode.c
@@ -24,6 +24,7 @@
#include <linux/parser.h>
#include <linux/fsnotify.h>
#include <linux/seq_file.h>
+#include <linux/fsuidgid.h>
#define DEVPTS_DEFAULT_MODE 0600
/*
@@ -277,7 +278,7 @@ static int parse_mount_options(char *data, int op, struct pts_mount_opts *opts)
case Opt_uid:
if (match_int(&args[0], &option))
return -EINVAL;
- uid = make_kuid(current_user_ns(), option);
+ uid = make_kfsuid(current_user_ns(), option);
if (!uid_valid(uid))
return -EINVAL;
opts->uid = uid;
@@ -286,7 +287,7 @@ static int parse_mount_options(char *data, int op, struct pts_mount_opts *opts)
case Opt_gid:
if (match_int(&args[0], &option))
return -EINVAL;
- gid = make_kgid(current_user_ns(), option);
+ gid = make_kfsgid(current_user_ns(), option);
if (!gid_valid(gid))
return -EINVAL;
opts->gid = gid;
@@ -410,7 +411,7 @@ static int devpts_show_options(struct seq_file *seq, struct dentry *root)
from_kuid_munged(&init_user_ns, opts->uid));
if (opts->setgid)
seq_printf(seq, ",gid=%u",
- from_kgid_munged(&init_user_ns, opts->gid));
+ from_kfsgid_munged(&init_user_ns, opts->gid));
seq_printf(seq, ",mode=%03o", opts->mode);
seq_printf(seq, ",ptmxmode=%03o", opts->ptmxmode);
if (opts->max < NR_UNIX98_PTY_MAX)
--
2.25.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox