All of lore.kernel.org
 help / color / mirror / Atom feed
* Policy capabilities: when to use and complications with using
@ 2017-05-03 16:14 Stephen Smalley
  2017-05-03 16:51 ` Dominick Grift
  2017-05-03 19:35 ` James Carter
  0 siblings, 2 replies; 17+ messages in thread
From: Stephen Smalley @ 2017-05-03 16:14 UTC (permalink / raw)
  To: Eric Paris, Paul Moore, Joshua Brindle, Chris PeBenito, James Carter
  Cc: selinux

Looking at adding a map permission for mmap [1], and thinking about
whether it needs to be wrapped by a policy capability.  On the one
hand, we made the open permission depend on a policy capability, but on
the other hand, we haven't done that for other cases where we simply
added a check of a new permission (recent examples include prlimit
checking and module_load).  Compatibility for new permission checks is
generally covered in Linux distributions via handle_unknown=allow, and
in Android where handle_unknown=deny, the kernel and policy are not
independently updated so we can keep them properly synchronized without
needing such compatibility measures.

It seems like we generally reserve the use of policy capabilities for
when there is a more fundamental change in logic that wouldn't be
covered by handle_unknown.  All of the current policy capabilities
except for open_perms seem to fit into that category; they change what
permissions/classes are checked rather than merely adding a new check,
or they change when checks are applied, or they alter labeling
behavior.  Even if that is our standard, we haven't consistently
defined new policy capabilities for all such changes, e.g. for
distinguishing non-init user namespace capability checks (commit
8e4ff6f228e4722cac74db716e308d1da33d744f) or for the earlier updating
of netlink socket classes (commit
6c6d2e9bde1c1c87a7ead806f8f5e2181d41a652).  Those changes don't break
existing policies because of handle_unknown=allow, but since they are
actually changing what is checked and not merely adding new checks,
they technically ought to have been wrapped (with handle_unknown=allow,
they could end up allowing what would have been previously denied due
to changes in the classes).

Part of the reason that we tend to not introduce a new policy
capability more often is that it is painful to do so currently.  We
have to patch libsepol to recognize the new capability and patch the
policy to declare it (although for the latter we can now declare them
via a CIL module without modifying the base policy).  And since the
policy or module won't build without the updated libsepol, we can't
turn on the capability by default in refpolicy without making it
dependent on a new libsepol version.  That's why extended_socket_class
isn't yet enabled in refpolicy, for example.  That causes enablement
and adoption to lag behind.  It also makes it harder to test the new
kernel feature in the first place.

We could possibly look into lighter weight support for policy
capabilities.  If the policy compiler toolchain left the capability
names as strings in the kernel policy (new binary format version), then
we would no longer need to update libsepol for each new policy
capability.  The kernel would then turn the list into the bitmap
internally.  The downside is that we would lose validation of the
capability names when policy is built, and it isn't clear how the
kernel should handle unknown names (presently the kernel will simply
ignore any unknown capabilities in the bitmap).  Failing at policy load
time would mean we can't enable the capability in policy without making
it depend on a particular kernel version.

The lack of any direct relationship between policy capabilities and the
classes/permissions they affect also can be misleading.  For example,
someone booting a recent kernel might see warnings about undefined
classes introduced by the extended_socket_class feature and add those
class definitions to their policy, which will silence the kernel
warning and make them think that they are actually using those classes
now.  But they won't actually get used until they declare the
capability too in their policy, and the kernel doesn't warn about that
(nor does it necessarily make sense to do so, since that may be a
conscious choice by the policy author).  The kernel could however log
each policy capability and its state as part of its normal logging and
leave it up to the reader to decide whether those values are correct or
not.  We also had talked originally about checkpolicy and friends
possibly warning on inconsistencies, while leaving it up to the policy
author.

So:

1) Should we investigate lighter weight support for policy
capabilities, and if so, how?

2) Should the kernel log information about enabled/disabled policy
capabilities in much the same manner as it does for undefined
classes/permissions?

3) Should the policy compiler toolchain warn the user if a policy
capability is not declared and classes/permissions are used in rules
that will only be used if that policy capability is declared?  And
similarly if a policy capability is declared but the corresponding
classes/permissions are not used in any rules?

4) Do we need/want a policy capability for map permission and other
cases where we are only adding a new permission check? Or should we
continue to reserve them for cases not addressed via handle_unknown?

[1] https://github.com/SELinuxProject/selinux-kernel/issues/13

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Policy capabilities: when to use and complications with using
  2017-05-03 16:14 Policy capabilities: when to use and complications with using Stephen Smalley
@ 2017-05-03 16:51 ` Dominick Grift
  2017-05-03 17:23   ` Stephen Smalley
  2017-05-04 15:50   ` Paul Moore
  2017-05-03 19:35 ` James Carter
  1 sibling, 2 replies; 17+ messages in thread
From: Dominick Grift @ 2017-05-03 16:51 UTC (permalink / raw)
  To: selinux

[-- Attachment #1: Type: text/plain, Size: 6069 bytes --]

On Wed, May 03, 2017 at 12:14:16PM -0400, Stephen Smalley wrote:
> Looking at adding a map permission for mmap [1], and thinking about
> whether it needs to be wrapped by a policy capability.  On the one
> hand, we made the open permission depend on a policy capability, but on
> the other hand, we haven't done that for other cases where we simply
> added a check of a new permission (recent examples include prlimit
> checking and module_load).  Compatibility for new permission checks is
> generally covered in Linux distributions via handle_unknown=allow, and
> in Android where handle_unknown=deny, the kernel and policy are not
> independently updated so we can keep them properly synchronized without
> needing such compatibility measures.
> 
> It seems like we generally reserve the use of policy capabilities for
> when there is a more fundamental change in logic that wouldn't be
> covered by handle_unknown.  All of the current policy capabilities
> except for open_perms seem to fit into that category; they change what
> permissions/classes are checked rather than merely adding a new check,
> or they change when checks are applied, or they alter labeling
> behavior.  Even if that is our standard, we haven't consistently
> defined new policy capabilities for all such changes, e.g. for
> distinguishing non-init user namespace capability checks (commit
> 8e4ff6f228e4722cac74db716e308d1da33d744f) or for the earlier updating
> of netlink socket classes (commit
> 6c6d2e9bde1c1c87a7ead806f8f5e2181d41a652).  Those changes don't break
> existing policies because of handle_unknown=allow, but since they are
> actually changing what is checked and not merely adding new checks,
> they technically ought to have been wrapped (with handle_unknown=allow,
> they could end up allowing what would have been previously denied due
> to changes in the classes).
> 
> Part of the reason that we tend to not introduce a new policy
> capability more often is that it is painful to do so currently.  We
> have to patch libsepol to recognize the new capability and patch the
> policy to declare it (although for the latter we can now declare them
> via a CIL module without modifying the base policy).  And since the
> policy or module won't build without the updated libsepol, we can't
> turn on the capability by default in refpolicy without making it
> dependent on a new libsepol version.  That's why extended_socket_class
> isn't yet enabled in refpolicy, for example.  That causes enablement
> and adoption to lag behind.  It also makes it harder to test the new
> kernel feature in the first place.

I would like to see Fedora package the RC's in Rawhide as well (other distributions could help by packaging the RC's in unstable as well). That would atleast make the RC's a bit more accessible.
In Fedora it is usually not the kernel that is the problem, it is user space that is generally to old. And as you've said policy is no longer a problem with CIL.

> 
> We could possibly look into lighter weight support for policy
> capabilities.  If the policy compiler toolchain left the capability
> names as strings in the kernel policy (new binary format version), then
> we would no longer need to update libsepol for each new policy
> capability.  The kernel would then turn the list into the bitmap
> internally.  The downside is that we would lose validation of the
> capability names when policy is built, and it isn't clear how the
> kernel should handle unknown names (presently the kernel will simply
> ignore any unknown capabilities in the bitmap).  Failing at policy load
> time would mean we can't enable the capability in policy without making
> it depend on a particular kernel version.
> 
> The lack of any direct relationship between policy capabilities and the
> classes/permissions they affect also can be misleading.  For example,
> someone booting a recent kernel might see warnings about undefined
> classes introduced by the extended_socket_class feature and add those
> class definitions to their policy, which will silence the kernel
> warning and make them think that they are actually using those classes
> now.  But they won't actually get used until they declare the
> capability too in their policy, and the kernel doesn't warn about that
> (nor does it necessarily make sense to do so, since that may be a
> conscious choice by the policy author).  The kernel could however log
> each policy capability and its state as part of its normal logging and
> leave it up to the reader to decide whether those values are correct or
> not.  We also had talked originally about checkpolicy and friends
> possibly warning on inconsistencies, while leaving it up to the policy
> author.
> 
> So:
> 
> 1) Should we investigate lighter weight support for policy
> capabilities, and if so, how?
> 
> 2) Should the kernel log information about enabled/disabled policy
> capabilities in much the same manner as it does for undefined
> classes/permissions?
> 
> 3) Should the policy compiler toolchain warn the user if a policy
> capability is not declared and classes/permissions are used in rules
> that will only be used if that policy capability is declared?  And
> similarly if a policy capability is declared but the corresponding
> classes/permissions are not used in any rules?
> 
> 4) Do we need/want a policy capability for map permission and other
> cases where we are only adding a new permission check? Or should we
> continue to reserve them for cases not addressed via handle_unknown?

Would the map permission "only add a new permission". The way i see it is that it will actually change the meaning of the "execute" existing permission

As a matter of fact: When we add map then what is execute for?

> 
> [1] https://github.com/SELinuxProject/selinux-kernel/issues/13

-- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Policy capabilities: when to use and complications with using
  2017-05-03 16:51 ` Dominick Grift
@ 2017-05-03 17:23   ` Stephen Smalley
  2017-05-04 15:50   ` Paul Moore
  1 sibling, 0 replies; 17+ messages in thread
From: Stephen Smalley @ 2017-05-03 17:23 UTC (permalink / raw)
  To: Dominick Grift, selinux

On Wed, 2017-05-03 at 18:51 +0200, Dominick Grift wrote:
> On Wed, May 03, 2017 at 12:14:16PM -0400, Stephen Smalley wrote:
> > Looking at adding a map permission for mmap [1], and thinking about
> > whether it needs to be wrapped by a policy capability.  On the one
> > hand, we made the open permission depend on a policy capability,
> > but on
> > the other hand, we haven't done that for other cases where we
> > simply
> > added a check of a new permission (recent examples include prlimit
> > checking and module_load).  Compatibility for new permission checks
> > is
> > generally covered in Linux distributions via handle_unknown=allow,
> > and
> > in Android where handle_unknown=deny, the kernel and policy are not
> > independently updated so we can keep them properly synchronized
> > without
> > needing such compatibility measures.
> > 
> > It seems like we generally reserve the use of policy capabilities
> > for
> > when there is a more fundamental change in logic that wouldn't be
> > covered by handle_unknown.  All of the current policy capabilities
> > except for open_perms seem to fit into that category; they change
> > what
> > permissions/classes are checked rather than merely adding a new
> > check,
> > or they change when checks are applied, or they alter labeling
> > behavior.  Even if that is our standard, we haven't consistently
> > defined new policy capabilities for all such changes, e.g. for
> > distinguishing non-init user namespace capability checks (commit
> > 8e4ff6f228e4722cac74db716e308d1da33d744f) or for the earlier
> > updating
> > of netlink socket classes (commit
> > 6c6d2e9bde1c1c87a7ead806f8f5e2181d41a652).  Those changes don't
> > break
> > existing policies because of handle_unknown=allow, but since they
> > are
> > actually changing what is checked and not merely adding new checks,
> > they technically ought to have been wrapped (with
> > handle_unknown=allow,
> > they could end up allowing what would have been previously denied
> > due
> > to changes in the classes).
> > 
> > Part of the reason that we tend to not introduce a new policy
> > capability more often is that it is painful to do so currently.  We
> > have to patch libsepol to recognize the new capability and patch
> > the
> > policy to declare it (although for the latter we can now declare
> > them
> > via a CIL module without modifying the base policy).  And since the
> > policy or module won't build without the updated libsepol, we can't
> > turn on the capability by default in refpolicy without making it
> > dependent on a new libsepol version.  That's why
> > extended_socket_class
> > isn't yet enabled in refpolicy, for example.  That causes
> > enablement
> > and adoption to lag behind.  It also makes it harder to test the
> > new
> > kernel feature in the first place.
> 
> I would like to see Fedora package the RC's in Rawhide as well (other
> distributions could help by packaging the RC's in unstable as well).
> That would atleast make the RC's a bit more accessible.
> In Fedora it is usually not the kernel that is the problem, it is
> user space that is generally to old. And as you've said policy is no
> longer a problem with CIL.
> 
> > 
> > We could possibly look into lighter weight support for policy
> > capabilities.  If the policy compiler toolchain left the capability
> > names as strings in the kernel policy (new binary format version),
> > then
> > we would no longer need to update libsepol for each new policy
> > capability.  The kernel would then turn the list into the bitmap
> > internally.  The downside is that we would lose validation of the
> > capability names when policy is built, and it isn't clear how the
> > kernel should handle unknown names (presently the kernel will
> > simply
> > ignore any unknown capabilities in the bitmap).  Failing at policy
> > load
> > time would mean we can't enable the capability in policy without
> > making
> > it depend on a particular kernel version.
> > 
> > The lack of any direct relationship between policy capabilities and
> > the
> > classes/permissions they affect also can be misleading.  For
> > example,
> > someone booting a recent kernel might see warnings about undefined
> > classes introduced by the extended_socket_class feature and add
> > those
> > class definitions to their policy, which will silence the kernel
> > warning and make them think that they are actually using those
> > classes
> > now.  But they won't actually get used until they declare the
> > capability too in their policy, and the kernel doesn't warn about
> > that
> > (nor does it necessarily make sense to do so, since that may be a
> > conscious choice by the policy author).  The kernel could however
> > log
> > each policy capability and its state as part of its normal logging
> > and
> > leave it up to the reader to decide whether those values are
> > correct or
> > not.  We also had talked originally about checkpolicy and friends
> > possibly warning on inconsistencies, while leaving it up to the
> > policy
> > author.
> > 
> > So:
> > 
> > 1) Should we investigate lighter weight support for policy
> > capabilities, and if so, how?
> > 
> > 2) Should the kernel log information about enabled/disabled policy
> > capabilities in much the same manner as it does for undefined
> > classes/permissions?
> > 
> > 3) Should the policy compiler toolchain warn the user if a policy
> > capability is not declared and classes/permissions are used in
> > rules
> > that will only be used if that policy capability is declared?  And
> > similarly if a policy capability is declared but the corresponding
> > classes/permissions are not used in any rules?
> > 
> > 4) Do we need/want a policy capability for map permission and other
> > cases where we are only adding a new permission check? Or should we
> > continue to reserve them for cases not addressed via
> > handle_unknown?
> 
> Would the map permission "only add a new permission". The way i see
> it is that it will actually change the meaning of the "execute"
> existing permission
> 
> As a matter of fact: When we add map then what is execute for?

No, map would be a new permission that controls the ability to mmap() a
file.  The existing read/write/execute checks during mmap() would
remain unchanged.  This is similar to open permission, which controls
the ability to open() a file, but does not replace the read/write
checks on it.  See the issue link below for the rationale for a
separate map check.

> 
> > 
> > [1] https://github.com/SELinuxProject/selinux-kernel/issues/13
> 
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Policy capabilities: when to use and complications with using
  2017-05-03 16:14 Policy capabilities: when to use and complications with using Stephen Smalley
  2017-05-03 16:51 ` Dominick Grift
@ 2017-05-03 19:35 ` James Carter
  2017-05-04 12:18   ` Chris PeBenito
  2017-05-09 17:49   ` Paul Moore
  1 sibling, 2 replies; 17+ messages in thread
From: James Carter @ 2017-05-03 19:35 UTC (permalink / raw)
  To: Stephen Smalley, Eric Paris, Paul Moore, Joshua Brindle, Chris PeBenito
  Cc: selinux

On 05/03/2017 12:14 PM, Stephen Smalley wrote:
> Looking at adding a map permission for mmap [1], and thinking about
> whether it needs to be wrapped by a policy capability.  On the one
> hand, we made the open permission depend on a policy capability, but on
> the other hand, we haven't done that for other cases where we simply
> added a check of a new permission (recent examples include prlimit
> checking and module_load).  Compatibility for new permission checks is
> generally covered in Linux distributions via handle_unknown=allow, and
> in Android where handle_unknown=deny, the kernel and policy are not
> independently updated so we can keep them properly synchronized without
> needing such compatibility measures.
> 
> It seems like we generally reserve the use of policy capabilities for
> when there is a more fundamental change in logic that wouldn't be
> covered by handle_unknown.  All of the current policy capabilities
> except for open_perms seem to fit into that category; they change what
> permissions/classes are checked rather than merely adding a new check,
> or they change when checks are applied, or they alter labeling
> behavior.  Even if that is our standard, we haven't consistently
> defined new policy capabilities for all such changes, e.g. for
> distinguishing non-init user namespace capability checks (commit
> 8e4ff6f228e4722cac74db716e308d1da33d744f) or for the earlier updating
> of netlink socket classes (commit
> 6c6d2e9bde1c1c87a7ead806f8f5e2181d41a652).  Those changes don't break
> existing policies because of handle_unknown=allow, but since they are
> actually changing what is checked and not merely adding new checks,
> they technically ought to have been wrapped (with handle_unknown=allow,
> they could end up allowing what would have been previously denied due
> to changes in the classes).
> 

I think that there are three cases to consider. (I am ignoring removing checks 
and/or permissions.)

Case 1: Additional checks using existing permissions

This case occurs when additional checks are added but they are very similar to 
checks already being done and there does not seem to be any benefit to adding a 
new permission. New policy is required to allow access where additional checks 
apply. If this requires giving a domain more access then desired, then probably 
a new permission should have been created.
Compatibility:
new kernel - old policy
     Previously unchecked accesses may now be denied. Potential compatibility 
issues.
old kernel - new policy
     No problems


Case 2: New checks using new permissions

This case occurs when additional checks are added and they are not similar to 
other checks, so a new permission would be beneficial. New policy is required to 
allow access where the new checks apply.
Compatibility:
new kernel - old policy
     SEPOL_ALLOW_UNKNOWN - No problems, but not getting the security benefits of 
the new check.
     SEPOL_DENY_UNKNOWN - Access attempts involving the new checks will be 
denied instead of always being allowed like they were before the new check was 
added. Potential compatibility issues.
     SEPOL_REJECT_UNKNOWN - Policy will not load.
old kernel - new policy
     No problem because no check will involve the new permission.


Case 3: Old checks now using new permissions

This case occurs when there is a new use-case that makes splitting an old 
permission into multiple ones worthwhile. New policy is required to allow access 
where the new permissions are required.
Compatibility:
new kernel - old policy
     SEPOL_ALLOW_UNKNOWN - Previously denied access attempts will now be 
allowed. Potential security issues.
     SEPOL_DENY_UNKNOWN - Accesses checked with the new permissions will be 
denied. Potential compatibility issues.
     SEPOL_REJECT_UNKNOWN - Policy will not load
old kernel - new policy
     Denials will occur where old permission is no longer allowed because only 
the new one needs to be allowed for a new kernel. Potential security issues.

Case 2 can be handled adequately by allowing unknown permissions, but the other 
two cases (and case 3 particularly) need a policycap to avoid problems.

This seems to fit with what the practice has been.

> Part of the reason that we tend to not introduce a new policy
> capability more often is that it is painful to do so currently.  We
> have to patch libsepol to recognize the new capability and patch the
> policy to declare it (although for the latter we can now declare them
> via a CIL module without modifying the base policy).  And since the
> policy or module won't build without the updated libsepol, we can't
> turn on the capability by default in refpolicy without making it
> dependent on a new libsepol version.  That's why extended_socket_class
> isn't yet enabled in refpolicy, for example.  That causes enablement
> and adoption to lag behind.  It also makes it harder to test the new
> kernel feature in the first place.
> 
> We could possibly look into lighter weight support for policy
> capabilities.  If the policy compiler toolchain left the capability
> names as strings in the kernel policy (new binary format version), then
> we would no longer need to update libsepol for each new policy
> capability.  The kernel would then turn the list into the bitmap
> internally.  The downside is that we would lose validation of the
> capability names when policy is built, and it isn't clear how the
> kernel should handle unknown names (presently the kernel will simply
> ignore any unknown capabilities in the bitmap).  Failing at policy load
> time would mean we can't enable the capability in policy without making
> it depend on a particular kernel version.
> 

I wonder if in the case of a new permission being added whether the kernel could 
revert to the previous behavior if the new permission is not in the policy. The 
new permission would be an implicit policy capability.

Even if we just included strings in the kernel policy it would still be nice to 
have something in libsepol, so that typos would be caught.

> The lack of any direct relationship between policy capabilities and the
> classes/permissions they affect also can be misleading.  For example,
> someone booting a recent kernel might see warnings about undefined
> classes introduced by the extended_socket_class feature and add those
> class definitions to their policy, which will silence the kernel
> warning and make them think that they are actually using those classes
> now.  But they won't actually get used until they declare the
> capability too in their policy, and the kernel doesn't warn about that
> (nor does it necessarily make sense to do so, since that may be a
> conscious choice by the policy author).  The kernel could however log
> each policy capability and its state as part of its normal logging and
> leave it up to the reader to decide whether those values are correct or
> not.  We also had talked originally about checkpolicy and friends
> possibly warning on inconsistencies, while leaving it up to the policy
> author.
> 

If the permissions acted as an implicit policy capability, that would take care 
of this problem. The kernel could warn that since the new permission is missing 
then these checks would be different.

> So:
> 
> 1) Should we investigate lighter weight support for policy
> capabilities, and if so, how?
> 
> 2) Should the kernel log information about enabled/disabled policy
> capabilities in much the same manner as it does for undefined
> classes/permissions?
> 
> 3) Should the policy compiler toolchain warn the user if a policy
> capability is not declared and classes/permissions are used in rules
> that will only be used if that policy capability is declared?  And
> similarly if a policy capability is declared but the corresponding
> classes/permissions are not used in any rules?
> 
> 4) Do we need/want a policy capability for map permission and other
> cases where we are only adding a new permission check? Or should we
> continue to reserve them for cases not addressed via handle_unknown?
> 

I know you also want this:
5) Should CIL support adding a permission to a new class, so we don't need to 
grab a source rpm and rebuild the whole policy from source just to test a new 
permission?

Jim

> [1] https://github.com/SELinuxProject/selinux-kernel/issues/13
> 


-- 
James Carter <jwcart2@tycho.nsa.gov>
National Security Agency

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Policy capabilities: when to use and complications with using
  2017-05-03 19:35 ` James Carter
