access(2) vs. SELinux

access(2) vs. SELinux

[Stephen Smalley]

Per [1], the fact that SELinux applies the same read/write/execute
checks in response to access(2) calls as it does for an actual open(2)
for read/write or execve(2) for execute leads to huge numbers of
spurious denials for applications such as nautilus that probe all files
being listed.  dontaudit'ing of these denials is viewed as unacceptable
as Dan wants to be able to see such denials upon the actual open(2) or
execve(2) attempts.

If access(2) only returned the result of the DAC access checks and e.g.
only checked getattr permission on access(2) calls (similar to stat(2)),
then the read/write/execute MAC denials would not happen, but that would
likely break the behavior of existing applications/libraries that use
access(2) to decide whether to follow a privileged code path or fall
back on an unprivileged code path (e.g. kerberos libraries).

Other suggested proposals included:
- Using the _noaudit interface to suppress auditing of the
read/write/execute checks when checking for access(2), although this
requires reworking the underlying implementation so that we can
distinguish access(2) from open(2) checking.  However, this could lead
to silent failure of applications with no way to obtain any audit
messages, and would provide an obvious way to probe for access without
triggering audit.

- Using a different class or set of permissions when checking for
access(2), likewise requiring reworking the underlying implementation so
that we can distinguish the checking from open(2).  Then one could
choose to use dontaudit rules to suppress the access(2) checking w/o
suppressing the open(2) checking.  However, this could present a major
consistency problem between what access(2) reports and what is actually
allowed upon open(2) or execve(2).

Thoughts?

[1] https://bugzilla.redhat.com/show_bug.cgi?id=495211


-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: access(2) vs. SELinux

[Eric Paris]

On Wed, 2009-04-15 at 11:51 -0400, Stephen Smalley wrote:
> - Using a different class or set of permissions when checking for
> access(2), likewise requiring reworking the underlying implementation so
> that we can distinguish the checking from open(2).  Then one could
> choose to use dontaudit rules to suppress the access(2) checking w/o
> suppressing the open(2) checking.  However, this could present a major
> consistency problem between what access(2) reports and what is actually
> allowed upon open(2) or execve(2).

Assuming I'm thinking about this the right way, to make this useful we
must keep the access and the open permissions in sync.  For every
file/read we MUST have an access/read etc.  Right?  So to solve Dan's
problem we'd need rules like:

allow my_app_t my_file_t:file read
allow my_app_t my_file_t:access read
dontaudit my_app_t my_file_t:access write

I'm worried that Dan would go wild adding
dontaudit my_app_t my_file_t:access *

