> -----Original Message-----
> From: Jann Horn [mailto:ja...@google.com]
> Sent: Tuesday, August 21, 2018 6:01 PM
> To: Schaufler, Casey <casey.schauf...@intel.com>
> Cc: Kernel Hardening <kernel-harden...@lists.openwall.com>; kernel list
> <linux-ker...@vger.kernel.org>; linux-security-module <linux-security-
> mod...@vger.kernel.org>; selinux@tycho.nsa.gov; Hansen, Dave
> <dave.han...@intel.com>; Dock, Deneen T <deneen.t.d...@intel.com>;
> kris...@linux.intel.com; Arjan van de Ven <ar...@linux.intel.com>
> Subject: Re: [PATCH v3 3/5] LSM: Security module checking for side-channel
> dangers
>
> On Wed, Aug 22, 2018 at 1:44 AM Schaufler, Casey
> <casey.schauf...@intel.com> wrote:
> >
> > > -----Original Message-----
> > > From: Jann Horn [mailto:ja...@google.com]
> > > Sent: Tuesday, August 21, 2018 10:24 AM
> > > To: Schaufler, Casey <casey.schauf...@intel.com>
> > > Cc: Kernel Hardening <kernel-harden...@lists.openwall.com>; kernel list
> > > <linux-ker...@vger.kernel.org>; linux-security-module <linux-security-
> > > mod...@vger.kernel.org>; selinux@tycho.nsa.gov; Hansen, Dave
> > > <dave.han...@intel.com>; Dock, Deneen T <deneen.t.d...@intel.com>;
> > > kris...@linux.intel.com; Arjan van de Ven <ar...@linux.intel.com>
> > > Subject: Re: [PATCH v3 3/5] LSM: Security module checking for side-channel
> > > dangers
> > >
> > > On Tue, Aug 21, 2018 at 2:05 AM Casey Schaufler
> > > <casey.schauf...@intel.com> wrote:
> > > >
> > > > The sidechannel LSM checks for cases where a side-channel
> > > > attack may be dangerous based on security attributes of tasks.
> > > > This includes:
> > > > Effective UID of the tasks is different
> > > > Capablity sets are different
> > > > Tasks are in different namespaces
> > > > An option is also provided to assert that task are never
> > > > to be considered safe. This is high paranoia, and expensive
> > > > as well.
> > > >
> > > > Signed-off-by: Casey Schaufler <casey.schauf...@intel.com>
> > > > ---
> > > [...]
> > > > diff --git a/security/sidechannel/Kconfig b/security/sidechannel/Kconfig
> > > > new file mode 100644
> > > > index 000000000000..af9396534128
> > > > --- /dev/null
> > > > +++ b/security/sidechannel/Kconfig
> > > [...]
> > > > +config SECURITY_SIDECHANNEL_CAPABILITIES
> > > > + bool "Sidechannel check on capability sets"
> > > > + depends on SECURITY_SIDECHANNEL
> > > > + default n
> > > > + help
> > > > + Assume that tasks with different sets of privilege may be
> > > > + subject to side-channel attacks. Potential interactions
> > > > + where the attacker lacks capabilities the attacked has
> > > > + are blocked.
> > > > +
> > > > + If you are unsure how to answer this question, answer N.
> > > > +
> > > > +config SECURITY_SIDECHANNEL_NAMESPACES
> > > > + bool "Sidechannel check on namespaces"
> > > > + depends on SECURITY_SIDECHANNEL
> > > > + depends on NAMESPACES
> > > > + default n
> > > > + help
> > > > + Assume that tasks in different namespaces may be
> > > > + subject to side-channel attacks. User, PID and cgroup
> > > > + namespaces are checked.
> > > > +
> > > > + If you are unsure how to answer this question, answer N.
> > > [...]
> > > > diff --git a/security/sidechannel/sidechannel.c
> > > b/security/sidechannel/sidechannel.c
> > > > new file mode 100644
> > > > index 000000000000..4da7d6dafdc5
> > > > --- /dev/null
> > > > +++ b/security/sidechannel/sidechannel.c
> > > [...]
> > > > +/*
> > > > + * safe_by_capability - Are task and current sidechannel safe?
> > > > + * @p: task to check on
> > > > + *
> > > > + * Returns 0 if the tasks are sidechannel safe, -EACCES otherwise.
> > > > + */
> > > > +#ifdef CONFIG_SECURITY_SIDECHANNEL_CAPABILITIES
> > > > +static int safe_by_capability(struct task_struct *p)
> > > > +{
> > > > + const struct cred *ccred = current_real_cred();
> > > > + const struct cred *pcred =
> > > > rcu_dereference_protected(p->real_cred,
> 1);
> > > > +
> > > > + /*
> > > > + * Capabilities checks. Considered safe if:
> > > > + * current has all the capabilities p does
> > > > + */
> > > > + if (ccred != pcred &&
> > > > + !cap_issubset(pcred->cap_effective, ccred->cap_effective))
> > > > + return -EACCES;
> > > > + return 0;
> > > > +}
> > >
> > > On its own (without safe_by_namespace()), this check makes no sense, I
> > > think. You're performing a test on the namespaced capability sets
> > > without looking at which user namespaces they are relative to. Maybe
> > > either introduce a configuration dependency or add an extra namespace
> > > check here?
> >
> > If you don't have namespaces the check is correct. If you do, and use
> > safe_by_namespace() you're also correct. If you use namespaces and
> > care about side-channel attacks you should enable the namespace checks.
>
> By "use namespaces", you mean "have CONFIG_USER_NS=y set in the kernel
> config", right?
That's correct.
> It doesn't matter much whether processes on your system are
> intentionally using namespaces;
Also correct.
> what matters is whether some random
> process can just use unshare(CLONE_NEWUSER) to increase its apparent
> capabilities and bypass the checks performed by this LSM.
Which puts it in a new user namespace, which gets caught by the
safe_by_namespace() check.
> My expectation is that unshare(CLONE_NEWUSER) should not increase the
> caller's abilities. Your patch seems to violate that expectation.
If you have CONFIG_USER_NS and not
CONFIG_SECURITY_SIDECHANNEL_NAMESPACES you do not increase the
caller's abilities from what you have without safesidechannel. If you have
CONFIG_SECURITY_SIDECHANNEL_NAMESPACES you have additional
restriction (assuming one considers setting the barrier a restriction) that
the tasks must be in the same namespace(s). As I said, if you care about
namespace implications you should configure the system accordingly.
> > I don't see real value in adding namespace checks in the capability checks
> > for the event where someone has said they don't want namespace checks.
>
> Capabilities are meaningless if you don't consider the namespaces
> relative to which they are effective.
Agreed. But if CONFIG_NAMESPACES is off you are always in the same
namespace and if it is on you should use the sidechannel namespace check.
> Anyone can get CAP_SYS_ADMIN or
> whatever other capabilities they want, by design - just not relative
> to objects they don't own. Look:
>
> $ grep ^Cap /proc/self/status
> CapInh: 0000000000000000
> CapPrm: 0000000000000000
> CapEff: 0000000000000000
> CapBnd: 0000003fffffffff
> CapAmb: 0000000000000000
> $ unshare -Ur grep ^Cap /proc/self/status
> CapInh: 0000000000000000
> CapPrm: 0000003fffffffff
> CapEff: 0000003fffffffff
> CapBnd: 0000003fffffffff
> CapAmb: 0000000000000000
>
> Ta-daa! Full capability set.
Yes, but in a different namespace. Hence the namespace check.
What I hear you saying is that you don't want the capability check
to be independent of the namespace check. This conflicts with the
strong desire expressed to me when I started this that the configuration
should be flexible. I can beef up the description of the various options.
Would that address the issue?
>
> > I got early feedback that configurability was considered important.
> > This is the correct behavior if you want namespace checks to be
> > separately configurable from capability checks. You could ask for
> > distinct configuration options for each kind of namespace, but, well, yuck.
> >
> > > > +static int safe_by_namespace(struct task_struct *p)
> > > > +{
> > > > + struct cgroup_namespace *ccgn = NULL;
> > > > + struct cgroup_namespace *pcgn = NULL;
> > > > + const struct cred *ccred;
> > > > + const struct cred *pcred;
> > > > +
> > > > + /*
> > > > + * Namespace checks. Considered safe if:
> > > > + * cgroup namespace is the same
> > > > + * User namespace is the same
> > > > + * PID namespace is the same
> > > > + */
> > > > + if (current->nsproxy)
> > > > + ccgn = current->nsproxy->cgroup_ns;
> > > > + if (p->nsproxy)
> > > > + pcgn = p->nsproxy->cgroup_ns;
> > > > + if (ccgn != pcgn)
> > > > + return -EACCES;
> > > > +
> > > > + ccred = current_real_cred();
> > > > + pcred = rcu_dereference_protected(p->real_cred, 1);
> > > > +
> > > > + if (ccred->user_ns != pcred->user_ns)
> > > > + return -EACCES;
> > > > + if (task_active_pid_ns(current) != task_active_pid_ns(p))
> > > > + return -EACCES;
> > > > + return 0;
> > > > +}
_______________________________________________
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