@ 2017-05-04 12:18   ` Chris PeBenito
  2017-05-04 16:07     ` Dominick Grift
  2017-05-09 17:49   ` Paul Moore
  1 sibling, 1 reply; 17+ messages in thread
From: Chris PeBenito @ 2017-05-04 12:18 UTC (permalink / raw)
  To: James Carter, Stephen Smalley, Eric Paris, Paul Moore, Joshua Brindle
  Cc: selinux

On 05/03/2017 03:35 PM, James Carter wrote:
> On 05/03/2017 12:14 PM, Stephen Smalley wrote:
>> Looking at adding a map permission for mmap [1], and thinking about
>> whether it needs to be wrapped by a policy capability.  On the one
>> hand, we made the open permission depend on a policy capability, but on
>> the other hand, we haven't done that for other cases where we simply
>> added a check of a new permission (recent examples include prlimit
>> checking and module_load).  Compatibility for new permission checks is
>> generally covered in Linux distributions via handle_unknown=allow, and
>> in Android where handle_unknown=deny, the kernel and policy are not
>> independently updated so we can keep them properly synchronized without
>> needing such compatibility measures.
>>
>> It seems like we generally reserve the use of policy capabilities for
>> when there is a more fundamental change in logic that wouldn't be
>> covered by handle_unknown.  All of the current policy capabilities
>> except for open_perms seem to fit into that category; they change what
>> permissions/classes are checked rather than merely adding a new check,
>> or they change when checks are applied, or they alter labeling
>> behavior.  Even if that is our standard, we haven't consistently
>> defined new policy capabilities for all such changes, e.g. for
>> distinguishing non-init user namespace capability checks (commit
>> 8e4ff6f228e4722cac74db716e308d1da33d744f) or for the earlier updating
>> of netlink socket classes (commit
>> 6c6d2e9bde1c1c87a7ead806f8f5e2181d41a652).  Those changes don't break
>> existing policies because of handle_unknown=allow, but since they are
>> actually changing what is checked and not merely adding new checks,
>> they technically ought to have been wrapped (with handle_unknown=allow,
>> they could end up allowing what would have been previously denied due
>> to changes in the classes).
>>
>
> I think that there are three cases to consider. (I am ignoring removing
> checks and/or permissions.)
>
> Case 1: Additional checks using existing permissions
>
> This case occurs when additional checks are added but they are very
> similar to checks already being done and there does not seem to be any
> benefit to adding a new permission. New policy is required to allow
> access where additional checks apply. If this requires giving a domain
> more access then desired, then probably a new permission should have
> been created.
> Compatibility:
> new kernel - old policy
>     Previously unchecked accesses may now be denied. Potential
> compatibility issues.
> old kernel - new policy
>     No problems
>
>
> Case 2: New checks using new permissions
>
> This case occurs when additional checks are added and they are not
> similar to other checks, so a new permission would be beneficial. New
> policy is required to allow access where the new checks apply.
> Compatibility:
> new kernel - old policy
>     SEPOL_ALLOW_UNKNOWN - No problems, but not getting the security
> benefits of the new check.
>     SEPOL_DENY_UNKNOWN - Access attempts involving the new checks will
> be denied instead of always being allowed like they were before the new
> check was added. Potential compatibility issues.
>     SEPOL_REJECT_UNKNOWN - Policy will not load.
> old kernel - new policy
>     No problem because no check will involve the new permission.
>
>
> Case 3: Old checks now using new permissions
>
> This case occurs when there is a new use-case that makes splitting an
> old permission into multiple ones worthwhile. New policy is required to
> allow access where the new permissions are required.
> Compatibility:
> new kernel - old policy
>     SEPOL_ALLOW_UNKNOWN - Previously denied access attempts will now be
> allowed. Potential security issues.
>     SEPOL_DENY_UNKNOWN - Accesses checked with the new permissions will
> be denied. Potential compatibility issues.
>     SEPOL_REJECT_UNKNOWN - Policy will not load
> old kernel - new policy
>     Denials will occur where old permission is no longer allowed because
> only the new one needs to be allowed for a new kernel. Potential
> security issues.
>
> Case 2 can be handled adequately by allowing unknown permissions, but
> the other two cases (and case 3 particularly) need a policycap to avoid
> problems.
>
> This seems to fit with what the practice has been.
>
>> Part of the reason that we tend to not introduce a new policy
>> capability more often is that it is painful to do so currently.  We
>> have to patch libsepol to recognize the new capability and patch the
>> policy to declare it (although for the latter we can now declare them
>> via a CIL module without modifying the base policy).  And since the
>> policy or module won't build without the updated libsepol, we can't
>> turn on the capability by default in refpolicy without making it
>> dependent on a new libsepol version.  That's why extended_socket_class
>> isn't yet enabled in refpolicy, for example.  That causes enablement
>> and adoption to lag behind.  It also makes it harder to test the new
>> kernel feature in the first place.
>>
>> We could possibly look into lighter weight support for policy
>> capabilities.  If the policy compiler toolchain left the capability
>> names as strings in the kernel policy (new binary format version), then
>> we would no longer need to update libsepol for each new policy
>> capability.  The kernel would then turn the list into the bitmap
>> internally.  The downside is that we would lose validation of the
>> capability names when policy is built, and it isn't clear how the
>> kernel should handle unknown names (presently the kernel will simply
>> ignore any unknown capabilities in the bitmap).  Failing at policy load
>> time would mean we can't enable the capability in policy without making
>> it depend on a particular kernel version.
>>
>
> I wonder if in the case of a new permission being added whether the
> kernel could revert to the previous behavior if the new permission is
> not in the policy. The new permission would be an implicit policy
> capability.

I'm not a fan of implicit behaviors.  It leads to unexplained behavior 
for the uninitiated.


> Even if we just included strings in the kernel policy it would still be
> nice to have something in libsepol, so that typos would be caught.
>
>> The lack of any direct relationship between policy capabilities and the
>> classes/permissions they affect also can be misleading.  For example,
>> someone booting a recent kernel might see warnings about undefined
>> classes introduced by the extended_socket_class feature and add those
>> class definitions to their policy, which will silence the kernel
>> warning and make them think that they are actually using those classes
>> now.  But they won't actually get used until they declare the
>> capability too in their policy, and the kernel doesn't warn about that
>> (nor does it necessarily make sense to do so, since that may be a
>> conscious choice by the policy author).  The kernel could however log
>> each policy capability and its state as part of its normal logging and
>> leave it up to the reader to decide whether those values are correct or
>> not.  We also had talked originally about checkpolicy and friends
>> possibly warning on inconsistencies, while leaving it up to the policy
>> author.
>>
>
> If the permissions acted as an implicit policy capability, that would
> take care of this problem. The kernel could warn that since the new
> permission is missing then these checks would be different.
>
>> So:
>>
>> 1) Should we investigate lighter weight support for policy
>> capabilities, and if so, how?
>>
>> 2) Should the kernel log information about enabled/disabled policy
>> capabilities in much the same manner as it does for undefined
>> classes/permissions?

Yes, I think it should.


>> 3) Should the policy compiler toolchain warn the user if a policy
>> capability is not declared and classes/permissions are used in rules
>> that will only be used if that policy capability is declared?  And
>> similarly if a policy capability is declared but the corresponding
>> classes/permissions are not used in any rules?

I think this is yes too.  For policy maintainers, it keeps reminding us 
there is something important to address (I admit I forgot about 
extended_socket_class).  For end users, it notifies them that their 
policy may not have the intended effect.


>> 4) Do we need/want a policy capability for map permission and other
>> cases where we are only adding a new permission check? Or should we
>> continue to reserve them for cases not addressed via handle_unknown?

I haven't come to a conclusion for a general rule, but for map, I'd 
expect that there wouldn't be a big impact by adding it in.  The (my 
guess) 99% would be handled by the one refpolicy interface that allows 
all the general shared lib access.


> I know you also want this:
> 5) Should CIL support adding a permission to a new class, so we don't
> need to grab a source rpm and rebuild the whole policy from source just
> to test a new permission?
>
> Jim
>
>> [1] https://github.com/SELinuxProject/selinux-kernel/issues/13
>>
>
>


-- 
Chris PeBenito

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Policy capabilities: when to use and complications with using
  2017-05-03 16:51 ` Dominick Grift
  2017-05-03 17:23   ` Stephen Smalley
@ 2017-05-04 15:50   ` Paul Moore
  2017-05-04 17:42     ` Dominick Grift
  1 sibling, 1 reply; 17+ messages in thread
From: Paul Moore @ 2017-05-04 15:50 UTC (permalink / raw)
  To: selinux

On Wed, May 3, 2017 at 12:51 PM, Dominick Grift <dac.override@gmail.com> wrote:
> On Wed, May 03, 2017 at 12:14:16PM -0400, Stephen Smalley wrote:
>> Part of the reason that we tend to not introduce a new policy
>> capability more often is that it is painful to do so currently.  We
>> have to patch libsepol to recognize the new capability and patch the
>> policy to declare it (although for the latter we can now declare them
>> via a CIL module without modifying the base policy).  And since the
>> policy or module won't build without the updated libsepol, we can't
>> turn on the capability by default in refpolicy without making it
>> dependent on a new libsepol version.  That's why extended_socket_class
>> isn't yet enabled in refpolicy, for example.  That causes enablement
>> and adoption to lag behind.  It also makes it harder to test the new
>> kernel feature in the first place.
>
> I would like to see Fedora package the RC's in Rawhide as well (other distributions could help by packaging the RC's in unstable as well). That would atleast make the RC's a bit more accessible.
> In Fedora it is usually not the kernel that is the problem, it is user space that is generally to old. And as you've said policy is no longer a problem with CIL.

[NOTE: I'm still thinking about the rest of Stephen's email, and the
follow up comments, but I wanted to reply to this particular comment
separately.]

I'm not sure I want to see SELinux userspace release candidates in
normal Rawhide, but I think creating a COPR repository to
build/distribute release candidates could be a good thing.  We already
do something similar for the kernel patches and it has been helpful in
my opinion.

https://copr.fedorainfracloud.org
https://copr.fedorainfracloud.org/coprs/pcmoore/kernel-secnext

-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Policy capabilities: when to use and complications with using
  2017-05-04 12:18   ` Chris PeBenito