Which would be just as bad for the end user experience as simply
dontauditing them in the kernel (which I won't do)  So if we go this
route, don't do that dan.


Now I could probably do some sort of fancy black magic in the kernel
whereby we actually check file/* for the permission but display access/*
for the audit message and access/* for the dontaudit rules, but that
seems very fragile and untenable.  Next is how audit2allow is going to
try to generate a policy with only access/*, which is quickly going to
fall down next on file/*

I sorta feel like the 'right' solution is going to be forcing userspace
to keep those 2 in sync.  I'm willing to put an in kernel check on
policy load that rejects policy when they are not in sync...   but it
feels like it should be an all userspace problem.  Best in my opinion
would be to make the userspace tool chain just create the access class
allow rules out of the file class allow rule, and vice versa.....

-Eric


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: access(2) vs. SELinux

[Stephen Smalley]

On Wed, 2009-04-15 at 12:41 -0400, Eric Paris wrote:
> On Wed, 2009-04-15 at 11:51 -0400, Stephen Smalley wrote:
> > - Using a different class or set of permissions when checking for
> > access(2), likewise requiring reworking the underlying implementation so
> > that we can distinguish the checking from open(2).  Then one could
> > choose to use dontaudit rules to suppress the access(2) checking w/o
> > suppressing the open(2) checking.  However, this could present a major
> > consistency problem between what access(2) reports and what is actually
> > allowed upon open(2) or execve(2).
> 
> Assuming I'm thinking about this the right way, to make this useful we
> must keep the access and the open permissions in sync.  For every
> file/read we MUST have an access/read etc.  Right?  So to solve Dan's
> problem we'd need rules like:
> 
> allow my_app_t my_file_t:file read
> allow my_app_t my_file_t:access read
> dontaudit my_app_t my_file_t:access write
> 
> I'm worried that Dan would go wild adding
> dontaudit my_app_t my_file_t:access *
> 
> Which would be just as bad for the end user experience as simply
> dontauditing them in the kernel (which I won't do)  So if we go this
> route, don't do that dan.
> 
> 
> Now I could probably do some sort of fancy black magic in the kernel
> whereby we actually check file/* for the permission but display access/*
> for the audit message and access/* for the dontaudit rules, but that
> seems very fragile and untenable.  Next is how audit2allow is going to
> try to generate a policy with only access/*, which is quickly going to
> fall down next on file/*
> 
> I sorta feel like the 'right' solution is going to be forcing userspace
> to keep those 2 in sync.  I'm willing to put an in kernel check on
> policy load that rejects policy when they are not in sync...   but it
> feels like it should be an all userspace problem.  Best in my opinion
> would be to make the userspace tool chain just create the access class
> allow rules out of the file class allow rule, and vice versa.....

That will generate a lot of policy bloat.  It would be far more compact
if we were to define new access_read, access_write, access_execute
permissions in each of the file classes, assuming we still have the
space, and forcibly set those bits whenever we set read, write, or
execute in the same access vector.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: access(2) vs. SELinux

[Stephen Smalley]

On Wed, 2009-04-15 at 16:13 -0400, Stephen Smalley wrote:
> On Wed, 2009-04-15 at 12:41 -0400, Eric Paris wrote:
> > On Wed, 2009-04-15 at 11:51 -0400, Stephen Smalley wrote:
> > > - Using a different class or set of permissions when checking for
> > > access(2), likewise requiring reworking the underlying implementation so
> > > that we can distinguish the checking from open(2).  Then one could
> > > choose to use dontaudit rules to suppress the access(2) checking w/o
> > > suppressing the open(2) checking.  However, this could present a major
> > > consistency problem between what access(2) reports and what is actually
> > > allowed upon open(2) or execve(2).
> > 
> > Assuming I'm thinking about this the right way, to make this useful we
> > must keep the access and the open permissions in sync.  For every
> > file/read we MUST have an access/read etc.  Right?  So to solve Dan's
> > problem we'd need rules like:
> > 
> > allow my_app_t my_file_t:file read
> > allow my_app_t my_file_t:access read
> > dontaudit my_app_t my_file_t:access write
> > 
> > I'm worried that Dan would go wild adding
> > dontaudit my_app_t my_file_t:access *
> > 
> > Which would be just as bad for the end user experience as simply
> > dontauditing them in the kernel (which I won't do)  So if we go this
> > route, don't do that dan.
> > 
> > 
> > Now I could probably do some sort of fancy black magic in the kernel
> > whereby we actually check file/* for the permission but display access/*
> > for the audit message and access/* for the dontaudit rules, but that
> > seems very fragile and untenable.  Next is how audit2allow is going to
> > try to generate a policy with only access/*, which is quickly going to
> > fall down next on file/*
> > 
> > I sorta feel like the 'right' solution is going to be forcing userspace
> > to keep those 2 in sync.  I'm willing to put an in kernel check on
> > policy load that rejects policy when they are not in sync...   but it
> > feels like it should be an all userspace problem.  Best in my opinion
> > would be to make the userspace tool chain just create the access class
> > allow rules out of the file class allow rule, and vice versa.....
> 
> That will generate a lot of policy bloat.  It would be far more compact
> if we were to define new access_read, access_write, access_execute
> permissions in each of the file classes, assuming we still have the
> space, and forcibly set those bits whenever we set read, write, or
> execute in the same access vector.

Note that migrating access(2) to new permissions also requires adding a
new policy capability flag to tell the kernel whether the policy expects
access(2) to be controlled via read/write/execute or via the new
permissions.  Otherwise, new kernels with old policies would always try
checking the new permissions, and these would either be always denied or
always allowed depending on the handle_unknown flag, thereby causing
access(2) to either always fail or always succeed (either of which could
break applications).

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: access(2) vs. SELinux

[Eric Paris]

On Thu, 2009-04-16 at 13:08 -0400, Stephen Smalley wrote:
> On Wed, 2009-04-15 at 16:13 -0400, Stephen Smalley wrote:
> > On Wed, 2009-04-15 at 12:41 -0400, Eric Paris wrote:
> > > On Wed, 2009-04-15 at 11:51 -0400, Stephen Smalley wrote:
> > > > - Using a different class or set of permissions when checking for
> > > > access(2), likewise requiring reworking the underlying implementation so
> > > > that we can distinguish the checking from open(2).  Then one could
> > > > choose to use dontaudit rules to suppress the access(2) checking w/o
> > > > suppressing the open(2) checking.  However, this could present a major
> > > > consistency problem between what access(2) reports and what is actually
> > > > allowed upon open(2) or execve(2).
> > > 
> > > Assuming I'm thinking about this the right way, to make this useful we
> > > must keep the access and the open permissions in sync.  For every
> > > file/read we MUST have an access/read etc.  Right?  So to solve Dan's
> > > problem we'd need rules like:
> > > 
> > > allow my_app_t my_file_t:file read
> > > allow my_app_t my_file_t:access read
> > > dontaudit my_app_t my_file_t:access write
> > > 
> > > I'm worried that Dan would go wild adding
> > > dontaudit my_app_t my_file_t:access *
> > > 
> > > Which would be just as bad for the end user experience as simply
> > > dontauditing them in the kernel (which I won't do)  So if we go this
> > > route, don't do that dan.
> > > 
> > > 
> > > Now I could probably do some sort of fancy black magic in the kernel
> > > whereby we actually check file/* for the permission but display access/*
> > > for the audit message and access/* for the dontaudit rules, but that
> > > seems very fragile and untenable.  Next is how audit2allow is going to
> > > try to generate a policy with only access/*, which is quickly going to
> > > fall down next on file/*
> > > 
> > > I sorta feel like the 'right' solution is going to be forcing userspace
> > > to keep those 2 in sync.  I'm willing to put an in kernel check on
> > > policy load that rejects policy when they are not in sync...   but it
> > > feels like it should be an all userspace problem.  Best in my opinion
> > > would be to make the userspace tool chain just create the access class
> > > allow rules out of the file class allow rule, and vice versa.....
> > 
> > That will generate a lot of policy bloat.  It would be far more compact
> > if we were to define new access_read, access_write, access_execute
> > permissions in each of the file classes, assuming we still have the
> > space, and forcibly set those bits whenever we set read, write, or
> > execute in the same access vector.
> 
> Note that migrating access(2) to new permissions also requires adding a
> new policy capability flag to tell the kernel whether the policy expects
> access(2) to be controlled via read/write/execute or via the new
> permissions.  Otherwise, new kernels with old policies would always try
> checking the new permissions, and these would either be always denied or
> always allowed depending on the handle_unknown flag, thereby causing
> access(2) to either always fail or always succeed (either of which could
> break applications).

Bah, implementation detail.  We could instead look to see if the
access_* bits were ever set in policy and if so set an in kernel flag
telling us to use those on access checks (but I'll probably use the
capability flag)  anyway...

He's some concerns/thoughts.

domain_t calls access(W_OK) on file_t.

This generates a denial

denied  { access_write } scontext=domain_t tcontext=file_t tclass=file

Which audit2allow will gladly turn into:

allow domain_t file_t:file { access_write }

Load that module and then, POP the next time

denied { write } scontext=domain_t tcontext=file_t tclass=file

So somewhere we need some sort of synchronization.

---

I suggested to Dan that audit2allow should should, given the above
access_write denial output:
allow domain_t file_t:file { write }

and the tool chain just automatically maps all write rule to both allow
bits for BOTH write and access_write.

---

We could also have the tool chain just run in both directions and leave
audit2allow alone.  Any rule with access_write would cause the tool
chain to add write and any rule with write would cause us to add
access_write.

>From the point of view of policy analysis I think think that makes
things worse.  Now the access_write rule really means something.  That's
fine with me, but I have a feeling the people who care about policy
analysis might not like that idea.....

-Eric


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: access(2) vs. SELinux

[Chad Sellers]

On 4/16/09 2:10 PM, "Eric Paris" <eparis@redhat.com> wrote:
> 
> He's some concerns/thoughts.
> 
> domain_t calls access(W_OK) on file_t.
> 
> This generates a denial
> 
> denied  { access_write } scontext=domain_t tcontext=file_t tclass=file
> 
> Which audit2allow will gladly turn into:
> 
> allow domain_t file_t:file { access_write }
> 
> Load that module and then, POP the next time
> 
> denied { write } scontext=domain_t tcontext=file_t tclass=file
> 
> So somewhere we need some sort of synchronization.
> 
> ---
> 
> I suggested to Dan that audit2allow should should, given the above
> access_write denial output:
> allow domain_t file_t:file { write }
> 
> and the tool chain just automatically maps all write rule to both allow
> bits for BOTH write and access_write.
> 
> ---
> 
> We could also have the tool chain just run in both directions and leave
> audit2allow alone.  Any rule with access_write would cause the tool
> chain to add write and any rule with write would cause us to add
> access_write.
> 
>> From the point of view of policy analysis I think think that makes
> things worse.  Now the access_write rule really means something.  That's
> fine with me, but I have a feeling the people who care about policy
> analysis might not like that idea.....
> 
I think these issues point to how inelegant this solution is. Relying on the
toolchain to special case this and modify all results is going to be
painful. Another example of this comes up when applying analysis tools to
the policy. SETools will now show rules in a binary policy that can only be
traced back to the source policy if you understand how the compiler mangling
works. Managed policy has allowed us to decrease the amount of policy
mangling we do, and this strategy seems to be going the other direction.

I'd like to put in a vote for having access call the _noaudit interface to
suppress the audit record. I agree with Alexey though that it would need to
have something in /selinux to toggle this on/off. Then, when debugging a
policy and not getting denials, you'd throw this switch when to the point of
disabling dontaudits. We could even have semodule -D toggle this as well, if
we really want to bake this into the toolchain somewhere. As far as probing
attacks go, systems with more stringent security requirements that want to
see all possible probing attacks could turn this flag on all the time.

Thanks,
Chad




--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: access(2) vs. SELinux

[Joshua Brindle]

Chad Sellers wrote:
> On 4/16/09 2:10 PM, "Eric Paris" <eparis@redhat.com> wrote:
>> He's some concerns/thoughts.
>>
>> domain_t calls access(W_OK) on file_t.
>>
>> This generates a denial
>>
>> denied  { access_write } scontext=domain_t tcontext=file_t tclass=file
>>
>> Which audit2allow will gladly turn into:
>>
>> allow domain_t file_t:file { access_write }
>>
>> Load that module and then, POP the next time
>>
>> denied { write } scontext=domain_t tcontext=file_t tclass=file
>>
>> So somewhere we need some sort of synchronization.
>>
>> ---
>>
>> I suggested to Dan that audit2allow should should, given the above
>> access_write denial output:
>> allow domain_t file_t:file { write }
>>
>> and the tool chain just automatically maps all write rule to both allow
>> bits for BOTH write and access_write.
>>
>> ---
>>
>> We could also have the tool chain just run in both directions and leave
>> audit2allow alone.  Any rule with access_write would cause the tool
>> chain to add write and any rule with write would cause us to add
>> access_write.
>>
>>> From the point of view of policy analysis I think think that makes
>> things worse.  Now the access_write rule really means something.  That's
>> fine with me, but I have a feeling the people who care about policy
>> analysis might not like that idea.....
>>
> I think these issues point to how inelegant this solution is. Relying on the
> toolchain to special case this and modify all results is going to be
> painful. Another example of this comes up when applying analysis tools to
> the policy. SETools will now show rules in a binary policy that can only be
> traced back to the source policy if you understand how the compiler mangling
> works. Managed policy has allowed us to decrease the amount of policy
> mangling we do, and this strategy seems to be going the other direction.
> 
> I'd like to put in a vote for having access call the _noaudit interface to
> suppress the audit record. I agree with Alexey though that it would need to
> have something in /selinux to toggle this on/off. Then, when debugging a
> policy and not getting denials, you'd throw this switch when to the point of
> disabling dontaudits. We could even have semodule -D toggle this as well, if
> we really want to bake this into the toolchain somewhere. As far as probing
> attacks go, systems with more stringent security requirements that want to
> see all possible probing attacks could turn this flag on all the time.
> 
> 

I agree with this, Eric's description of what needs to happen above makes me 
cringe. There is very limited utility to having 2 parallel sets of permissions 
that need to be synchronized by the toolchain and quite a bit of potential pain.

One example of horrific pain would be if both the access_write and write rules 
were present in policy, and someone wanted to remove the write access. They do 
the necessary searching and find where write is granted, remove it and viola - 
nothing happens, write access is still granted. Very confusing, I could see even 
experienced policy writers getting bitten by this enough to curse the day these 
permissions were added.

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: access(2) vs. SELinux

[Eric Paris]

On Mon, 2009-04-20 at 10:35 -0400, Joshua Brindle wrote:
> Chad Sellers wrote:
> > On 4/16/09 2:10 PM, "Eric Paris" <eparis@redhat.com> wrote:
> >> He's some concerns/thoughts.
> >>
> >> domain_t calls access(W_OK) on file_t.
> >>
> >> This generates a denial
> >>
> >> denied  { access_write } scontext=domain_t tcontext=file_t tclass=file
> >>
> >> Which audit2allow will gladly turn into:
> >>
> >> allow domain_t file_t:file { access_write }
> >>
> >> Load that module and then, POP the next time
> >>
> >> denied { write } scontext=domain_t tcontext=file_t tclass=file
> >>
> >> So somewhere we need some sort of synchronization.
> >>
> >> ---
> >>
> >> I suggested to Dan that audit2allow should should, given the above
> >> access_write denial output:
> >> allow domain_t file_t:file { write }
> >>
> >> and the tool chain just automatically maps all write rule to both allow
> >> bits for BOTH write and access_write.
> >>
> >> ---
> >>
> >> We could also have the tool chain just run in both directions and leave
> >> audit2allow alone.  Any rule with access_write would cause the tool
> >> chain to add write and any rule with write would cause us to add
> >> access_write.
> >>
> >>> From the point of view of policy analysis I think think that makes
> >> things worse.  Now the access_write rule really means something.  That's
> >> fine with me, but I have a feeling the people who care about policy
> >> analysis might not like that idea.....
> >>
> > I think these issues point to how inelegant this solution is. Relying on the
> > toolchain to special case this and modify all results is going to be
> > painful. Another example of this comes up when applying analysis tools to
> > the policy. SETools will now show rules in a binary policy that can only be
> > traced back to the source policy if you understand how the compiler mangling
> > works. Managed policy has allowed us to decrease the amount of policy
> > mangling we do, and this strategy seems to be going the other direction.
> > 
> > I'd like to put in a vote for having access call the _noaudit interface to
> > suppress the audit record. I agree with Alexey though that it would need to
> > have something in /selinux to toggle this on/off. Then, when debugging a
> > policy and not getting denials, you'd throw this switch when to the point of
> > disabling dontaudits. We could even have semodule -D toggle this as well, if
> > we really want to bake this into the toolchain somewhere. As far as probing
> > attacks go, systems with more stringent security requirements that want to
> > see all possible probing attacks could turn this flag on all the time.
> > 
> > 
> 
> I agree with this, Eric's description of what needs to happen above makes me 
> cringe. There is very limited utility to having 2 parallel sets of permissions 
> that need to be synchronized by the toolchain and quite a bit of potential pain.
> 
> One example of horrific pain would be if both the access_write and write rules 
> were present in policy, and someone wanted to remove the write access. They do 
> the necessary searching and find where write is granted, remove it and viola - 
> nothing happens, write access is still granted. Very confusing, I could see even 
> experienced policy writers getting bitten by this enough to curse the day these 
> permissions were added.

I strongly and vehemently oppose any solution that is a blanket
dontaudit on access calls, even if there is a flag to dontdonaudit.
This might be fine in "secure" shops where everyone understands and is
willing to suffer some extra SELinux pain but not here.  If SELinux gets
in the way it better scream to high heavens for my customers.

I agree that having bi-directional synchronization is a difficult idea.
That's why I suggested audit2allow never output access_* permissions,
and I can make the tool chain reject any allow rules that include
access_* permissions (even with a nice message telling you to not use
access_* in allow rules).

But maybe people would like a more kernel centric solution?  I don't,
because i think the userspace tool chain is the right place to do this.
And it'll be ugly as @#$^ but I'm sure I could fine some atrocious way
to get the kernel to check permission on "read" output denials that say
"read" but on access() call it would check dontaudit rules for
"access_read"     allow or auditallow rules with any of the access_*
permissions would be meaningless.  Something tells me sds and jmorris
are going to hate the level of encapsulation and layering violations
this is going to take....

I pretty strongly prefer a one way synch from "read" -> "access_read"
when going from the text to a binary policy and having the tool chain
reject any rules with access_* set in other types of rules.  But I think
audit2allow/why should automagically convert the other way for easy of
use.

Doesn't make policy analysis any harder since access_* basically means
nothing at all, solves the problem of us having to dontaudit
hot/dangerous code paths, doesn't screw up policy writers since you
can't have both types of permissions in the text version of policy....

Does it seem too fragile?

-Eric


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: access(2) vs. SELinux

[Joshua Brindle]

Eric Paris wrote:
> On Mon, 2009-04-20 at 10:35 -0400, Joshua Brindle wrote:
>> Chad Sellers wrote:
>>> On 4/16/09 2:10 PM, "Eric Paris" <eparis@redhat.com> wrote:
>>>> He's some concerns/thoughts.
>>>>
>>>> domain_t calls access(W_OK) on file_t.
>>>>
>>>> This generates a denial
>>>>
>>>> denied  { access_write } scontext=domain_t tcontext=file_t tclass=file
>>>>
>>>> Which audit2allow will gladly turn into:
>>>>
>>>> allow domain_t file_t:file { access_write }
>>>>
>>>> Load that module and then, POP the next time
>>>>
>>>> denied { write } scontext=domain_t tcontext=file_t tclass=file
>>>>
>>>> So somewhere we need some sort of synchronization.
>>>>
>>>> ---
>>>>
>>>> I suggested to Dan that audit2allow should should, given the above
>>>> access_write denial output:
>>>> allow domain_t file_t:file { write }
>>>>
>>>> and the tool chain just automatically maps all write rule to both allow
>>>> bits for BOTH write and access_write.
>>>>
>>>> ---
>>>>
>>>> We could also have the tool chain just run in both directions and leave
>>>> audit2allow alone.  Any rule with access_write would cause the tool
>>>> chain to add write and any rule with write would cause us to add
>>>> access_write.
>>>>
>>>>> From the point of view of policy analysis I think think that makes
>>>> things worse.  Now the access_write rule really means something.  That's
>>>> fine with me, but I have a feeling the people who care about policy
>>>> analysis might not like that idea.....
>>>>
>>> I think these issues point to how inelegant this solution is. Relying on the
>>> toolchain to special case this and modify all results is going to be
>>> painful. Another example of this comes up when applying analysis tools to
>>> the policy. SETools will now show rules in a binary policy that can only be
>>> traced back to the source policy if you understand how the compiler mangling
>>> works. Managed policy has allowed us to decrease the amount of policy
>>> mangling we do, and this strategy seems to be going the other direction.
>>>
>>> I'd like to put in a vote for having access call the _noaudit interface to
>>> suppress the audit record. I agree with Alexey though that it would need to
>>> have something in /selinux to toggle this on/off. Then, when debugging a
>>> policy and not getting denials, you'd throw this switch when to the point of
>>> disabling dontaudits. We could even have semodule -D toggle this as well, if
>>> we really want to bake this into the toolchain somewhere. As far as probing
>>> attacks go, systems with more stringent security requirements that want to
>>> see all possible probing attacks could turn this flag on all the time.
>>>
>>>
>> I agree with this, Eric's description of what needs to happen above makes me 
>> cringe. There is very limited utility to having 2 parallel sets of permissions 
>> that need to be synchronized by the toolchain and quite a bit of potential pain.
>>
>> One example of horrific pain would be if both the access_write and write rules 
>> were present in policy, and someone wanted to remove the write access. They do 
>> the necessary searching and find where write is granted, remove it and viola - 
>> nothing happens, write access is still granted. Very confusing, I could see even 
>> experienced policy writers getting bitten by this enough to curse the day these 
>> permissions were added.
> 
> I strongly and vehemently oppose any solution that is a blanket
> dontaudit on access calls, even if there is a flag to dontdonaudit.
> This might be fine in "secure" shops where everyone understands and is
> willing to suffer some extra SELinux pain but not here.  If SELinux gets
> in the way it better scream to high heavens for my customers.
> 
> I agree that having bi-directional synchronization is a difficult idea.
> That's why I suggested audit2allow never output access_* permissions,
> and I can make the tool chain reject any allow rules that include
> access_* permissions (even with a nice message telling you to not use
> access_* in allow rules).
> 
> But maybe people would like a more kernel centric solution?  I don't,
> because i think the userspace tool chain is the right place to do this.
> And it'll be ugly as @#$^ but I'm sure I could fine some atrocious way
> to get the kernel to check permission on "read" output denials that say
> "read" but on access() call it would check dontaudit rules for
> "access_read"     allow or auditallow rules with any of the access_*
> permissions would be meaningless.  Something tells me sds and jmorris
> are going to hate the level of encapsulation and layering violations
> this is going to take....
> 
> I pretty strongly prefer a one way synch from "read" -> "access_read"
> when going from the text to a binary policy and having the tool chain
> reject any rules with access_* set in other types of rules.  But I think
> audit2allow/why should automagically convert the other way for easy of
> use.
> 
> Doesn't make policy analysis any harder since access_* basically means
> nothing at all, solves the problem of us having to dontaudit
> hot/dangerous code paths, doesn't screw up policy writers since you
> can't have both types of permissions in the text version of policy....
> 
> Does it seem too fragile?
> 

It is an awkward abstraction and we are terrible at making leaky abstractions 
that confuse and harm. Even if audit2allow does the correct thing (transparently 
converts access_X to X) what about people writing policy manually? What about 
tools that search for permissions in refpolicy interfaces and try to map denials 
to interfaces? Is it really a good idea to push this abstraction onto everyone 
doing any kind of selinux work in userspace? We already have enough problems 
with people understanding policy, most people don't even understand the divide 
between refpolicy and the checkpolicy language.

I'm fairly unfamiliar with the audit subsystem in the kernel so this might be 
completely impossible but how hard would it be to add a field to the denial that 
specifies the original syscall used to generate a denial, that way tools can 
ignore things coming from uninteresting syscalls?

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: access(2) vs. SELinux

[Eric Paris]

On Mon, 2009-04-20 at 11:30 -0400, Joshua Brindle wrote:
> Eric Paris wrote:
> > On Mon, 2009-04-20 at 10:35 -0400, Joshua Brindle wrote:
> >> Chad Sellers wrote:
> >>> On 4/16/09 2:10 PM, "Eric Paris" <eparis@redhat.com> wrote:
> >>>> He's some concerns/thoughts.
> >>>>
> >>>> domain_t calls access(W_OK) on file_t.
> >>>>
> >>>> This generates a denial
> >>>>
> >>>> denied  { access_write } scontext=domain_t tcontext=file_t tclass=file
> >>>>
> >>>> Which audit2allow will gladly turn into:
> >>>>
> >>>> allow domain_t file_t:file { access_write }
> >>>>
> >>>> Load that module and then, POP the next time
> >>>>
> >>>> denied { write } scontext=domain_t tcontext=file_t tclass=file
> >>>>
> >>>> So somewhere we need some sort of synchronization.
> >>>>
> >>>> ---
> >>>>
> >>>> I suggested to Dan that audit2allow should should, given the above
> >>>> access_write denial output:
> >>>> allow domain_t file_t:file { write }
> >>>>
> >>>> and the tool chain just automatically maps all write rule to both allow
> >>>> bits for BOTH write and access_write.
> >>>>
> >>>> ---
> >>>>
> >>>> We could also have the tool chain just run in both directions and leave
> >>>> audit2allow alone.  Any rule with access_write would cause the tool
> >>>> chain to add write and any rule with write would cause us to add
> >>>> access_write.
> >>>>
> >>>>> From the point of view of policy analysis I think think that makes
> >>>> things worse.  Now the access_write rule really means something.  That's
> >>>> fine with me, but I have a feeling the people who care about policy
> >>>> analysis might not like that idea.....
> >>>>
> >>> I think these issues point to how inelegant this solution is. Relying on the
> >>> toolchain to special case this and modify all results is going to be
> >>> painful. Another example of this comes up when applying analysis tools to
> >>> the policy. SETools will now show rules in a binary policy that can only be
> >>> traced back to the source policy if you understand how the compiler mangling
> >>> works. Managed policy has allowed us to decrease the amount of policy
> >>> mangling we do, and this strategy seems to be going the other direction.
> >>>
> >>> I'd like to put in a vote for having access call the _noaudit interface to
> >>> suppress the audit record. I agree with Alexey though that it would need to
> >>> have something in /selinux to toggle this on/off. Then, when debugging a
> >>> policy and not getting denials, you'd throw this switch when to the point of
> >>> disabling dontaudits. We could even have semodule -D toggle this as well, if
> >>> we really want to bake this into the toolchain somewhere. As far as probing
> >>> attacks go, systems with more stringent security requirements that want to
> >>> see all possible probing attacks could turn this flag on all the time.
> >>>
> >>>
> >> I agree with this, Eric's description of what needs to happen above makes me 
> >> cringe. There is very limited utility to having 2 parallel sets of permissions 
> >> that need to be synchronized by the toolchain and quite a bit of potential pain.
> >>
> >> One example of horrific pain would be if both the access_write and write rules 
> >> were present in policy, and someone wanted to remove the write access. They do 
> >> the necessary searching and find where write is granted, remove it and viola - 
> >> nothing happens, write access is still granted. Very confusing, I could see even 
> >> experienced policy writers getting bitten by this enough to curse the day these 
> >> permissions were added.
> > 
> > I strongly and vehemently oppose any solution that is a blanket
> > dontaudit on access calls, even if there is a flag to dontdonaudit.
> > This might be fine in "secure" shops where everyone understands and is
> > willing to suffer some extra SELinux pain but not here.  If SELinux gets
> > in the way it better scream to high heavens for my customers.
> > 
> > I agree that having bi-directional synchronization is a difficult idea.
> > That's why I suggested audit2allow never output access_* permissions,
> > and I can make the tool chain reject any allow rules that include
> > access_* permissions (even with a nice message telling you to not use
> > access_* in allow rules).
> > 
> > But maybe people would like a more kernel centric solution?  I don't,
> > because i think the userspace tool chain is the right place to do this.
> > And it'll be ugly as @#$^ but I'm sure I could fine some atrocious way
> > to get the kernel to check permission on "read" output denials that say
> > "read" but on access() call it would check dontaudit rules for
> > "access_read"     allow or auditallow rules with any of the access_*
> > permissions would be meaningless.  Something tells me sds and jmorris
> > are going to hate the level of encapsulation and layering violations
> > this is going to take....
> > 
> > I pretty strongly prefer a one way synch from "read" -> "access_read"
> > when going from the text to a binary policy and having the tool chain
> > reject any rules with access_* set in other types of rules.  But I think
> > audit2allow/why should automagically convert the other way for easy of
> > use.
> > 
> > Doesn't make policy analysis any harder since access_* basically means
> > nothing at all, solves the problem of us having to dontaudit
> > hot/dangerous code paths, doesn't screw up policy writers since you
> > can't have both types of permissions in the text version of policy....
> > 
> > Does it seem too fragile?
> > 
> 
> It is an awkward abstraction and we are terrible at making leaky abstractions 
> that confuse and harm. Even if audit2allow does the correct thing (transparently 
> converts access_X to X) what about people writing policy manually? What about 
> tools that search for permissions in refpolicy interfaces and try to map denials 
> to interfaces? Is it really a good idea to push this abstraction onto everyone 
> doing any kind of selinux work in userspace?

I admit that people/everything knowing a denial for access_read would
require an allow for "read" is a pain, but you would only go down the
wrong path until you tried to compile and the tool chain told you that
you got the wrong permission....

>  We already have enough problems 
> with people understanding policy, most people don't even understand the divide 
> between refpolicy and the checkpolicy language.

I wouldn't expect anyone to understand that the checkpolicy language
included new rules they didn't intend.  It would be all behind the
scenes.  And the tools would complain if the user tried to create those
rules themselves...

> I'm fairly unfamiliar with the audit subsystem in the kernel so this might be 
> completely impossible but how hard would it be to add a field to the denial that 
> specifies the original syscall used to generate a denial, that way tools can 
> ignore things coming from uninteresting syscalls?

Yes, I guess that 99% of the time in real systems we have all this
information in userspace.  We don't generally have it in kernel.  So
we'd be writing denials in audit.log but the tools wouldn't be showing
them to people.  That'd be fine, but we're filling disks for no good
reason.

I'm going to spend a couple minutes and try to prototype an all in
kernel solution such that on access() we will check the permission:

allow process_t file_t:file read

if there is a dontaudit rule for EITHER "read" or "access_read" we will
NOT print an avc denial.

on the actual usage we will ONLY check for the "read" dontaudit.


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: access(2) vs. SELinux

[Chad Sellers]

On 4/20/09 1:21 PM, "Eric Paris" <eparis@redhat.com> wrote:
<snip>
> 
> I'm going to spend a couple minutes and try to prototype an all in
> kernel solution such that on access() we will check the permission:
> 
> allow process_t file_t:file read
> 
> if there is a dontaudit rule for EITHER "read" or "access_read" we will
> NOT print an avc denial.
> 
> on the actual usage we will ONLY check for the "read" dontaudit.
> 
That sounds like a better solution than anything we've talked about so far.
It would mean the policy author would only have to worry about the access_*
perms if they cared about the issue they're designed to solve. Hopefully you
can find an acceptable way to pull it off in the kernel. I'm rooting for
you!

Chad


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

[RFC PATCH] access(2) vs. SELinux

[Eric Paris]

On Mon, 2009-04-20 at 13:35 -0400, Chad Sellers wrote:
> On 4/20/09 1:21 PM, "Eric Paris" <eparis@redhat.com> wrote:
> <snip>
> > 
> > I'm going to spend a couple minutes and try to prototype an all in
> > kernel solution such that on access() we will check the permission:
> > 
> > allow process_t file_t:file read
> > 
> > if there is a dontaudit rule for EITHER "read" or "access_read" we will
> > NOT print an avc denial.
> > 
> > on the actual usage we will ONLY check for the "read" dontaudit.
> > 
> That sounds like a better solution than anything we've talked about so far.
> It would mean the policy author would only have to worry about the access_*
> perms if they cared about the issue they're designed to solve. Hopefully you
> can find an acceptable way to pull it off in the kernel. I'm rooting for
> you!

So James, Stephen what do we think of something like this?  Lot of code
bloat and pain on the access+denial path, but shouldn't hurt us any in
the hot case...

---

 fs/namei.c                                |   37 +++++++----
 fs/open.c                                 |    2 
 include/linux/fs.h                        |    1 
 include/linux/security.h                  |    7 +-
 security/capability.c                     |    2 
 security/security.c                       |    4 -
 security/selinux/hooks.c                  |  100 ++++++++++++++++++++++++++----
 security/selinux/include/av_permissions.h |    6 +
 security/smack/smack_lsm.c                |    2 
 9 files changed, 128 insertions(+), 33 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index b8433eb..5a5eeab 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -228,17 +228,7 @@ int generic_permission(struct inode *inode, int mask,
 	return -EACCES;
 }
 
-/**
- * inode_permission  -  check for access rights to a given inode
- * @inode:	inode to check permission on
- * @mask:	right to check for (%MAY_READ, %MAY_WRITE, %MAY_EXEC)
- *
- * Used to check for read/write/execute permissions on an inode.
- * We use "fsuid" for this, letting us set arbitrary permissions
- * for filesystem access without changing the "normal" uids which
- * are used for other things.
- */
-int inode_permission(struct inode *inode, int mask)
+static int __inode_permission(struct inode *inode, int mask, bool from_access)
 {
 	int retval;
 
@@ -272,7 +262,28 @@ int inode_permission(struct inode *inode, int mask)
 		return retval;
 
 	return security_inode_permission(inode,
-			mask & (MAY_READ|MAY_WRITE|MAY_EXEC|MAY_APPEND));
+			mask & (MAY_READ|MAY_WRITE|MAY_EXEC|MAY_APPEND),
+			from_access);
+}
+
+int inode_access_permission(struct inode *inode, int mask)
+{
+	return __inode_permission(inode, mask, 1);
+}
+
+/**
+ * inode_permission  -  check for access rights to a given inode
+ * @inode:	inode to check permission on
+ * @mask:	right to check for (%MAY_READ, %MAY_WRITE, %MAY_EXEC)
+ *
+ * Used to check for read/write/execute permissions on an inode.
+ * We use "fsuid" for this, letting us set arbitrary permissions
+ * for filesystem access without changing the "normal" uids which
+ * are used for other things.
+ */
+int inode_permission(struct inode *inode, int mask)
+{
+	return __inode_permission(inode, mask, 0);
 }
 
 /**
@@ -456,7 +467,7 @@ static int exec_permission_lite(struct inode *inode)
 
 	return -EACCES;
 ok:
-	return security_inode_permission(inode, MAY_EXEC);
+	return security_inode_permission(inode, MAY_EXEC, 0);
 }
 
 /*
diff --git a/fs/open.c b/fs/open.c
index 377eb25..485cfd8 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -493,7 +493,7 @@ SYSCALL_DEFINE3(faccessat, int, dfd, const char __user *, filename, int, mode)
 			goto out_path_release;
 	}
 
-	res = inode_permission(inode, mode | MAY_ACCESS);
+	res = inode_access_permission(inode, mode | MAY_ACCESS);
 	/* SuS v2 requires we report a read only fs too */
 	if (res || !(mode & S_IWOTH) || special_file(inode->i_mode))
 		goto out_path_release;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index e766be0..0ac6e6b 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2091,6 +2091,7 @@ extern int do_remount_sb(struct super_block *sb, int flags,
 extern sector_t bmap(struct inode *, sector_t);
 #endif
 extern int notify_change(struct dentry *, struct iattr *);
+extern int inode_access_permission(struct inode *, int);
 extern int inode_permission(struct inode *, int);
 extern int generic_permission(struct inode *, int,
 		int (*check_acl)(struct inode *, int));
diff --git a/include/linux/security.h b/include/linux/security.h
index d5fd616..98a0ac7 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1423,7 +1423,7 @@ struct security_operations {
 			     struct inode *new_dir, struct dentry *new_dentry);
 	int (*inode_readlink) (struct dentry *dentry);
 	int (*inode_follow_link) (struct dentry *dentry, struct nameidata *nd);
-	int (*inode_permission) (struct inode *inode, int mask);
+	int (*inode_permission) (struct inode *inode, int mask, bool from_access);
 	int (*inode_setattr)	(struct dentry *dentry, struct iattr *attr);
 	int (*inode_getattr) (struct vfsmount *mnt, struct dentry *dentry);
 	void (*inode_delete) (struct inode *inode);
@@ -1682,7 +1682,7 @@ int security_inode_rename(struct inode *old_dir, struct dentry *old_dentry,
 			  struct inode *new_dir, struct dentry *new_dentry);
 int security_inode_readlink(struct dentry *dentry);
 int security_inode_follow_link(struct dentry *dentry, struct nameidata *nd);
-int security_inode_permission(struct inode *inode, int mask);
+int security_inode_permission(struct inode *inode, int mask, bool from_access);
 int security_inode_setattr(struct dentry *dentry, struct iattr *attr);
 int security_inode_getattr(struct vfsmount *mnt, struct dentry *dentry);
 void security_inode_delete(struct inode *inode);
@@ -2095,7 +2095,8 @@ static inline int security_inode_follow_link(struct dentry *dentry,
 	return 0;
 }
 
-static inline int security_inode_permission(struct inode *inode, int mask)
+static inline int security_inode_permission(struct inode *inode, int mask,
+					    bool from_access)
 {
 	return 0;
 }
diff --git a/security/capability.c b/security/capability.c
index 21b6cea..672d87a 100644
--- a/security/capability.c
+++ b/security/capability.c
@@ -206,7 +206,7 @@ static int cap_inode_follow_link(struct dentry *dentry,
 	return 0;
 }
 
-static int cap_inode_permission(struct inode *inode, int mask)
+static int cap_inode_permission(struct inode *inode, int mask, bool from_access)
 {
 	return 0;
 }
diff --git a/security/security.c b/security/security.c
index 5284255..19e2d41 100644
--- a/security/security.c
+++ b/security/security.c
@@ -516,11 +516,11 @@ int security_inode_follow_link(struct dentry *dentry, struct nameidata *nd)
 	return security_ops->inode_follow_link(dentry, nd);
 }
 
