Re: [PATCH v5 5/5] sidechannel: Linux Security Module for sidechannel

2018-09-27 Thread James Morris 
On Wed, 26 Sep 2018, Casey Schaufler wrote:

> + /*
> +  * 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;
> + if (current->cred->user_ns != p->cred->user_ns)
> + return -EACCES;
> + if (task_active_pid_ns(current) != task_active_pid_ns(p))
> + return -EACCES;
> + return 0;

I really don't like the idea of hard-coding namespace security semantics 
in an LSM.  Also, I'm not sure if these semantics make any sense.

It least make it user configurable.


-- 
James Morris


___
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.

[PATCH 6/5] capability: Repair sidechannel test in ptrace

2018-09-27 Thread Casey Schaufler 
From: Casey Schaufler 

The PTRACE_MODE_SCHED check erroniously returns 0 in
all cases. It should be returning -EPERM. This fixes
the logic to correct that error.

Signed-off-by: Casey Schaufler 
---
 security/commoncap.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/security/commoncap.c b/security/commoncap.c
index e77457110d05..70a7e3d19c16 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -152,9 +152,8 @@ int cap_ptrace_access_check(struct task_struct *child, 
unsigned int mode)
if (cred->user_ns == child_cred->user_ns &&
cap_issubset(child_cred->cap_permitted, *caller_caps))
goto out;
-   if (mode & PTRACE_MODE_SCHED)
-   goto out;
-   if (ns_capable(child_cred->user_ns, CAP_SYS_PTRACE))
+   if (!(mode & PTRACE_MODE_SCHED) &&
+   ns_capable(child_cred->user_ns, CAP_SYS_PTRACE))
goto out;
ret = -EPERM;
 out:
-- 
2.17.1

___
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.

RE: [PATCH v5 3/5] SELinux: Prepare for PTRACE_MODE_SCHED

2018-09-27 Thread Schaufler, Casey 
> -Original Message-
> From: Stephen Smalley [mailto:s...@tycho.nsa.gov]
> Sent: Thursday, September 27, 2018 8:50 AM
> To: Schaufler, Casey ; kernel-
> harden...@lists.openwall.com; linux-ker...@vger.kernel.org; linux-security-
> mod...@vger.kernel.org; selinux@tycho.nsa.gov; Hansen, Dave
> ; Dock, Deneen T ;
> kris...@linux.intel.com; ar...@linux.intel.com; Paul Moore  moore.com>
> Subject: Re: [PATCH v5 3/5] SELinux: Prepare for PTRACE_MODE_SCHED
> 
> On 09/26/2018 04:34 PM, Casey Schaufler wrote:
> > From: Casey Schaufler 
> >
> > A ptrace access check with mode PTRACE_MODE_SCHED gets called
> > from process switching code. This precludes the use of audit or avc,
> > as the locking is incompatible. The only available check that
> > can be made without using avc is a comparison of the secids.
> > This is not very satisfactory as it will indicate possible
> > vulnerabilies much too aggressively.
> Can you document (in the patch description and/or in the inline
> documentation in lsm_hooks.h) what locks can be safely used when this
> hook is called with PTRACE_MODE_SCHED?  rcu_read_lock() seemingly must
> be safe since it is being called by task_sid() below. Are any other
> locking primitives safe?

Peter Zijlstra  had this comment on
locking in the SELinux ptrace path. 

avc_has_perm_noaudit()
  security_compute_av()
read_lock(>ss->policy_rwlock);
  avc_insert()
spin_lock_irqsave();
  avc_denied()
avc_update_node()
  spin_lock_irqsave();

under the scheduler's raw_spinlock_t, which are invalid lock nestings.

I don't know that it would be impossible to address these issues,
but as many people have noted over the years I am not now
nor have ever been an expert on locking.

> 
> Does the PTRACE_MODE_SCHED check have to occur while holding the
> scheduler lock, or could it be performed before taking the lock?

My understanding is that the lock is required.

 
> Can you cite the commit or patch posting (e.g. from lore or patchwork)
> that defines PTRACE_MODE_SCHED and its usage as part of the patch
> description for context?  Is this based on the v7 patchset, e.g.
> https://lore.kernel.org/lkml/nycvar.yfh.7.76.1809251437340.15...@cbobk.fhf
> r.pm/

Yes, that's the one. Sorry, I should have identified that.

> 
> >
> > Signed-off-by: Casey Schaufler 
> > ---
> >   security/selinux/hooks.c | 2 ++
> >   1 file changed, 2 insertions(+)
> >
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index ad9a9b8e9979..160239791007 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -2267,6 +2267,8 @@ static int selinux_ptrace_access_check(struct
> task_struct *child,
> > u32 sid = current_sid();
> > u32 csid = task_sid(child);
> >
> > +   if (mode & PTRACE_MODE_SCHED)
> > +   return sid == csid ? 0 : -EACCES;
> IIUC, this logic is essentially the same as the uid-based check,
> including the fact that even a "privileged" process is not given any
> special handling since they always return false from ptrace_has_cap()
> for PTRACE_MODE_SCHED. If they are ok with applying IBPB whenever uids
> differ, then doing so whenever sids/contexts differ does not seem like
> an onerous thing.
> 
> 
> > if (mode & PTRACE_MODE_READ)
> > return avc_has_perm(_state,
> > sid, csid, SECCLASS_FILE, FILE__READ,
> NULL);
> >


___
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.

Re: [PATCH v5 3/5] SELinux: Prepare for PTRACE_MODE_SCHED

2018-09-27 Thread Stephen Smalley 

On 09/26/2018 04:34 PM, Casey Schaufler wrote:

From: Casey Schaufler 

A ptrace access check with mode PTRACE_MODE_SCHED gets called
from process switching code. This precludes the use of audit or avc,
as the locking is incompatible. The only available check that
can be made without using avc is a comparison of the secids.
This is not very satisfactory as it will indicate possible
vulnerabilies much too aggressively.
Can you document (in the patch description and/or in the inline 
documentation in lsm_hooks.h) what locks can be safely used when this 
hook is called with PTRACE_MODE_SCHED?  rcu_read_lock() seemingly must 
be safe since it is being called by task_sid() below. Are any other 
locking primitives safe?


Does the PTRACE_MODE_SCHED check have to occur while holding the 
scheduler lock, or could it be performed before taking the lock?


Can you cite the commit or patch posting (e.g. from lore or patchwork) 
that defines PTRACE_MODE_SCHED and its usage as part of the patch 
description for context?  Is this based on the v7 patchset, e.g.

https://lore.kernel.org/lkml/nycvar.yfh.7.76.1809251437340.15...@cbobk.fhfr.pm/



Signed-off-by: Casey Schaufler 
---
  security/selinux/hooks.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index ad9a9b8e9979..160239791007 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2267,6 +2267,8 @@ static int selinux_ptrace_access_check(struct task_struct 
*child,
u32 sid = current_sid();
u32 csid = task_sid(child);
  
+	if (mode & PTRACE_MODE_SCHED)

+   return sid == csid ? 0 : -EACCES;
IIUC, this logic is essentially the same as the uid-based check, 
including the fact that even a "privileged" process is not given any 
special handling since they always return false from ptrace_has_cap() 
for PTRACE_MODE_SCHED. If they are ok with applying IBPB whenever uids 
differ, then doing so whenever sids/contexts differ does not seem like 
an onerous thing.




if (mode & PTRACE_MODE_READ)
return avc_has_perm(_state,
sid, csid, SECCLASS_FILE, FILE__READ, NULL);



___
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.

Re: [PATCH v5 2/5] Smack: Prepare for PTRACE_MODE_SCHED

2018-09-27 Thread Jann Horn via Selinux 
On Wed, Sep 26, 2018 at 10:35 PM Casey Schaufler
 wrote:
> A ptrace access check with mode PTRACE_MODE_SCHED gets called
> from process switching code. This precludes the use of audit,
> as the locking is incompatible. Don't do audit in the PTRACE_MODE_SCHED
> case.
>
> Signed-off-by: Casey Schaufler 
> ---
>  security/smack/smack_lsm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 340fc30ad85d..ffa95bcab599 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -422,7 +422,8 @@ static int smk_ptrace_rule_check(struct task_struct 
> *tracer,
> struct task_smack *tsp;
> struct smack_known *tracer_known;
>
> -   if ((mode & PTRACE_MODE_NOAUDIT) == 0) {
> +   if ((mode & PTRACE_MODE_NOAUDIT) == 0 &&
> +   (mode & PTRACE_MODE_SCHED) == 0) {

If you ORed PTRACE_MODE_NOAUDIT into the flags when calling the
security hook, you could drop this patch, right?
___
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.

Re: [PATCH v5 4/5] Capability: Complete PTRACE_MODE_SCHED

2018-09-27 Thread Jann Horn via Selinux 
On Wed, Sep 26, 2018 at 10:35 PM Casey Schaufler
 wrote:
> Allow a complete ptrace access check with mode PTRACE_MODE_SCHED.
> Disable the inappropriate privilege check in the capability code
> that does incompatible locking.

What's that locking you're talking about?

> Signed-off-by: Casey Schaufler 
> ---
>  kernel/ptrace.c  | 2 --
>  security/commoncap.c | 2 ++
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/ptrace.c b/kernel/ptrace.c
> index 99cfddde6a55..0b6a9df51c3b 100644
> --- a/kernel/ptrace.c
> +++ b/kernel/ptrace.c
> @@ -331,8 +331,6 @@ static int __ptrace_may_access(struct task_struct *task, 
> unsigned int mode)
>  !ptrace_has_cap(mm->user_ns, mode)))
> return -EPERM;
>
> -   if (mode & PTRACE_MODE_SCHED)
> -   return 0;
> return security_ptrace_access_check(task, mode);
>  }
>
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 2e489d6a3ac8..e77457110d05 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -152,6 +152,8 @@ int cap_ptrace_access_check(struct task_struct *child, 
> unsigned int mode)
> if (cred->user_ns == child_cred->user_ns &&
> cap_issubset(child_cred->cap_permitted, *caller_caps))
> goto out;
> +   if (mode & PTRACE_MODE_SCHED)
> +   goto out;

So for PTRACE_MODE_SCHED, this function always returns 0, right? If
that's intentional, perhaps you should instead just put "if (mode &
PTRACE_MODE_SCHED) return 0;" at the start of the function, to avoid
taking the RCU read lock in this case.

> if (ns_capable(child_cred->user_ns, CAP_SYS_PTRACE))
> goto out;
> ret = -EPERM;
___
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.

Re: [PATCH v5 1/5] AppArmor: Prepare for PTRACE_MODE_SCHED

2018-09-27 Thread Jann Horn via Selinux 
On Wed, Sep 26, 2018 at 11:16 PM Jann Horn  wrote:
>
> On Wed, Sep 26, 2018 at 10:35 PM Casey Schaufler
>  wrote:
> > A ptrace access check with mode PTRACE_MODE_SCHED gets called
> > from process switching code. This precludes the use of audit,
> > as the locking is incompatible. Don't do audit in the PTRACE_MODE_SCHED
> > case.
>
> Why is this separate from PTRACE_MODE_NOAUDIT? It looks like
> apparmor_ptrace_access_check() currently ignores PTRACE_MODE_NOAUDIT.
> Could you, instead of adding a new flag, fix the handling of
> PTRACE_MODE_NOAUDIT?

Er, after looking at more of the series, I see that PTRACE_MODE_SCHED
is necessary; but could you handle the "don't audit" part for AppArmor
using PTRACE_MODE_NOAUDIT instead?

> > Signed-off-by: Casey Schaufler 
> > ---
> >  security/apparmor/domain.c  | 2 +-
> >  security/apparmor/include/ipc.h | 2 +-
> >  security/apparmor/ipc.c | 8 +---
> >  security/apparmor/lsm.c | 5 +++--
> >  4 files changed, 10 insertions(+), 7 deletions(-)
> >
> > diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
> > index 08c88de0ffda..28300f4c3ef9 100644
> > --- a/security/apparmor/domain.c
> > +++ b/security/apparmor/domain.c
> > @@ -77,7 +77,7 @@ static int may_change_ptraced_domain(struct aa_label 
> > *to_label,
> > if (!tracer || unconfined(tracerl))
> > goto out;
> >
> > -   error = aa_may_ptrace(tracerl, to_label, PTRACE_MODE_ATTACH);
> > +   error = aa_may_ptrace(tracerl, to_label, PTRACE_MODE_ATTACH, true);
> >
> >  out:
> > rcu_read_unlock();
> > diff --git a/security/apparmor/include/ipc.h 
> > b/security/apparmor/include/ipc.h
> > index 5ffc218d1e74..299d1c45fef0 100644
> > --- a/security/apparmor/include/ipc.h
> > +++ b/security/apparmor/include/ipc.h
> > @@ -34,7 +34,7 @@ struct aa_profile;
> > "xcpu xfsz vtalrm prof winch io pwr sys emt lost"
> >
> >  int aa_may_ptrace(struct aa_label *tracer, struct aa_label *tracee,
> > - u32 request);
> > + u32 request, bool audit);
> >  int aa_may_signal(struct aa_label *sender, struct aa_label *target, int 
> > sig);
> >
> >  #endif /* __AA_IPC_H */
> > diff --git a/security/apparmor/ipc.c b/security/apparmor/ipc.c
> > index 527ea1557120..9ed110afc822 100644
> > --- a/security/apparmor/ipc.c
> > +++ b/security/apparmor/ipc.c
> > @@ -121,15 +121,17 @@ static int profile_tracer_perm(struct aa_profile 
> > *tracer,
> >   * Returns: %0 else error code if permission denied or error
> >   */
> >  int aa_may_ptrace(struct aa_label *tracer, struct aa_label *tracee,
> > - u32 request)
> > + u32 request, bool audit)
> >  {
> > struct aa_profile *profile;
> > u32 xrequest = request << PTRACE_PERM_SHIFT;
> > DEFINE_AUDIT_DATA(sa, LSM_AUDIT_DATA_NONE, OP_PTRACE);
> >
> > return xcheck_labels(tracer, tracee, profile,
> > -   profile_tracer_perm(profile, tracee, request, ),
> > -   profile_tracee_perm(profile, tracer, xrequest, 
> > ));
> > +   profile_tracer_perm(profile, tracee, request,
> > +   audit ?  : NULL),
> > +   profile_tracee_perm(profile, tracer, xrequest,
> > +   audit ?  : NULL));
> >  }
> >
> >
> > diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> > index 8b8b70620bbe..da9d0b228857 100644
> > --- a/security/apparmor/lsm.c
> > +++ b/security/apparmor/lsm.c
> > @@ -118,7 +118,8 @@ static int apparmor_ptrace_access_check(struct 
> > task_struct *child,
> > tracee = aa_get_task_label(child);
> > error = aa_may_ptrace(tracer, tracee,
> > (mode & PTRACE_MODE_READ) ? AA_PTRACE_READ
> > - : AA_PTRACE_TRACE);
> > + : AA_PTRACE_TRACE,
> > +   !(mode & PTRACE_MODE_SCHED));
> > aa_put_label(tracee);
> > end_current_label_crit_section(tracer);
> >
> > @@ -132,7 +133,7 @@ static int apparmor_ptrace_traceme(struct task_struct 
> > *parent)
> >
> > tracee = begin_current_label_crit_section();
> > tracer = aa_get_task_label(parent);
> > -   error = aa_may_ptrace(tracer, tracee, AA_PTRACE_TRACE);
> > +   error = aa_may_ptrace(tracer, tracee, AA_PTRACE_TRACE, true);
> > aa_put_label(tracer);
> > end_current_label_crit_section(tracee);
> >
> > --
> > 2.17.1
> >
___
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.