@ 2017-05-04 16:07     ` Dominick Grift
  0 siblings, 0 replies; 17+ messages in thread
From: Dominick Grift @ 2017-05-04 16:07 UTC (permalink / raw)
  To: selinux

[-- Attachment #1: Type: text/plain, Size: 10329 bytes --]

On Thu, May 04, 2017 at 08:18:10AM -0400, Chris PeBenito wrote:
> On 05/03/2017 03:35 PM, James Carter wrote:
> > On 05/03/2017 12:14 PM, Stephen Smalley wrote:
> > > Looking at adding a map permission for mmap [1], and thinking about
> > > whether it needs to be wrapped by a policy capability.  On the one
> > > hand, we made the open permission depend on a policy capability, but on
> > > the other hand, we haven't done that for other cases where we simply
> > > added a check of a new permission (recent examples include prlimit
> > > checking and module_load).  Compatibility for new permission checks is
> > > generally covered in Linux distributions via handle_unknown=allow, and
> > > in Android where handle_unknown=deny, the kernel and policy are not
> > > independently updated so we can keep them properly synchronized without
> > > needing such compatibility measures.
> > > 
> > > It seems like we generally reserve the use of policy capabilities for
> > > when there is a more fundamental change in logic that wouldn't be
> > > covered by handle_unknown.  All of the current policy capabilities
> > > except for open_perms seem to fit into that category; they change what
> > > permissions/classes are checked rather than merely adding a new check,
> > > or they change when checks are applied, or they alter labeling
> > > behavior.  Even if that is our standard, we haven't consistently
> > > defined new policy capabilities for all such changes, e.g. for
> > > distinguishing non-init user namespace capability checks (commit
> > > 8e4ff6f228e4722cac74db716e308d1da33d744f) or for the earlier updating
> > > of netlink socket classes (commit
> > > 6c6d2e9bde1c1c87a7ead806f8f5e2181d41a652).  Those changes don't break
> > > existing policies because of handle_unknown=allow, but since they are
> > > actually changing what is checked and not merely adding new checks,
> > > they technically ought to have been wrapped (with handle_unknown=allow,
> > > they could end up allowing what would have been previously denied due
> > > to changes in the classes).
> > > 
> > 
> > I think that there are three cases to consider. (I am ignoring removing
> > checks and/or permissions.)
> > 
> > Case 1: Additional checks using existing permissions
> > 
> > This case occurs when additional checks are added but they are very
> > similar to checks already being done and there does not seem to be any
> > benefit to adding a new permission. New policy is required to allow
> > access where additional checks apply. If this requires giving a domain
> > more access then desired, then probably a new permission should have
> > been created.
> > Compatibility:
> > new kernel - old policy
> >     Previously unchecked accesses may now be denied. Potential
> > compatibility issues.
> > old kernel - new policy
> >     No problems
> > 
> > 
> > Case 2: New checks using new permissions
> > 
> > This case occurs when additional checks are added and they are not
> > similar to other checks, so a new permission would be beneficial. New
> > policy is required to allow access where the new checks apply.
> > Compatibility:
> > new kernel - old policy
> >     SEPOL_ALLOW_UNKNOWN - No problems, but not getting the security
> > benefits of the new check.
> >     SEPOL_DENY_UNKNOWN - Access attempts involving the new checks will
> > be denied instead of always being allowed like they were before the new
> > check was added. Potential compatibility issues.
> >     SEPOL_REJECT_UNKNOWN - Policy will not load.
> > old kernel - new policy
> >     No problem because no check will involve the new permission.
> > 
> > 
> > Case 3: Old checks now using new permissions
> > 
> > This case occurs when there is a new use-case that makes splitting an
> > old permission into multiple ones worthwhile. New policy is required to
> > allow access where the new permissions are required.
> > Compatibility:
> > new kernel - old policy
> >     SEPOL_ALLOW_UNKNOWN - Previously denied access attempts will now be
> > allowed. Potential security issues.
> >     SEPOL_DENY_UNKNOWN - Accesses checked with the new permissions will
> > be denied. Potential compatibility issues.
> >     SEPOL_REJECT_UNKNOWN - Policy will not load
> > old kernel - new policy
> >     Denials will occur where old permission is no longer allowed because
> > only the new one needs to be allowed for a new kernel. Potential
> > security issues.
> > 
> > Case 2 can be handled adequately by allowing unknown permissions, but
> > the other two cases (and case 3 particularly) need a policycap to avoid
> > problems.
> > 
> > This seems to fit with what the practice has been.
> > 
> > > Part of the reason that we tend to not introduce a new policy
> > > capability more often is that it is painful to do so currently.  We
> > > have to patch libsepol to recognize the new capability and patch the
> > > policy to declare it (although for the latter we can now declare them
> > > via a CIL module without modifying the base policy).  And since the
> > > policy or module won't build without the updated libsepol, we can't
> > > turn on the capability by default in refpolicy without making it
> > > dependent on a new libsepol version.  That's why extended_socket_class
> > > isn't yet enabled in refpolicy, for example.  That causes enablement
> > > and adoption to lag behind.  It also makes it harder to test the new
> > > kernel feature in the first place.
> > > 
> > > We could possibly look into lighter weight support for policy
> > > capabilities.  If the policy compiler toolchain left the capability
> > > names as strings in the kernel policy (new binary format version), then
> > > we would no longer need to update libsepol for each new policy
> > > capability.  The kernel would then turn the list into the bitmap
> > > internally.  The downside is that we would lose validation of the
> > > capability names when policy is built, and it isn't clear how the
> > > kernel should handle unknown names (presently the kernel will simply
> > > ignore any unknown capabilities in the bitmap).  Failing at policy load
> > > time would mean we can't enable the capability in policy without making
> > > it depend on a particular kernel version.
> > > 
> > 
> > I wonder if in the case of a new permission being added whether the
> > kernel could revert to the previous behavior if the new permission is
> > not in the policy. The new permission would be an implicit policy
> > capability.
> 
> I'm not a fan of implicit behaviors.  It leads to unexplained behavior for
> the uninitiated.
> 
> 
> > Even if we just included strings in the kernel policy it would still be
> > nice to have something in libsepol, so that typos would be caught.
> > 
> > > The lack of any direct relationship between policy capabilities and the
> > > classes/permissions they affect also can be misleading.  For example,
> > > someone booting a recent kernel might see warnings about undefined
> > > classes introduced by the extended_socket_class feature and add those
> > > class definitions to their policy, which will silence the kernel
> > > warning and make them think that they are actually using those classes
> > > now.  But they won't actually get used until they declare the
> > > capability too in their policy, and the kernel doesn't warn about that
> > > (nor does it necessarily make sense to do so, since that may be a
> > > conscious choice by the policy author).  The kernel could however log
> > > each policy capability and its state as part of its normal logging and
> > > leave it up to the reader to decide whether those values are correct or
> > > not.  We also had talked originally about checkpolicy and friends
> > > possibly warning on inconsistencies, while leaving it up to the policy
> > > author.
> > > 
> > 
> > If the permissions acted as an implicit policy capability, that would
> > take care of this problem. The kernel could warn that since the new
> > permission is missing then these checks would be different.
> > 
> > > So:
> > > 
> > > 1) Should we investigate lighter weight support for policy
> > > capabilities, and if so, how?
> > > 
> > > 2) Should the kernel log information about enabled/disabled policy
> > > capabilities in much the same manner as it does for undefined
> > > classes/permissions?
> 
> Yes, I think it should.
> 
> 
> > > 3) Should the policy compiler toolchain warn the user if a policy
> > > capability is not declared and classes/permissions are used in rules
> > > that will only be used if that policy capability is declared?  And
> > > similarly if a policy capability is declared but the corresponding
> > > classes/permissions are not used in any rules?
> 
> I think this is yes too.  For policy maintainers, it keeps reminding us
> there is something important to address (I admit I forgot about
> extended_socket_class).  For end users, it notifies them that their policy

Don't forget the cgroup_seclabel polcap. Paul Moore does a really good job at writing about these changes in his blog here:

http://www.paul-moore.com/blog/d/2017/05/linux-v411.html



> may not have the intended effect.
> 
> 
> > > 4) Do we need/want a policy capability for map permission and other
> > > cases where we are only adding a new permission check? Or should we
> > > continue to reserve them for cases not addressed via handle_unknown?
> 
> I haven't come to a conclusion for a general rule, but for map, I'd expect
> that there wouldn't be a big impact by adding it in.  The (my guess) 99%
> would be handled by the one refpolicy interface that allows all the general
> shared lib access.
> 
> 
> > I know you also want this:
> > 5) Should CIL support adding a permission to a new class, so we don't
> > need to grab a source rpm and rebuild the whole policy from source just
> > to test a new permission?
> > 
> > Jim
> > 
> > > [1] https://github.com/SELinuxProject/selinux-kernel/issues/13
> > > 
> > 
> > 
> 
> 
> -- 
> Chris PeBenito

-- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Policy capabilities: when to use and complications with using
  2017-05-04 15:50   ` Paul Moore