-int security_inode_permission(struct inode *inode, int mask)
+int security_inode_permission(struct inode *inode, int mask, bool from_access)
 {
 	if (unlikely(IS_PRIVATE(inode)))
 		return 0;
-	return security_ops->inode_permission(inode, mask);
+	return security_ops->inode_permission(inode, mask, from_access);
 }
 
 int security_inode_setattr(struct dentry *dentry, struct iattr *attr)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index ba808ef..e791970 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -1518,23 +1518,93 @@ static int task_has_system(struct task_struct *tsk,
 			    SECCLASS_SYSTEM, perms, NULL);
 }
 
+/* given an avd | the access_XXX perm dontaudit bit onto the XXX bit */
+static void compute_access_dontaudit(struct av_decision *avd, u32 sclass)
+{
+	u32 auditdeny = avd->auditdeny;
+
+	switch (sclass) {
+	case SECCLASS_FILE:
+		if (auditdeny & FILE__ACCESS_READ)
+			auditdeny |= FILE__READ;
+		if (auditdeny & FILE__ACCESS_WRITE)
+			auditdeny |= FILE__WRITE;
+		if (auditdeny & FILE__ACCESS_EXECUTE)
+			auditdeny |= FILE__EXECUTE;
+		break;
+	case SECCLASS_DIR:
+		if (auditdeny & DIR__ACCESS_READ)
+			auditdeny |= DIR__READ;
+		if (auditdeny & DIR__ACCESS_WRITE)
+			auditdeny |= DIR__WRITE;
+		if (auditdeny & DIR__ACCESS_EXECUTE)
+			auditdeny |= DIR__EXECUTE;
+		break;
+/*
+	case SECCLASS_CHR_FILE:
+		if (auditdeny & CHR_FILE__ACCESS_READ)
+			auditdeny |= CHR_FILE__READ;
+		if (auditdeny & CHR_FILE__ACCESS_WRITE)
+			auditdeny |= CHR_FILE__WRITE;
+		if (auditdeny & CHR_FILE__ACCESS_EXECUTE)
+			auditdeny |= CHR_FILE__EXECUTE;
+		break;
+	case SECCLASS_BLK_FILE:
+		if (auditdeny & BLK_FILE__ACCESS_READ)
+			auditdeny |= BLK_FILE__READ;
+		if (auditdeny & BLK_FILE__ACCESS_WRITE)
+			auditdeny |= BLK_FILE__WRITE;
+		if (auditdeny & BLK_FILE__ACCESS_EXECUTE)
+			auditdeny |= BLK_FILE__EXECUTE;
+		break;
+	case SECCLASS_FIFO_FILE:
+		if (auditdeny & FIFO_FILE__ACCESS_READ)
+			auditdeny |= FIFO_FILE__READ;
+		if (auditdeny & FIFO_FILE__ACCESS_WRITE)
+			auditdeny |= FIFO_FILE__WRITE;
+		if (auditdeny & FIFO_FILE__ACCESS_EXECUTE)
+			auditdeny |= FIFO_FILE__EXECUTE;
+		break;
+	case SECCLASS_SOCK_FILE:
+		if (auditdeny & SOCK_FILE__ACCESS_READ)
+			auditdeny |= SOCK_FILE__READ;
+		if (auditdeny & SOCK_FILE__ACCESS_WRITE)
+			auditdeny |= SOCK_FILE__WRITE;
+		if (auditdeny & SOCK_FILE__ACCESS_EXECUTE)
+			auditdeny |= SOCK_FILE__EXECUTE;
+		break;
+	default:
+		BUG();
+*/
+	}
+
+	avd->auditdeny = auditdeny;
+}
+
 /* Check whether a task has a particular permission to an inode.
    The 'adp' parameter is optional and allows other audit
    data to be passed (e.g. the dentry). */
 static int inode_has_perm(const struct cred *cred,
 			  struct inode *inode,
 			  u32 perms,
-			  struct avc_audit_data *adp)
+			  struct avc_audit_data *adp,
+			  bool from_access)
 {
 	struct inode_security_struct *isec;
 	struct avc_audit_data ad;
-	u32 sid;
+        struct av_decision avd;
+        int rc;
+	u32 ssid, tsid;
+	u16 tclass;
 
 	if (unlikely(IS_PRIVATE(inode)))
 		return 0;
 
-	sid = cred_sid(cred);
+	ssid = cred_sid(cred);
+
 	isec = inode->i_security;
+	tsid = isec->sid;
+	tclass = isec->sclass;
 
 	if (!adp) {
 		adp = &ad;
@@ -1542,7 +1612,13 @@ static int inode_has_perm(const struct cred *cred,
 		ad.u.fs.inode = inode;
 	}
 
-	return avc_has_perm(sid, isec->sid, isec->sclass, perms, adp);
+        rc = avc_has_perm_noaudit(ssid, tsid, tclass, perms, 0, &avd);
+
+	if (unlikely(rc && from_access))
+		compute_access_dontaudit(&avd, tclass);
+
+        avc_audit(ssid, tsid, tclass, perms, &avd, rc, adp);
+        return rc;
 }
 
 /* Same as inode_has_perm, but pass explicit audit data containing
@@ -1559,7 +1635,7 @@ static inline int dentry_has_perm(const struct cred *cred,
 	AVC_AUDIT_DATA_INIT(&ad, FS);
 	ad.u.fs.path.mnt = mnt;
 	ad.u.fs.path.dentry = dentry;
-	return inode_has_perm(cred, inode, av, &ad);
+	return inode_has_perm(cred, inode, av, &ad, 0);
 }
 
 /* Check whether a task can use an open file descriptor to
@@ -1595,7 +1671,7 @@ static int file_has_perm(const struct cred *cred,
 	/* av is zero if only checking access to the descriptor. */
 	rc = 0;
 	if (av)
-		rc = inode_has_perm(cred, inode, av, &ad);
+		rc = inode_has_perm(cred, inode, av, &ad, 0);
 
 out:
 	return rc;
@@ -2255,8 +2331,8 @@ static inline void flush_unauthorized_files(const struct cred *cred,
 			   interested in the inode-based check here. */
 			file = list_first_entry(&tty->tty_files, struct file, f_u.fu_list);
 			inode = file->f_path.dentry->d_inode;
-			if (inode_has_perm(cred, inode,
-					   FILE__READ | FILE__WRITE, NULL)) {
+			if (inode_has_perm(cred, inode, FILE__READ | FILE__WRITE,
+					   NULL, 0)) {
 				drop_tty = 1;
 			}
 		}
@@ -2702,7 +2778,7 @@ static int selinux_inode_follow_link(struct dentry *dentry, struct nameidata *na
 	return dentry_has_perm(cred, NULL, dentry, FILE__READ);
 }
 
-static int selinux_inode_permission(struct inode *inode, int mask)
+static int selinux_inode_permission(struct inode *inode, int mask, bool from_access)
 {
 	const struct cred *cred = current_cred();
 
@@ -2711,8 +2787,8 @@ static int selinux_inode_permission(struct inode *inode, int mask)
 		return 0;
 	}
 
-	return inode_has_perm(cred, inode,
-			      file_mask_to_av(inode->i_mode, mask), NULL);
+	return inode_has_perm(cred, inode, file_mask_to_av(inode->i_mode, mask),
+			      NULL, from_access);
 }
 
 static int selinux_inode_setattr(struct dentry *dentry, struct iattr *iattr)
@@ -3204,7 +3280,7 @@ static int selinux_dentry_open(struct file *file, const struct cred *cred)
 	 * new inode label or new policy.
 	 * This check is not redundant - do not remove.
 	 */
-	return inode_has_perm(cred, inode, open_file_to_av(file), NULL);
+	return inode_has_perm(cred, inode, open_file_to_av(file), NULL, 0);
 }
 
 /* task security operations */