RE: [PATCH v5 2/5] Smack: Prepare for PTRACE_MODE_SCHED

2018-09-27 Thread Schaufler, Casey 
> -Original Message-
> From: Jann Horn [mailto:ja...@google.com]
> Sent: Wednesday, September 26, 2018 2:31 PM
> To: Schaufler, Casey 
> Cc: Kernel Hardening ; kernel list
> ; linux-security-module  mod...@vger.kernel.org>; selinux@tycho.nsa.gov; Hansen, Dave
> ; Dock, Deneen T ;
> kris...@linux.intel.com; Arjan van de Ven 
> Subject: Re: [PATCH v5 2/5] Smack: Prepare for PTRACE_MODE_SCHED
> 
> On Wed, Sep 26, 2018 at 10:35 PM Casey Schaufler
>  wrote:
> > A ptrace access check with mode PTRACE_MODE_SCHED gets called
> > from process switching code. This precludes the use of audit,
> > as the locking is incompatible. Don't do audit in the PTRACE_MODE_SCHED
> > case.
> >
> > Signed-off-by: Casey Schaufler 
> > ---
> >  security/smack/smack_lsm.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> > index 340fc30ad85d..ffa95bcab599 100644
> > --- a/security/smack/smack_lsm.c
> > +++ b/security/smack/smack_lsm.c
> > @@ -422,7 +422,8 @@ static int smk_ptrace_rule_check(struct task_struct
> *tracer,
> > struct task_smack *tsp;
> > struct smack_known *tracer_known;
> >
> > -   if ((mode & PTRACE_MODE_NOAUDIT) == 0) {
> > +   if ((mode & PTRACE_MODE_NOAUDIT) == 0 &&
> > +   (mode & PTRACE_MODE_SCHED) == 0) {
> 
> If you ORed PTRACE_MODE_NOAUDIT into the flags when calling the
> security hook, you could drop this patch, right?

Yes. Since the PTRACE_MODE_NOAUDIT was in PTRACE_MODE_IBPB
in Jiri's previous patch set and not in PTRACE_MODE_SCHED in this one
I assumed that there was a good reason for it.


___
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.

Re: [PATCH v5 2/5] Smack: Prepare for PTRACE_MODE_SCHED

2018-09-27 Thread Jann Horn via Selinux 
+Jiri

On Thu, Sep 27, 2018 at 12:54 AM Schaufler, Casey
 wrote:
> > -Original Message-
> > From: Jann Horn [mailto:ja...@google.com]
> > Sent: Wednesday, September 26, 2018 2:31 PM
> > To: Schaufler, Casey 
> > Cc: Kernel Hardening ; kernel list
> > ; linux-security-module  > mod...@vger.kernel.org>; selinux@tycho.nsa.gov; Hansen, Dave
> > ; Dock, Deneen T ;
> > kris...@linux.intel.com; Arjan van de Ven 
> > Subject: Re: [PATCH v5 2/5] Smack: Prepare for PTRACE_MODE_SCHED
> >
> > On Wed, Sep 26, 2018 at 10:35 PM Casey Schaufler
> >  wrote:
> > > A ptrace access check with mode PTRACE_MODE_SCHED gets called
> > > from process switching code. This precludes the use of audit,
> > > as the locking is incompatible. Don't do audit in the PTRACE_MODE_SCHED
> > > case.
> > >
> > > Signed-off-by: Casey Schaufler 
> > > ---
> > >  security/smack/smack_lsm.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> > > index 340fc30ad85d..ffa95bcab599 100644
> > > --- a/security/smack/smack_lsm.c
> > > +++ b/security/smack/smack_lsm.c
> > > @@ -422,7 +422,8 @@ static int smk_ptrace_rule_check(struct task_struct
> > *tracer,
> > > struct task_smack *tsp;
> > > struct smack_known *tracer_known;
> > >
> > > -   if ((mode & PTRACE_MODE_NOAUDIT) == 0) {
> > > +   if ((mode & PTRACE_MODE_NOAUDIT) == 0 &&
> > > +   (mode & PTRACE_MODE_SCHED) == 0) {
> >
> > If you ORed PTRACE_MODE_NOAUDIT into the flags when calling the
> > security hook, you could drop this patch, right?
>
> Yes. Since the PTRACE_MODE_NOAUDIT was in PTRACE_MODE_IBPB
> in Jiri's previous patch set and not in PTRACE_MODE_SCHED in this one
> I assumed that there was a good reason for it.

Jiri, was there a good reason for it, and if so, what was it?
___
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.

RE: [PATCH v5 4/5] Capability: Complete PTRACE_MODE_SCHED

2018-09-27 Thread Schaufler, Casey 
> -Original Message-
> From: Jann Horn [mailto:ja...@google.com]
> Sent: Wednesday, September 26, 2018 2:26 PM
> To: Schaufler, Casey 
> Cc: Kernel Hardening ; kernel list
> ; linux-security-module  mod...@vger.kernel.org>; selinux@tycho.nsa.gov; Hansen, Dave
> ; Dock, Deneen T ;
> kris...@linux.intel.com; Arjan van de Ven 
> Subject: Re: [PATCH v5 4/5] Capability: Complete PTRACE_MODE_SCHED
> 
> On Wed, Sep 26, 2018 at 10:35 PM Casey Schaufler
>  wrote:
> > Allow a complete ptrace access check with mode PTRACE_MODE_SCHED.
> > Disable the inappropriate privilege check in the capability code
> > that does incompatible locking.
> 
> What's that locking you're talking about?

ns_capable() eventually gets you to an audit call. The audit code
is going to do the locking. Fortunately, the preceding cap_issubset()
is the check that we really need here.

> 
> > Signed-off-by: Casey Schaufler 
> > ---
> >  kernel/ptrace.c  | 2 --
> >  security/commoncap.c | 2 ++
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/kernel/ptrace.c b/kernel/ptrace.c
> > index 99cfddde6a55..0b6a9df51c3b 100644
> > --- a/kernel/ptrace.c
> > +++ b/kernel/ptrace.c
> > @@ -331,8 +331,6 @@ static int __ptrace_may_access(struct task_struct
> *task, unsigned int mode)
> >  !ptrace_has_cap(mm->user_ns, mode)))
> > return -EPERM;
> >
> > -   if (mode & PTRACE_MODE_SCHED)
> > -   return 0;
> > return security_ptrace_access_check(task, mode);
> >  }
> >
> > diff --git a/security/commoncap.c b/security/commoncap.c
> > index 2e489d6a3ac8..e77457110d05 100644
> > --- a/security/commoncap.c
> > +++ b/security/commoncap.c
> > @@ -152,6 +152,8 @@ int cap_ptrace_access_check(struct task_struct
> *child, unsigned int mode)
> > if (cred->user_ns == child_cred->user_ns &&
> > cap_issubset(child_cred->cap_permitted, *caller_caps))
> > goto out;
> > +   if (mode & PTRACE_MODE_SCHED)
> > +   goto out;
> 
> So for PTRACE_MODE_SCHED, this function always returns 0, right? 

That can't be right, can it? Determining that we have PTRACE_MODE_SCHED
at this point should result in -EPERM. I mucked up on the logic flow. The
next revision will fix this.

> If that's intentional, perhaps you should instead just put "if (mode &
> PTRACE_MODE_SCHED) return 0;" at the start of the function, to avoid
> taking the RCU read lock in this case.
> 
> > if (ns_capable(child_cred->user_ns, CAP_SYS_PTRACE))
> > goto out;
> > ret = -EPERM;

___
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.

[PATCH 21/19] LSM: Cleanup and fixes from Tetsuo Handa

2018-09-27 Thread Casey Schaufler 
lsm_early_cred()/lsm_early_task() are called from only __init functions.

lsm_cred_alloc()/lsm_file_alloc() are called from only security/security.c .

lsm_early_inode() should be avoided because it is not appropriate to
call panic() when lsm_early_inode() is called after __init phase.

Since all free hooks are called when one of init hooks failed, each
free hook needs to check whether init hook was called.

The original changes are from Tetsuo Handa. I have made minor
changes in some places, but this is mostly his code.

Signed-off-by: Casey Schaufler 
---
 include/linux/lsm_hooks.h |  6 ++
 security/security.c   | 27 ---
 security/selinux/hooks.c  |  5 -
 security/selinux/include/objsec.h |  2 ++
 security/smack/smack_lsm.c|  8 +++-
 5 files changed, 19 insertions(+), 29 deletions(-)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 7e8b32fdf576..80146147531f 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -2095,13 +2095,11 @@ void __init loadpin_add_hooks(void);
 static inline void loadpin_add_hooks(void) { };
 #endif
 
-extern int lsm_cred_alloc(struct cred *cred, gfp_t gfp);
 extern int lsm_inode_alloc(struct inode *inode);
 
 #ifdef CONFIG_SECURITY
-void lsm_early_cred(struct cred *cred);
-void lsm_early_inode(struct inode *inode);
-void lsm_early_task(struct task_struct *task);
+void __init lsm_early_cred(struct cred *cred);
+void __init lsm_early_task(struct task_struct *task);
 #endif
 
 #endif /* ! __LINUX_LSM_HOOKS_H */
diff --git a/security/security.c b/security/security.c
index 76f7dc49b63c..d986045dd4c0 100644
--- a/security/security.c
+++ b/security/security.c
@@ -267,7 +267,7 @@ EXPORT_SYMBOL(unregister_lsm_notifier);
  *
  * Returns 0, or -ENOMEM if memory can't be allocated.
  */
-int lsm_cred_alloc(struct cred *cred, gfp_t gfp)
+static int lsm_cred_alloc(struct cred *cred, gfp_t gfp)
 {
if (blob_sizes.lbs_cred == 0) {
cred->security = NULL;
@@ -286,7 +286,7 @@ int lsm_cred_alloc(struct cred *cred, gfp_t gfp)
  *
  * Allocate the cred blob for all the modules if it's not already there
  */
-void lsm_early_cred(struct cred *cred)
+void __init lsm_early_cred(struct cred *cred)
 {
int rc;
 
@@ -344,7 +344,7 @@ void __init security_add_blobs(struct lsm_blob_sizes 
*needed)
  *
  * Returns 0, or -ENOMEM if memory can't be allocated.
  */
-int lsm_file_alloc(struct file *file)
+static int lsm_file_alloc(struct file *file)
 {
if (!lsm_file_cache) {
file->f_security = NULL;
@@ -378,25 +378,6 @@ int lsm_inode_alloc(struct inode *inode)
return 0;
 }
 
-/**
- * lsm_early_inode - during initialization allocate a composite inode blob
- * @inode: the inode that needs a blob
- *
- * Allocate the inode blob for all the modules if it's not already there
- */
-void lsm_early_inode(struct inode *inode)
-{
-   int rc;
-
-   if (inode == NULL)
-   panic("%s: NULL inode.\n", __func__);
-   if (inode->i_security != NULL)
-   return;
-   rc = lsm_inode_alloc(inode);
-   if (rc)
-   panic("%s: Early inode alloc failed.\n", __func__);
-}
-
 /**
  * lsm_task_alloc - allocate a composite task blob
  * @task: the task that needs a blob
@@ -466,7 +447,7 @@ int lsm_msg_msg_alloc(struct msg_msg *mp)
  *
  * Allocate the task blob for all the modules if it's not already there
  */
-void lsm_early_task(struct task_struct *task)
+void __init lsm_early_task(struct task_struct *task)
 {
int rc;
 
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 44337d2349d9..e54b7dbac775 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -332,8 +332,11 @@ static struct inode_security_struct 
*backing_inode_security(struct dentry *dentr
 static void inode_free_security(struct inode *inode)
 {
struct inode_security_struct *isec = selinux_inode(inode);
-   struct superblock_security_struct *sbsec = inode->i_sb->s_security;
+   struct superblock_security_struct *sbsec;
 
+   if (!isec)
+   return;
+   sbsec = inode->i_sb->s_security;
/*
 * As not all inode security structures are in a list, we check for
 * empty list outside of the lock to make sure that we won't waste
diff --git a/security/selinux/include/objsec.h 
b/security/selinux/include/objsec.h
index ee4471213909..8231ae02560e 100644
--- a/security/selinux/include/objsec.h
+++ b/security/selinux/include/objsec.h
@@ -180,6 +180,8 @@ static inline struct inode_security_struct *selinux_inode(
const struct inode *inode)
 {
 #ifdef CONFIG_SECURITY_STACKING
+   if (unlikely(!inode->i_security))
+   return NULL;
return inode->i_security + selinux_blob_sizes.lbs_inode;
 #else
return inode->i_security;
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c

[PATCH v4 20/19] LSM: Correct file blob free empty blob check

2018-09-27 Thread Casey Schaufler 
Instead of checking if the kmem_cache for file blobs
has been initialized check if the blob is NULL. This
allows non-blob using modules to do other kinds of
clean up in the security_file_free hooks.

Signed-off-by: Casey Schaufler 
---
 security/security.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/security/security.c b/security/security.c
index e7c8506041f1..76f7dc49b63c 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1202,14 +1202,13 @@ void security_file_free(struct file *file)
 {
void *blob;
 
-   if (!lsm_file_cache)
-   return;
-
call_void_hook(file_free_security, file);
 
blob = file->f_security;
-   file->f_security = NULL;
-   kmem_cache_free(lsm_file_cache, blob);
+   if (blob) {
+   file->f_security = NULL;
+   kmem_cache_free(lsm_file_cache, blob);
+   }
 }
 
 int security_file_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
-- 
2.17.1


___
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.

RE: [PATCH v5 1/5] AppArmor: Prepare for PTRACE_MODE_SCHED

2018-09-27 Thread Schaufler, Casey 
> -Original Message-
> From: Jann Horn [mailto:ja...@google.com]
> Sent: Wednesday, September 26, 2018 2:19 PM
> To: Schaufler, Casey 
> Cc: Kernel Hardening ; kernel list
> ; linux-security-module  mod...@vger.kernel.org>; selinux@tycho.nsa.gov; Hansen, Dave
> ; Dock, Deneen T ;
> kris...@linux.intel.com; Arjan van de Ven 
> Subject: Re: [PATCH v5 1/5] AppArmor: Prepare for PTRACE_MODE_SCHED
> 
> On Wed, Sep 26, 2018 at 11:16 PM Jann Horn  wrote:
> >
> > On Wed, Sep 26, 2018 at 10:35 PM Casey Schaufler
> >  wrote:
> > > A ptrace access check with mode PTRACE_MODE_SCHED gets called
> > > from process switching code. This precludes the use of audit,
> > > as the locking is incompatible. Don't do audit in the PTRACE_MODE_SCHED
> > > case.
> >
> > Why is this separate from PTRACE_MODE_NOAUDIT? It looks like
> > apparmor_ptrace_access_check() currently ignores
> PTRACE_MODE_NOAUDIT.
> > Could you, instead of adding a new flag, fix the handling of
> > PTRACE_MODE_NOAUDIT?
> 
> Er, after looking at more of the series, I see that PTRACE_MODE_SCHED
> is necessary; but could you handle the "don't audit" part for AppArmor
> using PTRACE_MODE_NOAUDIT instead?

I could have done it a number of ways, but this seemed to maintain
the apparmor AA_PTRACE abstraction the best. If aa_may_ptrace didn't
eschew PTRACE_MODE in favor of AA_PTRACE no change to the interface
would have been required. I'm reluctant to change something like that.

> > > Signed-off-by: Casey Schaufler 
> > > ---
> > >  security/apparmor/domain.c  | 2 +-
> > >  security/apparmor/include/ipc.h | 2 +-
> > >  security/apparmor/ipc.c | 8 +---
> > >  security/apparmor/lsm.c | 5 +++--
> > >  4 files changed, 10 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
> > > index 08c88de0ffda..28300f4c3ef9 100644
> > > --- a/security/apparmor/domain.c
> > > +++ b/security/apparmor/domain.c
> > > @@ -77,7 +77,7 @@ static int may_change_ptraced_domain(struct
> aa_label *to_label,
> > > if (!tracer || unconfined(tracerl))
> > > goto out;
> > >
> > > -   error = aa_may_ptrace(tracerl, to_label, PTRACE_MODE_ATTACH);
> > > +   error = aa_may_ptrace(tracerl, to_label, PTRACE_MODE_ATTACH,
> true);
> > >
> > >  out:
> > > rcu_read_unlock();
> > > diff --git a/security/apparmor/include/ipc.h
> b/security/apparmor/include/ipc.h
> > > index 5ffc218d1e74..299d1c45fef0 100644
> > > --- a/security/apparmor/include/ipc.h
> > > +++ b/security/apparmor/include/ipc.h
> > > @@ -34,7 +34,7 @@ struct aa_profile;
> > > "xcpu xfsz vtalrm prof winch io pwr sys emt lost"
> > >
> > >  int aa_may_ptrace(struct aa_label *tracer, struct aa_label *tracee,
> > > - u32 request);
> > > + u32 request, bool audit);
> > >  int aa_may_signal(struct aa_label *sender, struct aa_label *target, int 
> > > sig);
> > >
> > >  #endif /* __AA_IPC_H */
> > > diff --git a/security/apparmor/ipc.c b/security/apparmor/ipc.c
> > > index 527ea1557120..9ed110afc822 100644
> > > --- a/security/apparmor/ipc.c
> > > +++ b/security/apparmor/ipc.c
> > > @@ -121,15 +121,17 @@ static int profile_tracer_perm(struct aa_profile
> *tracer,
> > >   * Returns: %0 else error code if permission denied or error
> > >   */
> > >  int aa_may_ptrace(struct aa_label *tracer, struct aa_label *tracee,
> > > - u32 request)
> > > + u32 request, bool audit)
> > >  {
> > > struct aa_profile *profile;
> > > u32 xrequest = request << PTRACE_PERM_SHIFT;
> > > DEFINE_AUDIT_DATA(sa, LSM_AUDIT_DATA_NONE, OP_PTRACE);
> > >
> > > return xcheck_labels(tracer, tracee, profile,
> > > -   profile_tracer_perm(profile, tracee, request, 
> > > ),
> > > -   profile_tracee_perm(profile, tracer, xrequest, 
> > > ));
> > > +   profile_tracer_perm(profile, tracee, request,
> > > +   audit ?  : NULL),
> > > +   profile_tracee_perm(profile, tracer, xrequest,
> > > +   audit ?  : NULL));
> > >  }
> > >
> > >
> > > diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> > > index 8b8b70620bbe..da9d0b228857 100644
> > > --- a/security/apparmor/lsm.c
> > > +++ b/security/apparmor/lsm.c
> > > @@ -118,7 +118,8 @@ static int apparmor_ptrace_access_check(struct
> task_struct *child,
> > > tracee = aa_get_task_label(child);
> > > error = aa_may_ptrace(tracer, tracee,
> > > (mode & PTRACE_MODE_READ) ? AA_PTRACE_READ
> > > - : AA_PTRACE_TRACE);
> > > + : AA_PTRACE_TRACE,
> > > +   !(mode & PTRACE_MODE_SCHED));
> > > aa_put_label(tracee);
> > > end_current_label_crit_section(tracer);
> > >
> > > @@ -132,7

[PATCH v5 4/5] Capability: Complete PTRACE_MODE_SCHED

2018-09-27 Thread Casey Schaufler 
From: Casey Schaufler 

Allow a complete ptrace access check with mode PTRACE_MODE_SCHED.
Disable the inappropriate privilege check in the capability code
that does incompatible locking.

Signed-off-by: Casey Schaufler 
---
 kernel/ptrace.c  | 2 --
 security/commoncap.c | 2 ++
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 99cfddde6a55..0b6a9df51c3b 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -331,8 +331,6 @@ static int __ptrace_may_access(struct task_struct *task, 
unsigned int mode)
 !ptrace_has_cap(mm->user_ns, mode)))
return -EPERM;
 
-   if (mode & PTRACE_MODE_SCHED)
-   return 0;
return security_ptrace_access_check(task, mode);
 }
 
diff --git a/security/commoncap.c b/security/commoncap.c
index 2e489d6a3ac8..e77457110d05 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -152,6 +152,8 @@ int cap_ptrace_access_check(struct task_struct *child, 
unsigned int mode)
if (cred->user_ns == child_cred->user_ns &&
cap_issubset(child_cred->cap_permitted, *caller_caps))
goto out;
+   if (mode & PTRACE_MODE_SCHED)
+   goto out;
if (ns_capable(child_cred->user_ns, CAP_SYS_PTRACE))
goto out;
ret = -EPERM;
-- 
2.17.1

___
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.

[PATCH v5 2/5] Smack: Prepare for PTRACE_MODE_SCHED

2018-09-27 Thread Casey Schaufler 
From: Casey Schaufler 

A ptrace access check with mode PTRACE_MODE_SCHED gets called
from process switching code. This precludes the use of audit,
as the locking is incompatible. Don't do audit in the PTRACE_MODE_SCHED
case.

Signed-off-by: Casey Schaufler 
---
 security/smack/smack_lsm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 340fc30ad85d..ffa95bcab599 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -422,7 +422,8 @@ static int smk_ptrace_rule_check(struct task_struct *tracer,
struct task_smack *tsp;
struct smack_known *tracer_known;
 
-   if ((mode & PTRACE_MODE_NOAUDIT) == 0) {
+   if ((mode & PTRACE_MODE_NOAUDIT) == 0 &&
+   (mode & PTRACE_MODE_SCHED) == 0) {
smk_ad_init(, func, LSM_AUDIT_DATA_TASK);
smk_ad_setfield_u_tsk(, tracer);
saip = 
-- 
2.17.1

___
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.

[PATCH v5 5/5] sidechannel: Linux Security Module for sidechannel

2018-09-27 Thread Casey Schaufler 
From: Casey Schaufler 

This is a new Linux Security Module (LSM) that checks for
potential sidechannel issues that are not covered in the
ptrace PTRACE_MODE_SCHED option. Namespace differences are
checked in this intitial version. Additional checks should
be added when they are determined to be useful.

Signed-off-by: Casey Schaufler 
---
 include/linux/lsm_hooks.h  |  5 ++
 security/Kconfig   |  1 +
 security/Makefile  |  2 +
 security/security.c|  1 +
 security/sidechannel/Kconfig   | 13 +
 security/sidechannel/Makefile  |  1 +
 security/sidechannel/sidechannel.c | 88 ++
 7 files changed, 111 insertions(+)
 create mode 100644 security/sidechannel/Kconfig
 create mode 100644 security/sidechannel/Makefile
 create mode 100644 security/sidechannel/sidechannel.c

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 97a020c616ad..3cb6516dba3c 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -2081,5 +2081,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 d9aa521b5206..6b814a3f93ea 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -236,6 +236,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 736e78da1ab9..2129b0e31d7b 100644
--- a/security/security.c
+++ b/security/security.c
@@ -83,6 +83,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 ..653033027415
--- /dev/null
+++ b/security/sidechannel/Kconfig
@@ -0,0 +1,13 @@
+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.
+
diff --git a/security/sidechannel/Makefile b/security/sidechannel/Makefile
new file mode 100644
index ..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 ..18a67d19c020
--- /dev/null
+++ b/security/sidechannel/sidechannel.c
@@ -0,0 +1,88 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Side Channel Safety Security Module
+ *
+ * Copyright (C) 2018 Intel Corporation.
+ *
+ */
+
+#define pr_fmt(fmt) "SideChannel: " fmt
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#ifdef CONFIG_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;
+
+   /*
+* Namespace checks. Considered safe if:
+*  cgroup namespace is the same
+*  User namespace is the same
+*  PID namespace is the same
+*/
+   if (current->nsproxy)
+

[PATCH v5 1/5] AppArmor: Prepare for PTRACE_MODE_SCHED

2018-09-27 Thread Casey Schaufler 
From: Casey Schaufler 

A ptrace access check with mode PTRACE_MODE_SCHED gets called
from process switching code. This precludes the use of audit,
as the locking is incompatible. Don't do audit in the PTRACE_MODE_SCHED
case.

Signed-off-by: Casey Schaufler 
---
 security/apparmor/domain.c  | 2 +-
 security/apparmor/include/ipc.h | 2 +-
 security/apparmor/ipc.c | 8 +---
 security/apparmor/lsm.c | 5 +++--
 4 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index 08c88de0ffda..28300f4c3ef9 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -77,7 +77,7 @@ static int may_change_ptraced_domain(struct aa_label 
*to_label,
if (!tracer || unconfined(tracerl))
goto out;
 
-   error = aa_may_ptrace(tracerl, to_label, PTRACE_MODE_ATTACH);
+   error = aa_may_ptrace(tracerl, to_label, PTRACE_MODE_ATTACH, true);
 
 out:
rcu_read_unlock();
diff --git a/security/apparmor/include/ipc.h b/security/apparmor/include/ipc.h
index 5ffc218d1e74..299d1c45fef0 100644
--- a/security/apparmor/include/ipc.h
+++ b/security/apparmor/include/ipc.h
@@ -34,7 +34,7 @@ struct aa_profile;
"xcpu xfsz vtalrm prof winch io pwr sys emt lost"
 
 int aa_may_ptrace(struct aa_label *tracer, struct aa_label *tracee,
- u32 request);
+ u32 request, bool audit);
 int aa_may_signal(struct aa_label *sender, struct aa_label *target, int sig);
 
 #endif /* __AA_IPC_H */
diff --git a/security/apparmor/ipc.c b/security/apparmor/ipc.c
index 527ea1557120..9ed110afc822 100644
--- a/security/apparmor/ipc.c
+++ b/security/apparmor/ipc.c
@@ -121,15 +121,17 @@ static int profile_tracer_perm(struct aa_profile *tracer,
  * Returns: %0 else error code if permission denied or error
  */
 int aa_may_ptrace(struct aa_label *tracer, struct aa_label *tracee,
- u32 request)
+ u32 request, bool audit)
 {
struct aa_profile *profile;
u32 xrequest = request << PTRACE_PERM_SHIFT;
DEFINE_AUDIT_DATA(sa, LSM_AUDIT_DATA_NONE, OP_PTRACE);
 
return xcheck_labels(tracer, tracee, profile,
-   profile_tracer_perm(profile, tracee, request, ),
-   profile_tracee_perm(profile, tracer, xrequest, ));
+   profile_tracer_perm(profile, tracee, request,
+   audit ?  : NULL),
+   profile_tracee_perm(profile, tracer, xrequest,
+   audit ?  : NULL));
 }
 
 
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 8b8b70620bbe..da9d0b228857 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -118,7 +118,8 @@ static int apparmor_ptrace_access_check(struct task_struct 
*child,
tracee = aa_get_task_label(child);
error = aa_may_ptrace(tracer, tracee,
(mode & PTRACE_MODE_READ) ? AA_PTRACE_READ
- : AA_PTRACE_TRACE);
+ : AA_PTRACE_TRACE,
+   !(mode & PTRACE_MODE_SCHED));
aa_put_label(tracee);
end_current_label_crit_section(tracer);
 
@@ -132,7 +133,7 @@ static int apparmor_ptrace_traceme(struct task_struct 
*parent)
 
tracee = begin_current_label_crit_section();
tracer = aa_get_task_label(parent);
-   error = aa_may_ptrace(tracer, tracee, AA_PTRACE_TRACE);
+   error = aa_may_ptrace(tracer, tracee, AA_PTRACE_TRACE, true);
aa_put_label(tracer);
end_current_label_crit_section(tracee);
 
-- 
2.17.1

___
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.

Re: [PATCH v5 1/5] AppArmor: Prepare for PTRACE_MODE_SCHED

2018-09-27 Thread Jann Horn via Selinux 
On Wed, Sep 26, 2018 at 10:35 PM Casey Schaufler
 wrote:
> A ptrace access check with mode PTRACE_MODE_SCHED gets called
> from process switching code. This precludes the use of audit,
> as the locking is incompatible. Don't do audit in the PTRACE_MODE_SCHED
> case.

Why is this separate from PTRACE_MODE_NOAUDIT? It looks like
apparmor_ptrace_access_check() currently ignores PTRACE_MODE_NOAUDIT.
Could you, instead of adding a new flag, fix the handling of
PTRACE_MODE_NOAUDIT?

> Signed-off-by: Casey Schaufler 
> ---
>  security/apparmor/domain.c  | 2 +-
>  security/apparmor/include/ipc.h | 2 +-
>  security/apparmor/ipc.c | 8 +---
>  security/apparmor/lsm.c | 5 +++--
>  4 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
> index 08c88de0ffda..28300f4c3ef9 100644
> --- a/security/apparmor/domain.c
> +++ b/security/apparmor/domain.c
> @@ -77,7 +77,7 @@ static int may_change_ptraced_domain(struct aa_label 
> *to_label,
> if (!tracer || unconfined(tracerl))
> goto out;
>
> -   error = aa_may_ptrace(tracerl, to_label, PTRACE_MODE_ATTACH);
> +   error = aa_may_ptrace(tracerl, to_label, PTRACE_MODE_ATTACH, true);
>
>  out:
> rcu_read_unlock();
> diff --git a/security/apparmor/include/ipc.h b/security/apparmor/include/ipc.h
> index 5ffc218d1e74..299d1c45fef0 100644
> --- a/security/apparmor/include/ipc.h
> +++ b/security/apparmor/include/ipc.h
> @@ -34,7 +34,7 @@ struct aa_profile;
> "xcpu xfsz vtalrm prof winch io pwr sys emt lost"
>
>  int aa_may_ptrace(struct aa_label *tracer, struct aa_label *tracee,
> - u32 request);
> + u32 request, bool audit);
>  int aa_may_signal(struct aa_label *sender, struct aa_label *target, int sig);
>
>  #endif /* __AA_IPC_H */
> diff --git a/security/apparmor/ipc.c b/security/apparmor/ipc.c
> index 527ea1557120..9ed110afc822 100644
> --- a/security/apparmor/ipc.c
> +++ b/security/apparmor/ipc.c
> @@ -121,15 +121,17 @@ static int profile_tracer_perm(struct aa_profile 
> *tracer,
>   * Returns: %0 else error code if permission denied or error
>   */
>  int aa_may_ptrace(struct aa_label *tracer, struct aa_label *tracee,
> - u32 request)
> + u32 request, bool audit)
>  {
> struct aa_profile *profile;
> u32 xrequest = request << PTRACE_PERM_SHIFT;
> DEFINE_AUDIT_DATA(sa, LSM_AUDIT_DATA_NONE, OP_PTRACE);
>
> return xcheck_labels(tracer, tracee, profile,
> -   profile_tracer_perm(profile, tracee, request, ),
> -   profile_tracee_perm(profile, tracer, xrequest, ));
> +   profile_tracer_perm(profile, tracee, request,
> +   audit ?  : NULL),
> +   profile_tracee_perm(profile, tracer, xrequest,
> +   audit ?  : NULL));
>  }
>
>
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index 8b8b70620bbe..da9d0b228857 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -118,7 +118,8 @@ static int apparmor_ptrace_access_check(struct 
> task_struct *child,
> tracee = aa_get_task_label(child);
> error = aa_may_ptrace(tracer, tracee,
> (mode & PTRACE_MODE_READ) ? AA_PTRACE_READ
> - : AA_PTRACE_TRACE);
> + : AA_PTRACE_TRACE,
> +   !(mode & PTRACE_MODE_SCHED));
> aa_put_label(tracee);
> end_current_label_crit_section(tracer);
>
> @@ -132,7 +133,7 @@ static int apparmor_ptrace_traceme(struct task_struct 
> *parent)
>
> tracee = begin_current_label_crit_section();
> tracer = aa_get_task_label(parent);
> -   error = aa_may_ptrace(tracer, tracee, AA_PTRACE_TRACE);
> +   error = aa_may_ptrace(tracer, tracee, AA_PTRACE_TRACE, true);
> aa_put_label(tracer);
> end_current_label_crit_section(tracee);
>
> --
> 2.17.1
>
___
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.

[PATCH v5 3/5] SELinux: Prepare for PTRACE_MODE_SCHED

2018-09-27 Thread Casey Schaufler 
From: Casey Schaufler 

A ptrace access check with mode PTRACE_MODE_SCHED gets called
from process switching code. This precludes the use of audit or avc,
as the locking is incompatible. The only available check that
can be made without using avc is a comparison of the secids.
This is not very satisfactory as it will indicate possible
vulnerabilies much too aggressively.

Signed-off-by: Casey Schaufler 
---
 security/selinux/hooks.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index ad9a9b8e9979..160239791007 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2267,6 +2267,8 @@ static int selinux_ptrace_access_check(struct task_struct 
*child,
u32 sid = current_sid();
u32 csid = task_sid(child);
 
+   if (mode & PTRACE_MODE_SCHED)
+   return sid == csid ? 0 : -EACCES;
if (mode & PTRACE_MODE_READ)
return avc_has_perm(_state,
sid, csid, SECCLASS_FILE, FILE__READ, NULL);
-- 
2.17.1

___
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.

[PATCH v5 0/5] LSM: Support ptrace sidechannel access checks

2018-09-27 Thread Casey Schaufler 
v5: Revamped to match Jiri Kosina 
Harden spectrev2 userspace-userspace protection v7
Fixed locking issues in the LSM code.
Dropped the new LSM hook and use a ptrace hook instead.
v4: select namespace checks if user namespaces are enabled
and credential checks are request.
v3: get_task_cred wasn't a good choice due to refcounts.
Use lower level protection instead
v2: SELinux access policy corrected.
Use real_cred instead of cred.

This patchset provide a mechanism by which a security module
can advise the system about potential side-channel vulnerabilities.
The existing security modules have been updated to avoid locking
issues in the face of PTRACE_MODE_SCHED. A new security
module is provided to make determinations regarding task attributes
including namespaces.

Signed-off-by: Casey Schaufler 
---
 include/linux/lsm_hooks.h  |  5 +++
 kernel/ptrace.c|  2 -
 security/Kconfig   |  1 +
 security/Makefile  |  2 +
 security/apparmor/domain.c |  2 +-
 security/apparmor/include/ipc.h|  2 +-
 security/apparmor/ipc.c|  8 ++--
 security/apparmor/lsm.c|  5 ++-
 security/commoncap.c   |  2 +
 security/security.c|  1 +
 security/selinux/hooks.c   |  2 +
 security/sidechannel/Kconfig   | 13 ++
 security/sidechannel/Makefile  |  1 +
 security/sidechannel/sidechannel.c | 88 ++
 security/smack/smack_lsm.c |  3 +-
 15 files changed, 127 insertions(+), 10 deletions(-)
___
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.