@ 2017-05-04 17:42     ` Dominick Grift
  2017-05-04 17:50       ` Dominick Grift
  0 siblings, 1 reply; 17+ messages in thread
From: Dominick Grift @ 2017-05-04 17:42 UTC (permalink / raw)
  To: selinux

[-- Attachment #1: Type: text/plain, Size: 2414 bytes --]

On Thu, May 04, 2017 at 11:50:15AM -0400, Paul Moore wrote:
> On Wed, May 3, 2017 at 12:51 PM, Dominick Grift <dac.override@gmail.com> wrote:
> > On Wed, May 03, 2017 at 12:14:16PM -0400, Stephen Smalley wrote:
> >> Part of the reason that we tend to not introduce a new policy
> >> capability more often is that it is painful to do so currently.  We
> >> have to patch libsepol to recognize the new capability and patch the
> >> policy to declare it (although for the latter we can now declare them
> >> via a CIL module without modifying the base policy).  And since the
> >> policy or module won't build without the updated libsepol, we can't
> >> turn on the capability by default in refpolicy without making it
> >> dependent on a new libsepol version.  That's why extended_socket_class
> >> isn't yet enabled in refpolicy, for example.  That causes enablement
> >> and adoption to lag behind.  It also makes it harder to test the new
> >> kernel feature in the first place.
> >
> > I would like to see Fedora package the RC's in Rawhide as well (other distributions could help by packaging the RC's in unstable as well). That would atleast make the RC's a bit more accessible.
> > In Fedora it is usually not the kernel that is the problem, it is user space that is generally to old. And as you've said policy is no longer a problem with CIL.
> 
> [NOTE: I'm still thinking about the rest of Stephen's email, and the
> follow up comments, but I wanted to reply to this particular comment
> separately.]
> 
> I'm not sure I want to see SELinux userspace release candidates in
> normal Rawhide, but I think creating a COPR repository to
> build/distribute release candidates could be a good thing.  We already
> do something similar for the kernel patches and it has been helpful in
> my opinion.

Thanks, Yes i suppose you are right. Release Candidates would probably potentially cause too much disruption even in Rawhide.
COPR should do the job, although will not be as accessible as Rawhide. It won't get the same kind of attention, but it will do for me.

> 
> https://copr.fedorainfracloud.org
> https://copr.fedorainfracloud.org/coprs/pcmoore/kernel-secnext
> 
> -- 
> paul moore
> www.paul-moore.com

-- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Policy capabilities: when to use and complications with using
  2017-05-04 17:42     ` Dominick Grift
@ 2017-05-04 17:50       ` Dominick Grift
  2017-05-04 19:22         ` Petr Lautrbach
  0 siblings, 1 reply; 17+ messages in thread
From: Dominick Grift @ 2017-05-04 17:50 UTC (permalink / raw)
  To: selinux

[-- Attachment #1: Type: text/plain, Size: 2957 bytes --]

On Thu, May 04, 2017 at 07:42:40PM +0200, Dominick Grift wrote:
> On Thu, May 04, 2017 at 11:50:15AM -0400, Paul Moore wrote:
> > On Wed, May 3, 2017 at 12:51 PM, Dominick Grift <dac.override@gmail.com> wrote:
> > > On Wed, May 03, 2017 at 12:14:16PM -0400, Stephen Smalley wrote:
> > >> Part of the reason that we tend to not introduce a new policy
> > >> capability more often is that it is painful to do so currently.  We
> > >> have to patch libsepol to recognize the new capability and patch the
> > >> policy to declare it (although for the latter we can now declare them
> > >> via a CIL module without modifying the base policy).  And since the
> > >> policy or module won't build without the updated libsepol, we can't
> > >> turn on the capability by default in refpolicy without making it
> > >> dependent on a new libsepol version.  That's why extended_socket_class
> > >> isn't yet enabled in refpolicy, for example.  That causes enablement
> > >> and adoption to lag behind.  It also makes it harder to test the new
> > >> kernel feature in the first place.
> > >
> > > I would like to see Fedora package the RC's in Rawhide as well (other distributions could help by packaging the RC's in unstable as well). That would atleast make the RC's a bit more accessible.
> > > In Fedora it is usually not the kernel that is the problem, it is user space that is generally to old. And as you've said policy is no longer a problem with CIL.
> > 
> > [NOTE: I'm still thinking about the rest of Stephen's email, and the
> > follow up comments, but I wanted to reply to this particular comment
> > separately.]
> > 
> > I'm not sure I want to see SELinux userspace release candidates in
> > normal Rawhide, but I think creating a COPR repository to
> > build/distribute release candidates could be a good thing.  We already
> > do something similar for the kernel patches and it has been helpful in
> > my opinion.
> 
> Thanks, Yes i suppose you are right. Release Candidates would probably potentially cause too much disruption even in Rawhide.
> COPR should do the job, although will not be as accessible as Rawhide. It won't get the same kind of attention, but it will do for me.

With COPR though we might be able to package more frequent and not just RC's (weekly's/nightly's)? If that can somehow be automated  then we also do not have to worrie so much about keeping things maintained over time

> 
> > 
> > https://copr.fedorainfracloud.org
> > https://copr.fedorainfracloud.org/coprs/pcmoore/kernel-secnext
> > 
> > -- 
> > paul moore
> > www.paul-moore.com
> 
> -- 
> Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
> https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
> Dominick Grift



-- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Policy capabilities: when to use and complications with using
  2017-05-04 17:50       ` Dominick Grift
@ 2017-05-04 19:22         ` Petr Lautrbach
  2017-05-04 19:44           ` Dominick Grift
  2017-05-09 18:03           ` Paul Moore
  0 siblings, 2 replies; 17+ messages in thread
From: Petr Lautrbach @ 2017-05-04 19:22 UTC (permalink / raw)
  To: selinux

On 05/04/2017 07:50 PM, Dominick Grift wrote:
> On Thu, May 04, 2017 at 07:42:40PM +0200, Dominick Grift wrote:
>> On Thu, May 04, 2017 at 11:50:15AM -0400, Paul Moore wrote:
>>> On Wed, May 3, 2017 at 12:51 PM, Dominick Grift <dac.override@gmail.com> wrote:
>>>> On Wed, May 03, 2017 at 12:14:16PM -0400, Stephen Smalley wrote:
>>>>> Part of the reason that we tend to not introduce a new policy
>>>>> capability more often is that it is painful to do so currently.  We
>>>>> have to patch libsepol to recognize the new capability and patch the
>>>>> policy to declare it (although for the latter we can now declare them
>>>>> via a CIL module without modifying the base policy).  And since the
>>>>> policy or module won't build without the updated libsepol, we can't
>>>>> turn on the capability by default in refpolicy without making it
>>>>> dependent on a new libsepol version.  That's why extended_socket_class
>>>>> isn't yet enabled in refpolicy, for example.  That causes enablement
>>>>> and adoption to lag behind.  It also makes it harder to test the new
>>>>> kernel feature in the first place.
>>>>
>>>> I would like to see Fedora package the RC's in Rawhide as well (other distributions could help by packaging the RC's in unstable as well). That would atleast make the RC's a bit more accessible.
>>>> In Fedora it is usually not the kernel that is the problem, it is user space that is generally to old. And as you've said policy is no longer a problem with CIL.
>>>
>>> [NOTE: I'm still thinking about the rest of Stephen's email, and the
>>> follow up comments, but I wanted to reply to this particular comment
>>> separately.]
>>>
>>> I'm not sure I want to see SELinux userspace release candidates in
>>> normal Rawhide, but I think creating a COPR repository to
>>> build/distribute release candidates could be a good thing.  We already
>>> do something similar for the kernel patches and it has been helpful in
>>> my opinion.
>>
>> Thanks, Yes i suppose you are right. Release Candidates would probably potentially cause too much disruption even in Rawhide.
>> COPR should do the job, although will not be as accessible as Rawhide. It won't get the same kind of attention, but it will do for me.
> 
> With COPR though we might be able to package more frequent and not just RC's (weekly's/nightly's)? If that can somehow be automated  then we also do not have to worrie so much about keeping things maintained over time


I'm just building new set of updates in my COPR plautrba/selinux
repository [1]. It's based on latest upstream sources with some Fedora
patches on the top of it currently tracked in my github tree [2]. But
there are some problems and it's not ready yet.

I used to build vanilla upstream sources [3] but the latest build is 15
months old. I can restart this project if there's an interest.

Since COPR provides API with an authentication token, builds can
automated and I have few scripts I used before.

I think it could even work for Rawhide with less frequent update cycle.

[1] https://copr.fedorainfracloud.org/coprs/plautrba/selinux/
[2] https://github.com/bachradsusi/selinux/tree/WIP-master
[3] https://copr.fedorainfracloud.org/coprs/plautrba/selinux-master/builds/

Petr

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Policy capabilities: when to use and complications with using
  2017-05-04 19:22         ` Petr Lautrbach