diff --git a/security/selinux/include/av_permissions.h b/security/selinux/include/av_permissions.h
index d645192..48e9d5d 100644
--- a/security/selinux/include/av_permissions.h
+++ b/security/selinux/include/av_permissions.h
@@ -80,6 +80,9 @@
 #define DIR__SEARCH                               0x00100000UL
 #define DIR__RMDIR                                0x00200000UL
 #define DIR__OPEN                                 0x00400000UL
+#define DIR__ACCESS_READ                          0x00800000UL
+#define DIR__ACCESS_WRITE                         0x01000000UL
+#define DIR__ACCESS_EXECUTE                       0x02000000UL
 #define FILE__IOCTL                               0x00000001UL
 #define FILE__READ                                0x00000002UL
 #define FILE__WRITE                               0x00000004UL
@@ -101,6 +104,9 @@
 #define FILE__ENTRYPOINT                          0x00040000UL
 #define FILE__EXECMOD                             0x00080000UL
 #define FILE__OPEN                                0x00100000UL
+#define FILE__ACCESS_READ                         0x00200000UL
+#define FILE__ACCESS_WRITE                        0x00400000UL
+#define FILE__ACCESS_EXECUTE                      0x00800000UL
 #define LNK_FILE__IOCTL                           0x00000001UL
 #define LNK_FILE__READ                            0x00000002UL
 #define LNK_FILE__WRITE                           0x00000004UL
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 0d030b4..3c2d321 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -601,7 +601,7 @@ static int smack_inode_rename(struct inode *old_inode,
  *
  * Returns 0 if access is permitted, -EACCES otherwise
  */
-static int smack_inode_permission(struct inode *inode, int mask)
+static int smack_inode_permission(struct inode *inode, int mask, bool from_access)
 {
 	struct smk_audit_info ad;
 	/*



--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [RFC PATCH] access(2) vs. SELinux

[Stephen Smalley]

On Mon, 2009-04-20 at 14:11 -0400, Eric Paris wrote:
> On Mon, 2009-04-20 at 13:35 -0400, Chad Sellers wrote:
> > On 4/20/09 1:21 PM, "Eric Paris" <eparis@redhat.com> wrote:
> > <snip>
> > > 
> > > I'm going to spend a couple minutes and try to prototype an all in
> > > kernel solution such that on access() we will check the permission:
> > > 
> > > allow process_t file_t:file read
> > > 
> > > if there is a dontaudit rule for EITHER "read" or "access_read" we will
> > > NOT print an avc denial.
> > > 
> > > on the actual usage we will ONLY check for the "read" dontaudit.
> > > 
> > That sounds like a better solution than anything we've talked about so far.
> > It would mean the policy author would only have to worry about the access_*
> > perms if they cared about the issue they're designed to solve. Hopefully you
> > can find an acceptable way to pull it off in the kernel. I'm rooting for
> > you!
> 
> So James, Stephen what do we think of something like this?  Lot of code
> bloat and pain on the access+denial path, but shouldn't hurt us any in
> the hot case...
> 
> ---
> diff --git a/fs/open.c b/fs/open.c
> index 377eb25..485cfd8 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -493,7 +493,7 @@ SYSCALL_DEFINE3(faccessat, int, dfd, const char __user *, filename, int, mode)
>  			goto out_path_release;
>  	}
>  
> -	res = inode_permission(inode, mode | MAY_ACCESS);
> +	res = inode_access_permission(inode, mode | MAY_ACCESS);

I had forgotten that there is already a MAY_ACCESS flag defined and used
by the vfs.  So why can't we just pass that on to the security modules
and use that to distinguish access(2) from open(2)?  Looks like it gets
used on chdir(2) and chroot(2) as well, which is unfortunate, and I
don't quite understand why.  Maybe we need to look back at the origins
of the MAY_ACCESS flag and its rationale.

>  	/* SuS v2 requires we report a read only fs too */
>  	if (res || !(mode & S_IWOTH) || special_file(inode->i_mode))
>  		goto out_path_release;

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [RFC PATCH] access(2) vs. SELinux

[Eric Paris]

On Mon, 2009-04-20 at 15:41 -0400, Stephen Smalley wrote:
> On Mon, 2009-04-20 at 14:11 -0400, Eric Paris wrote:
> > On Mon, 2009-04-20 at 13:35 -0400, Chad Sellers wrote:
> > > On 4/20/09 1:21 PM, "Eric Paris" <eparis@redhat.com> wrote:
> > > <snip>
> > > > 
> > > > I'm going to spend a couple minutes and try to prototype an all in
> > > > kernel solution such that on access() we will check the permission:
> > > > 
> > > > allow process_t file_t:file read
> > > > 
> > > > if there is a dontaudit rule for EITHER "read" or "access_read" we will
> > > > NOT print an avc denial.
> > > > 
> > > > on the actual usage we will ONLY check for the "read" dontaudit.
> > > > 
> > > That sounds like a better solution than anything we've talked about so far.
> > > It would mean the policy author would only have to worry about the access_*
> > > perms if they cared about the issue they're designed to solve. Hopefully you
> > > can find an acceptable way to pull it off in the kernel. I'm rooting for
> > > you!
> > 
> > So James, Stephen what do we think of something like this?  Lot of code
> > bloat and pain on the access+denial path, but shouldn't hurt us any in
> > the hot case...
> > 
> > ---
> > diff --git a/fs/open.c b/fs/open.c
> > index 377eb25..485cfd8 100644
> > --- a/fs/open.c
> > +++ b/fs/open.c
> > @@ -493,7 +493,7 @@ SYSCALL_DEFINE3(faccessat, int, dfd, const char __user *, filename, int, mode)
> >  			goto out_path_release;
> >  	}
> >  
> > -	res = inode_permission(inode, mode | MAY_ACCESS);
> > +	res = inode_access_permission(inode, mode | MAY_ACCESS);
> 
> I had forgotten that there is already a MAY_ACCESS flag defined and used
> by the vfs.  So why can't we just pass that on to the security modules
> and use that to distinguish access(2) from open(2)?  Looks like it gets
> used on chdir(2) and chroot(2) as well, which is unfortunate, and I
> don't quite understand why.  Maybe we need to look back at the origins
> of the MAY_ACCESS flag and its rationale.

Actually since both of those should be S_ISDIR we should still be able
to tell in the case we care about.

Looks like there was a MAY_CHDIR flag, but viro realized having 2 flags
that didn't overlap was useless...

a110343f0d6d41f68b7cf8c00b57a3172c67f816


-Eric


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [RFC PATCH] access(2) vs. SELinux

[Eric Paris]

On Mon, 2009-04-20 at 18:24 -0400, Eric Paris wrote:
> On Mon, 2009-04-20 at 15:41 -0400, Stephen Smalley wrote:

> > > diff --git a/fs/open.c b/fs/open.c
> > > index 377eb25..485cfd8 100644
> > > --- a/fs/open.c
> > > +++ b/fs/open.c
> > > @@ -493,7 +493,7 @@ SYSCALL_DEFINE3(faccessat, int, dfd, const char __user *, filename, int, mode)
> > >  			goto out_path_release;
> > >  	}
> > >  
> > > -	res = inode_permission(inode, mode | MAY_ACCESS);
> > > +	res = inode_access_permission(inode, mode | MAY_ACCESS);
> > 
> > I had forgotten that there is already a MAY_ACCESS flag defined and used
> > by the vfs.  So why can't we just pass that on to the security modules
> > and use that to distinguish access(2) from open(2)?  Looks like it gets
> > used on chdir(2) and chroot(2) as well, which is unfortunate, and I
> > don't quite understand why.  Maybe we need to look back at the origins
> > of the MAY_ACCESS flag and its rationale.
> 
> Actually since both of those should be S_ISDIR we should still be able
> to tell in the case we care about.

