* Re: [PATCH 2/3] Teach SELinux about anonymous inodes
From: Stephen Smalley @ 2020-02-14 16:39 UTC (permalink / raw)
To: Daniel Colascione, timmurray, selinux, linux-security-module,
linux-fsdevel, linux-kernel, kvm, viro, paul, nnk, lokeshgidra
In-Reply-To: <20200214032635.75434-3-dancol@google.com>
On 2/13/20 10:26 PM, Daniel Colascione wrote:
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 1659b59fb5d7..6de0892620b3 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2915,6 +2915,62 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
> return 0;
> }
>
> +static int selinux_inode_init_security_anon(struct inode *inode,
> + const struct qstr *name,
> + const struct file_operations *fops,
> + const struct inode *context_inode)
> +{
> + const struct task_security_struct *tsec = selinux_cred(current_cred());
> + struct common_audit_data ad;
> + struct inode_security_struct *isec;
> + int rc;
> +
> + if (unlikely(IS_PRIVATE(inode)))
> + return 0;
This is not possible since the caller clears S_PRIVATE before calling
and it would be a bug to call the hook on an inode that was intended to
be private, so we shouldn't check it here.
> +
> + if (unlikely(!selinux_state.initialized))
> + return 0;
Are we doing this to defer initialization until selinux_complete_init()
- that's normally why we bail in the !initialized case? Not entirely
sure what will happen in such a situation since we won't have the
context_inode or the allocating task information at that time, so we
certainly won't get the same result - probably they would all be labeled
with whatever anon_inodefs is assigned via genfscon or
SECINITSID_UNLABELED by default. If we instead just drop this test and
proceed, we'll inherit the context inode SID if specified or we'll call
security_transition_sid(), which in the !initialized case will just
return the tsid i.e. tsec->sid, so it will be labeled with the creating
task SID (SECINITSID_KERNEL prior to initialization). Then the
avc_has_perm() call will pass because everything gets allowed until
initialization. So you could drop this check and userfaultfds created
before policy load would get the kernel SID, or you can keep it and they
will get the unlabeled SID. Preference?
> +
> + isec = selinux_inode(inode);
> +
> + /*
> + * We only get here once per ephemeral inode. The inode has
> + * been initialized via inode_alloc_security but is otherwise
> + * untouched.
> + */
> +
> + if (context_inode) {
> + struct inode_security_struct *context_isec =
> + selinux_inode(context_inode);
> + if (IS_ERR(context_isec))
> + return PTR_ERR(context_isec);
This isn't possible AFAICT so you don't need to test for it or handle
it. In fact, even the test for NULL in selinux_inode() is bogus and
should get dropped AFAICT; we always allocate an inode security blob
even before policy load so it would be a bug if we ever had a NULL there.
> + isec->sid = context_isec->sid;
> + } else {
> + rc = security_transition_sid(
> + &selinux_state, tsec->sid, tsec->sid,
> + SECCLASS_FILE, name, &isec->sid);
> + if (rc)
> + return rc;
> + }
Since you switched to using security_transition_sid(), you are not using
the fops parameter anymore nor comparing with userfaultfd_fops, so you
could drop the parameter from the hook and leave the latter static in
the first patch.
That's assuming you are ok with having to define these type_transition
rules for the userfaultfd case instead of having your own separate
security class. Wondering how many different anon inode names/classes
there are in the kernel today and how much they change over time; for a
small, relatively stable set, separate classes might be ok; for a large,
dynamic set, type transitions should scale better. We might still need
to create a mapping table in SELinux from the names to some stable
identifier for the policy lookup if we can't rely on the names being stable.
^ permalink raw reply
* Re: [PATCH v26 10/22] x86/sgx: Linux Enclave Driver
From: Sean Christopherson @ 2020-02-14 17:11 UTC (permalink / raw)
To: Jethro Beekman
Cc: Jarkko Sakkinen, linux-kernel, x86, linux-sgx, akpm, dave.hansen,
nhorman, npmccallum, haitao.huang, andriy.shevchenko, tglx,
kai.svahn, bp, josh, luto, kai.huang, rientjes, cedric.xing,
puiterwijk, linux-security-module, Haitao Huang
In-Reply-To: <a4d9a58d-6984-5894-f6c8-73f2b2b466aa@fortanix.com>
On Fri, Feb 14, 2020 at 10:24:10AM +0100, Jethro Beekman wrote:
> On 2020-02-13 19:07, Sean Christopherson wrote:
> > On Thu, Feb 13, 2020 at 02:59:52PM +0100, Jethro Beekman wrote:
> >> On 2020-02-09 22:25, Jarkko Sakkinen wrote:
> >>> +/**
> >>> + * struct sgx_enclave_add_pages - parameter structure for the
> >>> + * %SGX_IOC_ENCLAVE_ADD_PAGE ioctl
> >>> + * @src: start address for the page data
> >>> + * @offset: starting page offset
> >>> + * @length: length of the data (multiple of the page size)
> >>> + * @secinfo: address for the SECINFO data
> >>> + * @flags: page control flags
> >>> + * @count: number of bytes added (multiple of the page size)
> >>> + */
> >>> +struct sgx_enclave_add_pages {
> >>> + __u64 src;
> >>> + __u64 offset;
> >>> + __u64 length;
> >>> + __u64 secinfo;
> >>> + __u64 flags;
> >>> + __u64 count;
> >>> +};
> >>
> >> Compared to the last time I looked at the patch set, this API removes the
> >> ability to measure individual pages chunks. That is not acceptable.
> >
> > Why is it not acceptable? E.g. what specific use case do you have that
> > _requires_ on measuring partial 4k pages of an enclave?
>
> The use case is someone gives me an enclave and I want to load it. If I don't
> load it exactly as the enclave author specified, the enclave hash will be
> different, and it won't work.
And if our ABI says "thou shall measure in 4k chunks", then it's an invalid
enclave if its author generated MRENCLAVE using a different granularity.
^ permalink raw reply
* Re: [PATCH 2/3] Teach SELinux about anonymous inodes
From: Daniel Colascione @ 2020-02-14 17:21 UTC (permalink / raw)
To: Stephen Smalley
Cc: Tim Murray, SElinux list, LSM List, Linux FS Devel, linux-kernel,
kvm, Al Viro, paul, Nick Kralevich, Lokesh Gidra
In-Reply-To: <9ca03838-8686-0007-0971-ee63bf5031da@tycho.nsa.gov>
On Fri, Feb 14, 2020 at 8:38 AM Stephen Smalley <sds@tycho.nsa.gov> wrote:
>
> On 2/13/20 10:26 PM, Daniel Colascione wrote:
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index 1659b59fb5d7..6de0892620b3 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -2915,6 +2915,62 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
> > return 0;
> > }
> >
> > +static int selinux_inode_init_security_anon(struct inode *inode,
> > + const struct qstr *name,
> > + const struct file_operations *fops,
> > + const struct inode *context_inode)
> > +{
> > + const struct task_security_struct *tsec = selinux_cred(current_cred());
> > + struct common_audit_data ad;
> > + struct inode_security_struct *isec;
> > + int rc;
> > +
> > + if (unlikely(IS_PRIVATE(inode)))
> > + return 0;
>
> This is not possible since the caller clears S_PRIVATE before calling
> and it would be a bug to call the hook on an inode that was intended to
> be private, so we shouldn't check it here.
>
> > +
> > + if (unlikely(!selinux_state.initialized))
> > + return 0;
>
> Are we doing this to defer initialization until selinux_complete_init()
> - that's normally why we bail in the !initialized case? Not entirely
> sure what will happen in such a situation since we won't have the
> context_inode or the allocating task information at that time, so we
> certainly won't get the same result - probably they would all be labeled
> with whatever anon_inodefs is assigned via genfscon or
> SECINITSID_UNLABELED by default.
> If we instead just drop this test and
> proceed, we'll inherit the context inode SID if specified or we'll call
> security_transition_sid(), which in the !initialized case will just
> return the tsid i.e. tsec->sid, so it will be labeled with the creating
> task SID (SECINITSID_KERNEL prior to initialization). Then the
> avc_has_perm() call will pass because everything gets allowed until
> initialization. So you could drop this check and userfaultfds created
> before policy load would get the kernel SID, or you can keep it and they
> will get the unlabeled SID. Preference?
The kernel SID seems safer. Thanks for the explanation!
> > +
> > + isec = selinux_inode(inode);
> > +
> > + /*
> > + * We only get here once per ephemeral inode. The inode has
> > + * been initialized via inode_alloc_security but is otherwise
> > + * untouched.
> > + */
> > +
> > + if (context_inode) {
> > + struct inode_security_struct *context_isec =
> > + selinux_inode(context_inode);
> > + if (IS_ERR(context_isec))
> > + return PTR_ERR(context_isec);
>
> This isn't possible AFAICT so you don't need to test for it or handle
> it. In fact, even the test for NULL in selinux_inode() is bogus and
> should get dropped AFAICT; we always allocate an inode security blob
> even before policy load so it would be a bug if we ever had a NULL there.
Thanks. Will fix.
> > + isec->sid = context_isec->sid;
> > + } else {
> > + rc = security_transition_sid(
> > + &selinux_state, tsec->sid, tsec->sid,
> > + SECCLASS_FILE, name, &isec->sid);
> > + if (rc)
> > + return rc;
> > + }
>
> Since you switched to using security_transition_sid(), you are not using
> the fops parameter anymore nor comparing with userfaultfd_fops, so you
> could drop the parameter from the hook and leave the latter static in
> the first patch.
That's true, but I figured different LSMs might want different rules
that depend on the fops. I'm also okay with removing this parameter
for now, since we're not using it.
> That's assuming you are ok with having to define these type_transition
> rules for the userfaultfd case instead of having your own separate
> security class. Wondering how many different anon inode names/classes
> there are in the kernel today and how much they change over time; for a
> small, relatively stable set, separate classes might be ok; for a large,
> dynamic set, type transitions should scale better.
I think we can get away without a class per anonymous-inode-type. I do
wonder whether we need a class for all anonymous inodes, though: if we
just give them the file class and use the anon inode type name for the
type_transition rule, isn't it possible that the type_transition rule
might also fire for plain files with the same names in the last path
component and the same originating sid? (Maybe I'm not understanding
type_transition rules properly.) Using a class to encompass all
anonymous inodes would address this problem (assuming the problem
exists in the first place).
> We might still need
> to create a mapping table in SELinux from the names to some stable
> identifier for the policy lookup if we can't rely on the names being stable.
Sure. The anonymous inode type names have historically been stable,
though, so maybe we can just use the names from anon_inodes directly
for now and then add some kind of remapping if we want to change those
names in the core, remaping to the old name for SELinux
type_transition purposes.
^ permalink raw reply
* Re: [PATCH v26 10/22] x86/sgx: Linux Enclave Driver
From: Andy Lutomirski @ 2020-02-14 17:40 UTC (permalink / raw)
To: Sean Christopherson
Cc: Jethro Beekman, Jarkko Sakkinen, linux-kernel, x86, linux-sgx,
akpm, dave.hansen, nhorman, npmccallum, haitao.huang,
andriy.shevchenko, tglx, kai.svahn, bp, josh, luto, kai.huang,
rientjes, cedric.xing, puiterwijk, linux-security-module,
Haitao Huang
In-Reply-To: <20200214171146.GD20690@linux.intel.com>
> On Feb 14, 2020, at 9:11 AM, Sean Christopherson <sean.j.christopherson@intel.com> wrote:
>
> On Fri, Feb 14, 2020 at 10:24:10AM +0100, Jethro Beekman wrote:
>>> On 2020-02-13 19:07, Sean Christopherson wrote:
>>> On Thu, Feb 13, 2020 at 02:59:52PM +0100, Jethro Beekman wrote:
>>>> On 2020-02-09 22:25, Jarkko Sakkinen wrote:
>>>>> +/**
>>>>> + * struct sgx_enclave_add_pages - parameter structure for the
>>>>> + * %SGX_IOC_ENCLAVE_ADD_PAGE ioctl
>>>>> + * @src: start address for the page data
>>>>> + * @offset: starting page offset
>>>>> + * @length: length of the data (multiple of the page size)
>>>>> + * @secinfo: address for the SECINFO data
>>>>> + * @flags: page control flags
>>>>> + * @count: number of bytes added (multiple of the page size)
>>>>> + */
>>>>> +struct sgx_enclave_add_pages {
>>>>> + __u64 src;
>>>>> + __u64 offset;
>>>>> + __u64 length;
>>>>> + __u64 secinfo;
>>>>> + __u64 flags;
>>>>> + __u64 count;
>>>>> +};
>>>>
>>>> Compared to the last time I looked at the patch set, this API removes the
>>>> ability to measure individual pages chunks. That is not acceptable.
>>>
>>> Why is it not acceptable? E.g. what specific use case do you have that
>>> _requires_ on measuring partial 4k pages of an enclave?
>>
>> The use case is someone gives me an enclave and I want to load it. If I don't
>> load it exactly as the enclave author specified, the enclave hash will be
>> different, and it won't work.
>
> And if our ABI says "thou shall measure in 4k chunks", then it's an invalid
> enclave if its author generated MRENCLAVE using a different granularity.
ISTM, unless there’s a particularly compelling reason, if an enclave is valid, we should be able to load it.
^ permalink raw reply
* Re: [PATCH v26 10/22] x86/sgx: Linux Enclave Driver
From: Sean Christopherson @ 2020-02-14 17:52 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Jethro Beekman, Jarkko Sakkinen, linux-kernel, x86, linux-sgx,
akpm, dave.hansen, nhorman, npmccallum, haitao.huang,
andriy.shevchenko, tglx, kai.svahn, bp, josh, luto, kai.huang,
rientjes, cedric.xing, puiterwijk, linux-security-module,
Haitao Huang
In-Reply-To: <D18DD438-2458-4264-9D56-A05EB84BB545@amacapital.net>
On Fri, Feb 14, 2020 at 09:40:00AM -0800, Andy Lutomirski wrote:
>
>
> > On Feb 14, 2020, at 9:11 AM, Sean Christopherson <sean.j.christopherson@intel.com> wrote:
> >
> > On Fri, Feb 14, 2020 at 10:24:10AM +0100, Jethro Beekman wrote:
> >>> On 2020-02-13 19:07, Sean Christopherson wrote:
> >>> On Thu, Feb 13, 2020 at 02:59:52PM +0100, Jethro Beekman wrote:
> >>>> On 2020-02-09 22:25, Jarkko Sakkinen wrote:
> >>>>> +/**
> >>>>> + * struct sgx_enclave_add_pages - parameter structure for the
> >>>>> + * %SGX_IOC_ENCLAVE_ADD_PAGE ioctl
> >>>>> + * @src: start address for the page data
> >>>>> + * @offset: starting page offset
> >>>>> + * @length: length of the data (multiple of the page size)
> >>>>> + * @secinfo: address for the SECINFO data
> >>>>> + * @flags: page control flags
> >>>>> + * @count: number of bytes added (multiple of the page size)
> >>>>> + */
> >>>>> +struct sgx_enclave_add_pages {
> >>>>> + __u64 src;
> >>>>> + __u64 offset;
> >>>>> + __u64 length;
> >>>>> + __u64 secinfo;
> >>>>> + __u64 flags;
> >>>>> + __u64 count;
> >>>>> +};
> >>>>
> >>>> Compared to the last time I looked at the patch set, this API removes the
> >>>> ability to measure individual pages chunks. That is not acceptable.
> >>>
> >>> Why is it not acceptable? E.g. what specific use case do you have that
> >>> _requires_ on measuring partial 4k pages of an enclave?
> >>
> >> The use case is someone gives me an enclave and I want to load it. If I don't
> >> load it exactly as the enclave author specified, the enclave hash will be
> >> different, and it won't work.
> >
> > And if our ABI says "thou shall measure in 4k chunks", then it's an invalid
> > enclave if its author generated MRENCLAVE using a different granularity.
>
> ISTM, unless there’s a particularly compelling reason, if an enclave is
> valid, we should be able to load it.
That means we have to have a separate ioctl() for EEXTEND, otherwise we
can't handle EADD[0]->EADD[1]->EADD[2]->EEXTEND[0]->EEXTEND[1]->EEXTEND[2].
I think we'd still want to keep the MEASURE flag for SGX_IOC_ENCLAVE_ADD_PAGE
so that we can optimize EADD[0]->EEXTEND[0]->EADD[1]->EEXTEND[1].
^ permalink raw reply
* Re: [PATCH 2/3] Teach SELinux about anonymous inodes
From: Stephen Smalley @ 2020-02-14 18:02 UTC (permalink / raw)
To: Daniel Colascione
Cc: Tim Murray, SElinux list, LSM List, Linux FS Devel, linux-kernel,
kvm, Al Viro, paul, Nick Kralevich, Lokesh Gidra
In-Reply-To: <CAKOZuev-=7Lgu35E3tzpHQn0m_KAvvrqi+ZJr1dpqRjHERRSqg@mail.gmail.com>
On 2/14/20 12:21 PM, Daniel Colascione wrote:
> On Fri, Feb 14, 2020 at 8:38 AM Stephen Smalley <sds@tycho.nsa.gov> wrote:
>> That's assuming you are ok with having to define these type_transition
>> rules for the userfaultfd case instead of having your own separate
>> security class. Wondering how many different anon inode names/classes
>> there are in the kernel today and how much they change over time; for a
>> small, relatively stable set, separate classes might be ok; for a large,
>> dynamic set, type transitions should scale better.
>
> I think we can get away without a class per anonymous-inode-type. I do
> wonder whether we need a class for all anonymous inodes, though: if we
> just give them the file class and use the anon inode type name for the
> type_transition rule, isn't it possible that the type_transition rule
> might also fire for plain files with the same names in the last path
> component and the same originating sid? (Maybe I'm not understanding
> type_transition rules properly.) Using a class to encompass all
> anonymous inodes would address this problem (assuming the problem
> exists in the first place).
It shouldn't fire for non-anon inodes because on a (non-anon) file
creation, security_transition_sid() is passed the parent directory SID
as the second argument and we only assign task SIDs to /proc/pid
directories, which don't support (userspace) file creation anyway.
However, in the absence of a matching type_transition rule, we'll end up
defaulting to the task SID on the anon inode, and without a separate
class we won't be able to distinguish it from a /proc/pid inode. So
that might justify a separate anoninode or similar class.
This however reminded me that for the context_inode case, we not only
want to inherit the SID but also the sclass from the context_inode.
That is so that anon inodes created via device node ioctls inherit the
same SID/class pair as the device node and a single allowx rule can
govern all ioctl commands on that device.
^ permalink raw reply
* Re: [PATCH 2/3] Teach SELinux about anonymous inodes
From: Stephen Smalley @ 2020-02-14 18:08 UTC (permalink / raw)
To: Daniel Colascione
Cc: Tim Murray, SElinux list, LSM List, Linux FS Devel, linux-kernel,
kvm, Al Viro, paul, Nick Kralevich, Lokesh Gidra,
Jeffrey Vander Stoep
In-Reply-To: <23f725ca-5b5a-5938-fcc8-5bbbfc9ba9bc@tycho.nsa.gov>
On 2/14/20 1:02 PM, Stephen Smalley wrote:
> It shouldn't fire for non-anon inodes because on a (non-anon) file
> creation, security_transition_sid() is passed the parent directory SID
> as the second argument and we only assign task SIDs to /proc/pid
> directories, which don't support (userspace) file creation anyway.
>
> However, in the absence of a matching type_transition rule, we'll end up
> defaulting to the task SID on the anon inode, and without a separate
> class we won't be able to distinguish it from a /proc/pid inode. So
> that might justify a separate anoninode or similar class.
>
> This however reminded me that for the context_inode case, we not only
> want to inherit the SID but also the sclass from the context_inode. That
> is so that anon inodes created via device node ioctls inherit the same
> SID/class pair as the device node and a single allowx rule can govern
> all ioctl commands on that device.
At least that's the way our patch worked with the /dev/kvm example.
However, if we are introducing a separate anoninode class for the
type_transition case, maybe we should apply that to all anon inodes
regardless of how they are labeled (based on context_inode or
transition) and then we'd need to write two allowx rules, one for ioctls
on the original device node and one for those on anon inodes created
from it. Not sure how Android wants to handle that as the original
developer and primary user of SELinux ioctl whitelisting.
^ permalink raw reply
* [PATCH AUTOSEL 5.5 201/542] IMA: Check IMA policy flag
From: Sasha Levin @ 2020-02-14 15:43 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Lakshmi Ramasubramanian, Mimi Zohar, Sasha Levin, linux-integrity,
linux-security-module
In-Reply-To: <20200214154854.6746-1-sashal@kernel.org>
From: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
[ Upstream commit c5563bad88e07017e08cce1142903e501598c80c ]
process_buffer_measurement() may be called prior to IMA being
initialized (for instance, when the IMA hook is called when
a key is added to the .builtin_trusted_keys keyring), which
would result in a kernel panic.
This patch adds the check in process_buffer_measurement()
to return immediately if IMA is not initialized yet.
Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
security/integrity/ima/ima_main.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index d7e987baf1274..9b35db2fc777a 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -655,6 +655,9 @@ void process_buffer_measurement(const void *buf, int size,
int action = 0;
u32 secid;
+ if (!ima_policy_flag)
+ return;
+
/*
* Both LSM hooks and auxilary based buffer measurements are
* based on policy. To avoid code duplication, differentiate
--
2.20.1
^ permalink raw reply related
* [PATCH v2 02/28] proc: add /proc/<pid>/fsuid_map
From: Christian Brauner @ 2020-02-14 18:35 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Seth Forshee, Alexander Viro, Alexey Dobriyan,
Serge Hallyn, James Morris, Kees Cook, Jonathan Corbet,
Phil Estes, linux-kernel, linux-fsdevel, containers,
linux-security-module, linux-api, Christian Brauner
In-Reply-To: <20200214183554.1133805-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>
---
/* v2 */
unchanged
---
fs/proc/base.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index c7c64272b0fa..5fb28004663e 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -2970,6 +2970,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,
@@ -2994,6 +3001,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;
@@ -3176,6 +3193,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 v2 11/28] sys:__sys_setreuid(): handle fsid mappings
From: Christian Brauner @ 2020-02-14 18:35 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Seth Forshee, Alexander Viro, Alexey Dobriyan,
Serge Hallyn, James Morris, Kees Cook, Jonathan Corbet,
Phil Estes, linux-kernel, linux-fsdevel, containers,
linux-security-module, linux-api, Christian Brauner
In-Reply-To: <20200214183554.1133805-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.
The kfsid to cleanly handle userns visible filesystem is set as before.
We require that a user must have a valid fsid mapping for the target id. This
is consistent with how the setid calls work today without fsid mappings.
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v2 */
- Christian Brauner <christian.brauner@ubuntu.com>:
- set kfsid which is used when dealing with proc permission checking
---
kernel/sys.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/kernel/sys.c b/kernel/sys.c
index aa379fb5e93b..4697e010bbd7 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -504,7 +504,7 @@ 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);
@@ -535,6 +535,13 @@ long __sys_setreuid(uid_t ruid, uid_t euid)
!uid_eq(old->suid, keuid) &&
!ns_capable_setid(old->user_ns, CAP_SETUID))
goto error;
+ kfsuid = make_kfsuid(new->user_ns, euid);
+ } else {
+ kfsuid = kuid_to_kfsuid(new->user_ns, new->euid);
+ }
+ if (!uid_valid(kfsuid)) {
+ retval = -EINVAL;
+ goto error;
}
if (!uid_eq(new->uid, old->uid)) {
@@ -545,7 +552,8 @@ 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->kfsuid = 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 v2 15/28] fs: add is_userns_visible() helper
From: Christian Brauner @ 2020-02-14 18:35 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Seth Forshee, Alexander Viro, Alexey Dobriyan,
Serge Hallyn, James Morris, Kees Cook, Jonathan Corbet,
Phil Estes, linux-kernel, linux-fsdevel, containers,
linux-security-module, linux-api, Christian Brauner
In-Reply-To: <20200214183554.1133805-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>
---
/* v2 */
unchanged
---
include/linux/fs.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 3cd4fe6b845e..fdc8fb2d786b 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3651,4 +3651,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 v2 19/28] stat: handle fsid mappings
From: Christian Brauner @ 2020-02-14 18:35 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Seth Forshee, Alexander Viro, Alexey Dobriyan,
Serge Hallyn, James Morris, Kees Cook, Jonathan Corbet,
Phil Estes, linux-kernel, linux-fsdevel, containers,
linux-security-module, linux-api, Christian Brauner
In-Reply-To: <20200214183554.1133805-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>
---
/* v2 */
unchanged
---
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 030008796479..edd45678c4ed 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>
@@ -79,6 +80,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);
@@ -239,8 +242,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)
@@ -327,8 +335,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;
@@ -471,8 +484,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;
@@ -544,8 +562,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;
@@ -615,8 +638,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 v2 17/28] inode: inode_owner_or_capable(): handle fsid mappings
From: Christian Brauner @ 2020-02-14 18:35 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Seth Forshee, Alexander Viro, Alexey Dobriyan,
Serge Hallyn, James Morris, Kees Cook, Jonathan Corbet,
Phil Estes, linux-kernel, linux-fsdevel, containers,
linux-security-module, linux-api, Christian Brauner
In-Reply-To: <20200214183554.1133805-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>
---
/* v2 */
unchanged
---
fs/inode.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/fs/inode.c b/fs/inode.c
index 7d57068b6b7a..81d7a30b381d 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -21,6 +21,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"
@@ -2087,8 +2088,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 v2 16/28] namei: may_{o_}create(): handle fsid mappings
From: Christian Brauner @ 2020-02-14 18:35 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Seth Forshee, Alexander Viro, Alexey Dobriyan,
Serge Hallyn, James Morris, Kees Cook, Jonathan Corbet,
Phil Estes, linux-kernel, linux-fsdevel, containers,
linux-security-module, linux-api, Christian Brauner
In-Reply-To: <20200214183554.1133805-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.
Cc: Jann Horn <jannh@google.com>
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v2 */
- Jann Horn <jannh@google.com>:
- Ensure that the correct fsid is used when dealing with userns visible
filesystems like proc.
---
fs/namei.c | 36 ++++++++++++++++++++++++++++--------
1 file changed, 28 insertions(+), 8 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index db6565c99825..c5b014000f13 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"
@@ -287,6 +288,13 @@ static int check_acl(struct inode *inode, int mask)
return -EAGAIN;
}
+static inline kuid_t get_current_fsuid(const struct inode *inode)
+{
+ if (is_userns_visible(inode->i_sb->s_iflags))
+ return current_kfsuid();
+ return current_fsuid();
+}
+
/*
* This does the basic permission checking
*/
@@ -294,7 +302,7 @@ static int acl_permission_check(struct inode *inode, int mask)
{
unsigned int mode = inode->i_mode;
- if (likely(uid_eq(current_fsuid(), inode->i_uid)))
+ if (likely(uid_eq(get_current_fsuid(inode), inode->i_uid)))
mode >>= 6;
else {
if (IS_POSIXACL(inode) && (mode & S_IRWXG)) {
@@ -980,7 +988,7 @@ static inline int may_follow_link(struct nameidata *nd)
/* Allowed if owner and follower match. */
inode = nd->link_inode;
- if (uid_eq(current_cred()->fsuid, inode->i_uid))
+ if (uid_eq(get_current_fsuid(inode), inode->i_uid))
return 0;
/* Allowed if parent directory not sticky and world-writable. */
@@ -1097,7 +1105,7 @@ static int may_create_in_sticky(umode_t dir_mode, kuid_t dir_uid,
(!sysctl_protected_regular && S_ISREG(inode->i_mode)) ||
likely(!(dir_mode & S_ISVTX)) ||
uid_eq(inode->i_uid, dir_uid) ||
- uid_eq(current_fsuid(), inode->i_uid))
+ uid_eq(get_current_fsuid(inode), inode->i_uid))
return 0;
if (likely(dir_mode & 0002) ||
@@ -2832,7 +2840,7 @@ EXPORT_SYMBOL(kern_path_mountpoint);
int __check_sticky(struct inode *dir, struct inode *inode)
{
- kuid_t fsuid = current_fsuid();
+ kuid_t fsuid = get_current_fsuid(inode);
if (uid_eq(inode->i_uid, fsuid))
return 0;
@@ -2902,6 +2910,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_kfsuid()) ||
+ !kgid_has_mapping(ns, current_kfsgid()))
+ 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
@@ -2920,8 +2942,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);
}
@@ -3103,8 +3124,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 v2 14/28] sys:__sys_setresgid(): handle fsid mappings
From: Christian Brauner @ 2020-02-14 18:35 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Seth Forshee, Alexander Viro, Alexey Dobriyan,
Serge Hallyn, James Morris, Kees Cook, Jonathan Corbet,
Phil Estes, linux-kernel, linux-fsdevel, containers,
linux-security-module, linux-api, Christian Brauner
In-Reply-To: <20200214183554.1133805-1-christian.brauner@ubuntu.com>
Switch setresgid() 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 setresgid() the kfsgid is set to the kegid corresponding the egid that is
requested by userspace. If the requested egid is -1 the kfsgid is reset to the
current kegid. For the latter case this means we need to lookup the
corresponding userspace egid corresponding to the current kegid in the id
mappings and translate this egid into the corresponding kfsgid in the fsid
mappings.
The kfsid to cleanly handle userns visible filesystem is set as before.
We require that a user must have a valid fsid mapping for the target id. This
is consistent with how the setid calls work today without fsid mappings.
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v2 */
- Christian Brauner <christian.brauner@ubuntu.com>:
- set kfsid which is used when dealing with proc permission checking
---
kernel/sys.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/kernel/sys.c b/kernel/sys.c
index 54e072145146..78592deee2d8 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -756,7 +756,7 @@ long __sys_setresgid(gid_t rgid, gid_t egid, gid_t sgid)
const struct cred *old;
struct cred *new;
int retval;
- kgid_t krgid, kegid, ksgid;
+ kgid_t krgid, kegid, ksgid, kfsgid;
krgid = make_kgid(ns, rgid);
kegid = make_kgid(ns, egid);
@@ -789,11 +789,21 @@ long __sys_setresgid(gid_t rgid, gid_t egid, gid_t sgid)
if (rgid != (gid_t) -1)
new->gid = krgid;
- if (egid != (gid_t) -1)
+ if (egid != (gid_t) -1) {
new->egid = kegid;
+ kfsgid = make_kfsgid(ns, egid);
+ } else {
+ kfsgid = kgid_to_kfsgid(new->user_ns, new->egid);
+ }
+ if (!gid_valid(kfsgid)) {
+ retval = -EINVAL;
+ goto error;
+ }
+
if (sgid != (gid_t) -1)
new->sgid = ksgid;
- new->fsgid = new->egid;
+ new->kfsgid = new->egid;
+ new->fsgid = kfsgid;
return commit_creds(new);
--
2.25.0
^ permalink raw reply related
* [PATCH v2 18/28] capability: privileged_wrt_inode_uidgid(): handle fsid mappings
From: Christian Brauner @ 2020-02-14 18:35 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Seth Forshee, Alexander Viro, Alexey Dobriyan,
Serge Hallyn, James Morris, Kees Cook, Jonathan Corbet,
Phil Estes, linux-kernel, linux-fsdevel, containers,
linux-security-module, linux-api, Christian Brauner
In-Reply-To: <20200214183554.1133805-1-christian.brauner@ubuntu.com>
Switch privileged_wrt_inode_uidgid() 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>
---
/* v2 */
unchanged
---
kernel/capability.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/kernel/capability.c b/kernel/capability.c
index 1444f3954d75..2b0c1dc992e2 100644
--- a/kernel/capability.c
+++ b/kernel/capability.c
@@ -19,6 +19,8 @@
#include <linux/pid_namespace.h>
#include <linux/user_namespace.h>
#include <linux/uaccess.h>
+#include <linux/fsuidgid.h>
+#include <linux/fs.h>
/*
* Leveraged for setting/resetting capabilities
@@ -486,8 +488,12 @@ EXPORT_SYMBOL(file_ns_capable);
*/
bool privileged_wrt_inode_uidgid(struct user_namespace *ns, const struct inode *inode)
{
- return kuid_has_mapping(ns, inode->i_uid) &&
- kgid_has_mapping(ns, inode->i_gid);
+ if (is_userns_visible(inode->i_sb->s_iflags))
+ return kuid_has_mapping(ns, inode->i_uid) &&
+ kgid_has_mapping(ns, inode->i_gid);
+
+ return kfsuid_has_mapping(ns, inode->i_uid) &&
+ kfsgid_has_mapping(ns, inode->i_gid);
}
/**
--
2.25.0
^ permalink raw reply related
* [PATCH v2 13/28] sys:__sys_setresuid(): handle fsid mappings
From: Christian Brauner @ 2020-02-14 18:35 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Seth Forshee, Alexander Viro, Alexey Dobriyan,
Serge Hallyn, James Morris, Kees Cook, Jonathan Corbet,
Phil Estes, linux-kernel, linux-fsdevel, containers,
linux-security-module, linux-api, Christian Brauner
In-Reply-To: <20200214183554.1133805-1-christian.brauner@ubuntu.com>
Switch setresuid() 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 setresuid() 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.
The kfsid to cleanly handle userns visible filesystem is set as before.
We require that a user must have a valid fsid mapping for the target id. This
is consistent with how the setid calls work today without fsid mappings.
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v2 */
- Christian Brauner <christian.brauner@ubuntu.com>:
- set kfsid which is used when dealing with proc permission checking
---
kernel/sys.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/kernel/sys.c b/kernel/sys.c
index 22eea030d9e7..54e072145146 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -654,7 +654,7 @@ long __sys_setresuid(uid_t ruid, uid_t euid, uid_t suid)
const struct cred *old;
struct cred *new;
int retval;
- kuid_t kruid, keuid, ksuid;
+ kuid_t kruid, keuid, ksuid, kfsuid;
kruid = make_kuid(ns, ruid);
keuid = make_kuid(ns, euid);
@@ -696,11 +696,21 @@ long __sys_setresuid(uid_t ruid, uid_t euid, uid_t suid)
goto error;
}
}
- if (euid != (uid_t) -1)
+ if (euid != (uid_t) -1) {
new->euid = keuid;
+ kfsuid = make_kfsuid(ns, euid);
+ } else {
+ kfsuid = kuid_to_kfsuid(new->user_ns, new->euid);
+ }
+ if (!uid_valid(kfsuid)) {
+ return -EINVAL;
+ goto error;
+ }
+
if (suid != (uid_t) -1)
new->suid = ksuid;
- new->fsuid = new->euid;
+ new->kfsuid = new->euid;
+ new->fsuid = kfsuid;
retval = security_task_fix_setuid(new, old, LSM_SETID_RES);
if (retval < 0)
--
2.25.0
^ permalink raw reply related
* [PATCH v2 10/28] sys:__sys_setgid(): handle fsid mappings
From: Christian Brauner @ 2020-02-14 18:35 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Seth Forshee, Alexander Viro, Alexey Dobriyan,
Serge Hallyn, James Morris, Kees Cook, Jonathan Corbet,
Phil Estes, linux-kernel, linux-fsdevel, containers,
linux-security-module, linux-api, Christian Brauner
In-Reply-To: <20200214183554.1133805-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.
The kfsid to cleanly handle userns visible filesystem is set as before.
We require that a user must have a valid fsid mapping for the target id. This
is consistent with how the setid calls work today without fsid mappings.
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v2 */
- Christian Brauner <christian.brauner@ubuntu.com>:
- set kfsid which is used when dealing with proc permission checking
---
kernel/sys.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/kernel/sys.c b/kernel/sys.c
index a8eefd748327..aa379fb5e93b 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 = new->kfsgid = kgid;
+ new->fsgid = kfsgid;
+ } else if (gid_eq(kgid, old->gid) || gid_eq(kgid, old->sgid)) {
+ new->egid = new->kfsgid = kgid;
+ new->fsgid = kfsgid;
+ } else {
goto error;
+ }
return commit_creds(new);
--
2.25.0
^ permalink raw reply related
* [PATCH v2 09/28] sys:__sys_setuid(): handle fsid mappings
From: Christian Brauner @ 2020-02-14 18:35 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Seth Forshee, Alexander Viro, Alexey Dobriyan,
Serge Hallyn, James Morris, Kees Cook, Jonathan Corbet,
Phil Estes, linux-kernel, linux-fsdevel, containers,
linux-security-module, linux-api, Christian Brauner
In-Reply-To: <20200214183554.1133805-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.
The kfsid to cleanly handle userns visible filesystem is set as before.
We require that a user must have a valid fsid mapping for the target id. This
is consistent with how the setid calls work today without fsid mappings.
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v2 */
- Christian Brauner <christian.brauner@ubuntu.com>:
- set kfsid which is used when dealing with proc permission checking
---
kernel/sys.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/kernel/sys.c b/kernel/sys.c
index 864fa78f25a7..a8eefd748327 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->kfsuid = 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 v2 08/28] sys: __sys_setfsgid(): handle fsid mappings
From: Christian Brauner @ 2020-02-14 18:35 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Seth Forshee, Alexander Viro, Alexey Dobriyan,
Serge Hallyn, James Morris, Kees Cook, Jonathan Corbet,
Phil Estes, linux-kernel, linux-fsdevel, containers,
linux-security-module, linux-api, Christian Brauner
In-Reply-To: <20200214183554.1133805-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.
A caller can only setfs{g,u}id() to a given id if the id maps to a valid kid in
both the id and fsid maps of the caller's user namespace. This is always the
case when no id mappings and fsid mappings have been written. It is also always
the case when an id mapping has been written which includes the target id and
but no fsid mappings have been written. All non-fsid mapping aware workloads
will thus work just as before.
Requiring a valid mapping for the target id in both the id and fsid mappings of
the container simplifies permission checking for userns visible filesystems
such as proc.
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v2 */
- Christian Brauner <christian.brauner@ubuntu.com>:
- Set unmapped fsid as well.
---
kernel/sys.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/kernel/sys.c b/kernel/sys.c
index 13f790dbda71..864fa78f25a7 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -849,15 +849,19 @@ long __sys_setfsgid(gid_t gid)
const struct cred *old;
struct cred *new;
gid_t old_fsgid;
- kgid_t kgid;
+ kgid_t kgid, kfsgid;
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;
+ kfsgid = make_kgid(old->user_ns, gid);
+ if (!gid_valid(kfsgid))
+ return old_fsgid;
+
new = prepare_creds();
if (!new)
return old_fsgid;
@@ -867,6 +871,7 @@ long __sys_setfsgid(gid_t gid)
ns_capable(old->user_ns, CAP_SETGID)) {
if (!gid_eq(kgid, old->fsgid)) {
new->fsgid = kgid;
+ new->kfsgid = kfsgid;
goto change_okay;
}
}
--
2.25.0
^ permalink raw reply related
* [PATCH v2 12/28] sys:__sys_setregid(): handle fsid mappings
From: Christian Brauner @ 2020-02-14 18:35 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Seth Forshee, Alexander Viro, Alexey Dobriyan,
Serge Hallyn, James Morris, Kees Cook, Jonathan Corbet,
Phil Estes, linux-kernel, linux-fsdevel, containers,
linux-security-module, linux-api, Christian Brauner
In-Reply-To: <20200214183554.1133805-1-christian.brauner@ubuntu.com>
Switch setregid() 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 setregid() the kfsgid is set to the kegid corresponding the egid that is
requested by userspace. If the requested egid is -1 the kfsgid is reset to the
current kegid. For the latter case this means we need to lookup the
corresponding userspace egid corresponding to the current kegid in the id
mappings and translate this egid into the corresponding kfsgid in the fsid
mappings.
The kfsid to cleanly handle userns visible filesystem is set as before.
We require that a user must have a valid fsid mapping for the target id. This
is consistent with how the setid calls work today without fsid mappings.
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v2 */
- Christian Brauner <christian.brauner@ubuntu.com>:
- set kfsid which is used when dealing with proc permission checking
---
kernel/sys.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/kernel/sys.c b/kernel/sys.c
index 4697e010bbd7..22eea030d9e7 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -354,7 +354,7 @@ long __sys_setregid(gid_t rgid, gid_t egid)
const struct cred *old;
struct cred *new;
int retval;
- kgid_t krgid, kegid;
+ kgid_t krgid, kegid, kfsgid;
krgid = make_kgid(ns, rgid);
kegid = make_kgid(ns, egid);
@@ -386,12 +386,20 @@ long __sys_setregid(gid_t rgid, gid_t egid)
new->egid = kegid;
else
goto error;
+ kfsgid = make_kfsgid(ns, egid);
+ } else {
+ kfsgid = kgid_to_kfsgid(new->user_ns, new->egid);
+ }
+ if (!gid_valid(kfsgid)) {
+ retval = -EINVAL;
+ goto error;
}
if (rgid != (gid_t) -1 ||
(egid != (gid_t) -1 && !gid_eq(kegid, old->gid)))
new->sgid = new->egid;
- new->fsgid = new->egid;
+ new->kfsgid = new->egid;
+ new->fsgid = kfsgid;
return commit_creds(new);
--
2.25.0
^ permalink raw reply related
* [PATCH v2 06/28] cred: add kfs{g,u}id
From: Christian Brauner @ 2020-02-14 18:35 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Seth Forshee, Alexander Viro, Alexey Dobriyan,
Serge Hallyn, James Morris, Kees Cook, Jonathan Corbet,
Phil Estes, linux-kernel, linux-fsdevel, containers,
linux-security-module, linux-api, Christian Brauner
In-Reply-To: <20200214183554.1133805-1-christian.brauner@ubuntu.com>
After the introduction of fsid mappings we need to carefully handle
single-superblock filesystems that are visible in user namespaces. This
specifically concerns proc and sysfs. For those filesystems we want to continue
looking up fsid in the id mappings of the relevant user namespace. We can
either do this by dynamically translating between these fsids or we simply keep
them around with the other creds. The latter option is not just simpler but
also more performant since we don't need to do the translation from fsid
mappings into id mappings on the fly.
Link: https://lore.kernel.org/r/20200212145149.zohmc6d3x52bw6j6@wittgenstein
Cc: Jann Horn <jannh@google.com>
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v2 */
patch added
---
include/linux/cred.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/include/linux/cred.h b/include/linux/cred.h
index 18639c069263..604914d3fd51 100644
--- a/include/linux/cred.h
+++ b/include/linux/cred.h
@@ -125,6 +125,8 @@ struct cred {
kgid_t egid; /* effective GID of the task */
kuid_t fsuid; /* UID for VFS ops */
kgid_t fsgid; /* GID for VFS ops */
+ kuid_t kfsuid; /* UID for VFS ops for userns visible filesystems */
+ kgid_t kfsgid; /* GID for VFS ops for userns visible filesystems */
unsigned securebits; /* SUID-less security management */
kernel_cap_t cap_inheritable; /* caps our children can inherit */
kernel_cap_t cap_permitted; /* caps we're permitted */
@@ -384,6 +386,8 @@ static inline void put_cred(const struct cred *_cred)
#define current_sgid() (current_cred_xxx(sgid))
#define current_fsuid() (current_cred_xxx(fsuid))
#define current_fsgid() (current_cred_xxx(fsgid))
+#define current_kfsuid() (current_cred_xxx(kfsuid))
+#define current_kfsgid() (current_cred_xxx(kfsgid))
#define current_cap() (current_cred_xxx(cap_effective))
#define current_user() (current_cred_xxx(user))
--
2.25.0
^ permalink raw reply related
* [PATCH v2 07/28] sys: __sys_setfsuid(): handle fsid mappings
From: Christian Brauner @ 2020-02-14 18:35 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Seth Forshee, Alexander Viro, Alexey Dobriyan,
Serge Hallyn, James Morris, Kees Cook, Jonathan Corbet,
Phil Estes, linux-kernel, linux-fsdevel, containers,
linux-security-module, linux-api, Christian Brauner
In-Reply-To: <20200214183554.1133805-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.
A caller can only setfs{g,u}id() to a given id if the id maps to a valid kid in
both the id and fsid maps of the caller's user namespace. This is always the
case when no id mappings and fsid mappings have been written. It is also always
the case when an id mapping has been written which includes the target id and
but no fsid mappings have been written. All non-fsid mapping aware workloads
will thus work just as before.
Requiring a valid mapping for the target id in both the id and fsid mappings of
the container simplifies permission checking for userns visible filesystems
such as proc.
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v2 */
- Christian Brauner <christian.brauner@ubuntu.com>:
- Set unmapped fsid as well.
---
kernel/sys.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/kernel/sys.c b/kernel/sys.c
index f9bc5c303e3f..13f790dbda71 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>
@@ -799,15 +800,19 @@ long __sys_setfsuid(uid_t uid)
const struct cred *old;
struct cred *new;
uid_t old_fsuid;
- kuid_t kuid;
+ kuid_t kuid, kfsuid;
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;
+ kfsuid = make_kuid(old->user_ns, uid);
+ if (!uid_valid(kfsuid))
+ return old_fsuid;
+
new = prepare_creds();
if (!new)
return old_fsuid;
@@ -817,6 +822,7 @@ long __sys_setfsuid(uid_t uid)
ns_capable_setid(old->user_ns, CAP_SETUID)) {
if (!uid_eq(kuid, old->fsuid)) {
new->fsuid = kuid;
+ new->kfsuid = kfsuid;
if (security_task_fix_setuid(new, old, LSM_SETID_FS) == 0)
goto change_okay;
}
--
2.25.0
^ permalink raw reply related
* [PATCH v2 05/28] proc: task_state(): use from_kfs{g,u}id_munged
From: Christian Brauner @ 2020-02-14 18:35 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Seth Forshee, Alexander Viro, Alexey Dobriyan,
Serge Hallyn, James Morris, Kees Cook, Jonathan Corbet,
Phil Estes, linux-kernel, linux-fsdevel, containers,
linux-security-module, linux-api, Christian Brauner
In-Reply-To: <20200214183554.1133805-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>
---
/* v2 */
unchanged
---
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 v2 01/28] user_namespace: introduce fsid mappings infrastructure
From: Christian Brauner @ 2020-02-14 18:35 UTC (permalink / raw)
To: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn
Cc: smbarber, Seth Forshee, Alexander Viro, Alexey Dobriyan,
Serge Hallyn, James Morris, Kees Cook, Jonathan Corbet,
Phil Estes, linux-kernel, linux-fsdevel, containers,
linux-security-module, linux-api, Christian Brauner
In-Reply-To: <20200214183554.1133805-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>
---
/* v2 */
- Randy Dunlap <rdunlap@infradead.org>:
- Fix typo in USER_NS_FSID kconfig documentation.
---
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 6ef1c7109fc4..e44742b0cf8a 100644
--- a/include/linux/user_namespace.h
+++ b/include/linux/user_namespace.h
@@ -56,6 +56,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;
@@ -127,6 +131,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 cfee56c151f1..d4d0beeba48f 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1111,6 +1111,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
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