@ 2017-05-04 19:44           ` Dominick Grift
  2017-05-09 18:03           ` Paul Moore
  1 sibling, 0 replies; 17+ messages in thread
From: Dominick Grift @ 2017-05-04 19:44 UTC (permalink / raw)
  To: selinux

[-- Attachment #1: Type: text/plain, Size: 4164 bytes --]

On Thu, May 04, 2017 at 09:22:22PM +0200, Petr Lautrbach wrote:
> On 05/04/2017 07:50 PM, Dominick Grift wrote:
> > On Thu, May 04, 2017 at 07:42:40PM +0200, Dominick Grift wrote:
> >> On Thu, May 04, 2017 at 11:50:15AM -0400, Paul Moore wrote:
> >>> On Wed, May 3, 2017 at 12:51 PM, Dominick Grift <dac.override@gmail.com> wrote:
> >>>> On Wed, May 03, 2017 at 12:14:16PM -0400, Stephen Smalley wrote:
> >>>>> Part of the reason that we tend to not introduce a new policy
> >>>>> capability more often is that it is painful to do so currently.  We
> >>>>> have to patch libsepol to recognize the new capability and patch the
> >>>>> policy to declare it (although for the latter we can now declare them
> >>>>> via a CIL module without modifying the base policy).  And since the
> >>>>> policy or module won't build without the updated libsepol, we can't
> >>>>> turn on the capability by default in refpolicy without making it
> >>>>> dependent on a new libsepol version.  That's why extended_socket_class
> >>>>> isn't yet enabled in refpolicy, for example.  That causes enablement
> >>>>> and adoption to lag behind.  It also makes it harder to test the new
> >>>>> kernel feature in the first place.
> >>>>
> >>>> I would like to see Fedora package the RC's in Rawhide as well (other distributions could help by packaging the RC's in unstable as well). That would atleast make the RC's a bit more accessible.
> >>>> In Fedora it is usually not the kernel that is the problem, it is user space that is generally to old. And as you've said policy is no longer a problem with CIL.
> >>>
> >>> [NOTE: I'm still thinking about the rest of Stephen's email, and the
> >>> follow up comments, but I wanted to reply to this particular comment
> >>> separately.]
> >>>
> >>> I'm not sure I want to see SELinux userspace release candidates in
> >>> normal Rawhide, but I think creating a COPR repository to
> >>> build/distribute release candidates could be a good thing.  We already
> >>> do something similar for the kernel patches and it has been helpful in
> >>> my opinion.
> >>
> >> Thanks, Yes i suppose you are right. Release Candidates would probably potentially cause too much disruption even in Rawhide.
> >> COPR should do the job, although will not be as accessible as Rawhide. It won't get the same kind of attention, but it will do for me.
> > 
> > With COPR though we might be able to package more frequent and not just RC's (weekly's/nightly's)? If that can somehow be automated  then we also do not have to worrie so much about keeping things maintained over time
> 
> 
> I'm just building new set of updates in my COPR plautrba/selinux
> repository [1]. It's based on latest upstream sources with some Fedora
> patches on the top of it currently tracked in my github tree [2]. But
> there are some problems and it's not ready yet.

Most of the fedora specific patches I will probably not be able to test. I don't (can't) use semanage, policycoreutils-GUI etc.
I am mainly able to test libselinux, libsepol and policycoreutils

> 
> I used to build vanilla upstream sources [3] but the latest build is 15
> months old. I can restart this project if there's an interest.

That would be nice, especially if it could be automated. So that I have some kind of assurance that it is maintained.
I dont add COPRs much but when i do i intent to keep using them consistently. I wouldnt want to have dead COPR repositories to worry about.

> 
> Since COPR provides API with an authentication token, builds can
> automated and I have few scripts I used before.
> 
> I think it could even work for Rawhide with less frequent update cycle.

I would prefer Rawhide because then we reach a broader audience.

> 
> [1] https://copr.fedorainfracloud.org/coprs/plautrba/selinux/
> [2] https://github.com/bachradsusi/selinux/tree/WIP-master
> [3] https://copr.fedorainfracloud.org/coprs/plautrba/selinux-master/builds/
> 
> Petr

Thanks!

-- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Policy capabilities: when to use and complications with using
  2017-05-03 19:35 ` James Carter
  2017-05-04 12:18   ` Chris PeBenito
@ 2017-05-09 17:49   ` Paul Moore
  2017-05-09 20:39     ` Stephen Smalley
  1 sibling, 1 reply; 17+ messages in thread
From: Paul Moore @ 2017-05-09 17:49 UTC (permalink / raw)
  To: James Carter, Stephen Smalley
  Cc: Eric Paris, Joshua Brindle, Chris PeBenito, selinux

On Wed, May 3, 2017 at 3:35 PM, James Carter <jwcart2@tycho.nsa.gov> wrote:
> On 05/03/2017 12:14 PM, Stephen Smalley wrote:

...

> I think that there are three cases to consider. (I am ignoring removing
> checks and/or permissions.)
>
> Case 1: Additional checks using existing permissions
>
> This case occurs when additional checks are added but they are very similar
> to checks already being done and there does not seem to be any benefit to
> adding a new permission. New policy is required to allow access where
> additional checks apply. If this requires giving a domain more access then
> desired, then probably a new permission should have been created.
> Compatibility:
> new kernel - old policy
>     Previously unchecked accesses may now be denied. Potential compatibility
> issues.
> old kernel - new policy
>     No problems
>
>
> Case 2: New checks using new permissions
>
> This case occurs when additional checks are added and they are not similar
> to other checks, so a new permission would be beneficial. New policy is
> required to allow access where the new checks apply.
> Compatibility:
> new kernel - old policy
>     SEPOL_ALLOW_UNKNOWN - No problems, but not getting the security benefits
> of the new check.
>     SEPOL_DENY_UNKNOWN - Access attempts involving the new checks will be
> denied instead of always being allowed like they were before the new check
> was added. Potential compatibility issues.
>     SEPOL_REJECT_UNKNOWN - Policy will not load.
> old kernel - new policy
>     No problem because no check will involve the new permission.
>
>
> Case 3: Old checks now using new permissions
>
> This case occurs when there is a new use-case that makes splitting an old
> permission into multiple ones worthwhile. New policy is required to allow
> access where the new permissions are required.
> Compatibility:
> new kernel - old policy
>     SEPOL_ALLOW_UNKNOWN - Previously denied access attempts will now be
> allowed. Potential security issues.
>     SEPOL_DENY_UNKNOWN - Accesses checked with the new permissions will be
> denied. Potential compatibility issues.
>     SEPOL_REJECT_UNKNOWN - Policy will not load
> old kernel - new policy
>     Denials will occur where old permission is no longer allowed because
> only the new one needs to be allowed for a new kernel. Potential security
> issues.
>
> Case 2 can be handled adequately by allowing unknown permissions, but the
> other two cases (and case 3 particularly) need a policycap to avoid
> problems.
>
> This seems to fit with what the practice has been.

I agree with all of the above.

>> Part of the reason that we tend to not introduce a new policy
>> capability more often is that it is painful to do so currently.  We
>> have to patch libsepol to recognize the new capability and patch the
>> policy to declare it (although for the latter we can now declare them
>> via a CIL module without modifying the base policy).  And since the
>> policy or module won't build without the updated libsepol, we can't
>> turn on the capability by default in refpolicy without making it
>> dependent on a new libsepol version.  That's why extended_socket_class
>> isn't yet enabled in refpolicy, for example.  That causes enablement
>> and adoption to lag behind.  It also makes it harder to test the new
>> kernel feature in the first place.
>>
>> We could possibly look into lighter weight support for policy
>> capabilities.  If the policy compiler toolchain left the capability
>> names as strings in the kernel policy (new binary format version), then
>> we would no longer need to update libsepol for each new policy
>> capability.  The kernel would then turn the list into the bitmap
>> internally.  The downside is that we would lose validation of the
>> capability names when policy is built, and it isn't clear how the
>> kernel should handle unknown names (presently the kernel will simply
>> ignore any unknown capabilities in the bitmap).  Failing at policy load
>> time would mean we can't enable the capability in policy without making
>> it depend on a particular kernel version.
>>
>
> I wonder if in the case of a new permission being added whether the kernel
> could revert to the previous behavior if the new permission is not in the
> policy. The new permission would be an implicit policy capability.

Not all policy capabilities are due to new permissions, e.g. the
"always_check_network" capability.  The policy capability
functionality is a way of introducing backwards-incompatible changes
into the kernel in a manner which (hopefully) doesn't break existing
policies.  While new/changed permissions are the obvious things, I
don't want to rule out use of policy capabilities for working around
other changes we may want to make in the future.

>> So:
>>
>> 1) Should we investigate lighter weight support for policy
>> capabilities, and if so, how?

I agree that not having to update userspace for each new policy
capability is a desirable goal, but I'm hopeful that changes requiring
a new policy capability are kept to a minimum.

>> 2) Should the kernel log information about enabled/disabled policy
>> capabilities in much the same manner as it does for undefined
>> classes/permissions?

This seems like a very good idea to me.

* https://github.com/SELinuxProject/selinux-kernel/issues/32

>> 3) Should the policy compiler toolchain warn the user if a policy
>> capability is not declared and classes/permissions are used in rules
>> that will only be used if that policy capability is declared?  And
>> similarly if a policy capability is declared but the corresponding
>> classes/permissions are not used in any rules?

This seems to go against lighter weight policy capabilities, would it not?

>> 4) Do we need/want a policy capability for map permission and other
>> cases where we are only adding a new permission check? Or should we
>> continue to reserve them for cases not addressed via handle_unknown?

See James' earlier comments.  I think sticking with the current usage
is the "best practice", but I think we should reserve the right to
treat each change individually.  I'm hopeful that changes where we
consider new policy capabilities remain infrequent enough that we can
along without a concrete policy on their use.

> I know you also want this:
> 5) Should CIL support adding a permission to a new class, so we don't need
> to grab a source rpm and rebuild the whole policy from source just to test a
> new permission?

-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Policy capabilities: when to use and complications with using
  2017-05-04 19:22         ` Petr Lautrbach
  2017-05-04 19:44           ` Dominick Grift