I'm not sure what I was smoking when I thought this...  Can we just
pretend that line of that e-mail was never sent?


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: access(2) vs. SELinux

[James Carter]

On Mon, 2009-04-20 at 11:11 -0400, Eric Paris wrote:
> On Mon, 2009-04-20 at 10:35 -0400, Joshua Brindle wrote:
> > Chad Sellers wrote:
> > > On 4/16/09 2:10 PM, "Eric Paris" <eparis@redhat.com> wrote:
> > >> He's some concerns/thoughts.
> > >>
> > >> domain_t calls access(W_OK) on file_t.
> > >>
> > >> This generates a denial
> > >>
> > >> denied  { access_write } scontext=domain_t tcontext=file_t tclass=file
> > >>
> > >> Which audit2allow will gladly turn into:
> > >>
> > >> allow domain_t file_t:file { access_write }
> > >>
> > >> Load that module and then, POP the next time
> > >>
> > >> denied { write } scontext=domain_t tcontext=file_t tclass=file
> > >>
> > >> So somewhere we need some sort of synchronization.
> > >>
> > >> ---
> > >>
> > >> I suggested to Dan that audit2allow should should, given the above
> > >> access_write denial output:
> > >> allow domain_t file_t:file { write }
> > >>
> > >> and the tool chain just automatically maps all write rule to both allow
> > >> bits for BOTH write and access_write.
> > >>
> > >> ---
> > >>
> > >> We could also have the tool chain just run in both directions and leave
> > >> audit2allow alone.  Any rule with access_write would cause the tool
> > >> chain to add write and any rule with write would cause us to add
> > >> access_write.
> > >>
> > >>> From the point of view of policy analysis I think think that makes
> > >> things worse.  Now the access_write rule really means something.  That's
> > >> fine with me, but I have a feeling the people who care about policy
> > >> analysis might not like that idea.....
> > >>
> > > I think these issues point to how inelegant this solution is. Relying on the
> > > toolchain to special case this and modify all results is going to be
> > > painful. Another example of this comes up when applying analysis tools to
> > > the policy. SETools will now show rules in a binary policy that can only be
> > > traced back to the source policy if you understand how the compiler mangling
> > > works. Managed policy has allowed us to decrease the amount of policy
> > > mangling we do, and this strategy seems to be going the other direction.
> > > 
> > > I'd like to put in a vote for having access call the _noaudit interface to
> > > suppress the audit record. I agree with Alexey though that it would need to
> > > have something in /selinux to toggle this on/off. Then, when debugging a
> > > policy and not getting denials, you'd throw this switch when to the point of
> > > disabling dontaudits. We could even have semodule -D toggle this as well, if
> > > we really want to bake this into the toolchain somewhere. As far as probing
> > > attacks go, systems with more stringent security requirements that want to
> > > see all possible probing attacks could turn this flag on all the time.
> > > 
> > > 
> > 
> > I agree with this, Eric's description of what needs to happen above makes me 
> > cringe. There is very limited utility to having 2 parallel sets of permissions 
> > that need to be synchronized by the toolchain and quite a bit of potential pain.
> > 
> > One example of horrific pain would be if both the access_write and write rules 
> > were present in policy, and someone wanted to remove the write access. They do 
> > the necessary searching and find where write is granted, remove it and viola - 
> > nothing happens, write access is still granted. Very confusing, I could see even 
> > experienced policy writers getting bitten by this enough to curse the day these 
> > permissions were added.
> 
> I strongly and vehemently oppose any solution that is a blanket
> dontaudit on access calls, even if there is a flag to dontdonaudit.
> This might be fine in "secure" shops where everyone understands and is
> willing to suffer some extra SELinux pain but not here.  If SELinux gets
> in the way it better scream to high heavens for my customers.
> 

