selinux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Casey Schaufler <casey@schaufler-ca.com>
To: Stephen Smalley <sds@tycho.nsa.gov>,
	Nicholas Franck <nhfran2@tycho.nsa.gov>,
	paul@paul-moore.com
Cc: selinux@vger.kernel.org, linux-security-module@vger.kernel.org,
	luto@amacapital.net, jmorris@namei.org, keescook@chromium.org,
	serge@hallyn.com, john.johansen@canonical.com,
	mortonm@chromium.org, casey@schaufler-ca.com
Subject: Re: [RFC PATCH] security, capability: pass object information to security_capable
Date: Fri, 12 Jul 2019 12:54:02 -0700	[thread overview]
Message-ID: <16cade37-9467-ca7f-ddea-b8254c501f48@schaufler-ca.com> (raw)
In-Reply-To: <e8de4a1c-7e18-fc20-e372-67bbaa93fd42@tycho.nsa.gov>

On 7/12/2019 11:25 AM, Stephen Smalley wrote:
> On 7/12/19 1:58 PM, Casey Schaufler wrote:
>> On 7/12/2019 10:34 AM, Nicholas Franck wrote:
>>> At present security_capable does not pass any object information
>>> and therefore can neither audit the particular object nor take it
>>> into account. Augment the security_capable interface to support
>>> passing supplementary data. Use this facility initially to convey
>>> the inode for capability checks relevant to inodes. This only
>>> addresses capable_wrt_inode_uidgid calls; other capability checks
>>> relevant to inodes will be addressed in subsequent changes. In the
>>> future, this will be further extended to pass object information for
>>> other capability checks such as the target task for CAP_KILL.
>>
>> This seems wrong to me. The capability system has nothing to do
>> with objects. Passing object information through security_capable()
>> may be convenient, but isn't relevant to the purpose of the interface.
>> It appears that there are very few places where the object information
>> is actually useful.
>
> A fair number of capabilities are checked upon some attempted object access (often right after comparing UIDs or other per-object state), and the particular object can be very helpful in both audit and in access control.  More below.

I'm not disagreeing with that. What I'm saying is that the capability
check interface is not the right place to pass that information. The
capability check has no use for the object information. I would much
rather see a security_pass_object_data() hook that gets called after
(or before) the security_capable() hook in the places where you want
the extra information.

>>> In SELinux this new information is leveraged here to include the inode
>>> in the audit message. In the future, it could also be used to perform
>>> a per inode capability checks.
>>
>> I suggest that you want a mechanism for adding the inode information
>> to the audit record instead.
>
> That is part of what we want, but not the entire picture.  But with respect to audit, the problem today is that one sees SELinux dac_read_search, dac_override, etc denials with no indication as to the particular file, which is unfortunate both from a security auditing perspective and from a policy development perspective.

I can see how that is a problem.

> The only option today to gain that information is by enabling system call audit and setting at least one audit filter so that the audit framework will collect that information and include it in the audit records that are emitted upon syscall exit after any such AVC denial.  Unfortunately, that is all disabled by default on most systems due to its associated performance overhead, and one doesn't even have the option of enabling it on some systems, e.g. Android devices.  And even when one can enable syscall audit, one must correlate the syscall audit record to the associated AVC record to identify the object rather than having the information directly included in the same record.

None of which gives any rationale for adding the information
to the capability check. Sure, it's in the right place, but there
is no object interaction with the capability call.

>> What would a "per inode" capability check be? Capability checks are
>> process checks, not object checks.
>
> Ideally it would be possible to scope dac_override and other capabilities to specific objects rather than having to allow it for all or none.

That would require a major overhaul of the capability scheme,
and you're going to get arguments from people like me about
whether or not that would be ideal. Besides, isn't that what
SELinux is all about, providing that sort of privilege granularity?

> Just because a process needs to override DAC on one file or one set of files is not a reason to allow it to do so on every file it can access at all.

That's an argument for privilege bracketing within the process.
Not something I recommend (the oft referenced sendmail problems
being but one example) but the only way to do it properly without
delving into path based controls.

> If we want to apply least privilege, then this is a desirable facility.

The capability mechanism is object agnostic by design.

> I understand that doesn't mesh with Smack's mental modelbut it would probably be useful to both SELinux and AppArmor, among others.

I'm perfectly happy to have the information transmitted.
I think a separate interface for doing so is appropriate.



  reply	other threads:[~2019-07-12 19:54 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-12 17:34 [RFC PATCH] security,capability: pass object information to security_capable Nicholas Franck
2019-07-12 17:50 ` James Morris
2019-07-12 18:02   ` [RFC PATCH] security, capability: " Stephen Smalley
2019-07-15 18:42     ` Richard Guy Briggs
2019-07-12 17:58 ` [RFC PATCH] security,capability: " Casey Schaufler
2019-07-12 18:25   ` [RFC PATCH] security, capability: " Stephen Smalley
2019-07-12 19:54     ` Casey Schaufler [this message]
2019-07-12 20:21       ` Stephen Smalley
2019-07-12 22:37         ` Casey Schaufler
2019-07-13  4:35         ` James Morris
2019-07-13 18:46           ` Casey Schaufler
2019-07-13  4:29       ` James Morris
2019-07-16 14:03       ` Serge E. Hallyn
2019-07-16 14:21         ` Andy Lutomirski
2019-07-16 15:03           ` Casey Schaufler
2019-07-16 15:08           ` Stephen Smalley
2019-07-16 14:43         ` Casey Schaufler
2019-07-24 20:12     ` Paul Moore
2019-07-16 14:16 ` [RFC PATCH] security,capability: " Serge E. Hallyn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=16cade37-9467-ca7f-ddea-b8254c501f48@schaufler-ca.com \
    --to=casey@schaufler-ca.com \
    --cc=jmorris@namei.org \
    --cc=john.johansen@canonical.com \
    --cc=keescook@chromium.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=mortonm@chromium.org \
    --cc=nhfran2@tycho.nsa.gov \
    --cc=paul@paul-moore.com \
    --cc=sds@tycho.nsa.gov \
    --cc=selinux@vger.kernel.org \
    --cc=serge@hallyn.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).