@ 2017-05-09 18:03           ` Paul Moore
  1 sibling, 0 replies; 17+ messages in thread
From: Paul Moore @ 2017-05-09 18:03 UTC (permalink / raw)
  To: Petr Lautrbach; +Cc: selinux

On Thu, May 4, 2017 at 3:22 PM, Petr Lautrbach <plautrba@redhat.com> wrote:
> On 05/04/2017 07:50 PM, Dominick Grift wrote:
>> On Thu, May 04, 2017 at 07:42:40PM +0200, Dominick Grift wrote:
>>> On Thu, May 04, 2017 at 11:50:15AM -0400, Paul Moore wrote:
>>>> On Wed, May 3, 2017 at 12:51 PM, Dominick Grift <dac.override@gmail.com> wrote:
>>>>> On Wed, May 03, 2017 at 12:14:16PM -0400, Stephen Smalley wrote:
>>>>>> Part of the reason that we tend to not introduce a new policy
>>>>>> capability more often is that it is painful to do so currently.  We
>>>>>> have to patch libsepol to recognize the new capability and patch the
>>>>>> policy to declare it (although for the latter we can now declare them
>>>>>> via a CIL module without modifying the base policy).  And since the
>>>>>> policy or module won't build without the updated libsepol, we can't
>>>>>> turn on the capability by default in refpolicy without making it
>>>>>> dependent on a new libsepol version.  That's why extended_socket_class
>>>>>> isn't yet enabled in refpolicy, for example.  That causes enablement
>>>>>> and adoption to lag behind.  It also makes it harder to test the new
>>>>>> kernel feature in the first place.
>>>>>
>>>>> I would like to see Fedora package the RC's in Rawhide as well (other distributions could help by packaging the RC's in unstable as well). That would atleast make the RC's a bit more accessible.
>>>>> In Fedora it is usually not the kernel that is the problem, it is user space that is generally to old. And as you've said policy is no longer a problem with CIL.
>>>>
>>>> [NOTE: I'm still thinking about the rest of Stephen's email, and the
>>>> follow up comments, but I wanted to reply to this particular comment
>>>> separately.]
>>>>
>>>> I'm not sure I want to see SELinux userspace release candidates in
>>>> normal Rawhide, but I think creating a COPR repository to
>>>> build/distribute release candidates could be a good thing.  We already
>>>> do something similar for the kernel patches and it has been helpful in
>>>> my opinion.
>>>
>>> Thanks, Yes i suppose you are right. Release Candidates would probably potentially cause too much disruption even in Rawhide.
>>> COPR should do the job, although will not be as accessible as Rawhide. It won't get the same kind of attention, but it will do for me.
>>
>> With COPR though we might be able to package more frequent and not just RC's (weekly's/nightly's)? If that can somehow be automated  then we also do not have to worrie so much about keeping things maintained over time
>
> I'm just building new set of updates in my COPR plautrba/selinux
> repository [1]. It's based on latest upstream sources with some Fedora
> patches on the top of it currently tracked in my github tree [2]. But
> there are some problems and it's not ready yet.
>
> I used to build vanilla upstream sources [3] but the latest build is 15
> months old. I can restart this project if there's an interest.
>
> Since COPR provides API with an authentication token, builds can
> automated and I have few scripts I used before.
>
> I think it could even work for Rawhide with less frequent update cycle.

I used to use your upstream COPR builds on my primary test system, but
I dropped that repo when I had to rebuild that system a few months ago
and I realized the COPR repo had grown stale.

If you've got the time, I think it might be helpful to create a COPR
with the upstream userspace and the Fedora/Rawhide patches layered on
top; it would help get more testing/exposure and should help amortize
the work in keeping the Fedora/Rawhide patches current.  If it helps
any I keep my COPR patching/building scripts at the link below.  The
pcopr_srpm-kernel script is package specific but should be easy to
adopt to new packages; the pcopr_patch script should be generic enough
to work on any package.

* https://github.com/pcmoore/copr-pkg_scripts

-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Policy capabilities: when to use and complications with using
  2017-05-09 17:49   ` Paul Moore
@ 2017-05-09 20:39     ` Stephen Smalley
  2017-05-09 21:44       ` Paul Moore
  0 siblings, 1 reply; 17+ messages in thread
From: Stephen Smalley @ 2017-05-09 20:39 UTC (permalink / raw)
  To: Paul Moore, James Carter
  Cc: Eric Paris, Joshua Brindle, Chris PeBenito, selinux

On Tue, 2017-05-09 at 13:49 -0400, Paul Moore wrote:
> > On 05/03/2017 12:14 PM, Stephen Smalley wrote:
> > > 
> > > 1) Should we investigate lighter weight support for policy
> > > capabilities, and if so, how?
> 
> I agree that not having to update userspace for each new policy
> capability is a desirable goal, but I'm hopeful that changes
> requiring
> a new policy capability are kept to a minimum.
> 
> > > 2) Should the kernel log information about enabled/disabled
> > > policy
> > > capabilities in much the same manner as it does for undefined
> > > classes/permissions?
> 
> This seems like a very good idea to me.
> 
> * https://github.com/SELinuxProject/selinux-kernel/issues/32
> 
> > > 3) Should the policy compiler toolchain warn the user if a policy
> > > capability is not declared and classes/permissions are used in
> > > rules
> > > that will only be used if that policy capability is
> > > declared?  And
> > > similarly if a policy capability is declared but the
> > > corresponding
> > > classes/permissions are not used in any rules?
> 
> This seems to go against lighter weight policy capabilities, would it
> not?

Not necessarily.  Let's say that we left policy capabilities as strings
in the kernel policy.  Then when introducing a new policy capability,
we would just need to patch the kernel to implement it, and patch the
policy (or even just insert a CIL module) to define it for testing and
enablement, similar to what we do for new classes/permissions.  We
wouldn't need an updated libsepol for basic enablement (which likewise
doesn't need to be patched for new classes/permissions).   We could
update checkpolicy and/or libsepol to recognize particular capability
names and provide these warnings, but those would be to help catch
policy mistakes, not a prerequisite to using the new capability at all.
 
The downside however is that we'd lose on build-time detection of e.g.
a typo in a capability name.  Maybe there is a middle ground where we
could warn if unrecognized but let them through.

Even if we insist on libsepol validation of the policy capability names
(to ensure build-time detection of a typo), it might be helpful to add
the string form of the capability names to the kernel policy. 
Otherwise, the kernel can't log anything useful about unrecognized
capabilities besides their bit number in the ebitmap.

> 
> > > 4) Do we need/want a policy capability for map permission and
> > > other
> > > cases where we are only adding a new permission check? Or should
> > > we
> > > continue to reserve them for cases not addressed via
> > > handle_unknown?
> 
> See James' earlier comments.  I think sticking with the current usage
> is the "best practice", but I think we should reserve the right to
> treat each change individually.  I'm hopeful that changes where we
> consider new policy capabilities remain infrequent enough that we can
> along without a concrete policy on their use.

So what about the two commits I cited?  Were we correct in not
introducing new policy capabilities for them, or should we have done
so?  Each of them switched from checking an existing class to a new
one, so they wouldn't break existing userspace (i.e. cause any new
denials) due to handle_unknown, but they could have caused a regression
in policy enforcement (i.e. allow something that would have been denied
previously under the old class).

> 
> > I know you also want this:
> > 5) Should CIL support adding a permission to a new class, so we
> > don't need
> > to grab a source rpm and rebuild the whole policy from source just
> > to test a
> > new permission?
> 
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Policy capabilities: when to use and complications with using
  2017-05-09 20:39     ` Stephen Smalley
@ 2017-05-09 21:44       ` Paul Moore
  2017-05-10 12:58         ` Stephen Smalley
  0 siblings, 1 reply; 17+ messages in thread
From: Paul Moore @ 2017-05-09 21:44 UTC (permalink / raw)
  To: Stephen Smalley
  Cc: James Carter, Eric Paris, Joshua Brindle, Chris PeBenito, selinux

On Tue, May 9, 2017 at 4:39 PM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On Tue, 2017-05-09 at 13:49 -0400, Paul Moore wrote:
>> > On 05/03/2017 12:14 PM, Stephen Smalley wrote:
>> > >
>> > > 1) Should we investigate lighter weight support for policy
>> > > capabilities, and if so, how?
>>
>> I agree that not having to update userspace for each new policy
>> capability is a desirable goal, but I'm hopeful that changes
>> requiring
>> a new policy capability are kept to a minimum.
>>
>> > > 2) Should the kernel log information about enabled/disabled
>> > > policy
>> > > capabilities in much the same manner as it does for undefined
>> > > classes/permissions?
>>
>> This seems like a very good idea to me.
>>
>> * https://github.com/SELinuxProject/selinux-kernel/issues/32
>>
>> > > 3) Should the policy compiler toolchain warn the user if a policy
>> > > capability is not declared and classes/permissions are used in
>> > > rules
>> > > that will only be used if that policy capability is
>> > > declared?  And
>> > > similarly if a policy capability is declared but the
>> > > corresponding
>> > > classes/permissions are not used in any rules?
>>
>> This seems to go against lighter weight policy capabilities, would it
>> not?
>
> Not necessarily.  Let's say that we left policy capabilities as strings
> in the kernel policy.  Then when introducing a new policy capability,
> we would just need to patch the kernel to implement it, and patch the
> policy (or even just insert a CIL module) to define it for testing and
> enablement, similar to what we do for new classes/permissions.  We
> wouldn't need an updated libsepol for basic enablement (which likewise
> doesn't need to be patched for new classes/permissions).   We could
> update checkpolicy and/or libsepol to recognize particular capability
> names and provide these warnings, but those would be to help catch
> policy mistakes, not a prerequisite to using the new capability at all.

I was referring to the additional checking/warnings, not basic
enablement, as I thought that was the point of #3.

> The downside however is that we'd lose on build-time detection of e.g.
> a typo in a capability name.  Maybe there is a middle ground where we
> could warn if unrecognized but let them through.
>
> Even if we insist on libsepol validation of the policy capability names
> (to ensure build-time detection of a typo), it might be helpful to add
> the string form of the capability names to the kernel policy.
> Otherwise, the kernel can't log anything useful about unrecognized
> capabilities besides their bit number in the ebitmap.

My only thought on this is that we already have the capability bitmap,
and for compatibility reasons that needs to stay, at least for the
existing capabilities, and I have a strong desire to limit the amount
of legacy "stuff" we have to carry around in the kernel.

However, I understand your points about easier enablement in userspace
and I'm sympathetic (this problem isn't going to go away, arguably in
some way it could get worse if the number of address families grows
frequently).  If this is something you and the rest of the userspace
folks feel strongly about I can be persuaded, but you have to *really*
want this; it needs to be more than a "nice to have".

>> > > 4) Do we need/want a policy capability for map permission and
>> > > other
>> > > cases where we are only adding a new permission check? Or should
>> > > we
>> > > continue to reserve them for cases not addressed via
>> > > handle_unknown?
>>
>> See James' earlier comments.  I think sticking with the current usage
>> is the "best practice", but I think we should reserve the right to
>> treat each change individually.  I'm hopeful that changes where we
>> consider new policy capabilities remain infrequent enough that we can
>> along without a concrete policy on their use.
>
> So what about the two commits I cited?  Were we correct in not
> introducing new policy capabilities for them, or should we have done
> so?  Each of them switched from checking an existing class to a new
> one, so they wouldn't break existing userspace (i.e. cause any new
> denials) due to handle_unknown, but they could have caused a regression
> in policy enforcement (i.e. allow something that would have been denied
> previously under the old class).

Yes, in hindsight we should have introduced new policy capabilities
for both those changes.

Unfortunately, the netlink socket class update has been part of Linus'
releases since v4.2 and I fear that introducing that policy capability
now could introduce it's own problems.  What if someone has a policy
module that only uses the new object classes and doesn't set the
policy capability?  I worry that there is no good solution for this
problem.

The namespace checks are less bad in this sense since they have only
been shipping since v4.7, but I think a similar problem exists.

I imagine we might be able to do something clever in the kernel and
special cases these object classes in the handle_unknown/allow case,
but I would be curious to hear if anyone else has a better idea on how
to fix this.  Do you?

-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Policy capabilities: when to use and complications with using
  2017-05-09 21:44       ` Paul Moore