I think that what we need is a check to see if the domain is allowed to
call access() on the object.  If it is not allowed, then a denial is
generated; if it is, then the results of the desired permission check is
returned, but denials are not audited.

This would better reflect what is actually happening.  When a domain
calls access(), it is really reading the security properties of the
object.

I don't really see the need of access_read and friends either.  One
permission (Dan had suggested getattrmac in the Bugzilla) should be
enough.  In the policy all domains would be allowed to call access() on
all objects marked with the non_security_file_type attribute.

Let's not make the toolchain even more brittle than it already is.

> I agree that having bi-directional synchronization is a difficult idea.
> That's why I suggested audit2allow never output access_* permissions,
> and I can make the tool chain reject any allow rules that include
> access_* permissions (even with a nice message telling you to not use
> access_* in allow rules).
> 
> But maybe people would like a more kernel centric solution?  I don't,
> because i think the userspace tool chain is the right place to do this.
> And it'll be ugly as @#$^ but I'm sure I could fine some atrocious way
> to get the kernel to check permission on "read" output denials that say
> "read" but on access() call it would check dontaudit rules for
> "access_read"     allow or auditallow rules with any of the access_*
> permissions would be meaningless.  Something tells me sds and jmorris
> are going to hate the level of encapsulation and layering violations
> this is going to take....
> 
> I pretty strongly prefer a one way synch from "read" -> "access_read"
> when going from the text to a binary policy and having the tool chain
> reject any rules with access_* set in other types of rules.  But I think
> audit2allow/why should automagically convert the other way for easy of
> use.
> 
> Doesn't make policy analysis any harder since access_* basically means
> nothing at all, solves the problem of us having to dontaudit
> hot/dangerous code paths, doesn't screw up policy writers since you
> can't have both types of permissions in the text version of policy....
> 
> Does it seem too fragile?
> 
> -Eric
> 
> 
> --
> This message was distributed to subscribers of the selinux mailing list.
> If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
> the words "unsubscribe selinux" without quotes as the message.
-- 
James Carter <jwcart2@epoch.ncsc.mil>
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: access(2) vs. SELinux

