[PATCH RFC v2 5/5] SELinux: Support SELinux determination of side-channel vulnerability

-----Original Message-----
From: Stephen Smalley [mailto:sds at tycho.nsa.gov]
Sent: Monday, August 20, 2018 10:44 AM
To: Schaufler, Casey <redacted>; kernel-
hardening at lists.openwall.com; linux-kernel at vger.kernel.org; linux-security-
module at vger.kernel.org; selinux at tycho.nsa.gov; Hansen, Dave
[off-list ref]; Dock, Deneen T [off-list ref];
kristen at linux.intel.com; arjan at linux.intel.com
Subject: Re: [PATCH RFC v2 5/5] SELinux: Support SELinux determination of
side-channel vulnerability

On 08/20/2018 12:59 PM, Schaufler, Casey wrote:

quoted

quoted

-----Original Message-----
From: Stephen Smalley [mailto:sds at tycho.nsa.gov]
Sent: Monday, August 20, 2018 9:03 AM
To: Schaufler, Casey <redacted>; kernel-
hardening at lists.openwall.com; linux-kernel at vger.kernel.org; linux-security-
module at vger.kernel.org; selinux at tycho.nsa.gov; Hansen, Dave
[off-list ref]; Dock, Deneen T [off-list ref];
kristen at linux.intel.com; arjan at linux.intel.com
Subject: Re: [PATCH RFC v2 5/5] SELinux: Support SELinux determination of
side-channel vulnerability

On 08/17/2018 06:16 PM, Casey Schaufler wrote:

quoted

SELinux considers tasks to be side-channel safe if they
have PROCESS_SHARE access.

Now the description and the code no longer match.

You're right.

quoted

quoted

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

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index a8bf324130f5..7fbd7d7ac1cb 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c

@@ -4219,6 +4219,14 @@ static void selinux_task_to_inode(struct

task_struct *p,

quoted

   	spin_unlock(&isec->lock);
   }

+static int selinux_task_safe_sidechannel(struct task_struct *p)
+{
+	struct av_decision avd;
+
+	return avc_has_perm_noaudit(&selinux_state, current_sid(),

task_sid(p),

quoted

+				    SECCLASS_FILE, FILE__READ, 0, &avd);
+}

And my question from before still stands:  why do we need a new hook and
new security module instead of just using ptrace_may_access()?

Locking. The SELinux check, for example, will lock up solid while trying
to generate an audit record. There is no good reason aside from coding
convenience to assume that the same restrictions will apply for side-channel
as apply to ptrace. I'm actually a touch surprised you're not suggesting a
separate SECCLASS or access mode for the SELinux hook.

The PTRACE_MODE_NOAUDIT flag to ptrace_may_access() would address the
locking concern.

OK ...

Duplicating the ptrace access checking logic seems
prone to errors and inconsistencies.

That's true only if the ptrace logic and the safe-sidechannel logic
are identical. I don't believe that is a safe assumption. It would sure
be convenient. But I would hate to see a change made for either
ptrace or safe_sidechannel that interfered with the correct behavior
of the other.

I can't imagine policy writers
understanding what "safe sidechannel" means, much less deciding when to
allow it.

I can't argue with that. But then, I have always had trouble with the
SELinux policy scheme.