@ 2017-05-10 12:58         ` Stephen Smalley
  2017-05-10 21:21           ` Paul Moore
  0 siblings, 1 reply; 17+ messages in thread
From: Stephen Smalley @ 2017-05-10 12:58 UTC (permalink / raw)
  To: Paul Moore
  Cc: James Carter, Eric Paris, Joshua Brindle, Chris PeBenito, selinux

On Tue, 2017-05-09 at 17:44 -0400, Paul Moore wrote:
> On Tue, May 9, 2017 at 4:39 PM, Stephen Smalley <sds@tycho.nsa.gov>
> wrote:
> > On Tue, 2017-05-09 at 13:49 -0400, Paul Moore wrote:
> > > > On 05/03/2017 12:14 PM, Stephen Smalley wrote:
> > > > > 
> > > > > 1) Should we investigate lighter weight support for policy
> > > > > capabilities, and if so, how?
> > > 
> > > I agree that not having to update userspace for each new policy
> > > capability is a desirable goal, but I'm hopeful that changes
> > > requiring
> > > a new policy capability are kept to a minimum.
> > > 
> > > > > 2) Should the kernel log information about enabled/disabled
> > > > > policy
> > > > > capabilities in much the same manner as it does for undefined
> > > > > classes/permissions?
> > > 
> > > This seems like a very good idea to me.
> > > 
> > > * https://github.com/SELinuxProject/selinux-kernel/issues/32
> > > 
> > > > > 3) Should the policy compiler toolchain warn the user if a
> > > > > policy
> > > > > capability is not declared and classes/permissions are used
> > > > > in
> > > > > rules
> > > > > that will only be used if that policy capability is
> > > > > declared?  And
> > > > > similarly if a policy capability is declared but the
> > > > > corresponding
> > > > > classes/permissions are not used in any rules?
> > > 
> > > This seems to go against lighter weight policy capabilities,
> > > would it
> > > not?
> > 
> > Not necessarily.  Let's say that we left policy capabilities as
> > strings
> > in the kernel policy.  Then when introducing a new policy
> > capability,
> > we would just need to patch the kernel to implement it, and patch
> > the
> > policy (or even just insert a CIL module) to define it for testing
> > and
> > enablement, similar to what we do for new classes/permissions.  We
> > wouldn't need an updated libsepol for basic enablement (which
> > likewise
> > doesn't need to be patched for new classes/permissions).   We could
> > update checkpolicy and/or libsepol to recognize particular
> > capability
> > names and provide these warnings, but those would be to help catch
> > policy mistakes, not a prerequisite to using the new capability at
> > all.
> 
> I was referring to the additional checking/warnings, not basic
> enablement, as I thought that was the point of #3.
> 
> > The downside however is that we'd lose on build-time detection of
> > e.g.
> > a typo in a capability name.  Maybe there is a middle ground where
> > we
> > could warn if unrecognized but let them through.
> > 
> > Even if we insist on libsepol validation of the policy capability
> > names
> > (to ensure build-time detection of a typo), it might be helpful to
> > add
> > the string form of the capability names to the kernel policy.
> > Otherwise, the kernel can't log anything useful about unrecognized
> > capabilities besides their bit number in the ebitmap.
> 
> My only thought on this is that we already have the capability
> bitmap,
> and for compatibility reasons that needs to stay, at least for the
> existing capabilities, and I have a strong desire to limit the amount
> of legacy "stuff" we have to carry around in the kernel.
> 
> However, I understand your points about easier enablement in
> userspace
> and I'm sympathetic (this problem isn't going to go away, arguably in
> some way it could get worse if the number of address families grows
> frequently).  If this is something you and the rest of the userspace
> folks feel strongly about I can be persuaded, but you have to
> *really*
> want this; it needs to be more than a "nice to have".
> 
> > > > > 4) Do we need/want a policy capability for map permission and
> > > > > other
> > > > > cases where we are only adding a new permission check? Or
> > > > > should
> > > > > we
> > > > > continue to reserve them for cases not addressed via
> > > > > handle_unknown?
> > > 
> > > See James' earlier comments.  I think sticking with the current
> > > usage
> > > is the "best practice", but I think we should reserve the right
> > > to
> > > treat each change individually.  I'm hopeful that changes where
> > > we
> > > consider new policy capabilities remain infrequent enough that we
> > > can
> > > along without a concrete policy on their use.
> > 
> > So what about the two commits I cited?  Were we correct in not
> > introducing new policy capabilities for them, or should we have
> > done
> > so?  Each of them switched from checking an existing class to a new
> > one, so they wouldn't break existing userspace (i.e. cause any new
> > denials) due to handle_unknown, but they could have caused a
> > regression
> > in policy enforcement (i.e. allow something that would have been
> > denied
> > previously under the old class).
> 
> Yes, in hindsight we should have introduced new policy capabilities
> for both those changes.
> 
> Unfortunately, the netlink socket class update has been part of
> Linus'
> releases since v4.2 and I fear that introducing that policy
> capability
> now could introduce it's own problems.  What if someone has a policy
> module that only uses the new object classes and doesn't set the
> policy capability?  I worry that there is no good solution for this
> problem.
> 
> The namespace checks are less bad in this sense since they have only
> been shipping since v4.7, but I think a similar problem exists.
> 
> I imagine we might be able to do something clever in the kernel and
> special cases these object classes in the handle_unknown/allow case,
> but I would be curious to hear if anyone else has a better idea on
> how
> to fix this.  Do you?

I'm not proposing introducing policy capabilities for those commits
retroactively; I don't think that would be productive now that they are
already in upstream kernels and policies.  I just wanted to determine
whether or not we think similar changes in the future should be wrapped
with policy capabilities.

If so, then I think that motivates lighter weight policy capabilities,
as otherwise for each of these changes (and others too - e.g. probably
the prlimit change) we would have been in the same position as with
extended_socket_class, i.e. waiting for a release of libsepol that
defines the new policy capability, requiring refpolicy to add a
dependency on that specific libsepol version before it could be enabled
by default, waiting for Fedora to update to that version, etc.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Policy capabilities: when to use and complications with using
  2017-05-10 12:58         ` Stephen Smalley
@ 2017-05-10 21:21           ` Paul Moore
  0 siblings, 0 replies; 17+ messages in thread
From: Paul Moore @ 2017-05-10 21:21 UTC (permalink / raw)
  To: Stephen Smalley
  Cc: James Carter, Eric Paris, Joshua Brindle, Chris PeBenito, selinux

On Wed, May 10, 2017 at 8:58 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> I'm not proposing introducing policy capabilities for those commits
> retroactively; I don't think that would be productive now that they are
> already in upstream kernels and policies.  I just wanted to determine
> whether or not we think similar changes in the future should be wrapped
> with policy capabilities.
>
> If so, then I think that motivates lighter weight policy capabilities,
> as otherwise for each of these changes (and others too - e.g. probably
> the prlimit change) we would have been in the same position as with
> extended_socket_class, i.e. waiting for a release of libsepol that
> defines the new policy capability, requiring refpolicy to add a
> dependency on that specific libsepol version before it could be enabled
> by default, waiting for Fedora to update to that version, etc.

That's fine with me.  As I said earlier, I'm not opposed, I just
wanted to make sure this is a definite "must have".

-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2017-05-10 21:21 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-03 16:14 Policy capabilities: when to use and complications with using Stephen Smalley
2017-05-03 16:51 ` Dominick Grift
2017-05-03 17:23   ` Stephen Smalley
2017-05-04 15:50   ` Paul Moore
2017-05-04 17:42     ` Dominick Grift
2017-05-04 17:50       ` Dominick Grift
2017-05-04 19:22         ` Petr Lautrbach
2017-05-04 19:44           ` Dominick Grift
2017-05-09 18:03           ` Paul Moore
2017-05-03 19:35 ` James Carter
2017-05-04 12:18   ` Chris PeBenito
2017-05-04 16:07     ` Dominick Grift
2017-05-09 17:49   ` Paul Moore
2017-05-09 20:39     ` Stephen Smalley
2017-05-09 21:44       ` Paul Moore
2017-05-10 12:58         ` Stephen Smalley
2017-05-10 21:21           ` Paul Moore

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.