[Eric Paris]

On Mon, 2009-04-20 at 14:55 -0400, James Carter wrote:
> On Mon, 2009-04-20 at 11:11 -0400, Eric Paris wrote:
 
> > I strongly and vehemently oppose any solution that is a blanket
> > dontaudit on access calls, even if there is a flag to dontdonaudit.
> > This might be fine in "secure" shops where everyone understands and is
> > willing to suffer some extra SELinux pain but not here.  If SELinux gets
> > in the way it better scream to high heavens for my customers.
> > 
> 
> I think that what we need is a check to see if the domain is allowed to
> call access() on the object.  If it is not allowed, then a denial is
> generated; if it is, then the results of the desired permission check is
> returned, but denials are not audited.
> 
> This would better reflect what is actually happening.  When a domain
> calls access(), it is really reading the security properties of the
> object.

Your still just talking about a big global dontaudit hammer on the
EACCESS people get back from access().  Not only that, you propose a new
permission every domain needs of which I don't see any security benefit
(outside of maybe helping make sure people can't probe policy willy
nilly).

If access() had better return semantics than yes/no this might be a good
approach, but I don't see how a single extra perm helps anything and I'm
still railing against the global dontaudit hammer.

-Eric


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: access(2) vs. SELinux

[James Carter]

On Mon, 2009-04-20 at 15:08 -0400, Eric Paris wrote:
> On Mon, 2009-04-20 at 14:55 -0400, James Carter wrote:
> > On Mon, 2009-04-20 at 11:11 -0400, Eric Paris wrote:
>  
> > > I strongly and vehemently oppose any solution that is a blanket
> > > dontaudit on access calls, even if there is a flag to dontdonaudit.
> > > This might be fine in "secure" shops where everyone understands and is
> > > willing to suffer some extra SELinux pain but not here.  If SELinux gets
> > > in the way it better scream to high heavens for my customers.
> > > 
> > 
> > I think that what we need is a check to see if the domain is allowed to
> > call access() on the object.  If it is not allowed, then a denial is
> > generated; if it is, then the results of the desired permission check is
> > returned, but denials are not audited.
> > 
> > This would better reflect what is actually happening.  When a domain
> > calls access(), it is really reading the security properties of the
> > object.
> 
> Your still just talking about a big global dontaudit hammer on the
> EACCESS people get back from access().  Not only that, you propose a new
> permission every domain needs of which I don't see any security benefit
> (outside of maybe helping make sure people can't probe policy willy
> nilly).
> 
If you don't care whether or not someone is probing the policy, then why
not just always use the _noaudit interfaces?

What is the security benefit of the access_* permissions?  It looks like
they are only going to be used to determine if a denial should be
generated.
  
> If access() had better return semantics than yes/no this might be a good
> approach, but I don't see how a single extra perm helps anything and I'm
> still railing against the global dontaudit hammer.
> 
> -Eric
-- 
James Carter <jwcart2@epoch.ncsc.mil>
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: access(2) vs. SELinux

[Eric Paris]

On Mon, 2009-04-20 at 15:28 -0400, James Carter wrote:
> On Mon, 2009-04-20 at 15:08 -0400, Eric Paris wrote:
> > On Mon, 2009-04-20 at 14:55 -0400, James Carter wrote:
> > > On Mon, 2009-04-20 at 11:11 -0400, Eric Paris wrote:
> >  
> > > > I strongly and vehemently oppose any solution that is a blanket
> > > > dontaudit on access calls, even if there is a flag to dontdonaudit.
> > > > This might be fine in "secure" shops where everyone understands and is
> > > > willing to suffer some extra SELinux pain but not here.  If SELinux gets
> > > > in the way it better scream to high heavens for my customers.
> > > > 
> > > 
> > > I think that what we need is a check to see if the domain is allowed to
> > > call access() on the object.  If it is not allowed, then a denial is
> > > generated; if it is, then the results of the desired permission check is
> > > returned, but denials are not audited.
> > > 
> > > This would better reflect what is actually happening.  When a domain
> > > calls access(), it is really reading the security properties of the
> > > object.
> > 
> > Your still just talking about a big global dontaudit hammer on the
> > EACCESS people get back from access().  Not only that, you propose a new
> > permission every domain needs of which I don't see any security benefit
> > (outside of maybe helping make sure people can't probe policy willy
> > nilly).
> > 
> If you don't care whether or not someone is probing the policy, then why
> not just always use the _noaudit interfaces?

>From where I sit, I don't care if people probe the policy.  The problem
I want to solve is that we have dontaudit rules against what I consider
valuable resources.  (eg: shadow and krb5 files)

But, I certainly understand why others might care (which is the lesser
of the two reasons I don't like the _noaudit, interface on access()).
So I think my kernel patch prototype allows the policy to decide where
it cares about probing and where it doesn't.

> What is the security benefit of the access_* permissions?  It looks like
> they are only going to be used to determine if a denial should be
> generated.

Very little security benefit other than we can remove all of the
dontaudit rules trying to read shadow_t.  Instead we'll have dontaudit
rules for trying to probe if you can read shadow_t.  Actually trying to
read shadow_t would begin to generate a denial.  Today neither activity
would generate a denial.  access_* permissions would have NO meaning
other than in the dontaudit case (could also do auditallow)

Let me summarize where I'm coming from.

Problem:
Many programs use access() to chose which code path to use thus
requesting permissions granted by DAC, but we choose to deny through
SELinux policy.  Thus we currently must dontaudit all read, write, and
execute for these situations.

Eric's Restrictions on solutions:
If SELinux denies access SELinux must is some way inform the user that
it denied access.

Other's Restrictions:
Do not make tool chain significantly more brittle?
Maintain ability to detect probing of security policy?

Am I missing something?  Does my latest kernel prototype meet these
needs?


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: access(2) vs. SELinux

[James Carter]

On Mon, 2009-04-20 at 15:49 -0400, Eric Paris wrote:
> On Mon, 2009-04-20 at 15:28 -0400, James Carter wrote:
> > On Mon, 2009-04-20 at 15:08 -0400, Eric Paris wrote:
> > > On Mon, 2009-04-20 at 14:55 -0400, James Carter wrote:
> > > > On Mon, 2009-04-20 at 11:11 -0400, Eric Paris wrote:
> > >  
> > > > > I strongly and vehemently oppose any solution that is a blanket
> > > > > dontaudit on access calls, even if there is a flag to dontdonaudit.
> > > > > This might be fine in "secure" shops where everyone understands and is
> > > > > willing to suffer some extra SELinux pain but not here.  If SELinux gets
> > > > > in the way it better scream to high heavens for my customers.
> > > > > 
> > > > 
> > > > I think that what we need is a check to see if the domain is allowed to
> > > > call access() on the object.  If it is not allowed, then a denial is
> > > > generated; if it is, then the results of the desired permission check is
> > > > returned, but denials are not audited.
> > > > 
> > > > This would better reflect what is actually happening.  When a domain
> > > > calls access(), it is really reading the security properties of the
> > > > object.
> > > 
> > > Your still just talking about a big global dontaudit hammer on the
> > > EACCESS people get back from access().  Not only that, you propose a new
> > > permission every domain needs of which I don't see any security benefit
> > > (outside of maybe helping make sure people can't probe policy willy
> > > nilly).
> > > 
> > If you don't care whether or not someone is probing the policy, then why
> > not just always use the _noaudit interfaces?
> 
> >From where I sit, I don't care if people probe the policy.  The problem
> I want to solve is that we have dontaudit rules against what I consider
> valuable resources.  (eg: shadow and krb5 files)
> 
> But, I certainly understand why others might care (which is the lesser
> of the two reasons I don't like the _noaudit, interface on access()).
> So I think my kernel patch prototype allows the policy to decide where
> it cares about probing and where it doesn't.
> 
> > What is the security benefit of the access_* permissions?  It looks like
> > they are only going to be used to determine if a denial should be
> > generated.
> 
> Very little security benefit other than we can remove all of the
> dontaudit rules trying to read shadow_t.  Instead we'll have dontaudit
> rules for trying to probe if you can read shadow_t.  Actually trying to
> read shadow_t would begin to generate a denial.  Today neither activity
> would generate a denial.  access_* permissions would have NO meaning
> other than in the dontaudit case (could also do auditallow)
> 
> Let me summarize where I'm coming from.
> 
> Problem:
> Many programs use access() to chose which code path to use thus
> requesting permissions granted by DAC, but we choose to deny through
> SELinux policy.  Thus we currently must dontaudit all read, write, and
> execute for these situations.
> 
> Eric's Restrictions on solutions:
> If SELinux denies access SELinux must is some way inform the user that
> it denied access.
> 
> Other's Restrictions:
> Do not make tool chain significantly more brittle?
> Maintain ability to detect probing of security policy?
> 
> Am I missing something?  Does my latest kernel prototype meet these
> needs?
> 
I think that it would.  My comments about the tool chain refer to
earlier comments, rather than your patch, so that is not a problem.

The only issues are (1) whether we need access_read, access_write, and
access_execute, or if just "access" would work; and (2) I don't really
like the idea of permissions that are just for dontaudit rules. 

This is how the situation looks to me:
1) If we would never care when access() is called, then we would just
use the _noaudit interfaces.  (But we do sometimes care)
2) If we always care when access() is called, then we would just leave
things as they are.  (But we don't always care)
3a) If we sometimes care when access() is called, then we should have a
permission to check whether we should care.  (What you propose)
3b) If we sometimes care when access() is called, then we should control
whether it is called or not.  (What I propose)

