| Message ID | 20180821000444.7004-4-casey.schaufler@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | LSM: Add and use a hook for side-channel safety checks | expand |
On Tue, Aug 21, 2018 at 2:05 AM Casey Schaufler <casey.schaufler@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.schaufler@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? > +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; > +}
> -----Original Message----- > From: Jann Horn [mailto:jannh@google.com] > Sent: Tuesday, August 21, 2018 10:24 AM > To: Schaufler, Casey <casey.schaufler@intel.com> > Cc: Kernel Hardening <kernel-hardening@lists.openwall.com>; kernel list > <linux-kernel@vger.kernel.org>; linux-security-module <linux-security- > module@vger.kernel.org>; selinux@tycho.nsa.gov; Hansen, Dave > <dave.hansen@intel.com>; Dock, Deneen T <deneen.t.dock@intel.com>; > kristen@linux.intel.com; Arjan van de Ven <arjan@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.schaufler@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.schaufler@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. 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. 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; > > +}
On Wed, Aug 22, 2018 at 1:44 AM Schaufler, Casey <casey.schaufler@intel.com> wrote: > > > -----Original Message----- > > From: Jann Horn [mailto:jannh@google.com] > > Sent: Tuesday, August 21, 2018 10:24 AM > > To: Schaufler, Casey <casey.schaufler@intel.com> > > Cc: Kernel Hardening <kernel-hardening@lists.openwall.com>; kernel list > > <linux-kernel@vger.kernel.org>; linux-security-module <linux-security- > > module@vger.kernel.org>; selinux@tycho.nsa.gov; Hansen, Dave > > <dave.hansen@intel.com>; Dock, Deneen T <deneen.t.dock@intel.com>; > > kristen@linux.intel.com; Arjan van de Ven <arjan@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.schaufler@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.schaufler@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? It doesn't matter much whether processes on your system are intentionally using namespaces; 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. My expectation is that unshare(CLONE_NEWUSER) should not increase the caller's abilities. Your patch seems to violate that expectation. > 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. 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. > 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; > > > +}
> -----Original Message----- > From: Jann Horn [mailto:jannh@google.com] > Sent: Tuesday, August 21, 2018 6:01 PM > To: Schaufler, Casey <casey.schaufler@intel.com> > Cc: Kernel Hardening <kernel-hardening@lists.openwall.com>; kernel list > <linux-kernel@vger.kernel.org>; linux-security-module <linux-security- > module@vger.kernel.org>; selinux@tycho.nsa.gov; Hansen, Dave > <dave.hansen@intel.com>; Dock, Deneen T <deneen.t.dock@intel.com>; > kristen@linux.intel.com; Arjan van de Ven <arjan@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.schaufler@intel.com> wrote: > > > > > -----Original Message----- > > > From: Jann Horn [mailto:jannh@google.com] > > > Sent: Tuesday, August 21, 2018 10:24 AM > > > To: Schaufler, Casey <casey.schaufler@intel.com> > > > Cc: Kernel Hardening <kernel-hardening@lists.openwall.com>; kernel list > > > <linux-kernel@vger.kernel.org>; linux-security-module <linux-security- > > > module@vger.kernel.org>; selinux@tycho.nsa.gov; Hansen, Dave > > > <dave.hansen@intel.com>; Dock, Deneen T <deneen.t.dock@intel.com>; > > > kristen@linux.intel.com; Arjan van de Ven <arjan@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.schaufler@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.schaufler@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; > > > > +}
On Wed, Aug 22, 2018 at 6:39 PM Schaufler, Casey <casey.schaufler@intel.com> wrote: > > > -----Original Message----- > > From: Jann Horn [mailto:jannh@google.com] > > Sent: Tuesday, August 21, 2018 6:01 PM > > To: Schaufler, Casey <casey.schaufler@intel.com> > > Cc: Kernel Hardening <kernel-hardening@lists.openwall.com>; kernel list > > <linux-kernel@vger.kernel.org>; linux-security-module <linux-security- > > module@vger.kernel.org>; selinux@tycho.nsa.gov; Hansen, Dave > > <dave.hansen@intel.com>; Dock, Deneen T <deneen.t.dock@intel.com>; > > kristen@linux.intel.com; Arjan van de Ven <arjan@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.schaufler@intel.com> wrote: > > > > > > > -----Original Message----- > > > > From: Jann Horn [mailto:jannh@google.com] > > > > Sent: Tuesday, August 21, 2018 10:24 AM > > > > To: Schaufler, Casey <casey.schaufler@intel.com> > > > > Cc: Kernel Hardening <kernel-hardening@lists.openwall.com>; kernel list > > > > <linux-kernel@vger.kernel.org>; linux-security-module <linux-security- > > > > module@vger.kernel.org>; selinux@tycho.nsa.gov; Hansen, Dave > > > > <dave.hansen@intel.com>; Dock, Deneen T <deneen.t.dock@intel.com>; > > > > kristen@linux.intel.com; Arjan van de Ven <arjan@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.schaufler@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.schaufler@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. The capability check doesn't always require a namespace match, and I don't care about non-user namespaces here, but I would prefer it if A->B with A having some capabilities required A's user namespace to be ancestor-or-self of B's user namespace. But alternatively: > 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? It seems to me that it would make sense to express this as something like a Kconfig dependency. But I guess if you document that the combination of CONFIG_USER_NS=y, CONFIG_SECURITY_SIDECHANNEL_NAMESPACES=n and SECURITY_SIDECHANNEL_CAPABILITIES=y is nonsensical, that works too. I just don't see why you'd want to provide such a footgun? Configurability is nice, but if we know that one of the possible configurations doesn't make sense, it seems like a good idea to just not allow the system to be configured that way. You say that you were asked to make the configuration flexible. Did whoever told you that actually want the ability to compare raw capability sets on a system with CONFIG_USER_NS=y, and understand what semantics that has (and doesn't have)? Or was their intent more along the lines of "we want to flush if the new task has higher privileges, capability-wise, than the old task; but we don't explicitly care about namespaces"? > > > 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; > > > > > +}
> -----Original Message----- > From: Jann Horn [mailto:jannh@google.com] > Sent: Wednesday, August 22, 2018 10:04 AM > To: Schaufler, Casey <casey.schaufler@intel.com> > Cc: Kernel Hardening <kernel-hardening@lists.openwall.com>; kernel list > <linux-kernel@vger.kernel.org>; linux-security-module <linux-security- > module@vger.kernel.org>; selinux@tycho.nsa.gov; Hansen, Dave > <dave.hansen@intel.com>; Dock, Deneen T <deneen.t.dock@intel.com>; > kristen@linux.intel.com; Arjan van de Ven <arjan@linux.intel.com> > Subject: Re: [PATCH v3 3/5] LSM: Security module checking for side-channel > dangers > > [SNIP] > > 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. > > The capability check doesn't always require a namespace match, and I > don't care about non-user namespaces here, but I would prefer it if > A->B with A having some capabilities required A's user namespace to be > ancestor-or-self of B's user namespace. But alternatively: Looking at ancestor relations starts to get us pretty close to the point where the cost of checking will overwhelm the savings. This is something we have to be very careful of. > > 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? > > It seems to me that it would make sense to express this as something > like a Kconfig dependency. I thought about that. It could be the best choice. I will investigate further. > But I guess if you document that the > combination of CONFIG_USER_NS=y, > CONFIG_SECURITY_SIDECHANNEL_NAMESPACES=n and > SECURITY_SIDECHANNEL_CAPABILITIES=y is nonsensical, that works too. I > just don't see why you'd want to provide such a footgun? Point. > Configurability is nice, but if we know that one of the possible > configurations doesn't make sense, it seems like a good idea to just > not allow the system to be configured that way. Another point. > You say that you were asked to make the configuration flexible. Did > whoever told you that actually want the ability to compare raw > capability sets on a system with CONFIG_USER_NS=y, and understand what > semantics that has (and doesn't have)? Or was their intent more along > the lines of "we want to flush if the new task has higher privileges, > capability-wise, than the old task; but we don't explicitly care about > namespaces"? Without going into too much detail, it's a matter of people who understand chips and performance better than they understand security or the user experience.
diff --git a/MAINTAINERS b/MAINTAINERS index 3119bba7971c..d078d6a5b471 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -13066,6 +13066,12 @@ F: drivers/slimbus/ F: Documentation/devicetree/bindings/slimbus/ F: include/linux/slimbus.h +SIDECHANNEL SECURITY MODULE +M: Casey Schaufler <casey.schaufler@intel.com> +L: linux-security-module@vger.kernel.org +S: Maintained +F: security/sidechannel/ + SMACK SECURITY MODULE M: Casey Schaufler <casey@schaufler-ca.com> L: linux-security-module@vger.kernel.org diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index fd2a7e6beb01..d48e4a085fe2 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -2088,5 +2088,10 @@ void __init loadpin_add_hooks(void); #else static inline void loadpin_add_hooks(void) { }; #endif +#ifdef CONFIG_SECURITY_SIDECHANNEL +void __init sidechannel_add_hooks(void); +#else +static inline void sidechannel_add_hooks(void) { }; +#endif #endif /* ! __LINUX_LSM_HOOKS_H */ diff --git a/security/Kconfig b/security/Kconfig index c4302067a3ad..28cb7b2939ee 100644 --- a/security/Kconfig +++ b/security/Kconfig @@ -237,6 +237,7 @@ source security/tomoyo/Kconfig source security/apparmor/Kconfig source security/loadpin/Kconfig source security/yama/Kconfig +source security/sidechannel/Kconfig source security/integrity/Kconfig diff --git a/security/Makefile b/security/Makefile index 4d2d3782ddef..d0c9e1b227f9 100644 --- a/security/Makefile +++ b/security/Makefile @@ -10,6 +10,7 @@ subdir-$(CONFIG_SECURITY_TOMOYO) += tomoyo subdir-$(CONFIG_SECURITY_APPARMOR) += apparmor subdir-$(CONFIG_SECURITY_YAMA) += yama subdir-$(CONFIG_SECURITY_LOADPIN) += loadpin +subdir-$(CONFIG_SECURITY_SIDECHANNEL) += sidechannel # always enable default capabilities obj-y += commoncap.o @@ -25,6 +26,7 @@ obj-$(CONFIG_SECURITY_TOMOYO) += tomoyo/ obj-$(CONFIG_SECURITY_APPARMOR) += apparmor/ obj-$(CONFIG_SECURITY_YAMA) += yama/ obj-$(CONFIG_SECURITY_LOADPIN) += loadpin/ +obj-$(CONFIG_SECURITY_SIDECHANNEL) += sidechannel/ obj-$(CONFIG_CGROUP_DEVICE) += device_cgroup.o # Object integrity file lists diff --git a/security/security.c b/security/security.c index 353b711e635a..777919349751 100644 --- a/security/security.c +++ b/security/security.c @@ -80,6 +80,7 @@ int __init security_init(void) capability_add_hooks(); yama_add_hooks(); loadpin_add_hooks(); + sidechannel_add_hooks(); /* * Load all the remaining security modules. diff --git a/security/sidechannel/Kconfig b/security/sidechannel/Kconfig new file mode 100644 index 000000000000..af9396534128 --- /dev/null +++ b/security/sidechannel/Kconfig @@ -0,0 +1,60 @@ +config SECURITY_SIDECHANNEL + bool "Sidechannel attack safety extra checks" + depends on SECURITY + default n + help + Look for a variety of cases where a side-channel attack + could potentially be exploited. Instruct the switching + code to use the indirect_branch_prediction_barrier in + cases where the passed task and the current task may be + at risk. + + If you are unsure how to answer this question, answer N. + +config SECURITY_SIDECHANNEL_UIDS + bool "Sidechannel check on UID" + depends on SECURITY_SIDECHANNEL + default n + help + Assume that tasks with different effective UIDs may be + subject to side-channel attacks. As most task switching + occurs between tasks with different effective UIDs this + can have a significant performance impact. + + If you are unsure how to answer this question, answer N. + + +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. + +config SECURITY_SIDECHANNEL_ALWAYS + bool "Sidechannel assumed to always be possible" + depends on SECURITY_SIDECHANNEL + default n + help + Assume that all tasks may be subject to side-channel attacks. + Always instruct the system to use countermeasures regardless + of the potential impact. + + If you are unsure how to answer this question, answer N. diff --git a/security/sidechannel/Makefile b/security/sidechannel/Makefile new file mode 100644 index 000000000000..f61d83f28035 --- /dev/null +++ b/security/sidechannel/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_SECURITY_SIDECHANNEL) += sidechannel.o 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 @@ -0,0 +1,162 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Side Channel Safety Security Module + * + * Copyright (C) 2018 Intel Corporation. + * + */ + +#define pr_fmt(fmt) "SideChannel: " fmt + +#include <linux/types.h> +#include <linux/lsm_hooks.h> +#include <linux/capability.h> +#include <linux/cred.h> +#include <linux/sched.h> +#include <linux/string_helpers.h> +#include <linux/nsproxy.h> +#include <linux/pid_namespace.h> + +#ifdef CONFIG_SECURITY_SIDECHANNEL_ALWAYS +static int sidechannel_task_safe_sidechannel(struct task_struct *p) +{ + return -EACCES; +} +#else +/* + * safe_by_uid - 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_UIDS +static int safe_by_uid(struct task_struct *p) +{ + const struct cred *ccred = current_real_cred(); + const struct cred *pcred = rcu_dereference_protected(p->real_cred, 1); + + /* + * Credential checks. Considered safe if: + * UIDs are the same + */ + if (ccred != pcred && ccred->euid.val != pcred->euid.val) + return -EACCES; + return 0; +} +#else +static inline int safe_by_uid(struct task_struct *p) +{ + return 0; +} +#endif + +/* + * 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; +} +#else +static inline int safe_by_capability(struct task_struct *p) +{ + return 0; +} +#endif + +#ifdef CONFIG_SECURITY_SIDECHANNEL_NAMESPACES +/** + * safe_by_namespace - Are task and current sidechannel safe? + * @p: task to check on + * + * Returns 0 if the tasks are sidechannel safe, -EACCES otherwise. + */ +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; +} +#else +static inline int safe_by_namespace(struct task_struct *p) +{ + return 0; +} +#endif + +/** + * sidechannel_task_safe_sidechannel - Are task and current sidechannel safe? + * @p: task to check on + * + * Returns 0 if the tasks are sidechannel safe, -EACCES otherwise. + */ +static int sidechannel_task_safe_sidechannel(struct task_struct *p) +{ + int rc; + + /* + * Easy optimizations + */ + if (p == current || p->pid == current->pid) + return 0; + + rc = safe_by_uid(p); + if (rc) + return rc; + rc = safe_by_capability(p); + if (rc) + return rc; + rc = safe_by_namespace(p); + if (rc) + return rc; + return 0; +} +#endif /* CONFIG_SECURITY_SIDECHANNEL_ALWAYS */ + +static struct security_hook_list sidechannel_hooks[] __lsm_ro_after_init = { + LSM_HOOK_INIT(task_safe_sidechannel, sidechannel_task_safe_sidechannel), +}; + +void __init sidechannel_add_hooks(void) +{ + pr_info("Extra sidechannel checks enabled\n"); + security_add_hooks(sidechannel_hooks, ARRAY_SIZE(sidechannel_hooks), + "sidechannel"); +}
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.schaufler@intel.com> --- MAINTAINERS | 6 ++ include/linux/lsm_hooks.h | 5 + security/Kconfig | 1 + security/Makefile | 2 + security/security.c | 1 + security/sidechannel/Kconfig | 60 +++++++++++ security/sidechannel/Makefile | 1 + security/sidechannel/sidechannel.c | 162 +++++++++++++++++++++++++++++ 8 files changed, 238 insertions(+) create mode 100644 security/sidechannel/Kconfig create mode 100644 security/sidechannel/Makefile create mode 100644 security/sidechannel/sidechannel.c