There are advantages to your approach.  One major one being that nothing
happens if the domain calling access() has the requested permissions.

What I propose seems more logical to me for what is actually happening.
The process is seeking information about the security policy, so that
attempt at access should be controlled.

> 
> --
> This message was distributed to subscribers of the selinux mailing list.
> If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
> the words "unsubscribe selinux" without quotes as the message.
-- 
James Carter <jwcart2@epoch.ncsc.mil>
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: access(2) vs. SELinux

[selinux]

On Mon, Apr 20, 2009 at 03:49:49PM -0400, Eric Paris wrote:
> >From where I sit, I don't care if people probe the policy.  The problem
> I want to solve is that we have dontaudit rules against what I consider
> valuable resources.  (eg: shadow and krb5 files)
> 
> But, I certainly understand why others might care (which is the lesser
> of the two reasons I don't like the _noaudit, interface on access()).
> So I think my kernel patch prototype allows the policy to decide where
> it cares about probing and where it doesn't.
> 
> > What is the security benefit of the access_* permissions?  It looks like
> > they are only going to be used to determine if a denial should be
> > generated.
> 
> Very little security benefit other than we can remove all of the
> dontaudit rules trying to read shadow_t.  Instead we'll have dontaudit
> rules for trying to probe if you can read shadow_t.  Actually trying to
> read shadow_t would begin to generate a denial.  Today neither activity
> would generate a denial.  access_* permissions would have NO meaning
> other than in the dontaudit case (could also do auditallow)
> 
> Let me summarize where I'm coming from.
> 
> Problem:
> Many programs use access() to chose which code path to use thus
> requesting permissions granted by DAC, but we choose to deny through
> SELinux policy.  Thus we currently must dontaudit all read, write, and
> execute for these situations.
> 
> Eric's Restrictions on solutions:
> If SELinux denies access SELinux must is some way inform the user that
> it denied access.
> 
> Other's Restrictions:
> Do not make tool chain significantly more brittle?
> Maintain ability to detect probing of security policy?

I have had a better thought on this all and now I'm sure, that
it would be optimal to have single access permission that will not mean
permitting/denying access(2) calls, but will control the audit messages
generation (depending on the actual access we have got).

If we will just allow averyone silently call access(2) on every file we will
only have one problem that is an information leakage.
To prevent that information leak in some cases or in all cases except some cases
we will use access permission (will it be enough to have one single access?).

So, simply not having access permission will generate audit messages only when access(2)
 returns "NO ACCESS", but will generate nothing if there is access.
If access permission will be dontaudited OR allowed, then that will mean we don't care at all
if someone is probing permissions, that are not permitted to him (we trust him enough).
This is not typical for SELinux to have same meaning for dontaudit and allow, but see,
we are only trying to decide whether we are to AUDIT some actions,
not to permit/deny some access. Right?

Or you gonna make it possible to deny access(2) call at all? I wonder if that would be useful.

-- 
Alexey S

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: access(2) vs. SELinux

[Alexey S]

On Wed, Apr 22, 2009 at 04:31:05PM +0500, selinux@udmvt.ru wrote:
> I have had a better thought on this all and now I'm sure, that
> it would be optimal to have single access permission that will not mean
> permitting/denying access(2) calls, but will control the audit messages
> generation (depending on the actual access we have got).
> 
> If we will just allow averyone silently call access(2) on every file we will
> only have one problem that is an information leakage.
> To prevent that information leak in some cases or in all cases except some cases
> we will use access permission (will it be enough to have one single access?).
> 
> So, simply not having access permission will generate audit messages only when access(2)
>  returns "NO ACCESS", but will generate nothing if there is access.
> If access permission will be dontaudited OR allowed, then that will mean we don't care at all
> if someone is probing permissions, that are not permitted to him (we trust him enough).
> This is not typical for SELinux to have same meaning for dontaudit and allow, but see,
> we are only trying to decide whether we are to AUDIT some actions,
> not to permit/deny some access. Right?
Or there can be difference.
Since there are other permissions that are tested by access(2) and they can be just denied
or dontaudited. So having access(2) to noise about dontaudited permissions sometimes looks silly,
but sometimes is what required...

It looks unnatural that dontaudit still produces audit messages. But what if
  dontaudit domain_t file_t:class { access }; 
    will produce messages when permissions are probed that are denied (and not dontaudited)
 
  allow domain_t file_t:class { access };
    will never generate audit messages on any permission probe

  deny domain_t file_t:class { access };
    will audit about probes on permissions that are denied or dontaudited

  auditallow domain_t file_t:class { access };
    will audit about any probe attempt regardless it's result (should it also audit the result?)

  auditdeny domain_t file_t:class { access };
    will audit about probes on permissions that are dontaudited (and not denied) :) why do we need this?

And I prefer audit messages generated from access(2) to be different from audits of actual actions,
but still it would be useful if audit2allow will be able to generate allow for actual permissions,
not for access permission. And it should be controlled by a switch from command line.

PS: Sorry, I'm talking with myself.
-- 
Alexey S.

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: access(2) vs. SELinux

[Alexey S]

On Wed, Apr 15, 2009 at 11:51:50AM -0400, Stephen Smalley wrote:
> Per [1], the fact that SELinux applies the same read/write/execute
> checks in response to access(2) calls as it does for an actual open(2)
> for read/write or execve(2) for execute leads to huge numbers of
> spurious denials for applications such as nautilus that probe all files
> being listed.  dontaudit'ing of these denials is viewed as unacceptable
> as Dan wants to be able to see such denials upon the actual open(2) or
> execve(2) attempts.
> 
> If access(2) only returned the result of the DAC access checks and e.g.
> only checked getattr permission on access(2) calls (similar to stat(2)),
> then the read/write/execute MAC denials would not happen, but that would
> likely break the behavior of existing applications/libraries that use
> access(2) to decide whether to follow a privileged code path or fall
> back on an unprivileged code path (e.g. kerberos libraries).
> 
> Other suggested proposals included:
> - Using the _noaudit interface to suppress auditing of the
> read/write/execute checks when checking for access(2), although this
> requires reworking the underlying implementation so that we can
> distinguish access(2) from open(2) checking.  However, this could lead
> to silent failure of applications with no way to obtain any audit
> messages, and would provide an obvious way to probe for access without
> triggering audit.
I'm sure this is the only one correct solution, but it lacks some little piece:
it needs some global switch somewhere in /proc or /selinux that turns on/off
generation of audit messages by access(2) testing read/write/execute permissions.
This is the special case, that deserves it's own config option, IMHO.

...
> 
> Thoughts?
> 
> [1] https://bugzilla.redhat.com/show_bug.cgi?id=495211
> 
> 
> -- 
> Stephen Smalley
> National Security Agency
> 

-- 
Alexey S

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: access(2) vs. SELinux

[Stephen Smalley]

On Thu, 2009-04-16 at 11:46 +0500, Alexey S wrote:
> On Wed, Apr 15, 2009 at 11:51:50AM -0400, Stephen Smalley wrote:
> > Per [1], the fact that SELinux applies the same read/write/execute
> > checks in response to access(2) calls as it does for an actual open(2)
> > for read/write or execve(2) for execute leads to huge numbers of
> > spurious denials for applications such as nautilus that probe all files
> > being listed.  dontaudit'ing of these denials is viewed as unacceptable
> > as Dan wants to be able to see such denials upon the actual open(2) or
> > execve(2) attempts.
> > 
> > If access(2) only returned the result of the DAC access checks and e.g.
> > only checked getattr permission on access(2) calls (similar to stat(2)),
> > then the read/write/execute MAC denials would not happen, but that would
> > likely break the behavior of existing applications/libraries that use
> > access(2) to decide whether to follow a privileged code path or fall
> > back on an unprivileged code path (e.g. kerberos libraries).
> > 
> > Other suggested proposals included:
> > - Using the _noaudit interface to suppress auditing of the
> > read/write/execute checks when checking for access(2), although this
> > requires reworking the underlying implementation so that we can
> > distinguish access(2) from open(2) checking.  However, this could lead
> > to silent failure of applications with no way to obtain any audit
> > messages, and would provide an obvious way to probe for access without
> > triggering audit.
> I'm sure this is the only one correct solution, but it lacks some little piece:
> it needs some global switch somewhere in /proc or /selinux that turns on/off
> generation of audit messages by access(2) testing read/write/execute permissions.
> This is the special case, that deserves it's own config option, IMHO.

I was actually more inclined to the latter option (new permissions for
access(2), with some consistency guarantees provided by the policy
compiler).  Is there some reason you preferred this approach instead?

Yet another approach would be to argue that this is most properly
handled by the system call audit mechanism, which can already
distinguish access(2) from open(2).  However, it cannot distinguish MAC
failures from DAC failures at present as they do not use different errno
values (and doing so would raise issues of its own).

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.